From 8e03e97b84101c331d097236b22f444750850a18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=A5=96=E5=BB=BA?= Date: Tue, 28 Mar 2023 14:22:17 +0800 Subject: [PATCH] fix ovn-bridge-mappings deletion (#2564) --- pkg/daemon/init.go | 58 ++++++------------------- pkg/daemon/ovs.go | 105 +++++++++++++++++++++++++++++---------------- 2 files changed, 83 insertions(+), 80 deletions(-) diff --git a/pkg/daemon/init.go b/pkg/daemon/init.go index ce24a98f894..237cff8da59 100644 --- a/pkg/daemon/init.go +++ b/pkg/daemon/init.go @@ -136,56 +136,19 @@ func ovsInitProviderNetwork(provider, nic string, exchangeLinkName, macLearningF } func ovsCleanProviderNetwork(provider string) error { - output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-bridge-mappings") + mappings, err := getOvnMappings("ovn-bridge-mappings") if err != nil { - return fmt.Errorf("failed to get ovn-bridge-mappings, %v: %q", err, output) + return err } - var idx int - var m, brName string - mappingPrefix := provider + ":" - brMappings := strings.Split(output, ",") - for idx, m = range brMappings { - if strings.HasPrefix(m, mappingPrefix) { - brName = m[len(mappingPrefix):] - klog.V(3).Infof("found bridge name for provider %s: %s", provider, brName) - break - } - } - - if idx != len(brMappings) { - brMappings = append(brMappings[:idx], brMappings[idx+1:]...) - if len(brMappings) == 0 { - output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-bridge-mappings") - } else { - output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-bridge-mappings="+strings.Join(brMappings, ",")) - } - if err != nil { - return fmt.Errorf("failed to set ovn-bridge-mappings, %v: %q", err, output) - } + brName := mappings[provider] + if brName == "" { + return nil } - if output, err = ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-chassis-mac-mappings"); err != nil { - return fmt.Errorf("failed to get ovn-chassis-mac-mappings, %v: %q", err, output) - } - macMappings := strings.Split(output, ",") - for _, macMap := range macMappings { - if strings.HasPrefix(macMap, mappingPrefix) { - macMappings = util.RemoveString(macMappings, macMap) - break - } - } - if len(macMappings) == 0 { - output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", "ovn-chassis-mac-mappings") - } else { - output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-chassis-mac-mappings="+strings.Join(macMappings, ",")) - } + output, err := ovs.Exec("list-br") if err != nil { - return fmt.Errorf("failed to set ovn-chassis-mac-mappings, %v: %q", err, output) - } - - if output, err = ovs.Exec("list-br"); err != nil { - return fmt.Errorf("failed to list OVS bridge %v: %q", err, output) + return fmt.Errorf("failed to list OVS bridges: %v, %q", err, output) } if !util.ContainsString(strings.Split(output, "\n"), brName) { @@ -225,5 +188,12 @@ func ovsCleanProviderNetwork(provider string) error { } } + if err := removeOvnMapping("ovn-chassis-mac-mappings", provider); err != nil { + return err + } + if err := removeOvnMapping("ovn-bridge-mappings", provider); err != nil { + return err + } + return nil } diff --git a/pkg/daemon/ovs.go b/pkg/daemon/ovs.go index b2cb236c006..f5ecbe5c2cd 100644 --- a/pkg/daemon/ovs.go +++ b/pkg/daemon/ovs.go @@ -83,21 +83,12 @@ func configureEmptyMirror(portName string, mtu int) error { return configureMirrorLink(portName, mtu) } -func updateOvnMapping(name, key, value string) error { - output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:"+name) - if err != nil { - return fmt.Errorf("failed to get %s, %v: %q", name, err, output) +func decodeOvnMappings(s string) map[string]string { + if len(s) == 0 { + return map[string]string{} } - if len(output) == 0 { - s := fmt.Sprintf("external-ids:%s=%s:%s", name, key, value) - if output, err = ovs.Exec("set", "open", ".", s); err != nil { - return fmt.Errorf("failed to set %s, %v: %q", name, err, output) - } - return nil - } - - fields := strings.Split(output, ",") + fields := strings.Split(s, ",") mappings := make(map[string]string, len(fields)+1) for _, f := range fields { idx := strings.IndexRune(f, ':') @@ -107,17 +98,37 @@ func updateOvnMapping(name, key, value string) error { } mappings[f[:idx]] = f[idx+1:] } - mappings[key] = value + return mappings +} + +func encodeOvnMappings(mappings map[string]string) string { + if len(mappings) == 0 { + return "" + } - fields = make([]string, 0, len(mappings)) + fields := make([]string, 0, len(mappings)) for k, v := range mappings { - fields = append(fields, fmt.Sprintf("%s:%s", k, v)) + fields = append(fields, fmt.Sprintf("%s:%v", k, v)) } + return strings.Join(fields, ",") +} - if len(fields) == 0 { +func getOvnMappings(name string) (map[string]string, error) { + output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:"+name) + if err != nil { + return nil, fmt.Errorf("failed to get %s, %v: %q", name, err, output) + } + + return decodeOvnMappings(output), nil +} + +func setOvnMappings(name string, mappings map[string]string) error { + var err error + var output string + if s := encodeOvnMappings(mappings); len(s) == 0 { output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", name) } else { - output, err = ovs.Exec("set", "open", ".", fmt.Sprintf("external-ids:%s=%s", name, strings.Join(fields, ","))) + output, err = ovs.Exec("set", "open", ".", fmt.Sprintf("external-ids:%s=%s", name, s)) } if err != nil { return fmt.Errorf("failed to set %s, %v: %q", name, err, output) @@ -126,6 +137,42 @@ func updateOvnMapping(name, key, value string) error { return nil } +func addOvnMapping(name, key, value string, overwrite bool) error { + mappings, err := getOvnMappings(name) + if err != nil { + return err + } + + if mappings[key] == value || (mappings[key] != "" && !overwrite) { + return nil + } + + mappings[key] = value + if err = setOvnMappings(name, mappings); err != nil { + return err + } + + return nil +} + +func removeOvnMapping(name, key string) error { + mappings, err := getOvnMappings(name) + if err != nil { + return err + } + + length := len(mappings) + delete(mappings, key) + if len(mappings) == length { + return nil + } + if err = setOvnMappings(name, mappings); err != nil { + return err + } + + return nil +} + func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error { brExists, err := ovs.BridgeExists(bridge) if err != nil { @@ -164,7 +211,7 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea } } - if err = updateOvnMapping("ovn-bridge-mappings", provider, bridge); err != nil { + if err = addOvnMapping("ovn-bridge-mappings", provider, bridge, true); err != nil { klog.Error(err) return err } @@ -173,23 +220,9 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea } func initProviderChassisMac(provider string) error { - output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-chassis-mac-mappings") - if err != nil { - return fmt.Errorf("failed to get ovn-bridge-mappings, %v", err) - } - - for _, macMap := range strings.Split(output, ",") { - if len(macMap) == len(provider)+18 && strings.Contains(output, provider) { - return nil - } - } - - macMappings := fmt.Sprintf("%s:%s", provider, util.GenerateMac()) - if output != "" { - macMappings = fmt.Sprintf("%s,%s", output, macMappings) - } - if output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-chassis-mac-mappings="+macMappings); err != nil { - return fmt.Errorf("failed to set ovn-chassis-mac-mappings, %v: %q", err, output) + if err := addOvnMapping("ovn-chassis-mac-mappings", provider, util.GenerateMac(), false); err != nil { + klog.Error(err) + return err } return nil }