resolved pr comments

Author: rejain789

npm/pkg/dataplane/dataplane_windows.go

@@ -366,7 +366,7 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error)
 	}
 
 	// Filtering out any of the same endpoints between endpoints with state attached and attachedSharing
-	endpoints = removeCommonEndpoints(&endpoints, &endpointsAttached)
+	endpoints = GetUniqueEndpoints(endpoints, endpointsAttached)
 
 	epPointers := make([]*hcn.HostComputeEndpoint, 0, len(endpoints))
 	for k := range endpoints {
@@ -375,36 +375,22 @@ func (dp *DataPlane) getLocalPodEndpoints() ([]*hcn.HostComputeEndpoint, error)
 	return epPointers, nil
 }
 
-func removeCommonEndpoints(endpoints, endpointsAttached *[]hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint {
-	// Choose smaller and larger lists based on length for efficiency
-	smallerEndpointsList, largerEndpointsList := endpoints, endpointsAttached
-	if len(*endpoints) > len(*endpointsAttached) {
-		smallerEndpointsList, largerEndpointsList = endpointsAttached, endpoints
-	}
-
-	// Store IDs of smaller list in a map for quick lookup
-	idMap := make(map[string]struct{}, len(*smallerEndpointsList))
-	for i := 0; i < len(*smallerEndpointsList); i++ {
-		ep := &(*smallerEndpointsList)[i]
+func GetUniqueEndpoints(endpoints, endpointsAttached []hcn.HostComputeEndpoint) []hcn.HostComputeEndpoint {
+	// Store IDs of endpoints list in a map for quick lookup
+	idMap := make(map[string]struct{}, len(endpoints))
+	for i := 0; i < len(endpoints); i++ {
+		ep := &(endpoints)[i]
 		idMap[ep.Id] = struct{}{}
 	}
 
-	// Append endpoints from larger list and remove common IDs from map
-	result := []hcn.HostComputeEndpoint{}
-	for i := 0; i < len(*largerEndpointsList); i++ {
-		ep := (*largerEndpointsList)[i]
-		result = append(result, ep)
-		delete(idMap, ep.Id)
-	}
-
-	// Append remaining unique endpoints from smaller list to result
-	for i := 0; i < len(*smallerEndpointsList); i++ {
-		ep := (*smallerEndpointsList)[i]
-		if _, found := idMap[ep.Id]; found {
-			result = append(result, ep)
+	// Add endpointsAttached list endpoints in endpoints list if the endpoint is not in the map
+	for i := 0; i < len(endpointsAttached); i++ {
+		ep := endpointsAttached[i]
+		if _, ok := idMap[ep.Id]; !ok {
+			endpoints = append(endpoints, ep)
 		}
 	}
-	return result
+	return endpoints
 }
 
 // refreshPodEndpoints will refresh all the pod endpoints and create empty netpol references for new endpoints

npm/pkg/dataplane/dataplane_windows_test.go

@@ -2,7 +2,6 @@ package dataplane
 
 import (
 	"fmt"
-	"reflect"
 	"sync"
 	"testing"
 	"time"
@@ -12,6 +11,7 @@ import (
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets"
 	dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils"
 	"github.com/Microsoft/hcsshim/hcn"
+	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 )
@@ -99,19 +99,19 @@ func TestRemoveCommonEndpoints(t *testing.T) {
 			name:              "1 value same",
 			endpoints:         []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}},
 			endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}},
-			expected:          []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}},
+			expected:          []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}},
 		},
 		{
 			name:              "no values same",
 			endpoints:         []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}},
 			endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}},
-			expected:          []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}},
+			expected:          []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}},
 		},
 		{
 			name:              "1 value same",
 			endpoints:         []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}},
 			endpointsAttached: []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}},
-			expected:          []hcn.HostComputeEndpoint{{Id: "567890"}, {Id: "123456"}, {Id: "789012"}, {Id: "456901"}, {Id: "560971"}},
+			expected:          []hcn.HostComputeEndpoint{{Id: "456901"}, {Id: "123456"}, {Id: "560971"}, {Id: "567890"}, {Id: "789012"}},
 		},
 		{
 			name:              "two values same",
@@ -142,14 +142,10 @@ func TestRemoveCommonEndpoints(t *testing.T) {
 		tt := tt
 
 		t.Run(tt.name, func(t *testing.T) {
-			result := removeCommonEndpoints(&tt.endpoints, &tt.endpointsAttached)
-			// Use reflect.DeepEqual as a backup if require.Equal doesn't work as expected
-			if !reflect.DeepEqual(tt.expected, result) {
+			result := GetUniqueEndpoints(tt.endpoints, tt.endpointsAttached)
+			if !cmp.Equal(tt.expected, result) {
 				t.Errorf("Test %s failed: expected %v, got %v", tt.name, tt.expected, result)
 			}
-
-			// Or, if require.Equal works fine, it will display a descriptive error message
-			require.Equal(t, tt.expected, result, "expected array equals result")
 		})
 	}
 }