Fix multiple instance registration/deregistration (#60)

Author: astaticvoid

pkg/cloudmap/client.go

@@ -157,8 +157,10 @@ func (sdc *serviceDiscoveryClient) RegisterEndpoints(ctx context.Context, nsName
 	opCollector := NewOperationCollector()
 
 	for _, endpt := range endpts {
+		endptId := endpt.Id
+		endptAttrs := endpt.GetCloudMapAttributes()
 		opCollector.Add(func() (opId string, err error) {
-			return sdc.sdApi.RegisterInstance(ctx, svcId, endpt.Id, endpt.GetCloudMapAttributes())
+			return sdc.sdApi.RegisterInstance(ctx, svcId, endptId, endptAttrs)
 		})
 	}
 
@@ -195,8 +197,9 @@ func (sdc *serviceDiscoveryClient) DeleteEndpoints(ctx context.Context, nsName s
 	opCollector := NewOperationCollector()
 
 	for _, endpt := range endpts {
+		endptId := endpt.Id
 		opCollector.Add(func() (opId string, err error) {
-			return sdc.sdApi.DeregisterInstance(ctx, svcId, endpt.Id)
+			return sdc.sdApi.DeregisterInstance(ctx, svcId, endptId)
 		})
 	}
 

pkg/cloudmap/client_test.go

@@ -288,12 +288,70 @@ func TestServiceDiscoveryClient_GetService(t *testing.T) {
 	// TODO: Add unit tests
 }
 
-func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) {
+func TestServiceDiscoveryClient_GetService_CachedValues(t *testing.T) {
 	// TODO: Add unit tests
 }
 
+func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) {
+	mockController := gomock.NewController(t)
+	defer mockController.Finish()
+
+	sdApi := cloudmap.NewMockServiceDiscoveryApi(mockController)
+
+	sdc := getTestSdClient(t, sdApi)
+	sdc.namespaceCache.Add(test.NsName, test.NsId, time.Minute)
+	sdc.serviceIdCache.Add(fmt.Sprintf("%s/%s", test.NsName, test.SvcName), test.SvcId, time.Minute)
+
+	attrs1 := map[string]string{"AWS_INSTANCE_IPV4": test.EndptIp1, "AWS_INSTANCE_PORT": test.EndptPortStr1}
+	attrs2 := map[string]string{"AWS_INSTANCE_IPV4": test.EndptIp2, "AWS_INSTANCE_PORT": test.EndptPortStr2}
+
+	sdApi.EXPECT().RegisterInstance(context.TODO(), test.SvcId, test.EndptId1, attrs1).
+		Return(test.OpId1, nil)
+	sdApi.EXPECT().RegisterInstance(context.TODO(), test.SvcId, test.EndptId2, attrs2).
+		Return(test.OpId2, nil)
+	sdApi.EXPECT().ListOperations(context.TODO(), gomock.Any()).
+		Return(map[string]types.OperationStatus{
+			test.OpId1: types.OperationStatusSuccess,
+			test.OpId2: types.OperationStatusSuccess}, nil)
+
+	err := sdc.RegisterEndpoints(context.TODO(), test.NsName, test.SvcName,
+		[]*model.Endpoint{
+			{
+				Id:   test.EndptId1,
+				IP:   test.EndptIp1,
+				Port: test.EndptPort1,
+			},
+			{
+				Id:   test.EndptId2,
+				IP:   test.EndptIp2,
+				Port: test.EndptPort2,
+			},
+		})
+
+	assert.Nil(t, err)
+}
+
 func TestServiceDiscoveryClient_DeleteEndpoints(t *testing.T) {
-	// TODO: Add unit tests
+	mockController := gomock.NewController(t)
+	defer mockController.Finish()
+
+	sdApi := cloudmap.NewMockServiceDiscoveryApi(mockController)
+
+	sdc := getTestSdClient(t, sdApi)
+	sdc.namespaceCache.Add(test.NsName, test.NsId, time.Minute)
+	sdc.serviceIdCache.Add(fmt.Sprintf("%s/%s", test.NsName, test.SvcName), test.SvcId, time.Minute)
+
+	sdApi.EXPECT().DeregisterInstance(context.TODO(), test.SvcId, test.EndptId1).Return(test.OpId1, nil)
+	sdApi.EXPECT().DeregisterInstance(context.TODO(), test.SvcId, test.EndptId2).Return(test.OpId2, nil)
+	sdApi.EXPECT().ListOperations(context.TODO(), gomock.Any()).
+		Return(map[string]types.OperationStatus{
+			test.OpId1: types.OperationStatusSuccess,
+			test.OpId2: types.OperationStatusSuccess}, nil)
+
+	err := sdc.DeleteEndpoints(context.TODO(), test.NsName, test.SvcName,
+		[]*model.Endpoint{{Id: test.EndptId1}, {Id: test.EndptId2}})
+
+	assert.Nil(t, err)
 }
 
 func TestServiceDiscoveryClient_getNamespace_HappyCase(t *testing.T) {

pkg/controllers/cloudmap_controller.go

@@ -27,8 +27,7 @@ const (
 	// DerivedServiceAnnotation annotates a ServiceImport with derived Service name
 	DerivedServiceAnnotation = "multicluster.k8s.aws/derived-service"
 
-	// LabelServiceName is used to indicate the name of multi-cluster service
-	// that an EndpointSlice belongs to.
+	// LabelServiceImportName indicates the name of multi-cluster service that an EndpointSlice belongs to.
 	LabelServiceImportName = "multicluster.kubernetes.io/service-name"
 )
 

test/test-constants.go

@@ -12,8 +12,11 @@ const (
 	EndptId1      = "endpoint-id-1"
 	EndptId2      = "endpoint-id-2"
 	EndptIp1      = "192.168.0.1"
-	EndptPort1    = 2
-	EndptPortStr1 = "2"
+	EndptIp2      = "192.168.0.2"
+	EndptPort1    = 1
+	EndptPortStr1 = "1"
+	EndptPort2    = 2
+	EndptPortStr2 = "2"
 	OpId1         = "operation-id-1"
 	OpId2         = "operation-id-2"
 	OpStart       = 1