Cache namespace and service maps (#124)

Author: astaticvoid

integration/janitor/janitor.go

@@ -39,37 +39,31 @@ func NewDefaultJanitor() CloudMapJanitor {
 func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) {
 	fmt.Printf("Cleaning up all test resources in Cloud Map for namespace : %s\n", nsName)
 
-	nsList, err := j.sdApi.ListNamespaces(ctx)
+	nsMap, err := j.sdApi.GetNamespaceMap(ctx)
 	j.checkOrFail(err, "", "could not find namespace to clean")
 
-	var nsId string
-	for _, ns := range nsList {
-		if ns.Name == nsName {
-			nsId = ns.Id
-		}
-	}
-
-	if nsId == "" {
+	ns, found := nsMap[nsName]
+	if !found {
 		fmt.Println("namespace does not exist in account, nothing to clean")
 		return
 	}
 
-	fmt.Printf("found namespace to clean: %s\n", nsId)
+	fmt.Printf("found namespace to clean: %s\n", ns.Id)
 
-	svcs, err := j.sdApi.ListServices(ctx, nsId)
+	svcIdMap, err := j.sdApi.GetServiceIdMap(ctx, ns.Id)
 	j.checkOrFail(err,
-		fmt.Sprintf("namespace has %d services to clean", len(svcs)),
+		fmt.Sprintf("namespace has %d services to clean", len(svcIdMap)),
 		"could not find services to clean")
 
-	for _, svc := range svcs {
-		fmt.Printf("found service to clean: %s\n", svc.Id)
-		j.deregisterInstances(ctx, nsName, svc.Name, svc.Id)
+	for svcName, svcId := range svcIdMap {
+		fmt.Printf("found service to clean: %s\n", svcId)
+		j.deregisterInstances(ctx, nsName, svcName, svcId)
 
-		delSvcErr := j.sdApi.DeleteService(ctx, svc.Id)
+		delSvcErr := j.sdApi.DeleteService(ctx, svcId)
 		j.checkOrFail(delSvcErr, "service deleted", "could not cleanup service")
 	}
 
-	opId, err := j.sdApi.DeleteNamespace(ctx, nsId)
+	opId, err := j.sdApi.DeleteNamespace(ctx, ns.Id)
 	if err == nil {
 		fmt.Println("namespace delete in progress")
 		_, err = j.sdApi.PollNamespaceOperation(ctx, opId)

integration/janitor/janitor_test.go

@@ -28,10 +28,10 @@ func TestCleanupHappyCase(t *testing.T) {
 	tj := getTestJanitor(t)
 	defer tj.close()
 
-	tj.mockApi.EXPECT().ListNamespaces(context.TODO()).
-		Return([]*model.Namespace{{Id: test.HttpNsId, Name: test.HttpNsName}}, nil)
-	tj.mockApi.EXPECT().ListServices(context.TODO(), test.HttpNsId).
-		Return([]*model.Resource{{Id: test.SvcId, Name: test.SvcName}}, nil)
+	tj.mockApi.EXPECT().GetNamespaceMap(context.TODO()).
+		Return(map[string]*model.Namespace{test.HttpNsName: test.GetTestHttpNamespace()}, nil)
+	tj.mockApi.EXPECT().GetServiceIdMap(context.TODO(), test.HttpNsId).
+		Return(map[string]string{test.SvcName: test.SvcId}, nil)
 	tj.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName).
 		Return([]types.HttpInstanceSummary{{InstanceId: aws.String(test.EndptId1)}}, nil)
 
@@ -54,8 +54,8 @@ func TestCleanupNothingToClean(t *testing.T) {
 	tj := getTestJanitor(t)
 	defer tj.close()
 
-	tj.mockApi.EXPECT().ListNamespaces(context.TODO()).
-		Return([]*model.Namespace{}, nil)
+	tj.mockApi.EXPECT().GetNamespaceMap(context.TODO()).
+		Return(map[string]*model.Namespace{}, nil)
 
 	tj.janitor.Cleanup(context.TODO(), test.HttpNsName)
 	assert.False(t, *tj.failed)

pkg/cloudmap/api.go

@@ -20,11 +20,11 @@ const (
 // ServiceDiscoveryApi handles the AWS Cloud Map API request and response processing logic, and converts results to
 // internal data structures. It manages all interactions with the AWS SDK.
 type ServiceDiscoveryApi interface {
-	// ListNamespaces returns a list of all namespaces.
-	ListNamespaces(ctx context.Context) (namespaces []*model.Namespace, err error)
+	// GetNamespaceMap returns a map of all namespaces in the Cloud Map account indexed by namespace name.
+	GetNamespaceMap(ctx context.Context) (namespaces map[string]*model.Namespace, err error)
 
-	// ListServices returns a list of services for a given namespace.
-	ListServices(ctx context.Context, namespaceId string) (services []*model.Resource, err error)
+	// GetServiceIdMap returns a map of all service IDs for a given namespace indexed by service name.
+	GetServiceIdMap(ctx context.Context, namespaceId string) (serviceIdMap map[string]string, err error)
 
 	// DiscoverInstances returns a list of service instances registered to a given service.
 	DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error)
@@ -64,52 +64,53 @@ func NewServiceDiscoveryApiFromConfig(cfg *aws.Config) ServiceDiscoveryApi {
 	}
 }
 
-func (sdApi *serviceDiscoveryApi) ListNamespaces(ctx context.Context) (namespaces []*model.Namespace, err error) {
-	pages := sd.NewListNamespacesPaginator(sdApi.awsFacade, &sd.ListNamespacesInput{})
+func (sdApi *serviceDiscoveryApi) GetNamespaceMap(ctx context.Context) (map[string]*model.Namespace, error) {
+	namespaceMap := make(map[string]*model.Namespace)
 
+	pages := sd.NewListNamespacesPaginator(sdApi.awsFacade, &sd.ListNamespacesInput{})
 	for pages.HasMorePages() {
 		output, err := pages.NextPage(ctx)
 		if err != nil {
-			return namespaces, err
+			return nil, err
 		}
 
 		for _, ns := range output.Namespaces {
-			if namespaceType := model.ConvertNamespaceType(ns.Type); !namespaceType.IsUnsupported() {
-				namespaces = append(namespaces, &model.Namespace{
-					Id:   aws.ToString(ns.Id),
-					Name: aws.ToString(ns.Name),
-					Type: namespaceType,
-				})
+			namespaceType := model.ConvertNamespaceType(ns.Type)
+			if namespaceType.IsUnsupported() {
+				continue
+			}
+			namespaceMap[aws.ToString(ns.Name)] = &model.Namespace{
+				Id:   aws.ToString(ns.Id),
+				Name: aws.ToString(ns.Name),
+				Type: namespaceType,
 			}
 		}
 	}
 
-	return namespaces, nil
+	return namespaceMap, nil
 }
 
-func (sdApi *serviceDiscoveryApi) ListServices(ctx context.Context, nsId string) (svcs []*model.Resource, err error) {
+func (sdApi *serviceDiscoveryApi) GetServiceIdMap(ctx context.Context, nsId string) (map[string]string, error) {
+	serviceIdMap := make(map[string]string)
+
 	filter := types.ServiceFilter{
 		Name:   types.ServiceFilterNameNamespaceId,
 		Values: []string{nsId},
 	}
 
 	pages := sd.NewListServicesPaginator(sdApi.awsFacade, &sd.ListServicesInput{Filters: []types.ServiceFilter{filter}})
-
 	for pages.HasMorePages() {
 		output, err := pages.NextPage(ctx)
 		if err != nil {
-			return svcs, err
+			return nil, err
 		}
 
 		for _, svc := range output.Services {
-			svcs = append(svcs, &model.Resource{
-				Id:   aws.ToString(svc.Id),
-				Name: aws.ToString(svc.Name),
-			})
+			serviceIdMap[aws.ToString(svc.Name)] = aws.ToString(svc.Id)
 		}
 	}
 
-	return svcs, nil
+	return serviceIdMap, nil
 }
 
 func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error) {
@@ -127,8 +128,8 @@ func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName
 	return out.Instances, nil
 }
 
-func (sdApi *serviceDiscoveryApi) ListOperations(ctx context.Context, opFilters []types.OperationFilter) (opStatusMap map[string]types.OperationStatus, err error) {
-	opStatusMap = make(map[string]types.OperationStatus)
+func (sdApi *serviceDiscoveryApi) ListOperations(ctx context.Context, opFilters []types.OperationFilter) (map[string]types.OperationStatus, error) {
+	opStatusMap := make(map[string]types.OperationStatus)
 
 	pages := sd.NewListOperationsPaginator(sdApi.awsFacade, &sd.ListOperationsInput{
 		Filters: opFilters,
@@ -190,7 +191,7 @@ func (sdApi *serviceDiscoveryApi) CreateService(ctx context.Context, namespace m
 	}
 
 	svcId = aws.ToString(output.Service.Id)
-	sdApi.log.Info("service created", "svcId", svcId)
+	sdApi.log.Info("service created", "namespace", namespace.Name, "name", svcName, "id", svcId)
 	return svcId, nil
 }
 

pkg/cloudmap/api_test.go

@@ -8,7 +8,6 @@ import (
 
 	cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap"
 	"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common"
-	"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model"
 	"github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test"
 	"github.com/aws/aws-sdk-go-v2/aws"
 	sd "github.com/aws/aws-sdk-go-v2/service/servicediscovery"
@@ -23,7 +22,7 @@ func TestNewServiceDiscoveryApi(t *testing.T) {
 	assert.NotNil(t, sdc)
 }
 
-func TestServiceDiscoveryApi_ListNamespaces_HappyCase(t *testing.T) {
+func TestServiceDiscoveryApi_GetNamespaceMap_HappyCase(t *testing.T) {
 	mockController := gomock.NewController(t)
 	defer mockController.Finish()
 
@@ -39,12 +38,13 @@ func TestServiceDiscoveryApi_ListNamespaces_HappyCase(t *testing.T) {
 	awsFacade.EXPECT().ListNamespaces(context.TODO(), &sd.ListNamespacesInput{}).
 		Return(&sd.ListNamespacesOutput{Namespaces: []types.NamespaceSummary{ns}}, nil)
 
-	namespaces, _ := sdApi.ListNamespaces(context.TODO())
+	namespaces, err := sdApi.GetNamespaceMap(context.TODO())
+	assert.Nil(t, err, "No error for happy case")
 	assert.True(t, len(namespaces) == 1)
-	assert.Equal(t, test.GetTestDnsNamespace(), namespaces[0], "No error for happy case")
+	assert.Equal(t, test.GetTestDnsNamespace(), namespaces[test.DnsNsName])
 }
 
-func TestServiceDiscoveryApi_ListNamespaces_SkipPublicDNSNotSupported(t *testing.T) {
+func TestServiceDiscoveryApi_GetNamespaceMap_SkipPublicDNSNotSupported(t *testing.T) {
 	mockController := gomock.NewController(t)
 	defer mockController.Finish()
 
@@ -60,11 +60,12 @@ func TestServiceDiscoveryApi_ListNamespaces_SkipPublicDNSNotSupported(t *testing
 	awsFacade.EXPECT().ListNamespaces(context.TODO(), &sd.ListNamespacesInput{}).
 		Return(&sd.ListNamespacesOutput{Namespaces: []types.NamespaceSummary{ns}}, nil)
 
-	namespaces, _ := sdApi.ListNamespaces(context.TODO())
-	assert.True(t, len(namespaces) == 0, "Successfully skipped DNS_PUBLIC from the output")
+	namespaces, err := sdApi.GetNamespaceMap(context.TODO())
+	assert.Nil(t, err, "No error for happy case")
+	assert.Empty(t, namespaces, "Successfully skipped DNS_PUBLIC from the output")
 }
 
-func TestServiceDiscoveryApi_ListServices_HappyCase(t *testing.T) {
+func TestServiceDiscoveryApi_GetServiceIdMap_HappyCase(t *testing.T) {
 	mockController := gomock.NewController(t)
 	defer mockController.Finish()
 
@@ -81,10 +82,10 @@ func TestServiceDiscoveryApi_ListServices_HappyCase(t *testing.T) {
 			{Id: aws.String(test.SvcId), Name: aws.String(test.SvcName)},
 		}}, nil)
 
-	svcs, err := sdApi.ListServices(context.TODO(), test.HttpNsId)
+	svcs, err := sdApi.GetServiceIdMap(context.TODO(), test.HttpNsId)
 	assert.Nil(t, err, "No error for happy case")
 	assert.True(t, len(svcs) == 1)
-	assert.Equal(t, svcs[0], &model.Resource{Id: test.SvcId, Name: test.SvcName})
+	assert.Equal(t, svcs[test.SvcName], test.SvcId)
 }
 
 func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) {
@@ -184,7 +185,8 @@ func TestServiceDiscoveryApi_CreateService_CreateForHttpNamespace(t *testing.T)
 			},
 		}, nil)
 
-	retSvcId, _ := sdApi.CreateService(context.TODO(), *test.GetTestHttpNamespace(), svcName)
+	retSvcId, err := sdApi.CreateService(context.TODO(), *test.GetTestHttpNamespace(), svcName)
+	assert.Nil(t, err)
 	assert.Equal(t, svcId, retSvcId, "Successfully created service")
 }
 
@@ -212,7 +214,8 @@ func TestServiceDiscoveryApi_CreateService_CreateForDnsNamespace(t *testing.T) {
 			},
 		}, nil)
 
-	retSvcId, _ := sdApi.CreateService(context.TODO(), *test.GetTestDnsNamespace(), svcName)
+	retSvcId, err := sdApi.CreateService(context.TODO(), *test.GetTestDnsNamespace(), svcName)
+	assert.Nil(t, err)
 	assert.Equal(t, svcId, retSvcId, "Successfully created service")
 }