fix: caching NAD CRD should before all kubeovn crds and pod (#6198)

* fix: caching NAD CRD should before all kubeovn crds and pod

---------

Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com>

Author: zbb88888

pkg/controller/controller.go

@@ -695,14 +695,17 @@ func Run(ctx context.Context, config *Configuration) {
 	defer controller.shutdown()
 	klog.Info("Starting OVN controller")
 
+	// Start and sync NAD informer first, as many resources depend on NAD cache
+	// NAD CRD is optional, so we check if it exists before starting the informer
+	controller.StartNetAttachInformerFactory(ctx)
+
 	// Wait for the caches to be synced before starting workers
 	controller.informerFactory.Start(ctx.Done())
 	controller.cmInformerFactory.Start(ctx.Done())
 	controller.deployInformerFactory.Start(ctx.Done())
 	controller.kubeovnInformerFactory.Start(ctx.Done())
 	controller.anpInformerFactory.Start(ctx.Done())
 	controller.StartKubevirtInformerFactory(ctx, kubevirtInformerFactory)
-	controller.StartNetAttachInformerFactory(ctx)
 
 	klog.Info("Waiting for informer caches to sync")
 	cacheSyncs := []cache.InformerSynced{

pkg/controller/network_attachment.go

@@ -22,29 +22,54 @@ func (c *Controller) isNetAttachCRDInstalled() (bool, error) {
 	)
 }
 
-// the network-attachment-definition CRD is not required to be installed so
-// periodically check and see if we should start the informer.
+// startNetAttachInformer starts the NAD informer and waits for cache sync.
+func (c *Controller) startNetAttachInformer(ctx context.Context) {
+	c.netAttachInformerFactory.Start(ctx.Done())
+	if !cache.WaitForCacheSync(ctx.Done(), c.netAttachSynced) {
+		util.LogFatalAndExit(nil, "failed to wait for network attachment cache to sync")
+	}
+	klog.Info("Network attachment informer cache synced")
+}
+
+// tryStartNetAttachInformer checks if NAD CRD is installed and starts the informer if so.
+// Returns true if informer was started, false otherwise.
+func (c *Controller) tryStartNetAttachInformer(ctx context.Context) bool {
+	exists, err := c.isNetAttachCRDInstalled()
+	if err != nil {
+		klog.Warningf("failed to check if network attachment CRD exists: %v", err)
+		return false
+	}
+	if !exists {
+		return false
+	}
+	klog.Info("Network attachment CRD found, starting informer")
+	c.startNetAttachInformer(ctx)
+	return true
+}
+
+// StartNetAttachInformerFactory starts the network attachment definition (NAD) informer.
+// This MUST be called before other informers that depend on NAD cache (Pod, Subnet, VpcNatGateway, etc.)
+//
+// The NAD CRD is optional - if installed, we start the informer synchronously and wait for
+// cache sync before returning. This ensures NAD cache is ready when other controllers start
+// processing resources that reference NADs.
+//
+// If the CRD is not installed at startup, we start a background goroutine to periodically
+// check and start the informer when the CRD becomes available.
 func (c *Controller) StartNetAttachInformerFactory(ctx context.Context) {
+	if c.tryStartNetAttachInformer(ctx) {
+		return
+	}
+
+	// CRD not found at startup, start background check loop
+	klog.Info("Network attachment CRD not found at startup, will check periodically in background")
 	ticker := time.NewTicker(10 * time.Second)
 	go func() {
 		defer ticker.Stop()
 		for {
 			select {
 			case <-ticker.C:
-				exists, err := c.isNetAttachCRDInstalled()
-				if err != nil {
-					klog.Errorf("checking network attachment CRD exists: %v", err)
-					continue
-				}
-
-				if exists {
-					klog.Info("Start attachment informer")
-
-					c.netAttachInformerFactory.Start(ctx.Done())
-					if !cache.WaitForCacheSync(ctx.Done(), c.netAttachSynced) {
-						util.LogFatalAndExit(nil, "failed to wait for network attachment cache to sync")
-					}
-
+				if c.tryStartNetAttachInformer(ctx) {
 					return
 				}
 			case <-ctx.Done():

pkg/controller/vpc_nat_gateway.go

@@ -323,7 +323,11 @@ func (c *Controller) handleInitVpcNatGw(key string) error {
 	var interfaces []string
 	if c.config.EnableNonPrimaryCNI {
 		// extract external nad interface name
-		externalNadNs, externalNadName := c.getExternalSubnetNad(gw)
+		externalNadNs, externalNadName, nadErr := c.getExternalSubnetNad(gw)
+		if nadErr != nil {
+			klog.Errorf("failed to get external subnet NAD for gateway %s: %v", gw.Name, nadErr)
+			return nadErr
+		}
 		networkStatusAnnotations := pod.Annotations[nadv1.NetworkStatusAnnot]
 		externalNadFullName := fmt.Sprintf("%s/%s", externalNadNs, externalNadName)
 		externalNadIfName, err := util.GetNadInterfaceFromNetworkStatusAnnotation(networkStatusAnnotations, externalNadFullName)
@@ -848,7 +852,11 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1
 		annotations = maps.Clone(oldSts.Annotations)
 	}
 
-	externalNadNamespace, externalNadName := c.getExternalSubnetNad(gw)
+	externalNadNamespace, externalNadName, err := c.getExternalSubnetNad(gw)
+	if err != nil {
+		klog.Errorf("failed to get gw external subnet nad: %v", err)
+		return nil, err
+	}
 
 	eth0SubnetProvider, err := c.GetSubnetProvider(gw.Spec.Subnet)
 	if err != nil {
@@ -1063,17 +1071,28 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1
 
 // getExternalSubnetNad returns the namespace and name of the NetworkAttachmentDefinition associated with
 // an external network attached to a NAT gateway
-func (c *Controller) getExternalSubnetNad(gw *kubeovnv1.VpcNatGateway) (string, string) {
+func (c *Controller) getExternalSubnetNad(gw *kubeovnv1.VpcNatGateway) (string, string, error) {
 	externalNadNamespace := c.config.PodNamespace
-	externalNadName := util.GetNatGwExternalNetwork(gw.Spec.ExternalSubnets)
-	if externalSubnet, err := c.subnetsLister.Get(externalNadName); err == nil {
-		if name, namespace, ok := util.GetNadBySubnetProvider(externalSubnet.Spec.Provider); ok {
-			externalNadName = name
-			externalNadNamespace = namespace
-		}
+	// GetNatGwExternalNetwork returns the subnet name from ExternalSubnets, or "ovn-vpc-external-network" if empty
+	externalSubnetName := util.GetNatGwExternalNetwork(gw.Spec.ExternalSubnets)
+
+	externalSubnet, err := c.subnetsLister.Get(externalSubnetName)
+	if err != nil {
+		err = fmt.Errorf("failed to get external subnet %s for NAT gateway %s: %w", externalSubnetName, gw.Name, err)
+		klog.Error(err)
+		return "", "", err
+	}
+
+	// Try to parse NAD info from subnet's provider
+	if name, namespace, ok := util.GetNadBySubnetProvider(externalSubnet.Spec.Provider); ok {
+		return namespace, name, nil
 	}
 
-	return externalNadNamespace, externalNadName
+	// Provider cannot be parsed to NAD info (e.g., provider is "ovn" or empty)
+	// Fall back to default NAD name which is the same as subnet name for external subnets
+	klog.Warningf("subnet %s provider %q cannot be parsed to NAD info, using default NAD %s/%s",
+		externalSubnetName, externalSubnet.Spec.Provider, externalNadNamespace, externalSubnetName)
+	return externalNadNamespace, externalSubnetName, nil
 }
 
 func (c *Controller) cleanUpVpcNatGw() error {

pkg/controller/vpc_nat_gateway_test.go

@@ -123,3 +123,133 @@ func TestGetSubnetProvider(t *testing.T) {
 		assert.Error(t, err, "Should error for missing subnet")
 	})
 }
+
+func TestGetExternalSubnetNad(t *testing.T) {
+	tests := []struct {
+		name              string
+		gw                *kubeovnv1.VpcNatGateway
+		subnets           []*kubeovnv1.Subnet
+		podNamespace      string
+		expectedNamespace string
+		expectedName      string
+		expectError       bool
+	}{
+		{
+			name: "provider with 3 parts (name.namespace.ovn)",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{"external-subnet"}},
+			},
+			subnets: []*kubeovnv1.Subnet{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "external-subnet"},
+					Spec:       kubeovnv1.SubnetSpec{Provider: "real-eip.kube-system.ovn"},
+				},
+			},
+			podNamespace:      "kube-system",
+			expectedNamespace: "kube-system",
+			expectedName:      "real-eip",
+			expectError:       false,
+		},
+		{
+			name: "provider with 2 parts (name.namespace)",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{"external-subnet"}},
+			},
+			subnets: []*kubeovnv1.Subnet{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "external-subnet"},
+					Spec:       kubeovnv1.SubnetSpec{Provider: "my-nad.default"},
+				},
+			},
+			podNamespace:      "kube-system",
+			expectedNamespace: "default",
+			expectedName:      "my-nad",
+			expectError:       false,
+		},
+		{
+			name: "provider is ovn (fallback to subnet name)",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{"ovn-vpc-external-network"}},
+			},
+			subnets: []*kubeovnv1.Subnet{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "ovn-vpc-external-network"},
+					Spec:       kubeovnv1.SubnetSpec{Provider: util.OvnProvider},
+				},
+			},
+			podNamespace:      "kube-system",
+			expectedNamespace: "kube-system",
+			expectedName:      "ovn-vpc-external-network",
+			expectError:       false,
+		},
+		{
+			name: "empty provider (fallback to subnet name)",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{"my-external-subnet"}},
+			},
+			subnets: []*kubeovnv1.Subnet{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "my-external-subnet"},
+					Spec:       kubeovnv1.SubnetSpec{Provider: ""},
+				},
+			},
+			podNamespace:      "kube-system",
+			expectedNamespace: "kube-system",
+			expectedName:      "my-external-subnet",
+			expectError:       false,
+		},
+		{
+			name: "empty ExternalSubnets (use default)",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{}},
+			},
+			subnets: []*kubeovnv1.Subnet{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "ovn-vpc-external-network"},
+					Spec:       kubeovnv1.SubnetSpec{Provider: "external.default.ovn"},
+				},
+			},
+			podNamespace:      "kube-system",
+			expectedNamespace: "default",
+			expectedName:      "external",
+			expectError:       false,
+		},
+		{
+			name: "subnet not found",
+			gw: &kubeovnv1.VpcNatGateway{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-gw"},
+				Spec:       kubeovnv1.VpcNatGatewaySpec{ExternalSubnets: []string{"non-existent-subnet"}},
+			},
+			subnets:      []*kubeovnv1.Subnet{},
+			podNamespace: "kube-system",
+			expectError:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeController, err := newFakeControllerWithOptions(t, &FakeControllerOptions{
+				Subnets: tt.subnets,
+			})
+			require.NoError(t, err)
+			controller := fakeController.fakeController
+			controller.config.PodNamespace = tt.podNamespace
+
+			namespace, name, err := controller.getExternalSubnetNad(tt.gw)
+
+			if tt.expectError {
+				assert.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedNamespace, namespace, "namespace mismatch")
+			assert.Equal(t, tt.expectedName, name, "name mismatch")
+		})
+	}
+}