Merge pull request openshift#1437 from trozet/merge_12-12-22

OCPBUGS-4659: [DownstreamMerge] - 12-12-22

go-controller/pkg/libovsdbops/portgroup.go

@@ -75,6 +75,25 @@ func CreateOrUpdatePortGroups(nbClient libovsdbclient.Client, pgs ...*nbdb.PortG
 	return err
 }
 
+// GetPortGroup looks up a port group from the cache
+func GetPortGroup(nbClient libovsdbclient.Client, pg *nbdb.PortGroup) (*nbdb.PortGroup, error) {
+	found := []*nbdb.PortGroup{}
+	opModel := operationModel{
+		Model:          pg,
+		ExistingResult: &found,
+		ErrNotFound:    true,
+		BulkOp:         false,
+	}
+
+	m := newModelClient(nbClient)
+	err := m.Lookup(opModel)
+	if err != nil {
+		return nil, err
+	}
+
+	return found[0], nil
+}
+
 func AddPortsToPortGroupOps(nbClient libovsdbclient.Client, ops []libovsdb.Operation, name string, ports ...string) ([]libovsdb.Operation, error) {
 	if len(ports) == 0 {
 		return ops, nil

go-controller/pkg/network-controller-manager/network_controller_manager.go

@@ -59,6 +59,8 @@ type NetworkControllerManager struct {
 	sbClient libovsdbclient.Client
 	// has SCTP support
 	SCTPSupport bool
+	// Supports multicast?
+	multicastSupport bool
 
 	stopChan chan struct{}
 	wg       *sync.WaitGroup
@@ -194,6 +196,8 @@ func (cm *NetworkControllerManager) Init() error {
 		return err
 	}
 
+	cm.configureMulticastSupport()
+
 	err = cm.enableOVNLogicalDataPathGroups()
 	if err != nil {
 		return err
@@ -218,6 +222,17 @@ func (cm *NetworkControllerManager) configureSCTPSupport() error {
 	return nil
 }
 
+func (cm *NetworkControllerManager) configureMulticastSupport() {
+	cm.multicastSupport = config.EnableMulticast
+	if cm.multicastSupport {
+		if _, _, err := util.RunOVNSbctl("--columns=_uuid", "list", "IGMP_Group"); err != nil {
+			klog.Warningf("Multicast support enabled, however version of OVN in use does not support IGMP Group. " +
+				"Disabling Multicast Support")
+			cm.multicastSupport = false
+		}
+	}
+}
+
 // enableOVNLogicalDataPathGroups sets an OVN flag to enable logical datapath
 // groups on OVN 20.12 and later. The option is ignored if OVN doesn't
 // understand it. Logical datapath groups reduce the size of the southbound
@@ -242,9 +257,9 @@ func (cm *NetworkControllerManager) configureMetrics(stopChan <-chan struct{}) {
 
 // NewDefaultNetworkController creates and returns the controller for default network
 func (cm *NetworkControllerManager) NewDefaultNetworkController() {
-	bnc := ovn.NewBaseNetworkController(cm.client, cm.kube, cm.watchFactory, cm.recorder, cm.nbClient,
-		cm.sbClient, cm.podRecorder, cm.SCTPSupport)
-	defaultController := ovn.NewDefaultNetworkController(bnc)
+	cnci := ovn.NewCommonNetworkControllerInfo(cm.client, cm.kube, cm.watchFactory, cm.recorder, cm.nbClient,
+		cm.sbClient, cm.podRecorder, cm.SCTPSupport, cm.multicastSupport)
+	defaultController := ovn.NewDefaultNetworkController(cnci)
 	cm.ovnControllers[ovntypes.DefaultNetworkName] = defaultController
 }
 

go-controller/pkg/node/gateway_init.go

@@ -247,7 +247,7 @@ func configureSvcRouteViaInterface(iface string, gwIPs []net.IP) error {
 			mtu = config.Default.RoutableMTU
 		}
 
-		err = util.LinkRoutesAddOrUpdateSourceOrMTU(link, gwIP[0], []*net.IPNet{subnet}, mtu, nil)
+		err = util.LinkRoutesApply(link, gwIP[0], []*net.IPNet{subnet}, mtu, nil)
 		if err != nil {
 			return fmt.Errorf("unable to add/update route for service via %s for gwIP %s, error: %v", iface, gwIP[0].String(), err)
 		}

go-controller/pkg/node/gateway_shared_intf.go

@@ -1738,7 +1738,7 @@ func addMasqueradeRoute(netIfaceName, nodeName string, watchFactory factory.Node
 		}
 		if masqIPNet != nil {
 			klog.Infof("Setting OVN Masquerade route with source: %s", nodeIP)
-			err = util.LinkRoutesAddOrUpdateSourceOrMTU(netIfaceLink, nil, []*net.IPNet{masqIPNet},
+			err = util.LinkRoutesApply(netIfaceLink, nil, []*net.IPNet{masqIPNet},
 				mtu, nodeIP)
 			if err != nil {
 				return fmt.Errorf("unable to add OVN masquerade route to host, error: %v", err)

go-controller/pkg/node/management-port_linux.go

@@ -187,7 +187,7 @@ func setupManagementPortIPFamilyConfig(mpcfg *managementPortConfig, cfg *managem
 			return warnings, err
 		}
 
-		err = util.LinkRoutesAddOrUpdateSourceOrMTU(mpcfg.link, cfg.gwIP, []*net.IPNet{subnet}, config.Default.RoutableMTU, nil)
+		err = util.LinkRoutesApply(mpcfg.link, cfg.gwIP, []*net.IPNet{subnet}, config.Default.RoutableMTU, nil)
 		if err != nil {
 			return warnings, err
 		}

go-controller/pkg/node/node.go

@@ -544,7 +544,7 @@ func (n *OvnNode) Start(ctx context.Context) error {
 					} else {
 						gwIP = mgmtPortConfig.ipv6.gwIP
 					}
-					err := util.LinkRoutesAddOrUpdateSourceOrMTU(link, gwIP, []*net.IPNet{subnet}, config.Default.RoutableMTU, nil)
+					err := util.LinkRoutesApply(link, gwIP, []*net.IPNet{subnet}, config.Default.RoutableMTU, nil)
 					if err != nil {
 						return fmt.Errorf("unable to add legacy route for services via mp0, error: %v", err)
 					}

go-controller/pkg/ovn/address_set/address_set.go

@@ -24,7 +24,7 @@ const (
 	ipv6AddressSetSuffix = "_v6"
 )
 
-type AddressSetIterFunc func(hashedName, namespace, suffix string) error
+type AddressSetIterFunc func(hashedName, name string) error
 type AddressSetDoFunc func(as AddressSet) error
 
 // AddressSetFactory is an interface for managing address set objects
@@ -135,7 +135,9 @@ func (asf *ovnAddressSetFactory) EnsureAddressSet(name string) (AddressSet, erro
 	return &ovnAddressSets{nbClient: asf.nbClient, name: name, ipv4: v4set, ipv6: v6set}, nil
 }
 
-func forEachAddressSet(nbClient libovsdbclient.Client, do func(string) error) error {
+// forEachAddressSet executes a do function on each address set found to have ExternalIDs["name"].
+// do function should take parameters: hashed addr set name, real name
+func forEachAddressSet(nbClient libovsdbclient.Client, do func(string, string) error) error {
 	p := func(addrSet *nbdb.AddressSet) bool {
 		_, exists := addrSet.ExternalIDs["name"]
 		return exists
@@ -147,7 +149,7 @@ func forEachAddressSet(nbClient libovsdbclient.Client, do func(string) error) er
 
 	var errors []error
 	for _, addrSet := range addrSetList {
-		if err := do(addrSet.ExternalIDs["name"]); err != nil {
+		if err := do(addrSet.Name, addrSet.ExternalIDs["name"]); err != nil {
 			errors = append(errors, err)
 		}
 	}
@@ -159,12 +161,10 @@ func forEachAddressSet(nbClient libovsdbclient.Client, do func(string) error) er
 	return nil
 }
 
-// ProcessEachAddressSet will pass the unhashed address set name, namespace name
-// and the first suffix in the name to the 'iteratorFn' for every address_set in
-// OVN. (Unhashed address set names are of the form namespaceName[.suffix1.suffix2. .suffixN])
+// ProcessEachAddressSet will pass the hashed and unhashed address set name to iteratorFn for every address set.
 func (asf *ovnAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIterFunc) error {
 	processedAddressSets := sets.String{}
-	return forEachAddressSet(asf.nbClient, func(name string) error {
+	return forEachAddressSet(asf.nbClient, func(hashedName, name string) error {
 		// Remove the suffix from the address set name and normalize
 		addrSetName := truncateSuffixFromAddressSet(name)
 		if processedAddressSets.Has(addrSetName) {
@@ -174,13 +174,7 @@ func (asf *ovnAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIter
 			return nil
 		}
 		processedAddressSets.Insert(addrSetName)
-		names := strings.Split(addrSetName, ".")
-		addrSetNamespace := names[0]
-		nameSuffix := ""
-		if len(names) >= 2 {
-			nameSuffix = names[1]
-		}
-		return iteratorFn(addrSetName, addrSetNamespace, nameSuffix)
+		return iteratorFn(hashedName, addrSetName)
 	})
 }
 
@@ -496,6 +490,10 @@ func (as *ovnAddressSet) addIPs(ips []net.IP) ([]ovsdb.Operation, error) {
 
 	uniqIPs := ipsToStringUnique(ips)
 
+	if as.hasIPs(uniqIPs...) {
+		return nil, nil
+	}
+
 	klog.V(5).Infof("(%s) adding IPs (%s) to address set", asDetail(as), uniqIPs)
 
 	addrset := nbdb.AddressSet{
@@ -510,6 +508,20 @@ func (as *ovnAddressSet) addIPs(ips []net.IP) ([]ovsdb.Operation, error) {
 	return ops, nil
 }
 
+// hasIPs returns true if an address set contains all given IPs
+func (as *ovnAddressSet) hasIPs(ips ...string) bool {
+	existingIPs, err := as.getIPs()
+	if err != nil {
+		return false
+	}
+
+	if len(existingIPs) == 0 {
+		return false
+	}
+
+	return sets.NewString(existingIPs...).HasAll(ips...)
+}
+
 // deleteIPs removes selected IPs from the existing address_set
 func (as *ovnAddressSet) deleteIPs(ips []net.IP) ([]ovsdb.Operation, error) {
 	if len(ips) == 0 {

go-controller/pkg/ovn/address_set/address_set_cleanup.go

@@ -16,7 +16,7 @@ func NonDualStackAddressSetCleanup(nbClient libovsdbclient.Client) error {
 	const old = 0
 	const new = 1
 	addressSets := map[string][2]bool{}
-	err := forEachAddressSet(nbClient, func(name string) error {
+	err := forEachAddressSet(nbClient, func(hashedName, name string) error {
 		shortName := truncateSuffixFromAddressSet(name)
 		spec, found := addressSets[shortName]
 		if !found {

go-controller/pkg/ovn/address_set/address_set_test.go

@@ -64,22 +64,22 @@ var _ = ginkgo.Describe("OVN Address Set operations", func() {
 	})
 
 	ginkgo.Context("when iterating address sets", func() {
-		ginkgo.It("calls the iterator function for each address set with the given prefix", func() {
+		ginkgo.It("calls the iterator function for each address set", func() {
 			app.Action = func(ctx *cli.Context) error {
 				dbSetup := libovsdbtest.TestSetup{
 					NBData: []libovsdbtest.TestData{
 						&nbdb.AddressSet{
 							Name:        "1",
-							ExternalIDs: map[string]string{"name": "ns1.foo.bar"},
+							ExternalIDs: map[string]string{"name": "foo.bar"},
 						},
 						&nbdb.AddressSet{
 							Name:        "2",
-							ExternalIDs: map[string]string{"name": "ns2.test.test2"},
+							ExternalIDs: map[string]string{"name": "test.test2"},
 						},
 
 						&nbdb.AddressSet{
 							Name:        "3",
-							ExternalIDs: map[string]string{"name": "ns3"},
+							ExternalIDs: map[string]string{"name": "test3"},
 						},
 					},
 				}
@@ -91,33 +91,17 @@ var _ = ginkgo.Describe("OVN Address Set operations", func() {
 
 				_, err = config.InitConfig(ctx, nil, nil)
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())
-
-				namespaces := []testAddressSetName{
-					{
-						namespace: "ns1",
-						suffix:    []string{"foo", "bar"},
-					},
-					{
-						namespace: "ns2",
-						suffix:    []string{"test", "test2"},
-					},
-					{
-						namespace: "ns3",
-					},
+				expectedAddressSets := map[string]bool{
+					"foo.bar":    true,
+					"test.test2": true,
+					"test3":      true,
 				}
-
-				err = asFactory.ProcessEachAddressSet(func(addrSetName, namespaceName, nameSuffix string) error {
-					found := false
-					for _, n := range namespaces {
-						name := n.makeNames()
-						if addrSetName == name {
-							found = true
-							gomega.Expect(namespaceName).To(gomega.Equal(n.namespace))
-						}
-					}
-					gomega.Expect(found).To(gomega.BeTrue())
+				err = asFactory.ProcessEachAddressSet(func(hashedName, addrSetName string) error {
+					gomega.Expect(expectedAddressSets[addrSetName]).To(gomega.BeTrue())
+					delete(expectedAddressSets, addrSetName)
 					return nil
 				})
+				gomega.Expect(len(expectedAddressSets)).To(gomega.Equal(0))
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())
 				return nil
 			}

go-controller/pkg/ovn/address_set/fake_address_set.go

@@ -3,14 +3,12 @@ package addressset
 import (
 	"k8s.io/klog/v2"
 	"net"
-	"strings"
 	"sync"
 	"sync/atomic"
 
 	"github.com/ovn-org/libovsdb/ovsdb"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
 
-	"k8s.io/apimachinery/pkg/util/sets"
 	utilnet "k8s.io/utils/net"
 
 	"github.com/onsi/gomega"
@@ -73,21 +71,17 @@ func (f *FakeAddressSetFactory) EnsureAddressSet(name string) (AddressSet, error
 
 func (f *FakeAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIterFunc) error {
 	f.Lock()
-	defer f.Unlock()
-	asNames := sets.String{}
+	asNames := map[string]string{}
 	for _, set := range f.sets {
 		asName := truncateSuffixFromAddressSet(set.getName())
-		if asNames.Has(asName) {
+		if _, ok := asNames[asName]; ok {
 			continue
 		}
-		asNames.Insert(asName)
-		parts := strings.Split(asName, ".")
-		addrSetNamespace := parts[0]
-		nameSuffix := ""
-		if len(parts) >= 2 {
-			nameSuffix = parts[1]
-		}
-		if err := iteratorFn(asName, addrSetNamespace, nameSuffix); err != nil {
+		asNames[asName] = set.hashName
+	}
+	f.Unlock()
+	for asName, hashName := range asNames {
+		if err := iteratorFn(hashName, asName); err != nil {
 			return err
 		}
 	}