Skip to content

Commit f4f990b

Browse files
zhangzujianzhangzujian
committed
fix ovn-bridge-mappings deletion (#2564)
1 parent e4242a0 commit f4f990b

File tree

2 files changed

+83
-80
lines changed

2 files changed

+83
-80
lines changed

‎pkg/daemon/init.go

+14-44
Original file line numberDiff line numberDiff line change
@@ -135,56 +135,19 @@ func ovsInitProviderNetwork(provider, nic string, exchangeLinkName, macLearningF
135135
}
136136

137137
func ovsCleanProviderNetwork(provider string) error {
138-
output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-bridge-mappings")
138+
mappings, err := getOvnMappings("ovn-bridge-mappings")
139139
if err != nil {
140-
return fmt.Errorf("failed to get ovn-bridge-mappings, %v: %q", err, output)
140+
return err
141141
}
142142

143-
var idx int
144-
var m, brName string
145-
mappingPrefix := provider + ":"
146-
brMappings := strings.Split(output, ",")
147-
for idx, m = range brMappings {
148-
if strings.HasPrefix(m, mappingPrefix) {
149-
brName = m[len(mappingPrefix):]
150-
klog.V(3).Infof("found bridge name for provider %s: %s", provider, brName)
151-
break
152-
}
153-
}
154-
155-
if idx != len(brMappings) {
156-
brMappings = append(brMappings[:idx], brMappings[idx+1:]...)
157-
if len(brMappings) == 0 {
158-
output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-bridge-mappings")
159-
} else {
160-
output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-bridge-mappings="+strings.Join(brMappings, ","))
161-
}
162-
if err != nil {
163-
return fmt.Errorf("failed to set ovn-bridge-mappings, %v: %q", err, output)
164-
}
143+
brName := mappings[provider]
144+
if brName == "" {
145+
return nil
165146
}
166147

167-
if output, err = ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-chassis-mac-mappings"); err != nil {
168-
return fmt.Errorf("failed to get ovn-chassis-mac-mappings, %v: %q", err, output)
169-
}
170-
macMappings := strings.Split(output, ",")
171-
for _, macMap := range macMappings {
172-
if strings.HasPrefix(macMap, mappingPrefix) {
173-
macMappings = util.RemoveString(macMappings, macMap)
174-
break
175-
}
176-
}
177-
if len(macMappings) == 0 {
178-
output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-chassis-mac-mappings")
179-
} else {
180-
output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-chassis-mac-mappings="+strings.Join(macMappings, ","))
181-
}
148+
output, err := ovs.Exec("list-br")
182149
if err != nil {
183-
return fmt.Errorf("failed to set ovn-chassis-mac-mappings, %v: %q", err, output)
184-
}
185-
186-
if output, err = ovs.Exec("list-br"); err != nil {
187-
return fmt.Errorf("failed to list OVS bridge %v: %q", err, output)
150+
return fmt.Errorf("failed to list OVS bridges: %v, %q", err, output)
188151
}
189152

190153
if !util.ContainsString(strings.Split(output, "\n"), brName) {
@@ -223,5 +186,12 @@ func ovsCleanProviderNetwork(provider string) error {
223186
}
224187
}
225188

189+
if err := removeOvnMapping("ovn-chassis-mac-mappings", provider); err != nil {
190+
return err
191+
}
192+
if err := removeOvnMapping("ovn-bridge-mappings", provider); err != nil {
193+
return err
194+
}
195+
226196
return nil
227197
}

‎pkg/daemon/ovs.go

+69-36
Original file line numberDiff line numberDiff line change
@@ -71,21 +71,12 @@ func configureEmptyMirror(portName string, mtu int) error {
7171
return configureMirrorLink(portName, mtu)
7272
}
7373

74-
func updateOvnMapping(name, key, value string) error {
75-
output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:"+name)
76-
if err != nil {
77-
return fmt.Errorf("failed to get %s, %v: %q", name, err, output)
74+
func decodeOvnMappings(s string) map[string]string {
75+
if len(s) == 0 {
76+
return map[string]string{}
7877
}
7978

80-
if len(output) == 0 {
81-
s := fmt.Sprintf("external-ids:%s=%s:%s", name, key, value)
82-
if output, err = ovs.Exec("set", "open", ".", s); err != nil {
83-
return fmt.Errorf("failed to set %s, %v: %q", name, err, output)
84-
}
85-
return nil
86-
}
87-
88-
fields := strings.Split(output, ",")
79+
fields := strings.Split(s, ",")
8980
mappings := make(map[string]string, len(fields)+1)
9081
for _, f := range fields {
9182
idx := strings.IndexRune(f, ':')
@@ -95,17 +86,37 @@ func updateOvnMapping(name, key, value string) error {
9586
}
9687
mappings[f[:idx]] = f[idx+1:]
9788
}
98-
mappings[key] = value
89+
return mappings
90+
}
91+
92+
func encodeOvnMappings(mappings map[string]string) string {
93+
if len(mappings) == 0 {
94+
return ""
95+
}
9996

100-
fields = make([]string, 0, len(mappings))
97+
fields := make([]string, 0, len(mappings))
10198
for k, v := range mappings {
102-
fields = append(fields, fmt.Sprintf("%s:%s", k, v))
99+
fields = append(fields, fmt.Sprintf("%s:%v", k, v))
103100
}
101+
return strings.Join(fields, ",")
102+
}
104103

105-
if len(fields) == 0 {
104+
func getOvnMappings(name string) (map[string]string, error) {
105+
output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:"+name)
106+
if err != nil {
107+
return nil, fmt.Errorf("failed to get %s, %v: %q", name, err, output)
108+
}
109+
110+
return decodeOvnMappings(output), nil
111+
}
112+
113+
func setOvnMappings(name string, mappings map[string]string) error {
114+
var err error
115+
var output string
116+
if s := encodeOvnMappings(mappings); len(s) == 0 {
106117
output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", name)
107118
} else {
108-
output, err = ovs.Exec("set", "open", ".", fmt.Sprintf("external-ids:%s=%s", name, strings.Join(fields, ",")))
119+
output, err = ovs.Exec("set", "open", ".", fmt.Sprintf("external-ids:%s=%s", name, s))
109120
}
110121
if err != nil {
111122
return fmt.Errorf("failed to set %s, %v: %q", name, err, output)
@@ -114,6 +125,42 @@ func updateOvnMapping(name, key, value string) error {
114125
return nil
115126
}
116127

128+
func addOvnMapping(name, key, value string, overwrite bool) error {
129+
mappings, err := getOvnMappings(name)
130+
if err != nil {
131+
return err
132+
}
133+
134+
if mappings[key] == value || (mappings[key] != "" && !overwrite) {
135+
return nil
136+
}
137+
138+
mappings[key] = value
139+
if err = setOvnMappings(name, mappings); err != nil {
140+
return err
141+
}
142+
143+
return nil
144+
}
145+
146+
func removeOvnMapping(name, key string) error {
147+
mappings, err := getOvnMappings(name)
148+
if err != nil {
149+
return err
150+
}
151+
152+
length := len(mappings)
153+
delete(mappings, key)
154+
if len(mappings) == length {
155+
return nil
156+
}
157+
if err = setOvnMappings(name, mappings); err != nil {
158+
return err
159+
}
160+
161+
return nil
162+
}
163+
117164
func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error {
118165
brExists, err := ovs.BridgeExists(bridge)
119166
if err != nil {
@@ -152,7 +199,7 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea
152199
}
153200
}
154201

155-
if err = updateOvnMapping("ovn-bridge-mappings", provider, bridge); err != nil {
202+
if err = addOvnMapping("ovn-bridge-mappings", provider, bridge, true); err != nil {
156203
klog.Error(err)
157204
return err
158205
}
@@ -161,23 +208,9 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea
161208
}
162209

163210
func initProviderChassisMac(provider string) error {
164-
output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-chassis-mac-mappings")
165-
if err != nil {
166-
return fmt.Errorf("failed to get ovn-bridge-mappings, %v", err)
167-
}
168-
169-
for _, macMap := range strings.Split(output, ",") {
170-
if len(macMap) == len(provider)+18 && strings.Contains(output, provider) {
171-
return nil
172-
}
173-
}
174-
175-
macMappings := fmt.Sprintf("%s:%s", provider, util.GenerateMac())
176-
if output != "" {
177-
macMappings = fmt.Sprintf("%s,%s", output, macMappings)
178-
}
179-
if output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-chassis-mac-mappings="+macMappings); err != nil {
180-
return fmt.Errorf("failed to set ovn-chassis-mac-mappings, %v: %q", err, output)
211+
if err := addOvnMapping("ovn-chassis-mac-mappings", provider, util.GenerateMac(), false); err != nil {
212+
klog.Error(err)
213+
return err
181214
}
182215
return nil
183216
}

0 commit comments

Comments
 (0)