Merge pull request #5250 from martinkennelly/crash-no-ep

Node port watcher sync: ignore ns with no valid network

Author: trozet

go-controller/pkg/node/gateway_nftables.go

@@ -122,7 +122,12 @@ func recreateNFTSet(setName string, keepNFTElems []*knftables.Element) error {
 			tx.Add(elem)
 		}
 	}
-	return nft.Run(context.TODO(), tx)
+	err = nft.Run(context.TODO(), tx)
+	// no error if set is not created and we desire zero NFT elements
+	if knftables.IsNotFound(err) && len(keepNFTElems) == 0 {
+		return nil
+	}
+	return err
 }
 
 func recreateNFTMap(mapName string, keepNFTElems []*knftables.Element) error {
@@ -139,7 +144,12 @@ func recreateNFTMap(mapName string, keepNFTElems []*knftables.Element) error {
 			tx.Add(elem)
 		}
 	}
-	return nft.Run(context.TODO(), tx)
+	err = nft.Run(context.TODO(), tx)
+	// no error if set is not created and we desire zero NFT elements
+	if knftables.IsNotFound(err) && len(keepNFTElems) == 0 {
+		return nil
+	}
+	return err
 }
 
 // getGatewayNFTRules returns nftables rules for service. This must be used in conjunction

go-controller/pkg/node/gateway_shared_intf.go

@@ -1038,6 +1038,10 @@ func (npw *nodePortWatcher) SyncServices(services []interface{}) error {
 		}
 
 		netInfo, err := npw.networkManager.GetActiveNetworkForNamespace(service.Namespace)
+		// The InvalidPrimaryNetworkError is returned when the UDN is not found because it has already been deleted.
+		if util.IsInvalidPrimaryNetworkError(err) {
+			continue
+		}
 		if err != nil {
 			errors = append(errors, err)
 			continue

go-controller/pkg/node/gateway_udn_test.go

@@ -40,6 +40,7 @@ import (
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
 	coreinformermocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/k8s.io/client-go/informers/core/v1"
 	v1mocks "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/mocks/k8s.io/client-go/listers/core/v1"
+	fakenetworkmanager "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/networkmanager"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 
@@ -1554,6 +1555,34 @@ var _ = Describe("UserDefinedNetworkGateway", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(fexec.CalledMatchesExpected()).To(BeTrue(), fexec.ErrorDesc)
 	})
+
+	It("should sync node port watcher successfully if a namespaces network is invalid", func() {
+		// create new gateway, add ns with primary UDN, pod, expose pod via Node port service, delete pod, delete udn, ensure sync should succeeds
+		namespace := util.NewNamespace("udn")
+		config.OVNKubernetesFeature.EnableMultiNetwork = true
+		config.OVNKubernetesFeature.EnableNetworkSegmentation = true
+		namespace.Labels[types.RequiredUDNNamespaceLabel] = ""
+		service := newService("udn-svc", namespace.Name, "10.96.0.10", []corev1.ServicePort{{NodePort: int32(30300),
+			Protocol: corev1.ProtocolTCP, Port: int32(8080)}}, corev1.ServiceTypeNodePort, []string{}, corev1.ServiceStatus{},
+			true, false)
+		fakeClient := util.GetOVNClientset(service, namespace)
+		wf, err := factory.NewNodeWatchFactory(fakeClient.GetNodeClientset(), "node")
+		Expect(err).ToNot(HaveOccurred(), "must get new node watch factory")
+		Expect(wf.Start()).NotTo(HaveOccurred(), "must start Node watch factory")
+		defer func() {
+			wf.Shutdown()
+		}()
+		iptV4, iptV6 := util.SetFakeIPTablesHelpers()
+		nodenft.SetFakeNFTablesHelper()
+		fNPW := initFakeNodePortWatcher(iptV4, iptV6)
+		fNPW.watchFactory = wf
+		// in-order to simulate a namespace with an Invalid UDN (when GetActiveNamespace is called), we add an entry
+		// to the fake network manager but no specified network. GetActiveNetwork will return the appropriate error of Invalid Network for namespace.
+		// network manager may have a different implementation that fake network manager but both will return the same error.
+		fNPW.networkManager = &fakenetworkmanager.FakeNetworkManager{PrimaryNetworks: map[string]util.NetInfo{namespace.Name: nil}}
+		services := append([]interface{}{}, service)
+		Expect(fNPW.SyncServices(services)).NotTo(HaveOccurred(), "must sync services")
+	})
 })
 
 func TestConstructUDNVRFIPRules(t *testing.T) {

go-controller/pkg/testing/networkmanager/fake.go

@@ -46,6 +46,7 @@ func (fcm *FakeControllerManager) Reconcile(_ string, _, _ util.NetInfo) error {
 
 type FakeNetworkManager struct {
 	// namespace -> netInfo
+	// if netInfo is nil, it represents a namespace which contains the required UDN label but with no valid network. It will return invalid network error.
 	PrimaryNetworks map[string]util.NetInfo
 }
 
@@ -54,11 +55,15 @@ func (fnm *FakeNetworkManager) Start() error { return nil }
 func (fnm *FakeNetworkManager) Stop() {}
 
 func (fnm *FakeNetworkManager) GetActiveNetworkForNamespace(namespace string) (util.NetInfo, error) {
-	return fnm.GetActiveNetworkForNamespaceFast(namespace), nil
+	network := fnm.GetActiveNetworkForNamespaceFast(namespace)
+	if network == nil {
+		return nil, util.NewInvalidPrimaryNetworkError(namespace)
+	}
+	return network, nil
 }
 
 func (fnm *FakeNetworkManager) GetActiveNetworkForNamespaceFast(namespace string) util.NetInfo {
-	if primaryNetworks, ok := fnm.PrimaryNetworks[namespace]; ok && primaryNetworks != nil {
+	if primaryNetworks, ok := fnm.PrimaryNetworks[namespace]; ok {
 		return primaryNetworks
 	}
 	return &util.DefaultNetInfo{}