feat: compare node IP with NNCs node IP (#1527)

* feat: compare node IP with NNCs node IP

* fix: don't block reconciler from starting if nnc doesn't have status

Author: thatmattlong

cns/configuration/env.go

@@ -9,6 +9,8 @@ import (
 const (
 	// EnvNodeName is the NODENAME env var string key.
 	EnvNodeName = "NODENAME"
+	// EnvNodeIP is the IP of the node running this CNS binary
+	EnvNodeIP = "NODE_IP"
 )
 
 // ErrNodeNameUnset indicates the the $EnvNodeName variable is unset in the environment.
@@ -22,3 +24,8 @@ func NodeName() (string, error) {
 	}
 	return nodeName, nil
 }
+
+// NodeIP returns the value of the NODE_IP environment variable, or empty string if unset.
+func NodeIP() string {
+	return os.Getenv(EnvNodeIP)
+}

cns/kubecontroller/nodenetworkconfig/conversion_test.go

@@ -22,6 +22,7 @@ const (
 	subnetPrefixLen    = 24
 	testSecIP          = "10.0.0.2"
 	version            = 1
+	nodeIP             = "10.1.0.5"
 )
 
 var invalidStatusMultiNC = v1alpha.NodeNetworkConfigStatus{
@@ -46,6 +47,7 @@ var validSwiftNC = v1alpha.NetworkContainer{
 	DefaultGateway:     defaultGateway,
 	SubnetAddressSpace: subnetAddressSpace,
 	Version:            version,
+	NodeIP:             nodeIP,
 }
 
 var validSwiftStatus = v1alpha.NodeNetworkConfigStatus{
@@ -55,7 +57,8 @@ var validSwiftStatus = v1alpha.NodeNetworkConfigStatus{
 }
 
 var validSwiftRequest = &cns.CreateNetworkContainerRequest{
-	Version: strconv.FormatInt(version, 10),
+	HostPrimaryIP: nodeIP,
+	Version:       strconv.FormatInt(version, 10),
 	IPConfiguration: cns.IPConfiguration{
 		GatewayIPAddress: defaultGateway,
 		IPSubnet: cns.IPSubnet{
@@ -78,6 +81,7 @@ var validOverlayNC = v1alpha.NetworkContainer{
 	AssignmentMode:     v1alpha.Static,
 	Type:               v1alpha.Overlay,
 	PrimaryIP:          overlayPrimaryIP,
+	NodeIP:             nodeIP,
 	SubnetName:         subnetName,
 	SubnetAddressSpace: subnetAddressSpace,
 	Version:            version,
@@ -162,6 +166,7 @@ func TestCreateNCRequestFromDynamicNC(t *testing.T) {
 			input: v1alpha.NetworkContainer{
 				PrimaryIP: ipIsCIDR,
 				ID:        ncID,
+				NodeIP:    nodeIP,
 				IPAssignments: []v1alpha.IPAssignment{
 					{
 						Name: uuid,

cns/kubecontroller/nodenetworkconfig/reconciler.go

@@ -40,18 +40,20 @@ type Reconciler struct {
 	nnccli             nncGetter
 	once               sync.Once
 	started            chan interface{}
+	nodeIP             string
 }
 
 // NewReconciler creates a NodeNetworkConfig Reconciler which will get updates from the Kubernetes
 // apiserver for NNC events.
 // Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The
 // passed Listeners are notified in the order provided.
-func NewReconciler(cnscli cnsClient, nnccli nncGetter, ipampoolmonitorcli nodeNetworkConfigListener) *Reconciler {
+func NewReconciler(cnscli cnsClient, nnccli nncGetter, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler {
 	return &Reconciler{
 		cnscli:             cnscli,
 		ipampoolmonitorcli: ipampoolmonitorcli,
 		nnccli:             nnccli,
 		started:            make(chan interface{}),
+		nodeIP:             nodeIP,
 	}
 }
 
@@ -71,8 +73,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	logger.Printf("[cns-rc] CRD Spec: %+v", nnc.Spec)
 
 	ipAssignments := 0
+
 	// for each NC, parse it in to a CreateNCRequest and forward it to the appropriate Listener
 	for i := range nnc.Status.NetworkContainers {
+		// check if this NC matches the Node IP if we have one to check against
+		if r.nodeIP != "" {
+			if r.nodeIP != nnc.Status.NetworkContainers[i].NodeIP {
+				// skip this NC since it was created for a different node
+				logger.Debugf("[cns-rc] skipping network container %s found in NNC because node IP doesn't match, got %s, expected %s",
+					nnc.Status.NetworkContainers[i].ID, nnc.Status.NetworkContainers[i].NodeIP, r.nodeIP)
+				continue
+			}
+		}
+
 		var req *cns.CreateNetworkContainerRequest
 		var err error
 		switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case
@@ -98,6 +111,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		}
 		ipAssignments += len(req.SecondaryIPConfigs)
 	}
+
 	// record assigned IPs metric
 	allocatedIPs.Set(float64(ipAssignments))
 

cns/kubecontroller/nodenetworkconfig/reconciler_test.go

@@ -53,6 +53,7 @@ func TestReconcile(t *testing.T) {
 		in                 reconcile.Request
 		ncGetter           mockNCGetter
 		cnsClient          mockCNSClient
+		nodeIP             string
 		want               reconcile.Result
 		wantCNSClientState cnsClientState
 		wantErr            bool
@@ -145,11 +146,35 @@ func TestReconcile(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "node IP mismatch",
+			ncGetter: mockNCGetter{
+				get: func(context.Context, types.NamespacedName) (*v1alpha.NodeNetworkConfig, error) {
+					return &v1alpha.NodeNetworkConfig{
+						Status: validSwiftStatus,
+						Spec: v1alpha.NodeNetworkConfigSpec{
+							RequestedIPCount: 1,
+						},
+					}, nil
+				},
+			},
+			cnsClient: mockCNSClient{
+				createOrUpdateNC: func(*cns.CreateNetworkContainerRequest) cnstypes.ResponseCode {
+					return cnstypes.Success
+				},
+				update: func(*v1alpha.NodeNetworkConfig) error {
+					return nil
+				},
+			},
+			nodeIP:             "192.168.1.5", // nodeIP in above NNC status is 10.1.0.5
+			wantErr:            false,
+			wantCNSClientState: cnsClientState{}, // state should be empty since we should skip this NC
+		},
 	}
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			r := NewReconciler(&tt.cnsClient, &tt.ncGetter, &tt.cnsClient)
+			r := NewReconciler(&tt.cnsClient, &tt.ncGetter, &tt.cnsClient, tt.nodeIP)
 			got, err := r.Reconcile(context.Background(), tt.in)
 			if tt.wantErr {
 				require.Error(t, err)

cns/service/main.go

@@ -1080,8 +1080,11 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 		return errors.Wrapf(err, "failed to get node %s", nodeName)
 	}
 
+	// get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node
+	nodeIP := configuration.NodeIP()
+
 	// NodeNetworkConfig reconciler
-	nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, nnccli, poolMonitor)
+	nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, nnccli, poolMonitor, nodeIP)
 	// pass Node to the Reconciler for Controller xref
 	if err := nncReconciler.SetupWithManager(manager, node); err != nil { //nolint:govet // intentional shadow
 		return errors.Wrapf(err, "failed to setup nnc reconciler with manager")
@@ -1141,11 +1144,9 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn
 		}
 	}()
 	logger.Printf("initialized NodeNetworkConfig reconciler")
-	// wait for up to 10m for the Reconciler to run once.
-	timedCtx, cancel := context.WithTimeout(ctx, 10*time.Minute) //nolint:gomnd // default 10m
-	defer cancel()
-	if started := nncReconciler.Started(timedCtx); !started {
-		return errors.Errorf("timed out waiting for reconciler start")
+	// wait for the Reconciler to run once on a NNC that was made for this Node
+	if started := nncReconciler.Started(ctx); !started {
+		return errors.Errorf("context cancelled while waiting for reconciler start")
 	}
 	logger.Printf("started NodeNetworkConfig reconciler")