Bug 2059354: [4.10z] After reboot egress node, lr-policy-list was not correct

dist/images/Dockerfile.fedora

@@ -9,13 +9,18 @@
 # are built locally and included in the image (instead of the rpm)
 #
 
-FROM fedora:34
+FROM fedora:35
 
 USER root
 
 ENV PYTHONDONTWRITEBYTECODE yes
 
-ARG ovnver=ovn-21.09.0-4.fc34
+ARG ovnver=ovn-21.12.0-5.fc35
+# Automatically populated when using docker buildx
+ARG TARGETPLATFORM
+ARG BUILDPLATFORM
+
+RUN echo "Running on $BUILDPLATFORM, building for $TARGETPLATFORM"
 
 # install needed rpms - openvswitch must be 2.10.4 or higher
 RUN INSTALL_PKGS=" \
@@ -29,7 +34,9 @@ RUN INSTALL_PKGS=" \
 
 RUN mkdir -p /var/run/openvswitch
 
-RUN koji download-build $ovnver --arch=x86_64
+RUN if [ "$TARGETPLATFORM" = "linux/amd64" ] || [ -z "$TARGETPLATFORM"] ; then koji download-build $ovnver --arch=x86_64  ; \
+    else koji download-build $ovnver --arch=aarch64 ; fi 
+
 RUN rpm -Uhv --nodeps --force *.rpm
 
 # Built in ../../go_controller, then the binaries are copied here.

go-controller/pkg/kube/kube.go

@@ -235,7 +235,7 @@ func (k *Kube) UpdateEgressFirewall(egressfirewall *egressfirewall.EgressFirewal
 
 // UpdateEgressIP updates the EgressIP with the provided EgressIP data
 func (k *Kube) UpdateEgressIP(eIP *egressipv1.EgressIP) error {
-	klog.Infof("Updating status on EgressIP %s", eIP.Name)
+	klog.Infof("Updating status on EgressIP %s status %v", eIP.Name, eIP.Status)
 	_, err := k.EIPClient.K8sV1().EgressIPs().Update(context.TODO(), eIP, metav1.UpdateOptions{})
 	return err
 }

go-controller/pkg/libovsdbops/switch.go

@@ -13,8 +13,8 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 )
 
-// findSwitch looks up the switch in the cache and sets the UUID
-func findSwitch(nbClient libovsdbclient.Client, lswitch *nbdb.LogicalSwitch) error {
+// findSwitchUUID looks up the switch in the cache and sets the UUID
+func findSwitchUUID(nbClient libovsdbclient.Client, lswitch *nbdb.LogicalSwitch) error {
 	if lswitch.UUID != "" && !IsNamedUUID(lswitch.UUID) {
 		return nil
 	}
@@ -119,6 +119,24 @@ func FindAllNodeLocalSwitches(nbClient libovsdbclient.Client) ([]nbdb.LogicalSwi
 	return switches, nil
 }
 
+// FindSwitchByName finds switch with provided name. If more than one is found, it will error.
+func FindSwitchByName(nbClient libovsdbclient.Client, name string) (*nbdb.LogicalSwitch, error) {
+	nameSearch := func(item *nbdb.LogicalSwitch) bool {
+		return item.Name == name
+	}
+
+	switches, err := findSwitchesByPredicate(nbClient, nameSearch)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(switches) > 1 {
+		return nil, fmt.Errorf("unexpectedly found multiple switches with same name: %+v", switches)
+	}
+
+	return &switches[0], nil
+}
+
 func AddLoadBalancersToSwitchOps(nbClient libovsdbclient.Client, ops []libovsdb.Operation, lswitch *nbdb.LogicalSwitch, lbs ...*nbdb.LoadBalancer) ([]libovsdb.Operation, error) {
 	if ops == nil {
 		ops = []libovsdb.Operation{}
@@ -127,7 +145,7 @@ func AddLoadBalancersToSwitchOps(nbClient libovsdbclient.Client, ops []libovsdb.
 		return ops, nil
 	}
 
-	err := findSwitch(nbClient, lswitch)
+	err := findSwitchUUID(nbClient, lswitch)
 	if err != nil {
 		return nil, err
 	}
@@ -157,7 +175,7 @@ func RemoveLoadBalancersFromSwitchOps(nbClient libovsdbclient.Client, ops []libo
 		return ops, nil
 	}
 
-	err := findSwitch(nbClient, lswitch)
+	err := findSwitchUUID(nbClient, lswitch)
 	if err != nil {
 		return nil, err
 	}

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

@@ -19,6 +19,7 @@ import (
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
@@ -29,7 +30,7 @@ const (
 	ipv6AddressSetSuffix = "_v6"
 )
 
-type AddressSetIterFunc func(hashedName, namespace, suffix string)
+type AddressSetIterFunc func(hashedName, namespace, suffix string) error
 type AddressSetDoFunc func(as AddressSet) error
 
 // AddressSetFactory is an interface for managing address set objects
@@ -175,7 +176,7 @@ 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 {
+func forEachAddressSet(nbClient libovsdbclient.Client, do func(string) error) error {
 	addrSetList := &[]nbdb.AddressSet{}
 	ctx, cancel := context.WithTimeout(context.Background(), types.OVSDBTimeout)
 	defer cancel()
@@ -188,9 +189,17 @@ func forEachAddressSet(nbClient libovsdbclient.Client, do func(string)) error {
 		return fmt.Errorf("error reading address sets: %+v", err)
 	}
 
+	var errors []error
 	for _, addrSet := range *addrSetList {
-		do(addrSet.ExternalIDs["name"])
+		if err := do(addrSet.ExternalIDs["name"]); err != nil {
+			errors = append(errors, err)
+		}
+	}
+
+	if len(errors) > 0 {
+		return fmt.Errorf("failed to iterate address sets: %v", utilerrors.NewAggregate(errors))
 	}
+
 	return nil
 }
 
@@ -199,14 +208,14 @@ func forEachAddressSet(nbClient libovsdbclient.Client, do func(string)) error {
 // OVN. (Unhashed address set names are of the form namespaceName[.suffix1.suffix2. .suffixN])
 func (asf *ovnAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIterFunc) error {
 	processedAddressSets := sets.String{}
-	err := forEachAddressSet(asf.nbClient, func(name string) {
+	return forEachAddressSet(asf.nbClient, func(name string) error {
 		// Remove the suffix from the address set name and normalize
 		addrSetName := truncateSuffixFromAddressSet(name)
 		if processedAddressSets.Has(addrSetName) {
 			// We have already processed the address set. In case of dual stack we will have _v4 and _v6
 			// suffixes for address sets. Since we are normalizing these two address sets through this API
 			// we will process only one normalized address set name.
-			return
+			return nil
 		}
 		processedAddressSets.Insert(addrSetName)
 		names := strings.Split(addrSetName, ".")
@@ -215,10 +224,8 @@ func (asf *ovnAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIter
 		if len(names) >= 2 {
 			nameSuffix = names[1]
 		}
-		iteratorFn(addrSetName, addrSetNamespace, nameSuffix)
+		return iteratorFn(addrSetName, addrSetNamespace, nameSuffix)
 	})
-
-	return err
 }
 
 func truncateSuffixFromAddressSet(asName string) string {

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) {
+	err := forEachAddressSet(nbClient, func(name string) error {
 		shortName := truncateSuffixFromAddressSet(name)
 		spec, found := addressSets[shortName]
 		if !found {
@@ -30,6 +30,7 @@ func NonDualStackAddressSetCleanup(nbClient libovsdbclient.Client) error {
 			spec[new] = true
 		}
 		addressSets[shortName] = spec
+		return nil
 	})
 
 	if err != nil {

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

@@ -106,7 +106,7 @@ var _ = ginkgo.Describe("OVN Address Set operations", func() {
 					},
 				}
 
-				err = asFactory.ProcessEachAddressSet(func(addrSetName, namespaceName, nameSuffix string) {
+				err = asFactory.ProcessEachAddressSet(func(addrSetName, namespaceName, nameSuffix string) error {
 					found := false
 					for _, n := range namespaces {
 						name := n.makeNames()
@@ -116,6 +116,7 @@ var _ = ginkgo.Describe("OVN Address Set operations", func() {
 						}
 					}
 					gomega.Expect(found).To(gomega.BeTrue())
+					return nil
 				})
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())
 				return nil

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

@@ -86,7 +86,9 @@ func (f *FakeAddressSetFactory) ProcessEachAddressSet(iteratorFn AddressSetIterF
 		if len(parts) >= 2 {
 			nameSuffix = parts[1]
 		}
-		iteratorFn(asName, addrSetNamespace, nameSuffix)
+		if err := iteratorFn(asName, addrSetNamespace, nameSuffix); err != nil {
+			return err
+		}
 	}
 	return nil
 }

go-controller/pkg/ovn/egressfirewall.go

@@ -97,29 +97,32 @@ func newEgressFirewallRule(rawEgressFirewallRule egressfirewallapi.EgressFirewal
 
 // NOTE: Utilize the fact that we know that all egress firewall related setup must have a priority: types.MinimumReservedEgressFirewallPriority <= priority <= types.EgressFirewallStartPriority
 func (oc *Controller) syncEgressFirewall(egressFirewalls []interface{}) {
+	oc.syncWithRetry("syncEgressFirewall", func() error { return oc.syncEgressFirewallRetriable(egressFirewalls) })
+}
+
+// This function implements the main body of work of what is described by syncEgressFirewall.
+// Upon failure, it may be invoked multiple times in order to avoid a pod restart.
+func (oc *Controller) syncEgressFirewallRetriable(egressFirewalls []interface{}) error {
 	// Lookup all ACLs used for egress Firewalls
 	egressFirewallACLs, err := libovsdbops.FindACLsByPriorityRange(oc.nbClient, types.MinimumReservedEgressFirewallPriority, types.EgressFirewallStartPriority)
 	if err != nil {
-		klog.Errorf("Unable to list egress firewall ACLs, cannot cleanup old stale data, err: %v", err)
-		return
+		return fmt.Errorf("unable to list egress firewall ACLs, cannot cleanup old stale data, err: %v", err)
 	}
 
 	if config.Gateway.Mode == config.GatewayModeShared {
 		// Mode is shared gateway mode, make sure to delete all egfw ACLs on the node switches
 		if len(egressFirewallACLs) != 0 {
 			err = libovsdbops.RemoveACLsFromNodeSwitches(oc.nbClient, egressFirewallACLs)
 			if err != nil {
-				klog.Errorf("Failed to remove reject acl from node logical switches: %v", err)
-				return
+				return fmt.Errorf("failed to remove reject acl from node logical switches: %v", err)
 			}
 		}
 	} else if config.Gateway.Mode == config.GatewayModeLocal {
 		// Mode is local gateway mode, make sure to delete all egfw ACLs on the join switches
 		if len(egressFirewallACLs) != 0 {
 			err = libovsdbops.RemoveACLsFromJoinSwitch(oc.nbClient, egressFirewallACLs)
 			if err != nil {
-				klog.Errorf("Failed to remove reject acl from node logical switches: %v", err)
-				return
+				return fmt.Errorf("failed to remove reject acl from node logical switches: %v", err)
 			}
 		}
 	}
@@ -143,7 +146,7 @@ func (oc *Controller) syncEgressFirewall(egressFirewalls []interface{}) {
 			})
 		}
 		if _, err := oc.modelClient.CreateOrUpdate(opModels...); err != nil {
-			klog.Errorf("Unable to set ACL direction on egress firewall acls, cannot convert old ACL data err: %v", err)
+			return fmt.Errorf("unable to set ACL direction on egress firewall acls, cannot convert old ACL data err: %v", err)
 		}
 	}
 	// In any gateway mode, make sure to delete all LRPs on ovn_cluster_router.
@@ -170,7 +173,7 @@ func (oc *Controller) syncEgressFirewall(egressFirewalls []interface{}) {
 		},
 	}
 	if err := oc.modelClient.Delete(opModels...); err != nil {
-		klog.Errorf("Unable to remove egress firewall policy, cannot cleanup old stale data, err: %v", err)
+		return fmt.Errorf("unable to remove egress firewall policy, cannot cleanup old stale data, err: %v", err)
 	}
 
 	// sync the ovn and k8s egressFirewall states
@@ -189,7 +192,7 @@ func (oc *Controller) syncEgressFirewall(egressFirewalls []interface{}) {
 	// get all the k8s EgressFirewall Objects
 	egressFirewallList, err := oc.kube.GetEgressFirewalls()
 	if err != nil {
-		klog.Errorf("Cannot reconcile the state of egressfirewalls in ovn database and k8s. err: %v", err)
+		return fmt.Errorf("cannot reconcile the state of egressfirewalls in ovn database and k8s. err: %v", err)
 	}
 	// delete entries from the map that exist in k8s and ovn
 	for _, egressFirewall := range egressFirewallList.Items {
@@ -199,10 +202,10 @@ func (oc *Controller) syncEgressFirewall(egressFirewalls []interface{}) {
 	for spuriousEF := range ovnEgressFirewalls {
 		err := oc.deleteEgressFirewallRules(spuriousEF)
 		if err != nil {
-			klog.Errorf("Cannot fully reconcile the state of egressfirewalls ACLs for namespace %s still exist in ovn db: %v", spuriousEF, err)
-			return
+			return fmt.Errorf("cannot fully reconcile the state of egressfirewalls ACLs for namespace %s still exist in ovn db: %v", spuriousEF, err)
 		}
 	}
+	return nil
 }
 
 func (oc *Controller) addEgressFirewall(egressFirewall *egressfirewallapi.EgressFirewall) error {

go-controller/pkg/ovn/egressgw_test.go

@@ -2190,6 +2190,10 @@ var _ = ginkgo.Describe("OVN Egress Gateway Operations", func() {
 								Name: types.GWRouterPrefix + nodeName,
 								UUID: types.GWRouterPrefix + nodeName + "-UUID",
 							},
+							&nbdb.LogicalSwitch{
+								UUID: "node1",
+								Name: "node1",
+							},
 						},
 					},
 					&v1.NamespaceList{
@@ -2220,6 +2224,10 @@ var _ = ginkgo.Describe("OVN Egress Gateway Operations", func() {
 						Name:     ovntypes.GWRouterToJoinSwitchPrefix + ovntypes.GWRouterPrefix + nodeName,
 						Networks: []string{"100.64.0.4/32"},
 					},
+					&nbdb.LogicalSwitch{
+						UUID: "node1",
+						Name: "node1",
+					},
 				}
 				injectNode(fakeOvn)
 				fakeOvn.controller.WatchNamespaces()
@@ -2241,6 +2249,10 @@ var _ = ginkgo.Describe("OVN Egress Gateway Operations", func() {
 						Name:     ovntypes.GWRouterToJoinSwitchPrefix + ovntypes.GWRouterPrefix + nodeName,
 						Networks: []string{"100.64.0.4/32"},
 					},
+					&nbdb.LogicalSwitch{
+						UUID: "node1",
+						Name: "node1",
+					},
 				}
 				err = deletePerPodGRSNAT(fakeOvn.controller.nbClient, nodeName, extIPs, []*net.IPNet{fullMaskPodNet})
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())