minor fixes

Author: NihaNallappagari

cns/fakes/imdsclientfake.go

@@ -94,4 +94,4 @@ func (m *MockIMDSClient) GetIMDSVersions(ctx context.Context) (*imds.APIVersions
             "2025-07-24",
         },
     }, nil
-}
+}

cns/imds/client.go

@@ -114,8 +114,7 @@ func (c *Client) GetNetworkInterfaces(ctx context.Context) ([]NetworkInterface,
 			return errors.Wrap(err, "error getting IMDS network metadata")
 		}
 
-		// Try to parse the network metadata as the expected structure
-		// Convert the map to JSON and back to properly unmarshal into struct
+		// Parse the network metadata to the expected structure
 		jsonData, err := json.Marshal(networkInterfaces)
 		if err != nil {
 			return errors.Wrap(err, "error marshaling network metadata")
@@ -133,7 +132,8 @@ func (c *Client) GetNetworkInterfaces(ctx context.Context) ([]NetworkInterface,
 	return networkData.Interface, nil
 }
 
-func (c *Client) getInstanceMetadata(ctx context.Context, imdsMetadataPath string, imdsAPIVersion string) (map[string]any, error) {
+
+func (c *Client) getInstanceMetadata(ctx context.Context, imdsMetadataPath, imdsAPIVersion string) (map[string]any, error) {
 	imdsRequestURL, err := url.JoinPath(c.config.endpoint, imdsMetadataPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to build path to IMDS metadata for path"+imdsMetadataPath)

cns/imds/client_test.go

@@ -21,7 +21,6 @@ func TestGetVMUniqueID(t *testing.T) {
 	require.NoError(t, err, "error reading testdata compute metadata file")
 
 	mockIMDSServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// request header "Metadata: true" must be present
 		metadataHeader := r.Header.Get("Metadata")
 		assert.Equal(t, "true", metadataHeader)
 
@@ -271,7 +270,7 @@ func TestGetIMDSVersionsInternalServerError(t *testing.T) {
     versionsResp, err := imdsClient.GetIMDSVersions(context.Background())
 
     require.Error(t, err, "expected error for 500")
-    assert.Nil(t, versionsResp, "response should be nil on error")
+    assert.Nil(t, versionsResp, "response should be nil or error")
 }
 
 func TestGetIMDSVersionsMissingAPIVersionsField(t *testing.T) {
@@ -295,4 +294,4 @@ func TestGetIMDSVersionsInvalidEndpoint(t *testing.T) {
     imdsClient := imds.NewClient(imds.Endpoint(string([]byte{0x7f})), imds.RetryAttempts(1))
     _, err := imdsClient.GetIMDSVersions(context.Background())
     require.Error(t, err, "expected error for invalid endpoint")
-}       
+}

cns/restserver/internalapi.go

@@ -195,113 +195,113 @@ var errNonExistentContainerStatus = errors.New("nonExistantContainerstatus")
 // all NCs and update the CNS state accordingly. This function returns the the total number of NCs on this VM that have been programmed to
 // some version, NOT the number of NCs that are up-to-date.
 func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMode string) (int, error) {
-    outdatedNCs := map[string]struct{}{}
-    programmedNCs := map[string]struct{}{}
-    for idx := range service.state.ContainerStatus {
-        // Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain.
-        localNCVersion, err := strconv.Atoi(service.state.ContainerStatus[idx].HostVersion)
-        if err != nil {
-            logger.Errorf("Received err when change containerstatus.HostVersion %s to int, err msg %v", service.state.ContainerStatus[idx].HostVersion, err)
-            continue
-        }
-        dncNCVersion, err := strconv.Atoi(service.state.ContainerStatus[idx].CreateNetworkContainerRequest.Version)
-        if err != nil {
-            logger.Errorf("Received err when change nc version %s in containerstatus to int, err msg %v", service.state.ContainerStatus[idx].CreateNetworkContainerRequest.Version, err)
-            continue
-        }
-        // host NC version is the NC version from NMAgent, if it's smaller than NC version from DNC, then append it to indicate it needs update.
-        if localNCVersion < dncNCVersion {
-            outdatedNCs[service.state.ContainerStatus[idx].ID] = struct{}{}
-        } else if localNCVersion > dncNCVersion {
-            logger.Errorf("NC version from NMAgent is larger than DNC, NC version from NMAgent is %d, NC version from DNC is %d", localNCVersion, dncNCVersion)
-        }
+	outdatedNCs := map[string]struct{}{}
+	programmedNCs := map[string]struct{}{}
+	for idx := range service.state.ContainerStatus {
+		// Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain.
+		localNCVersion, err := strconv.Atoi(service.state.ContainerStatus[idx].HostVersion)
+		if err != nil {
+			logger.Errorf("Received err when change containerstatus.HostVersion %s to int, err msg %v", service.state.ContainerStatus[idx].HostVersion, err)
+			continue
+		}
+		dncNCVersion, err := strconv.Atoi(service.state.ContainerStatus[idx].CreateNetworkContainerRequest.Version)
+		if err != nil {
+			logger.Errorf("Received err when change nc version %s in containerstatus to int, err msg %v", service.state.ContainerStatus[idx].CreateNetworkContainerRequest.Version, err)
+			continue
+		}
+		// host NC version is the NC version from NMAgent, if it's smaller than NC version from DNC, then append it to indicate it needs update.
+		if localNCVersion < dncNCVersion {
+			outdatedNCs[service.state.ContainerStatus[idx].ID] = struct{}{}
+		} else if localNCVersion > dncNCVersion {
+			logger.Errorf("NC version from NMAgent is larger than DNC, NC version from NMAgent is %d, NC version from DNC is %d", localNCVersion, dncNCVersion)
+		}
 
-        if localNCVersion > -1 {
-            programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{}
-        }
-    }
-    if len(outdatedNCs) == 0 {
-        return len(programmedNCs), nil
-    }
-    ncVersionListResp, err := service.nma.GetNCVersionList(ctx)
-    if err != nil {
-        return len(programmedNCs), errors.Wrap(err, "failed to get nc version list from nmagent")
-    }
+		if localNCVersion > -1 {
+			programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{}
+		}
+	}
+	if len(outdatedNCs) == 0 {
+		return len(programmedNCs), nil
+	}
+	ncVersionListResp, err := service.nma.GetNCVersionList(ctx)
+	if err != nil {
+		return len(programmedNCs), errors.Wrap(err, "failed to get nc version list from nmagent")
+	}
 
-    // Get IMDS NC details for delegated NIC
-    imdsNCs, err := service.GetIMDSNCs(ctx)
-    if err != nil {
-        // If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map
-        imdsNCs = make(map[string]string)
-    }
+	// Get IMDS NC versions for delegated NIC scenarios
+	imdsNCVersions, err := service.GetIMDSNCVersions(ctx)
+	if err != nil {
+		// If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map
+		imdsNCVersions = make(map[string]string)
+	}
 
-    nmaNCs := map[string]string{}
-    for _, nc := range ncVersionListResp.Containers {
-        nmaNCs[strings.ToLower(nc.NetworkContainerID)] = nc.Version
-    }
+	nmaNCs := map[string]string{}
+	for _, nc := range ncVersionListResp.Containers {
+		nmaNCs[strings.ToLower(nc.NetworkContainerID)] = nc.Version
+	}
 
-    // Consolidate both nc's from NMA and IMDS calls
-    nmaProgrammedNCs := make(map[string]string)
-    for ncID, version := range nmaNCs {
-        nmaProgrammedNCs[ncID] = version
-    }
-    for ncID, version := range imdsNCs {
-        if _, exists := nmaProgrammedNCs[ncID]; !exists {
-            nmaProgrammedNCs[strings.ToLower(ncID)] = version
-        }
-    }
-    hasNC.Set(float64(len(nmaProgrammedNCs)))
-    for ncID := range outdatedNCs {
-        nmaProgrammedNCVersionStr, ok := nmaProgrammedNCs[ncID]
-        if !ok {
-            // Neither NMA nor IMDS has this NC that we need programmed yet, bail out
-            continue
-        }
-        nmaProgrammedNCVersion, err := strconv.Atoi(nmaProgrammedNCVersionStr)
-        if err != nil {
-            logger.Errorf("failed to parse container version of %s: %s", ncID, err)
-            continue
-        }
-        // Check whether it exist in service state and get the related nc info
-        ncInfo, exist := service.state.ContainerStatus[ncID]
-        if !exist {
-            // if we marked this NC as needs update, but it no longer exists in internal state when we reach
-            // this point, our internal state has changed unexpectedly and we should bail out and try again.
-            return len(programmedNCs), errors.Wrapf(errNonExistentContainerStatus, "can't find NC with ID %s in service state, stop updating this host NC version", ncID)
-        }
-        // if the NC still exists in state and is programmed to some version (doesn't have to be latest), add it to our set of NCs that have been programmed
-        if nmaProgrammedNCVersion > -1 {
-            programmedNCs[ncID] = struct{}{}
-        }
+	// Consolidate both nc's from NMA and IMDS calls
+	nmaProgrammedNCs := make(map[string]string)
+	for ncID, version := range nmaNCs {
+		nmaProgrammedNCs[ncID] = version
+	}
+	for ncID, version := range imdsNCVersions {
+		if _, exists := nmaProgrammedNCs[ncID]; !exists {
+			nmaProgrammedNCs[strings.ToLower(ncID)] = version
+		}
+	}
+	hasNC.Set(float64(len(nmaProgrammedNCs)))
+	for ncID := range outdatedNCs {
+		nmaProgrammedNCVersionStr, ok := nmaProgrammedNCs[ncID]
+		if !ok {
+			// Neither NMA nor IMDS has this NC that we need programmed yet, bail out
+			continue
+		}
+		nmaProgrammedNCVersion, err := strconv.Atoi(nmaProgrammedNCVersionStr)
+		if err != nil {
+			logger.Errorf("failed to parse container version of %s: %s", ncID, err)
+			continue
+		}
+		// Check whether it exist in service state and get the related nc info
+		ncInfo, exist := service.state.ContainerStatus[ncID]
+		if !exist {
+			// if we marked this NC as needs update, but it no longer exists in internal state when we reach
+			// this point, our internal state has changed unexpectedly and we should bail out and try again.
+			return len(programmedNCs), errors.Wrapf(errNonExistentContainerStatus, "can't find NC with ID %s in service state, stop updating this host NC version", ncID)
+		}
+		// if the NC still exists in state and is programmed to some version (doesn't have to be latest), add it to our set of NCs that have been programmed
+		if nmaProgrammedNCVersion > -1 {
+			programmedNCs[ncID] = struct{}{}
+		}
 
-        localNCVersion, err := strconv.Atoi(ncInfo.HostVersion)
-        if err != nil {
-            logger.Errorf("failed to parse host nc version string %s: %s", ncInfo.HostVersion, err)
-            continue
-        }
-        if localNCVersion > nmaProgrammedNCVersion {
-            //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
-            logger.Errorf("NC version from consolidated sources is decreasing: have %d, got %d", localNCVersion, nmaProgrammedNCVersion)
-            continue
-        }
-        if channelMode == cns.CRD {
-            service.MarkIpsAsAvailableUntransacted(ncInfo.ID, nmaProgrammedNCVersion)
-        }
-        //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
-        logger.Printf("Updating NC %s host version from %s to %s", ncID, ncInfo.HostVersion, nmaProgrammedNCVersionStr)
-        ncInfo.HostVersion = nmaProgrammedNCVersionStr
-        logger.Printf("Updated NC %s host version to %s", ncID, ncInfo.HostVersion)
-        service.state.ContainerStatus[ncID] = ncInfo
-        // if we successfully updated the NC, pop it from the needs update set.
-        delete(outdatedNCs, ncID)
-    }
-    // if we didn't empty out the needs update set, NMA has not programmed all the NCs we are expecting, and we
-    // need to return an error indicating that
-    if len(outdatedNCs) > 0 {
-        return len(programmedNCs), errors.Errorf("unable to update some NCs: %v, missing or bad response from NMA or IMDS", outdatedNCs)
-    }
+		localNCVersion, err := strconv.Atoi(ncInfo.HostVersion)
+		if err != nil {
+			logger.Errorf("failed to parse host nc version string %s: %s", ncInfo.HostVersion, err)
+			continue
+		}
+		if localNCVersion > nmaProgrammedNCVersion {
+			//nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
+			logger.Errorf("NC version from consolidated sources is decreasing: have %d, got %d", localNCVersion, nmaProgrammedNCVersion)
+			continue
+		}
+		if channelMode == cns.CRD {
+			service.MarkIpsAsAvailableUntransacted(ncInfo.ID, nmaProgrammedNCVersion)
+		}
+		//nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
+		logger.Printf("Updating NC %s host version from %s to %s", ncID, ncInfo.HostVersion, nmaProgrammedNCVersionStr)
+		ncInfo.HostVersion = nmaProgrammedNCVersionStr
+		logger.Printf("Updated NC %s host version to %s", ncID, ncInfo.HostVersion)
+		service.state.ContainerStatus[ncID] = ncInfo
+		// if we successfully updated the NC, pop it from the needs update set.
+		delete(outdatedNCs, ncID)
+	}
+	// if we didn't empty out the needs update set, NMA has not programmed all the NCs we are expecting, and we
+	// need to return an error indicating that
+	if len(outdatedNCs) > 0 {
+		return len(programmedNCs), errors.Errorf("unable to update some NCs: %v, missing or bad response from NMA or IMDS", outdatedNCs)
+	}
 
-    return len(programmedNCs), nil
+	return len(programmedNCs), nil
 }
 
 func (service *HTTPRestService) ReconcileIPAssignment(podInfoByIP map[string]cns.PodInfo, ncReqs []*cns.CreateNetworkContainerRequest) types.ResponseCode {

cns/restserver/internalapi_test.go

@@ -230,7 +230,6 @@ func TestSyncHostNCVersion(t *testing.T) {
     // cns.KubernetesCRD has one more logic compared to other orchestrator type, so test both of them
     orchestratorTypes := []string{cns.Kubernetes, cns.KubernetesCRD}
     for _, orchestratorType := range orchestratorTypes {
-        orchestratorType := orchestratorType
         t.Run(orchestratorType, func(t *testing.T) {
             req := createNCReqeustForSyncHostNCVersion(t)
             containerStatus := svc.state.ContainerStatus[req.NetworkContainerid]
@@ -531,7 +530,6 @@ func TestSyncHostNCVersionNMAgentAPICallFailed(t *testing.T) {
 }
 
 func TestSyncHostNCVersionSwiftV2APINotSupported(t *testing.T) {
-	// Test scenario where NMAgent doesn't support SwiftV2 API
 	req := createNCReqeustForSyncHostNCVersion(t)
 
 	svc.Lock()
@@ -581,7 +579,6 @@ func TestSyncHostNCVersionSwiftV2APINotSupported(t *testing.T) {
 func TestSyncHostNCVersionIMDSAPIVersionNotSupported(t *testing.T) {
     orchestratorTypes := []string{cns.Kubernetes, cns.KubernetesCRD}
     for _, orchestratorType := range orchestratorTypes {
-        orchestratorType := orchestratorType
         t.Run(orchestratorType, func(t *testing.T) {
             req := createNCReqeustForSyncHostNCVersion(t)