From 62bd318a658f359d875e012a124026d3f941b3d9 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Mon, 13 Jun 2022 21:24:08 +0000 Subject: [PATCH 01/30] Added managed by label - issue 110 --- pkg/controllers/cloudmap_controller_test.go | 1 + pkg/controllers/utils.go | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/controllers/cloudmap_controller_test.go b/pkg/controllers/cloudmap_controller_test.go index 023bd7db..d48d0ebe 100644 --- a/pkg/controllers/cloudmap_controller_test.go +++ b/pkg/controllers/cloudmap_controller_test.go @@ -68,6 +68,7 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { assert.NoError(t, err) endpointSlice := endpointSliceList.Items[0] assert.Equal(t, test.SvcName, endpointSlice.Labels["multicluster.kubernetes.io/service-name"], "Endpoint slice is created") + assert.Contains(t, endpointSlice.Labels, LabelEntityManagedBy, "Managed by label is added") assert.Equal(t, int32(test.Port1), *endpointSlice.Ports[0].Port) assert.Equal(t, test.EndptIp1, endpointSlice.Endpoints[0].Addresses[0]) } diff --git a/pkg/controllers/utils.go b/pkg/controllers/utils.go index 517a8f92..803c1ba0 100644 --- a/pkg/controllers/utils.go +++ b/pkg/controllers/utils.go @@ -20,6 +20,12 @@ const ( // LabelServiceImportName indicates the name of the multi-cluster service that an EndpointSlice belongs to. LabelServiceImportName = "multicluster.kubernetes.io/service-name" + + // LabelEntityManagedBy indicates the name of the entity that manages the EndpointSlice. + LabelEntityManagedBy = "multicluster.kubernetes.io/managed-by" + + // ValueEntityManagedBy indicates the name of the entity that manages the EndpointSlice. + ValueEntityManagedBy = "aws-cloud-map-mcs-controller-for-k8s" ) // ServicePortToPort converts a k8s service port to internal model port @@ -203,8 +209,12 @@ func CreateEndpointSliceStruct(svc *v1.Service, svcImportName string) *discovery return &discovery.EndpointSlice{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - discovery.LabelServiceName: svc.Name, // derived Service name - LabelServiceImportName: svcImportName, // original ServiceImport name + // derived Service name + discovery.LabelServiceName: svc.Name, + // original ServiceImport name + LabelServiceImportName: svcImportName, + // 'managed-by' label set to controller + LabelEntityManagedBy: ValueEntityManagedBy, }, GenerateName: svc.Name + "-", OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(svc, schema.GroupVersionKind{ From 6101a8b573ecf0fffeb75aaa731274f459d8502a Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 29 Jun 2022 21:33:17 +0000 Subject: [PATCH 02/30] migration to multigroup structure --- PROJECT | 1 + integration/scenarios/export_service.go | 2 +- main.go | 4 ++-- pkg/{api => apis/multicluster}/v1alpha1/groupversion_info.go | 0 .../multicluster}/v1alpha1/serviceexport_types.go | 0 .../multicluster}/v1alpha1/serviceimport_types.go | 0 .../multicluster}/v1alpha1/zz_generated.deepcopy.go | 0 pkg/controllers/{ => multicluster}/cloudmap_controller.go | 2 +- .../{ => multicluster}/cloudmap_controller_test.go | 2 +- pkg/controllers/{ => multicluster}/controllers_common_test.go | 2 +- pkg/controllers/{ => multicluster}/endpointslice_plan.go | 0 pkg/controllers/{ => multicluster}/endpointslice_plan_test.go | 0 .../{ => multicluster}/serviceexport_controller.go | 2 +- .../{ => multicluster}/serviceexport_controller_test.go | 2 +- pkg/controllers/{ => multicluster}/suite_test.go | 4 ++-- pkg/controllers/{ => multicluster}/utils.go | 2 +- pkg/controllers/{ => multicluster}/utils_test.go | 2 +- 17 files changed, 13 insertions(+), 12 deletions(-) rename pkg/{api => apis/multicluster}/v1alpha1/groupversion_info.go (100%) rename pkg/{api => apis/multicluster}/v1alpha1/serviceexport_types.go (100%) rename pkg/{api => apis/multicluster}/v1alpha1/serviceimport_types.go (100%) rename pkg/{api => apis/multicluster}/v1alpha1/zz_generated.deepcopy.go (100%) rename pkg/controllers/{ => multicluster}/cloudmap_controller.go (99%) rename pkg/controllers/{ => multicluster}/cloudmap_controller_test.go (97%) rename pkg/controllers/{ => multicluster}/controllers_common_test.go (96%) rename pkg/controllers/{ => multicluster}/endpointslice_plan.go (100%) rename pkg/controllers/{ => multicluster}/endpointslice_plan_test.go (100%) rename pkg/controllers/{ => multicluster}/serviceexport_controller.go (99%) rename pkg/controllers/{ => multicluster}/serviceexport_controller_test.go (98%) rename pkg/controllers/{ => multicluster}/suite_test.go (92%) rename pkg/controllers/{ => multicluster}/utils.go (98%) rename pkg/controllers/{ => multicluster}/utils_test.go (99%) diff --git a/PROJECT b/PROJECT index 60ca5e0f..65905c73 100644 --- a/PROJECT +++ b/PROJECT @@ -3,6 +3,7 @@ layout: - go.kubebuilder.io/v3 projectName: aws-cloud-map-mcs-controller-for-k8s repo: github.com/aws/aws-cloud-map-mcs-controller-for-k8s +multigroup: true resources: - api: crdVersion: v1 diff --git a/integration/scenarios/export_service.go b/integration/scenarios/export_service.go index 51826cf1..8ae3a571 100644 --- a/integration/scenarios/export_service.go +++ b/integration/scenarios/export_service.go @@ -8,7 +8,7 @@ import ( "time" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers" + controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" "github.com/aws/aws-sdk-go-v2/aws" v1 "k8s.io/api/core/v1" diff --git a/main.go b/main.go index 757383f7..21ca9881 100644 --- a/main.go +++ b/main.go @@ -22,8 +22,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" - multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers" + multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" + controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" // +kubebuilder:scaffold:imports ) diff --git a/pkg/api/v1alpha1/groupversion_info.go b/pkg/apis/multicluster/v1alpha1/groupversion_info.go similarity index 100% rename from pkg/api/v1alpha1/groupversion_info.go rename to pkg/apis/multicluster/v1alpha1/groupversion_info.go diff --git a/pkg/api/v1alpha1/serviceexport_types.go b/pkg/apis/multicluster/v1alpha1/serviceexport_types.go similarity index 100% rename from pkg/api/v1alpha1/serviceexport_types.go rename to pkg/apis/multicluster/v1alpha1/serviceexport_types.go diff --git a/pkg/api/v1alpha1/serviceimport_types.go b/pkg/apis/multicluster/v1alpha1/serviceimport_types.go similarity index 100% rename from pkg/api/v1alpha1/serviceimport_types.go rename to pkg/apis/multicluster/v1alpha1/serviceimport_types.go diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/multicluster/v1alpha1/zz_generated.deepcopy.go similarity index 100% rename from pkg/api/v1alpha1/zz_generated.deepcopy.go rename to pkg/apis/multicluster/v1alpha1/zz_generated.deepcopy.go diff --git a/pkg/controllers/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go similarity index 99% rename from pkg/controllers/cloudmap_controller.go rename to pkg/controllers/multicluster/cloudmap_controller.go index 2cd53655..56f9ce99 100644 --- a/pkg/controllers/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -5,7 +5,7 @@ import ( "fmt" "time" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/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" diff --git a/pkg/controllers/cloudmap_controller_test.go b/pkg/controllers/multicluster/cloudmap_controller_test.go similarity index 97% rename from pkg/controllers/cloudmap_controller_test.go rename to pkg/controllers/multicluster/cloudmap_controller_test.go index f4c97e7e..5fa12beb 100644 --- a/pkg/controllers/cloudmap_controller_test.go +++ b/pkg/controllers/multicluster/cloudmap_controller_test.go @@ -6,7 +6,7 @@ import ( "testing" 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/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "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" diff --git a/pkg/controllers/controllers_common_test.go b/pkg/controllers/multicluster/controllers_common_test.go similarity index 96% rename from pkg/controllers/controllers_common_test.go rename to pkg/controllers/multicluster/controllers_common_test.go index 90a6c81e..19f4f38e 100644 --- a/pkg/controllers/controllers_common_test.go +++ b/pkg/controllers/multicluster/controllers_common_test.go @@ -1,7 +1,7 @@ package controllers import ( - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test" "github.com/aws/aws-sdk-go-v2/aws" v1 "k8s.io/api/core/v1" diff --git a/pkg/controllers/endpointslice_plan.go b/pkg/controllers/multicluster/endpointslice_plan.go similarity index 100% rename from pkg/controllers/endpointslice_plan.go rename to pkg/controllers/multicluster/endpointslice_plan.go diff --git a/pkg/controllers/endpointslice_plan_test.go b/pkg/controllers/multicluster/endpointslice_plan_test.go similarity index 100% rename from pkg/controllers/endpointslice_plan_test.go rename to pkg/controllers/multicluster/endpointslice_plan_test.go diff --git a/pkg/controllers/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go similarity index 99% rename from pkg/controllers/serviceexport_controller.go rename to pkg/controllers/multicluster/serviceexport_controller.go index 0d77d265..38294c27 100644 --- a/pkg/controllers/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -24,7 +24,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" ) const ( diff --git a/pkg/controllers/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go similarity index 98% rename from pkg/controllers/serviceexport_controller_test.go rename to pkg/controllers/multicluster/serviceexport_controller_test.go index d158c421..0fb3b69f 100644 --- a/pkg/controllers/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -4,7 +4,7 @@ import ( "context" 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/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "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" diff --git a/pkg/controllers/suite_test.go b/pkg/controllers/multicluster/suite_test.go similarity index 92% rename from pkg/controllers/suite_test.go rename to pkg/controllers/multicluster/suite_test.go index 56672340..c37817c3 100644 --- a/pkg/controllers/suite_test.go +++ b/pkg/controllers/multicluster/suite_test.go @@ -14,7 +14,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" // +kubebuilder:scaffold:imports ) @@ -38,7 +38,7 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, } diff --git a/pkg/controllers/utils.go b/pkg/controllers/multicluster/utils.go similarity index 98% rename from pkg/controllers/utils.go rename to pkg/controllers/multicluster/utils.go index 5465a3c6..eb4c70d1 100644 --- a/pkg/controllers/utils.go +++ b/pkg/controllers/multicluster/utils.go @@ -5,7 +5,7 @@ import ( "encoding/base32" "strings" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" diff --git a/pkg/controllers/utils_test.go b/pkg/controllers/multicluster/utils_test.go similarity index 99% rename from pkg/controllers/utils_test.go rename to pkg/controllers/multicluster/utils_test.go index b4e1bda6..71a7c59f 100644 --- a/pkg/controllers/utils_test.go +++ b/pkg/controllers/multicluster/utils_test.go @@ -4,7 +4,7 @@ import ( "reflect" "testing" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/api/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test" v1 "k8s.io/api/core/v1" From 51f7bfd8b9ef4aecffcf0e2717c60716b56502db Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 29 Jun 2022 23:38:27 +0000 Subject: [PATCH 03/30] updated paths in PROJECT --- PROJECT | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PROJECT b/PROJECT index 65905c73..83e31c52 100644 --- a/PROJECT +++ b/PROJECT @@ -12,7 +12,7 @@ resources: domain: x-k8s.io group: multicluster kind: ServiceExport - path: github.com/aws/aws-cloud-map-mcs-controller-for-k8s/api/v1alpha1 + path: github.com/aws/aws-cloud-map-mcs-controller-for-k8s/apis/multicluster/v1alpha1 version: v1alpha1 - api: crdVersion: v1 @@ -21,6 +21,6 @@ resources: domain: x-k8s.io group: multicluster kind: ServiceImport - path: github.com/aws/aws-cloud-map-mcs-controller-for-k8s/api/v1alpha1 + path: github.com/aws/aws-cloud-map-mcs-controller-for-k8s/apis/multicluster/v1alpha1 version: v1alpha1 version: "3" From fe4778202f7c2683d8430f7e1a0e901282bf470c Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 29 Jun 2022 23:42:43 +0000 Subject: [PATCH 04/30] updated paths in .golangci & .codecov --- .github/.codecov.yml | 2 +- .golangci.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/.codecov.yml b/.github/.codecov.yml index 4e479a10..1ab3b5b2 100644 --- a/.github/.codecov.yml +++ b/.github/.codecov.yml @@ -22,7 +22,7 @@ comment: ignore: - "config/**/*" - - "pkg/api/**/*" + - "pkg/apis/**/*" - "mocks/**/*" - "integration/scenarios/**/*" - "pkg/common/logger.go" diff --git a/.golangci.yaml b/.golangci.yaml index 376f5fee..aae8ba43 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -32,4 +32,4 @@ run: skip: - .*_mock.go - mocks/ - - pkg/api/ + - pkg/apis/ From b2cd85282328eea5c0e4911027eb26cc51163d19 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 16:55:13 +0000 Subject: [PATCH 05/30] ClusterID recognized & watched for changes + sync --- .../bases/about.k8s.io_clusterproperties.yaml | 65 ++++++++++ config/crd/kustomization.yaml | 6 + .../annotation_for_clusterproperties.yaml | 7 ++ .../cainjection_in_clusterproperties.yaml | 7 ++ .../patches/webhook_in_clusterproperties.yaml | 16 +++ .../about_v1alpha1_clusterproperty.yaml | 9 ++ go.mod | 2 +- go.sum | 2 + main.go | 9 +- .../about/v1alpha1/clusterproperty_types.go | 52 ++++++++ pkg/apis/about/v1alpha1/groupversion_info.go | 20 +++ .../about/v1alpha1/zz_generated.deepcopy.go | 115 ++++++++++++++++++ .../multicluster/cloudmap_controller.go | 16 +++ .../multicluster/serviceexport_controller.go | 53 +++++++- .../serviceexport_controller_test.go | 18 ++- 15 files changed, 385 insertions(+), 12 deletions(-) create mode 100644 config/crd/bases/about.k8s.io_clusterproperties.yaml create mode 100644 config/crd/patches/annotation_for_clusterproperties.yaml create mode 100644 config/crd/patches/cainjection_in_clusterproperties.yaml create mode 100644 config/crd/patches/webhook_in_clusterproperties.yaml create mode 100644 config/samples/about_v1alpha1_clusterproperty.yaml create mode 100644 pkg/apis/about/v1alpha1/clusterproperty_types.go create mode 100644 pkg/apis/about/v1alpha1/groupversion_info.go create mode 100644 pkg/apis/about/v1alpha1/zz_generated.deepcopy.go diff --git a/config/crd/bases/about.k8s.io_clusterproperties.yaml b/config/crd/bases/about.k8s.io_clusterproperties.yaml new file mode 100644 index 00000000..71b36cf2 --- /dev/null +++ b/config/crd/bases/about.k8s.io_clusterproperties.yaml @@ -0,0 +1,65 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: clusterproperties.about.k8s.io +spec: + group: about.k8s.io + names: + kind: ClusterProperty + listKind: ClusterPropertyList + plural: clusterproperties + singular: clusterproperty + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.value + name: value + type: string + - jsonPath: .metadata.creationTimestamp + name: age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: ClusterProperty is the Schema for the clusterproperties API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: ClusterPropertySpec defines the desired state of ClusterProperty + properties: + value: + description: ClusterProperty value + minLength: 1 + type: string + required: + - value + type: object + status: + description: ClusterPropertyStatus defines the observed state of ClusterProperty + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8dac89bc..6dc8a039 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -2,6 +2,7 @@ # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default resources: +- bases/about.k8s.io_clusterproperties.yaml - bases/multicluster.x-k8s.io_serviceexports.yaml - bases/multicluster.x-k8s.io_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizeresource @@ -9,16 +10,21 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD +#- patches/webhook_in_clusterproperties.yaml #- patches/webhook_in_serviceexports.yaml #- patches/webhook_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD +#- patches/cainjection_in_clusterproperties.yaml #- patches/cainjection_in_serviceexports.yaml #- patches/cainjection_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch +# Patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +- patches/annotation_for_clusterproperties.yaml + # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: - kustomizeconfig.yaml diff --git a/config/crd/patches/annotation_for_clusterproperties.yaml b/config/crd/patches/annotation_for_clusterproperties.yaml new file mode 100644 index 00000000..9ceefc79 --- /dev/null +++ b/config/crd/patches/annotation_for_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.kubernetes.io: "https://github.com/kubernetes/kubernetes/pull/78458" + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/cainjection_in_clusterproperties.yaml b/config/crd/patches/cainjection_in_clusterproperties.yaml new file mode 100644 index 00000000..31e5f39b --- /dev/null +++ b/config/crd/patches/cainjection_in_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/webhook_in_clusterproperties.yaml b/config/crd/patches/webhook_in_clusterproperties.yaml new file mode 100644 index 00000000..c1880095 --- /dev/null +++ b/config/crd/patches/webhook_in_clusterproperties.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: clusterproperties.about.k8s.io +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml new file mode 100644 index 00000000..818301ef --- /dev/null +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -0,0 +1,9 @@ +# An example object of `id.k8s.io ClusterProperty` +# using a kube-system ns uuid as the id value: + +apiVersion: about.k8s.io/v1alpha1 +kind: ClusterProperty +metadata: + name: id.k8s.io +spec: + value: matthew-cluster-1 diff --git a/go.mod b/go.mod index 7a681e33..4cd07628 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( github.com/google/gnostic v0.5.7-v3refs // indirect github.com/google/go-cmp v0.5.8 // indirect github.com/google/gofuzz v1.1.0 // indirect - github.com/google/uuid v1.1.2 // indirect + github.com/google/uuid v1.3.0 // indirect github.com/imdario/mergo v0.3.12 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect diff --git a/go.sum b/go.sum index 1ee146f4..936fb02c 100644 --- a/go.sum +++ b/go.sum @@ -282,6 +282,8 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLe github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/main.go b/main.go index 21ca9881..b483ec82 100644 --- a/main.go +++ b/main.go @@ -22,8 +22,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" - controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" + multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" // +kubebuilder:scaffold:imports ) @@ -36,6 +37,8 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(multiclusterv1alpha1.AddToScheme(scheme)) + + utilruntime.Must(aboutv1alpha1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } @@ -85,7 +88,7 @@ func main() { log.Info("Running with AWS region", "AWS_REGION", awsCfg.Region) serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg) - if err = (&controllers.ServiceExportReconciler{ + if err = (&multiclustercontrollers.ServiceExportReconciler{ Client: mgr.GetClient(), Log: common.NewLogger("controllers", "ServiceExport"), Scheme: mgr.GetScheme(), @@ -95,7 +98,7 @@ func main() { os.Exit(1) } - cloudMapReconciler := &controllers.CloudMapReconciler{ + cloudMapReconciler := &multiclustercontrollers.CloudMapReconciler{ Client: mgr.GetClient(), Cloudmap: serviceDiscoveryClient, Log: common.NewLogger("controllers", "Cloudmap"), diff --git a/pkg/apis/about/v1alpha1/clusterproperty_types.go b/pkg/apis/about/v1alpha1/clusterproperty_types.go new file mode 100644 index 00000000..f3fd599e --- /dev/null +++ b/pkg/apis/about/v1alpha1/clusterproperty_types.go @@ -0,0 +1,52 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ClusterPropertySpec defines the desired state of ClusterProperty +type ClusterPropertySpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ClusterProperty value + // +kubebuilder:validation:Maxlength=128000 + // +kubebuilder:validation:MinLength=1 + Value string `json:"value"` +} + +// ClusterPropertyStatus defines the observed state of ClusterProperty +type ClusterPropertyStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:resource:scope=Cluster + +// ClusterProperty is the Schema for the clusterproperties API +// +kubebuilder:printcolumn:name="value",type=string,JSONPath=`.spec.value` +// +kubebuilder:printcolumn:name="age",type=date,JSONPath=`.metadata.creationTimestamp` +type ClusterProperty struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ClusterPropertySpec `json:"spec,omitempty"` + Status ClusterPropertyStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// ClusterPropertyList contains a list of ClusterProperty +type ClusterPropertyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ClusterProperty `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ClusterProperty{}, &ClusterPropertyList{}) +} diff --git a/pkg/apis/about/v1alpha1/groupversion_info.go b/pkg/apis/about/v1alpha1/groupversion_info.go new file mode 100644 index 00000000..82f56067 --- /dev/null +++ b/pkg/apis/about/v1alpha1/groupversion_info.go @@ -0,0 +1,20 @@ +// Package v1alpha1 contains API Schema definitions for the about v1alpha1 API group +//+kubebuilder:object:generate=true +//+groupName=about.k8s.io +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "about.k8s.io", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 00000000..476fb37f --- /dev/null +++ b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,115 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterProperty) DeepCopyInto(out *ClusterProperty) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterProperty. +func (in *ClusterProperty) DeepCopy() *ClusterProperty { + if in == nil { + return nil + } + out := new(ClusterProperty) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterProperty) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyList) DeepCopyInto(out *ClusterPropertyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterProperty, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyList. +func (in *ClusterPropertyList) DeepCopy() *ClusterPropertyList { + if in == nil { + return nil + } + out := new(ClusterPropertyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterPropertyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertySpec) DeepCopyInto(out *ClusterPropertySpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertySpec. +func (in *ClusterPropertySpec) DeepCopy() *ClusterPropertySpec { + if in == nil { + return nil + } + out := new(ClusterPropertySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyStatus) DeepCopyInto(out *ClusterPropertyStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyStatus. +func (in *ClusterPropertyStatus) DeepCopy() *ClusterPropertyStatus { + if in == nil { + return nil + } + out := new(ClusterPropertyStatus) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 56f9ce99..48389ca0 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -117,6 +117,18 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se importedSvcPorts := ExtractServicePorts(svc.Endpoints) + r.Log.Debug("service ports", "ports", importedSvcPorts) + + // service ports: {"ports": [{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]} + + // separate importedSvcPorts by unique ClusterID + importedSvcPortsByClusterID := make(map[string][]model.Port) + for _, p := range importedSvcPorts { + importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]] = append(importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]], *p) + } + + r.Log.Debug("service ports by cluster ID", "ports", importedSvcPortsByClusterID) + svcImport, err := r.getServiceImport(ctx, svc.Namespace, svc.Name) if err != nil { if !errors.IsNotFound(err) { @@ -129,6 +141,10 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } + // For each port in importedSvcPortsByClusterID, create derived services + // importedSvcPortsByClusterID = {"ports": {"matthew-cluster-1":[{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]}} + + derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 38294c27..7eb1238a 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/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" @@ -24,10 +25,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" + multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" ) const ( + ClusterIdName = "id.k8s.io" + ClusterIdAttr = "CLUSTER_ID" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" @@ -54,7 +57,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques name := req.NamespacedName r.Log.Debug("reconciling ServiceExport", "Namespace", namespace, "Name", name) - serviceExport := v1alpha1.ServiceExport{} + serviceExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(ctx, name, &serviceExport); err != nil { if errors.IsNotFound(err) { r.Log.Debug("no ServiceExport found", @@ -92,7 +95,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.handleUpdate(ctx, &serviceExport, &service) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *v1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -185,7 +188,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context return cmService, nil } -func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *v1alpha1.ServiceExport) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport) (ctrl.Result, error) { if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -225,6 +228,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. return nil, err } + clusterId := &aboutv1alpha1.ClusterProperty{} + err = r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + } + servicePortMap := make(map[string]model.Port) for _, svcPort := range svc.Spec.Ports { servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) @@ -241,6 +250,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. if version.GetVersion() != "" { attributes[K8sVersionAttr] = version.PackageName + " " + version.GetVersion() } + + // Apply clusterID as endpoint attribute if it exists + if clusterId.Spec.Value != "" { + attributes[ClusterIdAttr] = clusterId.Spec.Value + } + // TODO extract attributes - pod, node and other useful details if possible port := EndpointPortToPort(endpointPort) @@ -261,7 +276,7 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.ServiceExport{}). + For(&multiclusterv1alpha1.ServiceExport{}). // Watch for the changes to the EndpointSlice object. This object is bound to be // updated when Service or Deployment are updated. There is also a filtering logic // to enqueue those EndpointSlice event which have corresponding ServiceExport @@ -270,6 +285,12 @@ func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.endpointSliceEventHandler()), builder.WithPredicates(r.endpointSliceFilter()), ). + // Watch for changes to ClusterProperty objects. If a ClusterProperty object is + // created, updated or deleted, the controller will reconcile all service exports + Watches( + &source.Kind{Type: &aboutv1alpha1.ClusterProperty{}}, + handler.EnqueueRequestsFromMapFunc(r.clusterPropertyEventHandler()), + ). Complete(r) } @@ -286,6 +307,26 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { } } +func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { + // Return reconcile requests for all services + return func(object client.Object) []reconcile.Request { + services := &v1.ServiceList{} + if err := r.Client.List(context.TODO(), services); err != nil { + r.Log.Error(err, "error listing services") + return nil + } + + result := make([]reconcile.Request, 0) + for _, service := range services.Items { + result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{ + Name: service.Name, + Namespace: service.Namespace, + }}) + } + return result + } +} + func (r *ServiceExportReconciler) endpointSliceFilter() predicate.Funcs { return predicate.Funcs{ GenericFunc: func(e event.GenericEvent) bool { @@ -310,7 +351,7 @@ func (r *ServiceExportReconciler) doesEndpointSliceHaveServiceExport(object clie Name: serviceName, Namespace: object.GetNamespace(), } - svcExport := v1alpha1.ServiceExport{} + svcExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(context.TODO(), ns, &svcExport); err != nil { return false } diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 0fb3b69f..bc91ee4f 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -4,6 +4,7 @@ import ( "context" cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -27,7 +29,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -47,7 +49,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) mock.EXPECT().RegisterEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, - []*model.Endpoint{test.GetTestEndpoint1()}).Return(nil).Times(1) + []*model.Endpoint{test.GetTestEndpoint1WithAttr()}).Return(nil).Times(1) reconciler := getServiceExportReconciler(t, mock, fakeClient) @@ -164,6 +166,7 @@ func getServiceExportScheme() *runtime.Scheme { scheme.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) return scheme } @@ -175,3 +178,14 @@ func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServi CloudMap: mockClient, } } + +func clusterIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: "test_clusterid", + }, + } +} From 0c64aa07590ab1f1c59c697cf152875bfb5c787f Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 16:58:43 +0000 Subject: [PATCH 06/30] clusterid sync with cloudmap + recognized --- .../bases/about.k8s.io_clusterproperties.yaml | 65 ++++++++++ config/crd/kustomization.yaml | 6 + .../annotation_for_clusterproperties.yaml | 7 ++ .../cainjection_in_clusterproperties.yaml | 7 ++ .../patches/webhook_in_clusterproperties.yaml | 16 +++ .../about_v1alpha1_clusterproperty.yaml | 9 ++ go.mod | 2 +- go.sum | 2 + main.go | 9 +- .../about/v1alpha1/clusterproperty_types.go | 52 ++++++++ pkg/apis/about/v1alpha1/groupversion_info.go | 20 +++ .../about/v1alpha1/zz_generated.deepcopy.go | 115 ++++++++++++++++++ .../multicluster/serviceexport_controller.go | 53 +++++++- .../serviceexport_controller_test.go | 18 ++- 14 files changed, 369 insertions(+), 12 deletions(-) create mode 100644 config/crd/bases/about.k8s.io_clusterproperties.yaml create mode 100644 config/crd/patches/annotation_for_clusterproperties.yaml create mode 100644 config/crd/patches/cainjection_in_clusterproperties.yaml create mode 100644 config/crd/patches/webhook_in_clusterproperties.yaml create mode 100644 config/samples/about_v1alpha1_clusterproperty.yaml create mode 100644 pkg/apis/about/v1alpha1/clusterproperty_types.go create mode 100644 pkg/apis/about/v1alpha1/groupversion_info.go create mode 100644 pkg/apis/about/v1alpha1/zz_generated.deepcopy.go diff --git a/config/crd/bases/about.k8s.io_clusterproperties.yaml b/config/crd/bases/about.k8s.io_clusterproperties.yaml new file mode 100644 index 00000000..71b36cf2 --- /dev/null +++ b/config/crd/bases/about.k8s.io_clusterproperties.yaml @@ -0,0 +1,65 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: clusterproperties.about.k8s.io +spec: + group: about.k8s.io + names: + kind: ClusterProperty + listKind: ClusterPropertyList + plural: clusterproperties + singular: clusterproperty + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.value + name: value + type: string + - jsonPath: .metadata.creationTimestamp + name: age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: ClusterProperty is the Schema for the clusterproperties API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: ClusterPropertySpec defines the desired state of ClusterProperty + properties: + value: + description: ClusterProperty value + minLength: 1 + type: string + required: + - value + type: object + status: + description: ClusterPropertyStatus defines the observed state of ClusterProperty + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8dac89bc..6dc8a039 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -2,6 +2,7 @@ # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default resources: +- bases/about.k8s.io_clusterproperties.yaml - bases/multicluster.x-k8s.io_serviceexports.yaml - bases/multicluster.x-k8s.io_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizeresource @@ -9,16 +10,21 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD +#- patches/webhook_in_clusterproperties.yaml #- patches/webhook_in_serviceexports.yaml #- patches/webhook_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD +#- patches/cainjection_in_clusterproperties.yaml #- patches/cainjection_in_serviceexports.yaml #- patches/cainjection_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch +# Patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +- patches/annotation_for_clusterproperties.yaml + # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: - kustomizeconfig.yaml diff --git a/config/crd/patches/annotation_for_clusterproperties.yaml b/config/crd/patches/annotation_for_clusterproperties.yaml new file mode 100644 index 00000000..9ceefc79 --- /dev/null +++ b/config/crd/patches/annotation_for_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.kubernetes.io: "https://github.com/kubernetes/kubernetes/pull/78458" + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/cainjection_in_clusterproperties.yaml b/config/crd/patches/cainjection_in_clusterproperties.yaml new file mode 100644 index 00000000..31e5f39b --- /dev/null +++ b/config/crd/patches/cainjection_in_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/webhook_in_clusterproperties.yaml b/config/crd/patches/webhook_in_clusterproperties.yaml new file mode 100644 index 00000000..c1880095 --- /dev/null +++ b/config/crd/patches/webhook_in_clusterproperties.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: clusterproperties.about.k8s.io +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml new file mode 100644 index 00000000..818301ef --- /dev/null +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -0,0 +1,9 @@ +# An example object of `id.k8s.io ClusterProperty` +# using a kube-system ns uuid as the id value: + +apiVersion: about.k8s.io/v1alpha1 +kind: ClusterProperty +metadata: + name: id.k8s.io +spec: + value: matthew-cluster-1 diff --git a/go.mod b/go.mod index 7a681e33..4cd07628 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( github.com/google/gnostic v0.5.7-v3refs // indirect github.com/google/go-cmp v0.5.8 // indirect github.com/google/gofuzz v1.1.0 // indirect - github.com/google/uuid v1.1.2 // indirect + github.com/google/uuid v1.3.0 // indirect github.com/imdario/mergo v0.3.12 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect diff --git a/go.sum b/go.sum index 1ee146f4..936fb02c 100644 --- a/go.sum +++ b/go.sum @@ -282,6 +282,8 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLe github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/main.go b/main.go index 21ca9881..b483ec82 100644 --- a/main.go +++ b/main.go @@ -22,8 +22,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" - controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" + multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" // +kubebuilder:scaffold:imports ) @@ -36,6 +37,8 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(multiclusterv1alpha1.AddToScheme(scheme)) + + utilruntime.Must(aboutv1alpha1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } @@ -85,7 +88,7 @@ func main() { log.Info("Running with AWS region", "AWS_REGION", awsCfg.Region) serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg) - if err = (&controllers.ServiceExportReconciler{ + if err = (&multiclustercontrollers.ServiceExportReconciler{ Client: mgr.GetClient(), Log: common.NewLogger("controllers", "ServiceExport"), Scheme: mgr.GetScheme(), @@ -95,7 +98,7 @@ func main() { os.Exit(1) } - cloudMapReconciler := &controllers.CloudMapReconciler{ + cloudMapReconciler := &multiclustercontrollers.CloudMapReconciler{ Client: mgr.GetClient(), Cloudmap: serviceDiscoveryClient, Log: common.NewLogger("controllers", "Cloudmap"), diff --git a/pkg/apis/about/v1alpha1/clusterproperty_types.go b/pkg/apis/about/v1alpha1/clusterproperty_types.go new file mode 100644 index 00000000..f3fd599e --- /dev/null +++ b/pkg/apis/about/v1alpha1/clusterproperty_types.go @@ -0,0 +1,52 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ClusterPropertySpec defines the desired state of ClusterProperty +type ClusterPropertySpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ClusterProperty value + // +kubebuilder:validation:Maxlength=128000 + // +kubebuilder:validation:MinLength=1 + Value string `json:"value"` +} + +// ClusterPropertyStatus defines the observed state of ClusterProperty +type ClusterPropertyStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:resource:scope=Cluster + +// ClusterProperty is the Schema for the clusterproperties API +// +kubebuilder:printcolumn:name="value",type=string,JSONPath=`.spec.value` +// +kubebuilder:printcolumn:name="age",type=date,JSONPath=`.metadata.creationTimestamp` +type ClusterProperty struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ClusterPropertySpec `json:"spec,omitempty"` + Status ClusterPropertyStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// ClusterPropertyList contains a list of ClusterProperty +type ClusterPropertyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ClusterProperty `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ClusterProperty{}, &ClusterPropertyList{}) +} diff --git a/pkg/apis/about/v1alpha1/groupversion_info.go b/pkg/apis/about/v1alpha1/groupversion_info.go new file mode 100644 index 00000000..82f56067 --- /dev/null +++ b/pkg/apis/about/v1alpha1/groupversion_info.go @@ -0,0 +1,20 @@ +// Package v1alpha1 contains API Schema definitions for the about v1alpha1 API group +//+kubebuilder:object:generate=true +//+groupName=about.k8s.io +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "about.k8s.io", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 00000000..476fb37f --- /dev/null +++ b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,115 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterProperty) DeepCopyInto(out *ClusterProperty) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterProperty. +func (in *ClusterProperty) DeepCopy() *ClusterProperty { + if in == nil { + return nil + } + out := new(ClusterProperty) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterProperty) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyList) DeepCopyInto(out *ClusterPropertyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterProperty, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyList. +func (in *ClusterPropertyList) DeepCopy() *ClusterPropertyList { + if in == nil { + return nil + } + out := new(ClusterPropertyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterPropertyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertySpec) DeepCopyInto(out *ClusterPropertySpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertySpec. +func (in *ClusterPropertySpec) DeepCopy() *ClusterPropertySpec { + if in == nil { + return nil + } + out := new(ClusterPropertySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyStatus) DeepCopyInto(out *ClusterPropertyStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyStatus. +func (in *ClusterPropertyStatus) DeepCopy() *ClusterPropertyStatus { + if in == nil { + return nil + } + out := new(ClusterPropertyStatus) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 38294c27..7eb1238a 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/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" @@ -24,10 +25,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" + multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" ) const ( + ClusterIdName = "id.k8s.io" + ClusterIdAttr = "CLUSTER_ID" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" @@ -54,7 +57,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques name := req.NamespacedName r.Log.Debug("reconciling ServiceExport", "Namespace", namespace, "Name", name) - serviceExport := v1alpha1.ServiceExport{} + serviceExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(ctx, name, &serviceExport); err != nil { if errors.IsNotFound(err) { r.Log.Debug("no ServiceExport found", @@ -92,7 +95,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.handleUpdate(ctx, &serviceExport, &service) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *v1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -185,7 +188,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context return cmService, nil } -func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *v1alpha1.ServiceExport) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport) (ctrl.Result, error) { if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -225,6 +228,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. return nil, err } + clusterId := &aboutv1alpha1.ClusterProperty{} + err = r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + } + servicePortMap := make(map[string]model.Port) for _, svcPort := range svc.Spec.Ports { servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) @@ -241,6 +250,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. if version.GetVersion() != "" { attributes[K8sVersionAttr] = version.PackageName + " " + version.GetVersion() } + + // Apply clusterID as endpoint attribute if it exists + if clusterId.Spec.Value != "" { + attributes[ClusterIdAttr] = clusterId.Spec.Value + } + // TODO extract attributes - pod, node and other useful details if possible port := EndpointPortToPort(endpointPort) @@ -261,7 +276,7 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.ServiceExport{}). + For(&multiclusterv1alpha1.ServiceExport{}). // Watch for the changes to the EndpointSlice object. This object is bound to be // updated when Service or Deployment are updated. There is also a filtering logic // to enqueue those EndpointSlice event which have corresponding ServiceExport @@ -270,6 +285,12 @@ func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.endpointSliceEventHandler()), builder.WithPredicates(r.endpointSliceFilter()), ). + // Watch for changes to ClusterProperty objects. If a ClusterProperty object is + // created, updated or deleted, the controller will reconcile all service exports + Watches( + &source.Kind{Type: &aboutv1alpha1.ClusterProperty{}}, + handler.EnqueueRequestsFromMapFunc(r.clusterPropertyEventHandler()), + ). Complete(r) } @@ -286,6 +307,26 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { } } +func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { + // Return reconcile requests for all services + return func(object client.Object) []reconcile.Request { + services := &v1.ServiceList{} + if err := r.Client.List(context.TODO(), services); err != nil { + r.Log.Error(err, "error listing services") + return nil + } + + result := make([]reconcile.Request, 0) + for _, service := range services.Items { + result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{ + Name: service.Name, + Namespace: service.Namespace, + }}) + } + return result + } +} + func (r *ServiceExportReconciler) endpointSliceFilter() predicate.Funcs { return predicate.Funcs{ GenericFunc: func(e event.GenericEvent) bool { @@ -310,7 +351,7 @@ func (r *ServiceExportReconciler) doesEndpointSliceHaveServiceExport(object clie Name: serviceName, Namespace: object.GetNamespace(), } - svcExport := v1alpha1.ServiceExport{} + svcExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(context.TODO(), ns, &svcExport); err != nil { return false } diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 0fb3b69f..bc91ee4f 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -4,6 +4,7 @@ import ( "context" cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -27,7 +29,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -47,7 +49,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) mock.EXPECT().RegisterEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, - []*model.Endpoint{test.GetTestEndpoint1()}).Return(nil).Times(1) + []*model.Endpoint{test.GetTestEndpoint1WithAttr()}).Return(nil).Times(1) reconciler := getServiceExportReconciler(t, mock, fakeClient) @@ -164,6 +166,7 @@ func getServiceExportScheme() *runtime.Scheme { scheme.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) return scheme } @@ -175,3 +178,14 @@ func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServi CloudMap: mockClient, } } + +func clusterIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: "test_clusterid", + }, + } +} From 317683f9d1072b06b03329e77dd91c2fe24e4f72 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:03:19 +0000 Subject: [PATCH 07/30] fix commit - clusterid recon + synced --- .../bases/about.k8s.io_clusterproperties.yaml | 65 ++++++++++ config/crd/kustomization.yaml | 6 + .../annotation_for_clusterproperties.yaml | 7 ++ .../cainjection_in_clusterproperties.yaml | 7 ++ .../patches/webhook_in_clusterproperties.yaml | 16 +++ .../about_v1alpha1_clusterproperty.yaml | 9 ++ go.mod | 2 +- go.sum | 2 + main.go | 9 +- .../about/v1alpha1/clusterproperty_types.go | 52 ++++++++ pkg/apis/about/v1alpha1/groupversion_info.go | 20 +++ .../about/v1alpha1/zz_generated.deepcopy.go | 115 ++++++++++++++++++ .../multicluster/serviceexport_controller.go | 53 +++++++- .../serviceexport_controller_test.go | 18 ++- 14 files changed, 369 insertions(+), 12 deletions(-) create mode 100644 config/crd/bases/about.k8s.io_clusterproperties.yaml create mode 100644 config/crd/patches/annotation_for_clusterproperties.yaml create mode 100644 config/crd/patches/cainjection_in_clusterproperties.yaml create mode 100644 config/crd/patches/webhook_in_clusterproperties.yaml create mode 100644 config/samples/about_v1alpha1_clusterproperty.yaml create mode 100644 pkg/apis/about/v1alpha1/clusterproperty_types.go create mode 100644 pkg/apis/about/v1alpha1/groupversion_info.go create mode 100644 pkg/apis/about/v1alpha1/zz_generated.deepcopy.go diff --git a/config/crd/bases/about.k8s.io_clusterproperties.yaml b/config/crd/bases/about.k8s.io_clusterproperties.yaml new file mode 100644 index 00000000..71b36cf2 --- /dev/null +++ b/config/crd/bases/about.k8s.io_clusterproperties.yaml @@ -0,0 +1,65 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: clusterproperties.about.k8s.io +spec: + group: about.k8s.io + names: + kind: ClusterProperty + listKind: ClusterPropertyList + plural: clusterproperties + singular: clusterproperty + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.value + name: value + type: string + - jsonPath: .metadata.creationTimestamp + name: age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: ClusterProperty is the Schema for the clusterproperties API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: ClusterPropertySpec defines the desired state of ClusterProperty + properties: + value: + description: ClusterProperty value + minLength: 1 + type: string + required: + - value + type: object + status: + description: ClusterPropertyStatus defines the observed state of ClusterProperty + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8dac89bc..6dc8a039 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -2,6 +2,7 @@ # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default resources: +- bases/about.k8s.io_clusterproperties.yaml - bases/multicluster.x-k8s.io_serviceexports.yaml - bases/multicluster.x-k8s.io_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizeresource @@ -9,16 +10,21 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD +#- patches/webhook_in_clusterproperties.yaml #- patches/webhook_in_serviceexports.yaml #- patches/webhook_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD +#- patches/cainjection_in_clusterproperties.yaml #- patches/cainjection_in_serviceexports.yaml #- patches/cainjection_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch +# Patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +- patches/annotation_for_clusterproperties.yaml + # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: - kustomizeconfig.yaml diff --git a/config/crd/patches/annotation_for_clusterproperties.yaml b/config/crd/patches/annotation_for_clusterproperties.yaml new file mode 100644 index 00000000..9ceefc79 --- /dev/null +++ b/config/crd/patches/annotation_for_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.kubernetes.io: "https://github.com/kubernetes/kubernetes/pull/78458" + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/cainjection_in_clusterproperties.yaml b/config/crd/patches/cainjection_in_clusterproperties.yaml new file mode 100644 index 00000000..31e5f39b --- /dev/null +++ b/config/crd/patches/cainjection_in_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/webhook_in_clusterproperties.yaml b/config/crd/patches/webhook_in_clusterproperties.yaml new file mode 100644 index 00000000..c1880095 --- /dev/null +++ b/config/crd/patches/webhook_in_clusterproperties.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: clusterproperties.about.k8s.io +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml new file mode 100644 index 00000000..818301ef --- /dev/null +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -0,0 +1,9 @@ +# An example object of `id.k8s.io ClusterProperty` +# using a kube-system ns uuid as the id value: + +apiVersion: about.k8s.io/v1alpha1 +kind: ClusterProperty +metadata: + name: id.k8s.io +spec: + value: matthew-cluster-1 diff --git a/go.mod b/go.mod index 7a681e33..4cd07628 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( github.com/google/gnostic v0.5.7-v3refs // indirect github.com/google/go-cmp v0.5.8 // indirect github.com/google/gofuzz v1.1.0 // indirect - github.com/google/uuid v1.1.2 // indirect + github.com/google/uuid v1.3.0 // indirect github.com/imdario/mergo v0.3.12 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect diff --git a/go.sum b/go.sum index 1ee146f4..936fb02c 100644 --- a/go.sum +++ b/go.sum @@ -282,6 +282,8 @@ github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLe github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/main.go b/main.go index 21ca9881..b483ec82 100644 --- a/main.go +++ b/main.go @@ -22,8 +22,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" - controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" + multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" // +kubebuilder:scaffold:imports ) @@ -36,6 +37,8 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(multiclusterv1alpha1.AddToScheme(scheme)) + + utilruntime.Must(aboutv1alpha1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } @@ -85,7 +88,7 @@ func main() { log.Info("Running with AWS region", "AWS_REGION", awsCfg.Region) serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg) - if err = (&controllers.ServiceExportReconciler{ + if err = (&multiclustercontrollers.ServiceExportReconciler{ Client: mgr.GetClient(), Log: common.NewLogger("controllers", "ServiceExport"), Scheme: mgr.GetScheme(), @@ -95,7 +98,7 @@ func main() { os.Exit(1) } - cloudMapReconciler := &controllers.CloudMapReconciler{ + cloudMapReconciler := &multiclustercontrollers.CloudMapReconciler{ Client: mgr.GetClient(), Cloudmap: serviceDiscoveryClient, Log: common.NewLogger("controllers", "Cloudmap"), diff --git a/pkg/apis/about/v1alpha1/clusterproperty_types.go b/pkg/apis/about/v1alpha1/clusterproperty_types.go new file mode 100644 index 00000000..f3fd599e --- /dev/null +++ b/pkg/apis/about/v1alpha1/clusterproperty_types.go @@ -0,0 +1,52 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ClusterPropertySpec defines the desired state of ClusterProperty +type ClusterPropertySpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ClusterProperty value + // +kubebuilder:validation:Maxlength=128000 + // +kubebuilder:validation:MinLength=1 + Value string `json:"value"` +} + +// ClusterPropertyStatus defines the observed state of ClusterProperty +type ClusterPropertyStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:resource:scope=Cluster + +// ClusterProperty is the Schema for the clusterproperties API +// +kubebuilder:printcolumn:name="value",type=string,JSONPath=`.spec.value` +// +kubebuilder:printcolumn:name="age",type=date,JSONPath=`.metadata.creationTimestamp` +type ClusterProperty struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ClusterPropertySpec `json:"spec,omitempty"` + Status ClusterPropertyStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// ClusterPropertyList contains a list of ClusterProperty +type ClusterPropertyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ClusterProperty `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ClusterProperty{}, &ClusterPropertyList{}) +} diff --git a/pkg/apis/about/v1alpha1/groupversion_info.go b/pkg/apis/about/v1alpha1/groupversion_info.go new file mode 100644 index 00000000..82f56067 --- /dev/null +++ b/pkg/apis/about/v1alpha1/groupversion_info.go @@ -0,0 +1,20 @@ +// Package v1alpha1 contains API Schema definitions for the about v1alpha1 API group +//+kubebuilder:object:generate=true +//+groupName=about.k8s.io +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "about.k8s.io", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 00000000..476fb37f --- /dev/null +++ b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,115 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterProperty) DeepCopyInto(out *ClusterProperty) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterProperty. +func (in *ClusterProperty) DeepCopy() *ClusterProperty { + if in == nil { + return nil + } + out := new(ClusterProperty) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterProperty) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyList) DeepCopyInto(out *ClusterPropertyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterProperty, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyList. +func (in *ClusterPropertyList) DeepCopy() *ClusterPropertyList { + if in == nil { + return nil + } + out := new(ClusterPropertyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterPropertyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertySpec) DeepCopyInto(out *ClusterPropertySpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertySpec. +func (in *ClusterPropertySpec) DeepCopy() *ClusterPropertySpec { + if in == nil { + return nil + } + out := new(ClusterPropertySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyStatus) DeepCopyInto(out *ClusterPropertyStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyStatus. +func (in *ClusterPropertyStatus) DeepCopy() *ClusterPropertyStatus { + if in == nil { + return nil + } + out := new(ClusterPropertyStatus) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 38294c27..7eb1238a 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/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" @@ -24,10 +25,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" + multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" ) const ( + ClusterIdName = "id.k8s.io" + ClusterIdAttr = "CLUSTER_ID" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" @@ -54,7 +57,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques name := req.NamespacedName r.Log.Debug("reconciling ServiceExport", "Namespace", namespace, "Name", name) - serviceExport := v1alpha1.ServiceExport{} + serviceExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(ctx, name, &serviceExport); err != nil { if errors.IsNotFound(err) { r.Log.Debug("no ServiceExport found", @@ -92,7 +95,7 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.handleUpdate(ctx, &serviceExport, &service) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *v1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -185,7 +188,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context return cmService, nil } -func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *v1alpha1.ServiceExport) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport) (ctrl.Result, error) { if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -225,6 +228,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. return nil, err } + clusterId := &aboutv1alpha1.ClusterProperty{} + err = r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + } + servicePortMap := make(map[string]model.Port) for _, svcPort := range svc.Spec.Ports { servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) @@ -241,6 +250,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. if version.GetVersion() != "" { attributes[K8sVersionAttr] = version.PackageName + " " + version.GetVersion() } + + // Apply clusterID as endpoint attribute if it exists + if clusterId.Spec.Value != "" { + attributes[ClusterIdAttr] = clusterId.Spec.Value + } + // TODO extract attributes - pod, node and other useful details if possible port := EndpointPortToPort(endpointPort) @@ -261,7 +276,7 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.ServiceExport{}). + For(&multiclusterv1alpha1.ServiceExport{}). // Watch for the changes to the EndpointSlice object. This object is bound to be // updated when Service or Deployment are updated. There is also a filtering logic // to enqueue those EndpointSlice event which have corresponding ServiceExport @@ -270,6 +285,12 @@ func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.endpointSliceEventHandler()), builder.WithPredicates(r.endpointSliceFilter()), ). + // Watch for changes to ClusterProperty objects. If a ClusterProperty object is + // created, updated or deleted, the controller will reconcile all service exports + Watches( + &source.Kind{Type: &aboutv1alpha1.ClusterProperty{}}, + handler.EnqueueRequestsFromMapFunc(r.clusterPropertyEventHandler()), + ). Complete(r) } @@ -286,6 +307,26 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { } } +func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { + // Return reconcile requests for all services + return func(object client.Object) []reconcile.Request { + services := &v1.ServiceList{} + if err := r.Client.List(context.TODO(), services); err != nil { + r.Log.Error(err, "error listing services") + return nil + } + + result := make([]reconcile.Request, 0) + for _, service := range services.Items { + result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{ + Name: service.Name, + Namespace: service.Namespace, + }}) + } + return result + } +} + func (r *ServiceExportReconciler) endpointSliceFilter() predicate.Funcs { return predicate.Funcs{ GenericFunc: func(e event.GenericEvent) bool { @@ -310,7 +351,7 @@ func (r *ServiceExportReconciler) doesEndpointSliceHaveServiceExport(object clie Name: serviceName, Namespace: object.GetNamespace(), } - svcExport := v1alpha1.ServiceExport{} + svcExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(context.TODO(), ns, &svcExport); err != nil { return false } diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 0fb3b69f..bc91ee4f 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -4,6 +4,7 @@ import ( "context" cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -27,7 +29,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -47,7 +49,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) mock.EXPECT().RegisterEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, - []*model.Endpoint{test.GetTestEndpoint1()}).Return(nil).Times(1) + []*model.Endpoint{test.GetTestEndpoint1WithAttr()}).Return(nil).Times(1) reconciler := getServiceExportReconciler(t, mock, fakeClient) @@ -164,6 +166,7 @@ func getServiceExportScheme() *runtime.Scheme { scheme.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) return scheme } @@ -175,3 +178,14 @@ func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServi CloudMap: mockClient, } } + +func clusterIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: "test_clusterid", + }, + } +} From 83efd8ace476afe7367356f9b897d036a738d33e Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:05:11 +0000 Subject: [PATCH 08/30] fix controller --- pkg/controllers/multicluster/cloudmap_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 56f9ce99..e9fae9e2 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -129,6 +129,9 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } + // For each port in importedSvcPortsByClusterID, create derived services + // importedSvcPortsByClusterID = {"ports": {"matthew-cluster-1":[{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]}} + derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { From ecb982a12a9e8d6afa686f56d91d243e2787dc83 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:11:25 +0000 Subject: [PATCH 09/30] test constants --- test/test-constants.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/test-constants.go b/test/test-constants.go index efd10116..4ead3969 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -13,6 +13,8 @@ const ( DnsNsId = "dns-ns-id" SvcName = "svc-name" SvcId = "svc-id" + ClusterIdName = "CLUSTER_ID" + ClusterIdValue = "test_clusterid" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -104,6 +106,27 @@ func GetTestEndpoint2() *model.Endpoint { } } +func GetTestEndpoint1WithAttr() *model.Endpoint { + return &model.Endpoint{ + Id: EndptId1, + IP: EndptIp1, + EndpointPort: model.Port{ + Name: PortName1, + Port: Port1, + Protocol: Protocol1, + }, + ServicePort: model.Port{ + Name: PortName1, + Port: ServicePort1, + TargetPort: PortStr1, + Protocol: Protocol1, + }, + Attributes: map[string]string{ + ClusterIdName: ClusterIdValue, + }, + } +} + func GetTestEndpoints(count int) (endpts []*model.Endpoint) { // use +3 offset go avoid collision with test endpoint 1 and 2 for i := 3; i < count+3; i++ { From 875f1a6cd86a7b93d75eec753ae4049294a9448c Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 18:05:06 +0000 Subject: [PATCH 10/30] recon --- .../multicluster/cloudmap_controller.go | 19 +++++++++++++++++++ pkg/controllers/multicluster/utils.go | 4 +++- pkg/model/types.go | 1 + 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 56f9ce99..1cfff084 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -117,6 +117,18 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se importedSvcPorts := ExtractServicePorts(svc.Endpoints) + r.Log.Debug("service ports", "ports", importedSvcPorts) + + // service ports: {"ports": [{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]} + + // separate importedSvcPorts by unique ClusterID + importedSvcPortsByClusterID := make(map[string][]model.Port) + for _, p := range importedSvcPorts { + importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]] = append(importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]], *p) + } + + r.Log.Debug("service ports by cluster ID", "ports", importedSvcPortsByClusterID) + svcImport, err := r.getServiceImport(ctx, svc.Namespace, svc.Name) if err != nil { if !errors.IsNotFound(err) { @@ -129,6 +141,12 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } + // For each id in importedSvcPortsByClusterID, create derived services + for clusterID, importedSvcPortsbyId := range importedSvcPortsByClusterID { + r.Log.Debug("der-syncing cluster", "clusterID", clusterID) + r.Log.Debug("der-service ports", "ports", importedSvcPortsbyId) + } + derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { @@ -171,6 +189,7 @@ func (r *CloudMapReconciler) createAndGetServiceImport(ctx context.Context, name } func (r *CloudMapReconciler) getDerivedService(ctx context.Context, namespace string, name string) (*v1.Service, error) { + r.Log.Debug("fetching derived service", "namespace", namespace, "name", name) existingService := &v1.Service{} err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, existingService) return existingService, err diff --git a/pkg/controllers/multicluster/utils.go b/pkg/controllers/multicluster/utils.go index eb4c70d1..6f670f29 100644 --- a/pkg/controllers/multicluster/utils.go +++ b/pkg/controllers/multicluster/utils.go @@ -89,7 +89,9 @@ func PortToEndpointPort(port model.Port) discovery.EndpointPort { func ExtractServicePorts(endpoints []*model.Endpoint) (servicePorts []*model.Port) { uniquePorts := make(map[string]model.Port) for _, ep := range endpoints { - uniquePorts[ep.ServicePort.GetID()] = ep.ServicePort + servicePort := ep.ServicePort + servicePort.Attributes = ep.Attributes + uniquePorts[ep.ServicePort.GetID()] = servicePort } for _, servicePort := range uniquePorts { portRef := servicePort diff --git a/pkg/model/types.go b/pkg/model/types.go index edbca8ff..4b04ebfa 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -47,6 +47,7 @@ type Port struct { Port int32 TargetPort string Protocol string // TCP, UDP, SCTP + Attributes map[string]string } // Cloudmap Instances IP and Port is supposed to be AWS_INSTANCE_IPV4 and AWS_INSTANCE_PORT From 225dc65989be59c13f17160313045588fe5455b5 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:02:01 +0000 Subject: [PATCH 11/30] changes to test --- config/samples/about_v1alpha1_clusterproperty.yaml | 2 +- pkg/controllers/multicluster/serviceexport_controller.go | 9 +++++++++ pkg/controllers/multicluster/suite_test.go | 8 +++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml index 818301ef..4a547e03 100644 --- a/config/samples/about_v1alpha1_clusterproperty.yaml +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -6,4 +6,4 @@ kind: ClusterProperty metadata: name: id.k8s.io spec: - value: matthew-cluster-1 + value: sample-mcs-clusterid diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 7eb1238a..1001f5eb 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -72,6 +72,15 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Mark ServiceExport to be deleted, which is indicated by the deletion timestamp being set. isServiceExportMarkedForDelete := serviceExport.GetDeletionTimestamp() != nil + clusterId := &aboutv1alpha1.ClusterProperty{} + err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + return ctrl.Result{}, err + } else { + r.Log.Info("ClusterID found", "ClusterID", clusterId) + } + service := v1.Service{} namespacedName := types.NamespacedName{Namespace: serviceExport.Namespace, Name: serviceExport.Name} if err := r.Client.Get(ctx, namespacedName, &service); err != nil { diff --git a/pkg/controllers/multicluster/suite_test.go b/pkg/controllers/multicluster/suite_test.go index c37817c3..952f7d0f 100644 --- a/pkg/controllers/multicluster/suite_test.go +++ b/pkg/controllers/multicluster/suite_test.go @@ -37,8 +37,14 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") + + crds := []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceexports.yaml"), + filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceimports.yaml"), + } + testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, + CRDDirectoryPaths: crds, ErrorIfCRDPathMissing: true, } From 1ebd09cc33439ba6f00842445095c1cf4ecc85c5 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:08:23 +0000 Subject: [PATCH 12/30] cm controller --- .../multicluster/cloudmap_controller.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 1cfff084..3360cbb4 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -117,18 +117,6 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se importedSvcPorts := ExtractServicePorts(svc.Endpoints) - r.Log.Debug("service ports", "ports", importedSvcPorts) - - // service ports: {"ports": [{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]} - - // separate importedSvcPorts by unique ClusterID - importedSvcPortsByClusterID := make(map[string][]model.Port) - for _, p := range importedSvcPorts { - importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]] = append(importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]], *p) - } - - r.Log.Debug("service ports by cluster ID", "ports", importedSvcPortsByClusterID) - svcImport, err := r.getServiceImport(ctx, svc.Namespace, svc.Name) if err != nil { if !errors.IsNotFound(err) { @@ -141,12 +129,6 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } - // For each id in importedSvcPortsByClusterID, create derived services - for clusterID, importedSvcPortsbyId := range importedSvcPortsByClusterID { - r.Log.Debug("der-syncing cluster", "clusterID", clusterID) - r.Log.Debug("der-service ports", "ports", importedSvcPortsbyId) - } - derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { From eb48ca3f5806e2787cfd1176c0d4d92f3a1b3e2e Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:13:46 +0000 Subject: [PATCH 13/30] controllers --- integration/scenarios/export_service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/scenarios/export_service.go b/integration/scenarios/export_service.go index 8ae3a571..45b0b864 100644 --- a/integration/scenarios/export_service.go +++ b/integration/scenarios/export_service.go @@ -8,7 +8,7 @@ import ( "time" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" - controllers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" + multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" "github.com/aws/aws-sdk-go-v2/aws" v1 "k8s.io/api/core/v1" @@ -107,7 +107,7 @@ func (e *exportServiceScenario) compareEndpoints(cmEndpoints []*model.Endpoint) match := false for _, actual := range cmEndpoints { // Ignore K8S instance attribute for the purpose of this test. - delete(actual.Attributes, controllers.K8sVersionAttr) + delete(actual.Attributes, multiclustercontrollers.K8sVersionAttr) if expected.Equals(actual) { match = true break From da594385088f1eff8c130825febbd7d6080ed002 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:03:19 +0000 Subject: [PATCH 14/30] fix commit - clusterid recon + synced --- .../bases/about.k8s.io_clusterproperties.yaml | 65 ++++++++++ config/crd/kustomization.yaml | 6 + .../annotation_for_clusterproperties.yaml | 7 ++ .../cainjection_in_clusterproperties.yaml | 7 ++ .../patches/webhook_in_clusterproperties.yaml | 16 +++ .../about_v1alpha1_clusterproperty.yaml | 9 ++ main.go | 3 + .../about/v1alpha1/clusterproperty_types.go | 52 ++++++++ pkg/apis/about/v1alpha1/groupversion_info.go | 20 +++ .../about/v1alpha1/zz_generated.deepcopy.go | 115 ++++++++++++++++++ 10 files changed, 300 insertions(+) create mode 100644 config/crd/bases/about.k8s.io_clusterproperties.yaml create mode 100644 config/crd/patches/annotation_for_clusterproperties.yaml create mode 100644 config/crd/patches/cainjection_in_clusterproperties.yaml create mode 100644 config/crd/patches/webhook_in_clusterproperties.yaml create mode 100644 config/samples/about_v1alpha1_clusterproperty.yaml create mode 100644 pkg/apis/about/v1alpha1/clusterproperty_types.go create mode 100644 pkg/apis/about/v1alpha1/groupversion_info.go create mode 100644 pkg/apis/about/v1alpha1/zz_generated.deepcopy.go diff --git a/config/crd/bases/about.k8s.io_clusterproperties.yaml b/config/crd/bases/about.k8s.io_clusterproperties.yaml new file mode 100644 index 00000000..71b36cf2 --- /dev/null +++ b/config/crd/bases/about.k8s.io_clusterproperties.yaml @@ -0,0 +1,65 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: clusterproperties.about.k8s.io +spec: + group: about.k8s.io + names: + kind: ClusterProperty + listKind: ClusterPropertyList + plural: clusterproperties + singular: clusterproperty + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.value + name: value + type: string + - jsonPath: .metadata.creationTimestamp + name: age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: ClusterProperty is the Schema for the clusterproperties API + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: ClusterPropertySpec defines the desired state of ClusterProperty + properties: + value: + description: ClusterProperty value + minLength: 1 + type: string + required: + - value + type: object + status: + description: ClusterPropertyStatus defines the observed state of ClusterProperty + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 8dac89bc..6dc8a039 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -2,6 +2,7 @@ # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default resources: +- bases/about.k8s.io_clusterproperties.yaml - bases/multicluster.x-k8s.io_serviceexports.yaml - bases/multicluster.x-k8s.io_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizeresource @@ -9,16 +10,21 @@ resources: patchesStrategicMerge: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD +#- patches/webhook_in_clusterproperties.yaml #- patches/webhook_in_serviceexports.yaml #- patches/webhook_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD +#- patches/cainjection_in_clusterproperties.yaml #- patches/cainjection_in_serviceexports.yaml #- patches/cainjection_in_serviceimports.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch +# Patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +- patches/annotation_for_clusterproperties.yaml + # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: - kustomizeconfig.yaml diff --git a/config/crd/patches/annotation_for_clusterproperties.yaml b/config/crd/patches/annotation_for_clusterproperties.yaml new file mode 100644 index 00000000..9ceefc79 --- /dev/null +++ b/config/crd/patches/annotation_for_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds an annotation to pass protected groups approval required to use domain "k8s.io" +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.kubernetes.io: "https://github.com/kubernetes/kubernetes/pull/78458" + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/cainjection_in_clusterproperties.yaml b/config/crd/patches/cainjection_in_clusterproperties.yaml new file mode 100644 index 00000000..31e5f39b --- /dev/null +++ b/config/crd/patches/cainjection_in_clusterproperties.yaml @@ -0,0 +1,7 @@ +# The following patch adds a directive for certmanager to inject CA into the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + name: clusterproperties.about.k8s.io diff --git a/config/crd/patches/webhook_in_clusterproperties.yaml b/config/crd/patches/webhook_in_clusterproperties.yaml new file mode 100644 index 00000000..c1880095 --- /dev/null +++ b/config/crd/patches/webhook_in_clusterproperties.yaml @@ -0,0 +1,16 @@ +# The following patch enables a conversion webhook for the CRD +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: clusterproperties.about.k8s.io +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: system + name: webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml new file mode 100644 index 00000000..4a547e03 --- /dev/null +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -0,0 +1,9 @@ +# An example object of `id.k8s.io ClusterProperty` +# using a kube-system ns uuid as the id value: + +apiVersion: about.k8s.io/v1alpha1 +kind: ClusterProperty +metadata: + name: id.k8s.io +spec: + value: sample-mcs-clusterid diff --git a/main.go b/main.go index 23483879..b483ec82 100644 --- a/main.go +++ b/main.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" // +kubebuilder:scaffold:imports @@ -36,6 +37,8 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(multiclusterv1alpha1.AddToScheme(scheme)) + + utilruntime.Must(aboutv1alpha1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } diff --git a/pkg/apis/about/v1alpha1/clusterproperty_types.go b/pkg/apis/about/v1alpha1/clusterproperty_types.go new file mode 100644 index 00000000..f3fd599e --- /dev/null +++ b/pkg/apis/about/v1alpha1/clusterproperty_types.go @@ -0,0 +1,52 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ClusterPropertySpec defines the desired state of ClusterProperty +type ClusterPropertySpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ClusterProperty value + // +kubebuilder:validation:Maxlength=128000 + // +kubebuilder:validation:MinLength=1 + Value string `json:"value"` +} + +// ClusterPropertyStatus defines the observed state of ClusterProperty +type ClusterPropertyStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file +} + +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:resource:scope=Cluster + +// ClusterProperty is the Schema for the clusterproperties API +// +kubebuilder:printcolumn:name="value",type=string,JSONPath=`.spec.value` +// +kubebuilder:printcolumn:name="age",type=date,JSONPath=`.metadata.creationTimestamp` +type ClusterProperty struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ClusterPropertySpec `json:"spec,omitempty"` + Status ClusterPropertyStatus `json:"status,omitempty"` +} + +//+kubebuilder:object:root=true + +// ClusterPropertyList contains a list of ClusterProperty +type ClusterPropertyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ClusterProperty `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ClusterProperty{}, &ClusterPropertyList{}) +} diff --git a/pkg/apis/about/v1alpha1/groupversion_info.go b/pkg/apis/about/v1alpha1/groupversion_info.go new file mode 100644 index 00000000..82f56067 --- /dev/null +++ b/pkg/apis/about/v1alpha1/groupversion_info.go @@ -0,0 +1,20 @@ +// Package v1alpha1 contains API Schema definitions for the about v1alpha1 API group +//+kubebuilder:object:generate=true +//+groupName=about.k8s.io +package v1alpha1 + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/scheme" +) + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "about.k8s.io", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + + // AddToScheme adds the types in this group-version to the given scheme. + AddToScheme = SchemeBuilder.AddToScheme +) diff --git a/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 00000000..476fb37f --- /dev/null +++ b/pkg/apis/about/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,115 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterProperty) DeepCopyInto(out *ClusterProperty) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + out.Status = in.Status +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterProperty. +func (in *ClusterProperty) DeepCopy() *ClusterProperty { + if in == nil { + return nil + } + out := new(ClusterProperty) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterProperty) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyList) DeepCopyInto(out *ClusterPropertyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterProperty, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyList. +func (in *ClusterPropertyList) DeepCopy() *ClusterPropertyList { + if in == nil { + return nil + } + out := new(ClusterPropertyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ClusterPropertyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertySpec) DeepCopyInto(out *ClusterPropertySpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertySpec. +func (in *ClusterPropertySpec) DeepCopy() *ClusterPropertySpec { + if in == nil { + return nil + } + out := new(ClusterPropertySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ClusterPropertyStatus) DeepCopyInto(out *ClusterPropertyStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPropertyStatus. +func (in *ClusterPropertyStatus) DeepCopy() *ClusterPropertyStatus { + if in == nil { + return nil + } + out := new(ClusterPropertyStatus) + in.DeepCopyInto(out) + return out +} From e2ecb4c47ac8d0460e5dceae190cf76c1a3a5915 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:05:11 +0000 Subject: [PATCH 15/30] fix controller --- pkg/controllers/multicluster/cloudmap_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index a9fe22fd..bb3cfa6f 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -129,6 +129,9 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } + // For each port in importedSvcPortsByClusterID, create derived services + // importedSvcPortsByClusterID = {"ports": {"matthew-cluster-1":[{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]}} + derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { From afc57c5ef6e478be51a05b0ea869b2a2b7d2f14a Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 16:58:43 +0000 Subject: [PATCH 16/30] clusterid sync with cloudmap + recognized --- .../multicluster/serviceexport_controller.go | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index c6a31685..7eb1238a 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/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" @@ -28,6 +29,8 @@ import ( ) const ( + ClusterIdName = "id.k8s.io" + ClusterIdAttr = "CLUSTER_ID" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" @@ -225,6 +228,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. return nil, err } + clusterId := &aboutv1alpha1.ClusterProperty{} + err = r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + } + servicePortMap := make(map[string]model.Port) for _, svcPort := range svc.Spec.Ports { servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) @@ -241,6 +250,12 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. if version.GetVersion() != "" { attributes[K8sVersionAttr] = version.PackageName + " " + version.GetVersion() } + + // Apply clusterID as endpoint attribute if it exists + if clusterId.Spec.Value != "" { + attributes[ClusterIdAttr] = clusterId.Spec.Value + } + // TODO extract attributes - pod, node and other useful details if possible port := EndpointPortToPort(endpointPort) @@ -270,6 +285,12 @@ func (r *ServiceExportReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.endpointSliceEventHandler()), builder.WithPredicates(r.endpointSliceFilter()), ). + // Watch for changes to ClusterProperty objects. If a ClusterProperty object is + // created, updated or deleted, the controller will reconcile all service exports + Watches( + &source.Kind{Type: &aboutv1alpha1.ClusterProperty{}}, + handler.EnqueueRequestsFromMapFunc(r.clusterPropertyEventHandler()), + ). Complete(r) } @@ -286,6 +307,26 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { } } +func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { + // Return reconcile requests for all services + return func(object client.Object) []reconcile.Request { + services := &v1.ServiceList{} + if err := r.Client.List(context.TODO(), services); err != nil { + r.Log.Error(err, "error listing services") + return nil + } + + result := make([]reconcile.Request, 0) + for _, service := range services.Items { + result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{ + Name: service.Name, + Namespace: service.Namespace, + }}) + } + return result + } +} + func (r *ServiceExportReconciler) endpointSliceFilter() predicate.Funcs { return predicate.Funcs{ GenericFunc: func(e event.GenericEvent) bool { From b927ad93e68ca9f8d2f932eb96ba22b916f0b405 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 16:55:13 +0000 Subject: [PATCH 17/30] ClusterID recognized & watched for changes + sync --- .../multicluster/cloudmap_controller.go | 12 ++++++++++++ .../serviceexport_controller_test.go | 17 +++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index bb3cfa6f..9187eb24 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -117,6 +117,18 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se importedSvcPorts := ExtractServicePorts(svc.Endpoints) + r.Log.Debug("service ports", "ports", importedSvcPorts) + + // service ports: {"ports": [{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]} + + // separate importedSvcPorts by unique ClusterID + importedSvcPortsByClusterID := make(map[string][]model.Port) + for _, p := range importedSvcPorts { + importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]] = append(importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]], *p) + } + + r.Log.Debug("service ports by cluster ID", "ports", importedSvcPortsByClusterID) + svcImport, err := r.getServiceImport(ctx, svc.Namespace, svc.Name) if err != nil { if !errors.IsNotFound(err) { diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 327c9693..52242bec 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -27,7 +28,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -47,7 +48,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) mock.EXPECT().RegisterEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, - []*model.Endpoint{test.GetTestEndpoint1()}).Return(nil).Times(1) + []*model.Endpoint{test.GetTestEndpoint1WithAttr()}).Return(nil).Times(1) reconciler := getServiceExportReconciler(t, mock, fakeClient) @@ -164,6 +165,7 @@ func getServiceExportScheme() *runtime.Scheme { scheme.AddKnownTypes(multiclusterv1alpha1.GroupVersion, &multiclusterv1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) return scheme } @@ -175,3 +177,14 @@ func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServi CloudMap: mockClient, } } + +func clusterIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: "test_clusterid", + }, + } +} From 94691178e3eaf02cb5182b308a93d069fe205258 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 17:11:25 +0000 Subject: [PATCH 18/30] test constants --- test/test-constants.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/test-constants.go b/test/test-constants.go index efd10116..4ead3969 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -13,6 +13,8 @@ const ( DnsNsId = "dns-ns-id" SvcName = "svc-name" SvcId = "svc-id" + ClusterIdName = "CLUSTER_ID" + ClusterIdValue = "test_clusterid" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -104,6 +106,27 @@ func GetTestEndpoint2() *model.Endpoint { } } +func GetTestEndpoint1WithAttr() *model.Endpoint { + return &model.Endpoint{ + Id: EndptId1, + IP: EndptIp1, + EndpointPort: model.Port{ + Name: PortName1, + Port: Port1, + Protocol: Protocol1, + }, + ServicePort: model.Port{ + Name: PortName1, + Port: ServicePort1, + TargetPort: PortStr1, + Protocol: Protocol1, + }, + Attributes: map[string]string{ + ClusterIdName: ClusterIdValue, + }, + } +} + func GetTestEndpoints(count int) (endpts []*model.Endpoint) { // use +3 offset go avoid collision with test endpoint 1 and 2 for i := 3; i < count+3; i++ { From 7c38f361efbe8115083c6d0218515763fd5b7543 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 7 Jul 2022 18:05:06 +0000 Subject: [PATCH 19/30] recon --- pkg/controllers/multicluster/cloudmap_controller.go | 1 + pkg/controllers/multicluster/utils.go | 4 +++- pkg/model/types.go | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 9187eb24..ea4801bc 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -186,6 +186,7 @@ func (r *CloudMapReconciler) createAndGetServiceImport(ctx context.Context, name } func (r *CloudMapReconciler) getDerivedService(ctx context.Context, namespace string, name string) (*v1.Service, error) { + r.Log.Debug("fetching derived service", "namespace", namespace, "name", name) existingService := &v1.Service{} err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, existingService) return existingService, err diff --git a/pkg/controllers/multicluster/utils.go b/pkg/controllers/multicluster/utils.go index 75acbb94..54d46726 100644 --- a/pkg/controllers/multicluster/utils.go +++ b/pkg/controllers/multicluster/utils.go @@ -89,7 +89,9 @@ func PortToEndpointPort(port model.Port) discovery.EndpointPort { func ExtractServicePorts(endpoints []*model.Endpoint) (servicePorts []*model.Port) { uniquePorts := make(map[string]model.Port) for _, ep := range endpoints { - uniquePorts[ep.ServicePort.GetID()] = ep.ServicePort + servicePort := ep.ServicePort + servicePort.Attributes = ep.Attributes + uniquePorts[ep.ServicePort.GetID()] = servicePort } for _, servicePort := range uniquePorts { portRef := servicePort diff --git a/pkg/model/types.go b/pkg/model/types.go index edbca8ff..4b04ebfa 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -47,6 +47,7 @@ type Port struct { Port int32 TargetPort string Protocol string // TCP, UDP, SCTP + Attributes map[string]string } // Cloudmap Instances IP and Port is supposed to be AWS_INSTANCE_IPV4 and AWS_INSTANCE_PORT From 0807debe3e408db2090a33dcf64a9c20fddc7a85 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:02:01 +0000 Subject: [PATCH 20/30] changes to test --- pkg/controllers/multicluster/serviceexport_controller.go | 9 +++++++++ pkg/controllers/multicluster/suite_test.go | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 7eb1238a..1001f5eb 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -72,6 +72,15 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Mark ServiceExport to be deleted, which is indicated by the deletion timestamp being set. isServiceExportMarkedForDelete := serviceExport.GetDeletionTimestamp() != nil + clusterId := &aboutv1alpha1.ClusterProperty{} + err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterId") + return ctrl.Result{}, err + } else { + r.Log.Info("ClusterID found", "ClusterID", clusterId) + } + service := v1.Service{} namespacedName := types.NamespacedName{Namespace: serviceExport.Namespace, Name: serviceExport.Name} if err := r.Client.Get(ctx, namespacedName, &service); err != nil { diff --git a/pkg/controllers/multicluster/suite_test.go b/pkg/controllers/multicluster/suite_test.go index c37817c3..952f7d0f 100644 --- a/pkg/controllers/multicluster/suite_test.go +++ b/pkg/controllers/multicluster/suite_test.go @@ -37,8 +37,14 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") + + crds := []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceexports.yaml"), + filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceimports.yaml"), + } + testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, + CRDDirectoryPaths: crds, ErrorIfCRDPathMissing: true, } From e617634948a260cb0c4f981e60fd141ee5cdad4e Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:08:23 +0000 Subject: [PATCH 21/30] cm controller --- .../multicluster/cloudmap_controller.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index ea4801bc..bfbccc3f 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -117,18 +117,6 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se importedSvcPorts := ExtractServicePorts(svc.Endpoints) - r.Log.Debug("service ports", "ports", importedSvcPorts) - - // service ports: {"ports": [{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]} - - // separate importedSvcPorts by unique ClusterID - importedSvcPortsByClusterID := make(map[string][]model.Port) - for _, p := range importedSvcPorts { - importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]] = append(importedSvcPortsByClusterID[p.Attributes["CLUSTER_ID"]], *p) - } - - r.Log.Debug("service ports by cluster ID", "ports", importedSvcPortsByClusterID) - svcImport, err := r.getServiceImport(ctx, svc.Namespace, svc.Name) if err != nil { if !errors.IsNotFound(err) { @@ -141,9 +129,6 @@ func (r *CloudMapReconciler) reconcileService(ctx context.Context, svc *model.Se } } - // For each port in importedSvcPortsByClusterID, create derived services - // importedSvcPortsByClusterID = {"ports": {"matthew-cluster-1":[{"Name":"","Port":80,"TargetPort":"80","Protocol":"TCP","Attributes":{"CLUSTER_ID":"matthew-cluster-1","K8S_CONTROLLER":"aws-cloud-map-mcs-controller-for-k8s 0.2.3-21-gfe47782-dirty (fe47782-dirty)"}}]}} - derivedService, err := r.getDerivedService(ctx, svc.Namespace, svcImport.Annotations[DerivedServiceAnnotation]) if err != nil { if !errors.IsNotFound(err) { From 0bbf75428c30efef9266df3379ce17b3a19e9650 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:53:50 +0000 Subject: [PATCH 22/30] rm changes --- .../multicluster/cloudmap_controller.go | 1 - .../serviceexport_controller_test.go | 16 ++----------- pkg/controllers/multicluster/utils.go | 4 +--- pkg/model/types.go | 1 - test/test-constants.go | 23 ------------------- 5 files changed, 3 insertions(+), 42 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index bfbccc3f..a9fe22fd 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -171,7 +171,6 @@ func (r *CloudMapReconciler) createAndGetServiceImport(ctx context.Context, name } func (r *CloudMapReconciler) getDerivedService(ctx context.Context, namespace string, name string) (*v1.Service, error) { - r.Log.Debug("fetching derived service", "namespace", namespace, "name", name) existingService := &v1.Service{} err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, existingService) return existingService, err diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 52242bec..d65d98a6 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -28,7 +28,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -48,7 +48,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) mock.EXPECT().RegisterEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, - []*model.Endpoint{test.GetTestEndpoint1WithAttr()}).Return(nil).Times(1) + []*model.Endpoint{test.GetTestEndpoint1()}).Return(nil).Times(1) reconciler := getServiceExportReconciler(t, mock, fakeClient) @@ -165,7 +165,6 @@ func getServiceExportScheme() *runtime.Scheme { scheme.AddKnownTypes(multiclusterv1alpha1.GroupVersion, &multiclusterv1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) - scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) return scheme } @@ -177,14 +176,3 @@ func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServi CloudMap: mockClient, } } - -func clusterIdForTest() *aboutv1alpha1.ClusterProperty { - return &aboutv1alpha1.ClusterProperty{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterIdName, - }, - Spec: aboutv1alpha1.ClusterPropertySpec{ - Value: "test_clusterid", - }, - } -} diff --git a/pkg/controllers/multicluster/utils.go b/pkg/controllers/multicluster/utils.go index 54d46726..75acbb94 100644 --- a/pkg/controllers/multicluster/utils.go +++ b/pkg/controllers/multicluster/utils.go @@ -89,9 +89,7 @@ func PortToEndpointPort(port model.Port) discovery.EndpointPort { func ExtractServicePorts(endpoints []*model.Endpoint) (servicePorts []*model.Port) { uniquePorts := make(map[string]model.Port) for _, ep := range endpoints { - servicePort := ep.ServicePort - servicePort.Attributes = ep.Attributes - uniquePorts[ep.ServicePort.GetID()] = servicePort + uniquePorts[ep.ServicePort.GetID()] = ep.ServicePort } for _, servicePort := range uniquePorts { portRef := servicePort diff --git a/pkg/model/types.go b/pkg/model/types.go index 4b04ebfa..edbca8ff 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -47,7 +47,6 @@ type Port struct { Port int32 TargetPort string Protocol string // TCP, UDP, SCTP - Attributes map[string]string } // Cloudmap Instances IP and Port is supposed to be AWS_INSTANCE_IPV4 and AWS_INSTANCE_PORT diff --git a/test/test-constants.go b/test/test-constants.go index 4ead3969..efd10116 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -13,8 +13,6 @@ const ( DnsNsId = "dns-ns-id" SvcName = "svc-name" SvcId = "svc-id" - ClusterIdName = "CLUSTER_ID" - ClusterIdValue = "test_clusterid" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -106,27 +104,6 @@ func GetTestEndpoint2() *model.Endpoint { } } -func GetTestEndpoint1WithAttr() *model.Endpoint { - return &model.Endpoint{ - Id: EndptId1, - IP: EndptIp1, - EndpointPort: model.Port{ - Name: PortName1, - Port: Port1, - Protocol: Protocol1, - }, - ServicePort: model.Port{ - Name: PortName1, - Port: ServicePort1, - TargetPort: PortStr1, - Protocol: Protocol1, - }, - Attributes: map[string]string{ - ClusterIdName: ClusterIdValue, - }, - } -} - func GetTestEndpoints(count int) (endpts []*model.Endpoint) { // use +3 offset go avoid collision with test endpoint 1 and 2 for i := 3; i < count+3; i++ { From 56514cd9e6b7c285006e29998f3f50de78b5d52e Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Wed, 13 Jul 2022 23:57:19 +0000 Subject: [PATCH 23/30] rm meta --- pkg/controllers/multicluster/serviceexport_controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index d65d98a6..327c9693 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" From 87b0dbf99517a69fcbd45f663843ee7c361fca2a Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 14 Jul 2022 00:50:08 +0000 Subject: [PATCH 24/30] rm return --- pkg/controllers/multicluster/serviceexport_controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 1001f5eb..d6c3d79e 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -76,7 +76,6 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) if err != nil { r.Log.Error(err, "error fetching ClusterId") - return ctrl.Result{}, err } else { r.Log.Info("ClusterID found", "ClusterID", clusterId) } From a8da27923c555e66ca3da8c9636e0f8dfde5ff65 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Sat, 16 Jul 2022 02:04:33 +0000 Subject: [PATCH 25/30] unit tests + clusterid error --- .../about_v1alpha1_clusterproperty.yaml | 1 - pkg/cloudmap/client_test.go | 4 ++ .../multicluster/controllers_common_test.go | 12 ++++++ .../multicluster/serviceexport_controller.go | 17 +++----- .../serviceexport_controller_test.go | 41 +++++++++++++++++-- pkg/model/types.go | 1 + test/test-constants.go | 10 ++++- 7 files changed, 69 insertions(+), 17 deletions(-) diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty.yaml index 4a547e03..76bc3b02 100644 --- a/config/samples/about_v1alpha1_clusterproperty.yaml +++ b/config/samples/about_v1alpha1_clusterproperty.yaml @@ -1,5 +1,4 @@ # An example object of `id.k8s.io ClusterProperty` -# using a kube-system ns uuid as the id value: apiVersion: about.k8s.io/v1alpha1 kind: ClusterProperty diff --git a/pkg/cloudmap/client_test.go b/pkg/cloudmap/client_test.go index c22b580e..a9daed77 100644 --- a/pkg/cloudmap/client_test.go +++ b/pkg/cloudmap/client_test.go @@ -283,6 +283,7 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { tc.mockCache.EXPECT().GetServiceIdMap(test.HttpNsName).Return(getServiceIdMapForTest(), true) attrs1 := map[string]string{ + model.ClusterIdAttrName: test.ClusterIdValue, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -293,6 +294,7 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { model.ServiceTargetPortAttr: test.PortStr1, } attrs2 := map[string]string{ + model.ClusterIdAttrName: test.ClusterIdValue, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, @@ -362,6 +364,7 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { { InstanceId: aws.String(test.EndptId1), Attributes: map[string]string{ + model.ClusterIdAttrName: test.ClusterIdValue, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -375,6 +378,7 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { { InstanceId: aws.String(test.EndptId2), Attributes: map[string]string{ + model.ClusterIdAttrName: test.ClusterIdValue, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, diff --git a/pkg/controllers/multicluster/controllers_common_test.go b/pkg/controllers/multicluster/controllers_common_test.go index 2917376a..36cccc60 100644 --- a/pkg/controllers/multicluster/controllers_common_test.go +++ b/pkg/controllers/multicluster/controllers_common_test.go @@ -1,6 +1,7 @@ package controllers import ( + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test" "github.com/aws/aws-sdk-go-v2/aws" @@ -83,3 +84,14 @@ func endpointSliceWithIpsAndPortsForTest(ips []string, ports []discovery.Endpoin return slice } + +func clusterPropertyWithIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: "test_clusterid", + }, + } +} diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index d6c3d79e..e23ca7e8 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -76,8 +76,9 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) if err != nil { r.Log.Error(err, "error fetching ClusterId") + return ctrl.Result{}, err } else { - r.Log.Info("ClusterID found", "ClusterID", clusterId) + r.Log.Debug("ClusterID found", "ClusterID", clusterId) } service := v1.Service{} @@ -100,10 +101,10 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.handleDelete(ctx, &serviceExport) } - return r.handleUpdate(ctx, &serviceExport, &service) + return r.handleUpdate(ctx, &serviceExport, &service, clusterId) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service, clusterId *aboutv1alpha1.ClusterProperty) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -134,7 +135,7 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor return ctrl.Result{}, err } - endpoints, err := r.extractEndpoints(ctx, service) + endpoints, err := r.extractEndpoints(ctx, service, clusterId) if err != nil { r.Log.Error(err, "error extracting Endpoints", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -225,7 +226,7 @@ func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExpor return ctrl.Result{}, nil } -func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service) ([]*model.Endpoint, error) { +func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service, clusterId *aboutv1alpha1.ClusterProperty) ([]*model.Endpoint, error) { result := make([]*model.Endpoint, 0) endpointSlices := discovery.EndpointSliceList{} @@ -236,12 +237,6 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. return nil, err } - clusterId := &aboutv1alpha1.ClusterProperty{} - err = r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) - if err != nil { - r.Log.Error(err, "error fetching ClusterId") - } - servicePortMap := make(map[string]model.Port) for _, svcPort := range svc.Spec.Ports { servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 327c9693..a4277445 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -2,8 +2,10 @@ package controllers import ( "context" + "fmt" cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" @@ -27,7 +29,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterPropertyWithIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -75,7 +77,7 @@ func TestServiceExportReconciler_Reconcile_ExistingServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterPropertyWithIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -122,7 +124,7 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { serviceExportObj.Finalizers = []string{ServiceExportFinalizer} fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(serviceExportObj). + WithObjects(serviceExportObj, clusterPropertyWithIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -159,8 +161,41 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { assert.Empty(t, serviceExport.Finalizers, "Finalizer removed from the service export") } +func TestServiceExportReconciler_Reconcile_NoClusterId(t *testing.T) { + // create a fake controller client and add some objects + fakeClient := fake.NewClientBuilder(). + WithScheme(getServiceExportScheme()). + WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithLists(&discovery.EndpointSliceList{ + Items: []discovery.EndpointSlice{*endpointSliceForTest()}, + }). + Build() + + // create a mock cloudmap service discovery client + mockController := gomock.NewController(t) + defer mockController.Finish() + + mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) + + reconciler := getServiceExportReconciler(t, mock, fakeClient) + + request := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: test.HttpNsName, + Name: test.SvcName, + }, + } + + // Reconciling should throw an error + got, err := reconciler.Reconcile(context.Background(), request) + expectedError := fmt.Errorf("clusterproperties.about.k8s.io \"id.k8s.io\" not found") + assert.ErrorContains(t, err, expectedError.Error()) + assert.Equal(t, ctrl.Result{}, got, "Result should be empty") +} + func getServiceExportScheme() *runtime.Scheme { scheme := runtime.NewScheme() + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) scheme.AddKnownTypes(multiclusterv1alpha1.GroupVersion, &multiclusterv1alpha1.ServiceExport{}) scheme.AddKnownTypes(v1.SchemeGroupVersion, &v1.Service{}) scheme.AddKnownTypes(discovery.SchemeGroupVersion, &discovery.EndpointSlice{}, &discovery.EndpointSliceList{}) diff --git a/pkg/model/types.go b/pkg/model/types.go index edbca8ff..cf514014 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -56,6 +56,7 @@ const ( EndpointPortAttr = "AWS_INSTANCE_PORT" EndpointPortNameAttr = "ENDPOINT_PORT_NAME" EndpointProtocolAttr = "ENDPOINT_PROTOCOL" + ClusterIdAttrName = "CLUSTER_ID" ServicePortNameAttr = "SERVICE_PORT_NAME" ServicePortAttr = "SERVICE_PORT" ServiceTargetPortAttr = "SERVICE_TARGET_PORT" diff --git a/test/test-constants.go b/test/test-constants.go index efd10116..477de060 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -13,6 +13,8 @@ const ( DnsNsId = "dns-ns-id" SvcName = "svc-name" SvcId = "svc-id" + ClusterIdName = "CLUSTER_ID" + ClusterIdValue = "test_clusterid" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -81,7 +83,9 @@ func GetTestEndpoint1() *model.Endpoint { TargetPort: PortStr1, Protocol: Protocol1, }, - Attributes: make(map[string]string), + Attributes: map[string]string{ + ClusterIdName: ClusterIdValue, + }, } } @@ -100,7 +104,9 @@ func GetTestEndpoint2() *model.Endpoint { TargetPort: PortStr2, Protocol: Protocol2, }, - Attributes: make(map[string]string), + Attributes: map[string]string{ + ClusterIdName: ClusterIdValue, + }, } } From b35bc75af21bfd685252b7216173fb3c3eb4fb29 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Thu, 21 Jul 2022 00:35:07 +0000 Subject: [PATCH 26/30] addition of clustersetid and other changes --- ...t_v1alpha1_clusterproperty_clusterId.yaml} | 0 ...v1alpha1_clusterproperty_clustersetId.yaml | 8 +++ integration/janitor/janitor.go | 10 +-- integration/janitor/janitor_test.go | 6 +- integration/janitor/runner/main.go | 7 +- .../shared/scenarios/export_service.go | 2 +- pkg/cloudmap/api.go | 7 +- pkg/cloudmap/api_test.go | 6 +- pkg/cloudmap/client.go | 16 ++--- pkg/cloudmap/client_test.go | 34 +++++---- .../multicluster/cloudmap_controller.go | 15 +++- .../multicluster/cloudmap_controller_test.go | 6 +- .../multicluster/controllers_common_test.go | 15 +++- .../multicluster/serviceexport_controller.go | 70 +++++++++++-------- .../serviceexport_controller_test.go | 14 ++-- pkg/model/types.go | 27 ++++++- pkg/model/types_test.go | 52 ++++++++++++++ test/test-constants.go | 17 ++--- 18 files changed, 221 insertions(+), 91 deletions(-) rename config/samples/{about_v1alpha1_clusterproperty.yaml => about_v1alpha1_clusterproperty_clusterId.yaml} (100%) create mode 100644 config/samples/about_v1alpha1_clusterproperty_clustersetId.yaml diff --git a/config/samples/about_v1alpha1_clusterproperty.yaml b/config/samples/about_v1alpha1_clusterproperty_clusterId.yaml similarity index 100% rename from config/samples/about_v1alpha1_clusterproperty.yaml rename to config/samples/about_v1alpha1_clusterproperty_clusterId.yaml diff --git a/config/samples/about_v1alpha1_clusterproperty_clustersetId.yaml b/config/samples/about_v1alpha1_clusterproperty_clustersetId.yaml new file mode 100644 index 00000000..10d3e82c --- /dev/null +++ b/config/samples/about_v1alpha1_clusterproperty_clustersetId.yaml @@ -0,0 +1,8 @@ +# An example object of `clusterset.k8s.io ClusterProperty`: + +apiVersion: about.k8s.io/v1alpha1 +kind: ClusterProperty +metadata: + name: clusterset.k8s.io +spec: + value: sample-mcs-clustersetid \ No newline at end of file diff --git a/integration/janitor/janitor.go b/integration/janitor/janitor.go index 24127bdf..a117c428 100644 --- a/integration/janitor/janitor.go +++ b/integration/janitor/janitor.go @@ -13,7 +13,7 @@ import ( // CloudMapJanitor handles AWS Cloud Map resource cleanup during integration tests. type CloudMapJanitor interface { // Cleanup removes all instances, services and the namespace from AWS Cloud Map for a given namespace name. - Cleanup(ctx context.Context, nsName string) + Cleanup(ctx context.Context, nsName string, clustersetId string) } type cloudMapJanitor struct { @@ -36,7 +36,7 @@ func NewDefaultJanitor() CloudMapJanitor { } } -func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) { +func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string, clustersetId string) { fmt.Printf("Cleaning up all test resources in Cloud Map for namespace : %s\n", nsName) nsMap, err := j.sdApi.GetNamespaceMap(ctx) @@ -57,7 +57,7 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) { for svcName, svcId := range svcIdMap { fmt.Printf("found service to clean: %s\n", svcId) - j.deregisterInstances(ctx, nsName, svcName, svcId) + j.deregisterInstances(ctx, nsName, svcName, svcId, clustersetId) delSvcErr := j.sdApi.DeleteService(ctx, svcId) j.checkOrFail(delSvcErr, "service deleted", "could not cleanup service") @@ -71,8 +71,8 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) { j.checkOrFail(err, "clean up successful", "could not cleanup namespace") } -func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, nsName string, svcName string, svcId string) { - insts, err := j.sdApi.DiscoverInstances(ctx, nsName, svcName) +func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, nsName string, svcName string, svcId string, clustersetId string) { + insts, err := j.sdApi.DiscoverInstances(ctx, nsName, svcName, clustersetId) j.checkOrFail(err, fmt.Sprintf("service has %d instances to clean", len(insts)), "could not list instances to cleanup") diff --git a/integration/janitor/janitor_test.go b/integration/janitor/janitor_test.go index 196125e4..0e2bbe3e 100644 --- a/integration/janitor/janitor_test.go +++ b/integration/janitor/janitor_test.go @@ -32,7 +32,7 @@ func TestCleanupHappyCase(t *testing.T) { 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). + tj.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). Return([]types.HttpInstanceSummary{{InstanceId: aws.String(test.EndptId1)}}, nil) tj.mockApi.EXPECT().DeregisterInstance(context.TODO(), test.SvcId, test.EndptId1). @@ -46,7 +46,7 @@ func TestCleanupHappyCase(t *testing.T) { tj.mockApi.EXPECT().PollNamespaceOperation(context.TODO(), test.OpId2). Return(test.HttpNsId, nil) - tj.janitor.Cleanup(context.TODO(), test.HttpNsName) + tj.janitor.Cleanup(context.TODO(), test.HttpNsName, test.ClustersetId) assert.False(t, *tj.failed) } @@ -57,7 +57,7 @@ func TestCleanupNothingToClean(t *testing.T) { tj.mockApi.EXPECT().GetNamespaceMap(context.TODO()). Return(map[string]*model.Namespace{}, nil) - tj.janitor.Cleanup(context.TODO(), test.HttpNsName) + tj.janitor.Cleanup(context.TODO(), test.HttpNsName, test.ClustersetId) assert.False(t, *tj.failed) } diff --git a/integration/janitor/runner/main.go b/integration/janitor/runner/main.go index 9711bfed..33a004b9 100644 --- a/integration/janitor/runner/main.go +++ b/integration/janitor/runner/main.go @@ -9,12 +9,13 @@ import ( ) func main() { - if len(os.Args) != 2 { - fmt.Println("Expected single namespace name argument") + if len(os.Args) != 3 { + fmt.Println("Expected single namespace name and clustersetId arguments") os.Exit(1) } j := janitor.NewDefaultJanitor() nsName := os.Args[1] - j.Cleanup(context.TODO(), nsName) + clustersetId := os.Args[2] + j.Cleanup(context.TODO(), nsName, clustersetId) } diff --git a/integration/shared/scenarios/export_service.go b/integration/shared/scenarios/export_service.go index 45b0b864..859ea8fa 100644 --- a/integration/shared/scenarios/export_service.go +++ b/integration/shared/scenarios/export_service.go @@ -82,7 +82,7 @@ func (e *exportServiceScenario) Run() error { return wait.Poll(defaultScenarioPollInterval, defaultScenarioPollTimeout, func() (done bool, err error) { fmt.Println("Polling service...") - cmSvc, err := e.sdClient.GetService(context.TODO(), e.expectedSvc.Namespace, e.expectedSvc.Name) + cmSvc, err := e.sdClient.GetService(context.TODO(), e.expectedSvc.Namespace, e.expectedSvc.Name, "") if err != nil { return true, err } diff --git a/pkg/cloudmap/api.go b/pkg/cloudmap/api.go index 38bad30f..5dd235f3 100644 --- a/pkg/cloudmap/api.go +++ b/pkg/cloudmap/api.go @@ -27,7 +27,7 @@ type ServiceDiscoveryApi interface { 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) + DiscoverInstances(ctx context.Context, nsName string, svcName string, clustersetId string) (insts []types.HttpInstanceSummary, err error) // ListOperations returns a map of operations to their status matching a list of filters. ListOperations(ctx context.Context, opFilters []types.OperationFilter) (operationStatusMap map[string]types.OperationStatus, err error) @@ -113,12 +113,15 @@ func (sdApi *serviceDiscoveryApi) GetServiceIdMap(ctx context.Context, nsId stri return serviceIdMap, nil } -func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error) { +func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string, clustersetId string) (insts []types.HttpInstanceSummary, err error) { out, err := sdApi.awsFacade.DiscoverInstances(ctx, &sd.DiscoverInstancesInput{ NamespaceName: aws.String(nsName), ServiceName: aws.String(svcName), HealthStatus: types.HealthStatusFilterAll, MaxResults: aws.Int32(1000), + QueryParameters: map[string]string{ + model.ClustersetIdAttr: clustersetId, + }, }) if err != nil { diff --git a/pkg/cloudmap/api_test.go b/pkg/cloudmap/api_test.go index 8d70d9c0..55a256a7 100644 --- a/pkg/cloudmap/api_test.go +++ b/pkg/cloudmap/api_test.go @@ -8,6 +8,7 @@ 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" @@ -101,6 +102,9 @@ func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) { ServiceName: aws.String(test.SvcName), HealthStatus: types.HealthStatusFilterAll, MaxResults: aws.Int32(1000), + QueryParameters: map[string]string{ + model.ClustersetIdAttr: test.ClustersetId, + }, }). Return(&sd.DiscoverInstancesOutput{ Instances: []types.HttpInstanceSummary{ @@ -109,7 +113,7 @@ func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) { }, }, nil) - insts, err := sdApi.DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName) + insts, err := sdApi.DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) assert.Nil(t, err, "No error for happy case") assert.True(t, len(insts) == 2) assert.Equal(t, test.EndptId1, *insts[0].InstanceId) diff --git a/pkg/cloudmap/client.go b/pkg/cloudmap/client.go index 66eae577..a9d10042 100644 --- a/pkg/cloudmap/client.go +++ b/pkg/cloudmap/client.go @@ -14,13 +14,13 @@ import ( // multi-cluster service discovery for Kubernetes controller. It maintains local caches for all AWS Cloud Map resources. type ServiceDiscoveryClient interface { // ListServices returns all services and their endpoints for a given namespace. - ListServices(ctx context.Context, namespaceName string) ([]*model.Service, error) + ListServices(ctx context.Context, namespaceName string, clustersetId string) ([]*model.Service, error) // CreateService creates a Cloud Map service resource, and namespace if necessary. CreateService(ctx context.Context, namespaceName string, serviceName string) error // GetService returns a service resource fetched from AWS Cloud Map or nil if not found. - GetService(ctx context.Context, namespaceName string, serviceName string) (*model.Service, error) + GetService(ctx context.Context, namespaceName string, serviceName string, clustersetId string) (*model.Service, error) // RegisterEndpoints registers all endpoints for given service. RegisterEndpoints(ctx context.Context, namespaceName string, serviceName string, endpoints []*model.Endpoint) error @@ -53,14 +53,14 @@ func NewServiceDiscoveryClientWithCustomCache(cfg *aws.Config, cacheConfig *SdCa } } -func (sdc *serviceDiscoveryClient) ListServices(ctx context.Context, nsName string) (svcs []*model.Service, err error) { +func (sdc *serviceDiscoveryClient) ListServices(ctx context.Context, nsName string, clustersetId string) (svcs []*model.Service, err error) { svcIdMap, err := sdc.getServiceIds(ctx, nsName) if err != nil { return svcs, err } for svcName := range svcIdMap { - endpts, endptsErr := sdc.getEndpoints(ctx, nsName, svcName) + endpts, endptsErr := sdc.getEndpoints(ctx, nsName, svcName, clustersetId) if endptsErr != nil { return svcs, endptsErr } @@ -102,7 +102,7 @@ func (sdc *serviceDiscoveryClient) CreateService(ctx context.Context, nsName str return nil } -func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string, svcName string) (svc *model.Service, err error) { +func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string, svcName string, clustersetId string) (svc *model.Service, err error) { sdc.log.Info("fetching a service", "namespace", nsName, "name", svcName) endpts, cacheHit := sdc.cache.GetEndpoints(nsName, svcName) @@ -123,7 +123,7 @@ func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string return nil, nil } - endpts, err = sdc.getEndpoints(ctx, nsName, svcName) + endpts, err = sdc.getEndpoints(ctx, nsName, svcName, clustersetId) if err != nil { return nil, err @@ -221,13 +221,13 @@ func (sdc *serviceDiscoveryClient) DeleteEndpoints(ctx context.Context, nsName s return nil } -func (sdc *serviceDiscoveryClient) getEndpoints(ctx context.Context, nsName string, svcName string) (endpts []*model.Endpoint, err error) { +func (sdc *serviceDiscoveryClient) getEndpoints(ctx context.Context, nsName string, svcName string, clustersetId string) (endpts []*model.Endpoint, err error) { endpts, cacheHit := sdc.cache.GetEndpoints(nsName, svcName) if cacheHit { return endpts, nil } - insts, err := sdc.sdApi.DiscoverInstances(ctx, nsName, svcName) + insts, err := sdc.sdApi.DiscoverInstances(ctx, nsName, svcName, clustersetId) if err != nil { return nil, err } diff --git a/pkg/cloudmap/client_test.go b/pkg/cloudmap/client_test.go index a9daed77..2ed8efd3 100644 --- a/pkg/cloudmap/client_test.go +++ b/pkg/cloudmap/client_test.go @@ -42,12 +42,12 @@ func TestServiceDiscoveryClient_ListServices_HappyCase(t *testing.T) { tc.mockCache.EXPECT().CacheServiceIdMap(test.HttpNsName, getServiceIdMapForTest()) tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return(nil, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). Return(getHttpInstanceSummaryForTest(), nil) tc.mockCache.EXPECT().CacheEndpoints(test.HttpNsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) assert.Equal(t, []*model.Service{test.GetTestService()}, svcs) assert.Nil(t, err, "No error for happy case") } @@ -64,7 +64,7 @@ func TestServiceDiscoveryClient_ListServices_HappyCaseCachedResults(t *testing.T tc.mockCache.EXPECT().GetEndpoints(test.DnsNsName, test.SvcName). Return([]*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}, true) - svcs, err := tc.client.ListServices(context.TODO(), test.DnsNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.DnsNsName, test.ClustersetId) assert.Equal(t, []*model.Service{dnsService}, svcs) assert.Nil(t, err, "No error for happy case") } @@ -79,7 +79,7 @@ func TestServiceDiscoveryClient_ListServices_NamespaceError(t *testing.T) { tc.mockCache.EXPECT().GetNamespaceMap().Return(nil, false) tc.mockApi.EXPECT().GetNamespaceMap(context.TODO()).Return(nil, nsErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) assert.Equal(t, nsErr, err) assert.Empty(t, svcs) } @@ -96,7 +96,7 @@ func TestServiceDiscoveryClient_ListServices_ServiceError(t *testing.T) { tc.mockApi.EXPECT().GetServiceIdMap(context.TODO(), test.HttpNsId). Return(nil, svcErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) assert.Equal(t, svcErr, err) assert.Empty(t, svcs) } @@ -109,10 +109,10 @@ func TestServiceDiscoveryClient_ListServices_InstanceError(t *testing.T) { endptErr := errors.New("error listing endpoints") tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return(nil, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). Return([]types.HttpInstanceSummary{}, endptErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) assert.Equal(t, endptErr, err) assert.Empty(t, svcs) } @@ -124,7 +124,7 @@ func TestServiceDiscoveryClient_ListServices_NamespaceNotFound(t *testing.T) { tc.mockCache.EXPECT().GetServiceIdMap(test.HttpNsName).Return(nil, false) tc.mockCache.EXPECT().GetNamespaceMap().Return(nil, true) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) assert.Empty(t, svcs) assert.Nil(t, err, "No error for namespace not found") } @@ -254,12 +254,12 @@ func TestServiceDiscoveryClient_GetService_HappyCase(t *testing.T) { tc.mockCache.EXPECT().CacheServiceIdMap(test.HttpNsName, getServiceIdMapForTest()) tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return([]*model.Endpoint{}, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). Return(getHttpInstanceSummaryForTest(), nil) tc.mockCache.EXPECT().CacheEndpoints(test.HttpNsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}) - svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName) + svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) assert.Nil(t, err) assert.Equal(t, test.GetTestService(), svc) } @@ -271,7 +271,7 @@ func TestServiceDiscoveryClient_GetService_CachedValues(t *testing.T) { tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName). Return([]*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}, true) - svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName) + svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) assert.Nil(t, err) assert.Equal(t, test.GetTestService(), svc) } @@ -283,7 +283,8 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { tc.mockCache.EXPECT().GetServiceIdMap(test.HttpNsName).Return(getServiceIdMapForTest(), true) attrs1 := map[string]string{ - model.ClusterIdAttrName: test.ClusterIdValue, + model.ClusterIdAttr: test.ClusterId, + model.ClustersetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -294,7 +295,8 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { model.ServiceTargetPortAttr: test.PortStr1, } attrs2 := map[string]string{ - model.ClusterIdAttrName: test.ClusterIdValue, + model.ClusterIdAttr: test.ClusterId, + model.ClustersetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, @@ -364,7 +366,8 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { { InstanceId: aws.String(test.EndptId1), Attributes: map[string]string{ - model.ClusterIdAttrName: test.ClusterIdValue, + model.ClusterIdAttr: test.ClusterId, + model.ClustersetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -378,7 +381,8 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { { InstanceId: aws.String(test.EndptId2), Attributes: map[string]string{ - model.ClusterIdAttrName: test.ClusterIdValue, + model.ClusterIdAttr: test.ClusterId, + model.ClustersetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index a9fe22fd..c0a87455 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" @@ -53,6 +54,14 @@ func (r *CloudMapReconciler) Start(ctx context.Context) error { // Reconcile triggers a single reconciliation round func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { + clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} + err := r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) + if err != nil { + r.Log.Error(err, "error getting ClusterProperty for clustersetId") + return err + } + clustersetId := clusterPropertyForClustersetId.Spec.Value + namespaces := v1.NamespaceList{} if err := r.Client.List(ctx, &namespaces); err != nil { r.Log.Error(err, "unable to list cluster namespaces") @@ -60,7 +69,7 @@ func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { } for _, ns := range namespaces.Items { - if err := r.reconcileNamespace(ctx, ns.Name); err != nil { + if err := r.reconcileNamespace(ctx, ns.Name, clustersetId); err != nil { return err } } @@ -68,10 +77,10 @@ func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { return nil } -func (r *CloudMapReconciler) reconcileNamespace(ctx context.Context, namespaceName string) error { +func (r *CloudMapReconciler) reconcileNamespace(ctx context.Context, namespaceName string, clustersetId string) error { r.Log.Debug("syncing namespace", "namespace", namespaceName) - desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName) + desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName, clustersetId) if err != nil { r.Log.Error(err, "failed to fetch the list Services") return err diff --git a/pkg/controllers/multicluster/cloudmap_controller_test.go b/pkg/controllers/multicluster/cloudmap_controller_test.go index d1c42bcd..e113772e 100644 --- a/pkg/controllers/multicluster/cloudmap_controller_test.go +++ b/pkg/controllers/multicluster/cloudmap_controller_test.go @@ -6,6 +6,7 @@ import ( "testing" cloudmapMock "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/mocks/pkg/cloudmap" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" @@ -28,8 +29,9 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { s := scheme.Scheme s.AddKnownTypes(multiclusterv1alpha1.GroupVersion, &multiclusterv1alpha1.ServiceImportList{}, &multiclusterv1alpha1.ServiceImport{}) + s.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(objs...).WithObjects(clusterIdForTest(), clustersetIdForTest()).Build() // create a mock cloudmap service discovery client mockController := gomock.NewController(t) @@ -37,7 +39,7 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { mockSDClient := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // The service model in the Cloudmap - mockSDClient.EXPECT().ListServices(context.TODO(), test.HttpNsName). + mockSDClient.EXPECT().ListServices(context.TODO(), test.HttpNsName, test.ClustersetId). Return([]*model.Service{test.GetTestServiceWithEndpoint([]*model.Endpoint{test.GetTestEndpoint1()})}, nil) reconciler := getReconciler(t, mockSDClient, fakeClient) diff --git a/pkg/controllers/multicluster/controllers_common_test.go b/pkg/controllers/multicluster/controllers_common_test.go index 36cccc60..df045948 100644 --- a/pkg/controllers/multicluster/controllers_common_test.go +++ b/pkg/controllers/multicluster/controllers_common_test.go @@ -85,13 +85,24 @@ func endpointSliceWithIpsAndPortsForTest(ips []string, ports []discovery.Endpoin return slice } -func clusterPropertyWithIdForTest() *aboutv1alpha1.ClusterProperty { +func clusterIdForTest() *aboutv1alpha1.ClusterProperty { return &aboutv1alpha1.ClusterProperty{ ObjectMeta: metav1.ObjectMeta{ Name: ClusterIdName, }, Spec: aboutv1alpha1.ClusterPropertySpec{ - Value: "test_clusterid", + Value: test.ClusterId, + }, + } +} + +func clustersetIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: ClustersetIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: test.ClustersetId, }, } } diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index e23ca7e8..12f2c428 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -30,7 +30,7 @@ import ( const ( ClusterIdName = "id.k8s.io" - ClusterIdAttr = "CLUSTER_ID" + ClustersetIdName = "clusterset.k8s.io" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" @@ -72,17 +72,32 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Mark ServiceExport to be deleted, which is indicated by the deletion timestamp being set. isServiceExportMarkedForDelete := serviceExport.GetDeletionTimestamp() != nil - clusterId := &aboutv1alpha1.ClusterProperty{} - err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterId) - if err != nil { - r.Log.Error(err, "error fetching ClusterId") - return ctrl.Result{}, err - } else { - r.Log.Debug("ClusterID found", "ClusterID", clusterId) + clusterPropertyForClusterId := &aboutv1alpha1.ClusterProperty{} + clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} + if !isServiceExportMarkedForDelete { + // Fetch ClusterProperty with name "id.k8s.io" + err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterPropertyForClusterId) + if err != nil { + r.Log.Error(err, "error fetching ClusterProperty with name "+ClusterIdName) + return ctrl.Result{}, err + } else { + r.Log.Debug("ClusterProperty with ClusterId found", "ClusterId", clusterPropertyForClusterId.Spec.Value) + } + + // Fetch ClusterProperty with name "clusterset.k8s.io" + err = r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) + if err != nil { + r.Log.Error(err, "error fetching ClusterProperty with name "+ClustersetIdName) + return ctrl.Result{}, err + } else { + r.Log.Debug("ClusterProperty with ClustersetId found", "ClustersetId", clusterPropertyForClustersetId.Spec.Value) + } } service := v1.Service{} namespacedName := types.NamespacedName{Namespace: serviceExport.Namespace, Name: serviceExport.Name} + clusterId := clusterPropertyForClusterId.Spec.Value + clustersetId := clusterPropertyForClustersetId.Spec.Value if err := r.Client.Get(ctx, namespacedName, &service); err != nil { if errors.IsNotFound(err) { r.Log.Info("no Service found, deleting the ServiceExport", @@ -98,13 +113,13 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Check if the service export is marked to be deleted if isServiceExportMarkedForDelete { - return r.handleDelete(ctx, &serviceExport) + return r.handleDelete(ctx, &serviceExport, clustersetId) } - return r.handleUpdate(ctx, &serviceExport, &service, clusterId) + return r.handleUpdate(ctx, &serviceExport, &service, clusterId, clustersetId) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service, clusterId *aboutv1alpha1.ClusterProperty) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service, clusterId string, clustersetId string) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -128,14 +143,14 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor } r.Log.Info("updating Cloud Map service", "namespace", service.Namespace, "name", service.Name) - cmService, err := r.createOrGetCloudMapService(ctx, service) + cmService, err := r.createOrGetCloudMapService(ctx, service, clustersetId) if err != nil { r.Log.Error(err, "error fetching Service from Cloud Map", "namespace", service.Namespace, "name", service.Name) return ctrl.Result{}, err } - endpoints, err := r.extractEndpoints(ctx, service, clusterId) + endpoints, err := r.extractEndpoints(ctx, service, clusterId, clustersetId) if err != nil { r.Log.Error(err, "error extracting Endpoints", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -176,8 +191,8 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor return ctrl.Result{}, nil } -func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context, service *v1.Service) (*model.Service, error) { - cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name) +func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context, service *v1.Service, clustersetId string) (*model.Service, error) { + cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name, clustersetId) if err != nil { return nil, err } @@ -189,7 +204,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context "namespace", service.Namespace, "name", service.Name) return nil, err } - if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name); err != nil { + if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name, clustersetId); err != nil { return nil, err } } @@ -197,11 +212,11 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context return cmService, nil } -func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, clustersetId string) (ctrl.Result, error) { if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) - cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name) + cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name, clustersetId) if err != nil { r.Log.Error(err, "error fetching Service from Cloud Map", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -226,7 +241,7 @@ func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExpor return ctrl.Result{}, nil } -func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service, clusterId *aboutv1alpha1.ClusterProperty) ([]*model.Endpoint, error) { +func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service, clusterId string, clustersetId string) ([]*model.Endpoint, error) { result := make([]*model.Endpoint, 0) endpointSlices := discovery.EndpointSliceList{} @@ -254,11 +269,6 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. attributes[K8sVersionAttr] = version.PackageName + " " + version.GetVersion() } - // Apply clusterID as endpoint attribute if it exists - if clusterId.Spec.Value != "" { - attributes[ClusterIdAttr] = clusterId.Spec.Value - } - // TODO extract attributes - pod, node and other useful details if possible port := EndpointPortToPort(endpointPort) @@ -267,6 +277,8 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. IP: IP, EndpointPort: port, ServicePort: servicePortMap[*endpointPort.Name], + ClusterId: clusterId, + ClustersetId: clustersetId, Attributes: attributes, }) } @@ -313,17 +325,17 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { // Return reconcile requests for all services return func(object client.Object) []reconcile.Request { - services := &v1.ServiceList{} - if err := r.Client.List(context.TODO(), services); err != nil { + serviceExports := &multiclusterv1alpha1.ServiceExportList{} + if err := r.Client.List(context.TODO(), serviceExports); err != nil { r.Log.Error(err, "error listing services") return nil } result := make([]reconcile.Request, 0) - for _, service := range services.Items { + for _, serviceExport := range serviceExports.Items { result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{ - Name: service.Name, - Namespace: service.Namespace, + Name: serviceExport.Name, + Namespace: serviceExport.Namespace, }}) } return result diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index a4277445..8becf37d 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -29,7 +29,7 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterPropertyWithIdForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest(), clustersetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -43,8 +43,8 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // expected interactions with the Cloud Map client // The first get call is expected to return nil, then second call after the creation of service is // supposed to return the value - first := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil, nil) - second := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). + first := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId).Return(nil, nil) + second := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). Return(&model.Service{Namespace: test.HttpNsName, Name: test.SvcName}, nil) gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) @@ -77,7 +77,7 @@ func TestServiceExportReconciler_Reconcile_ExistingServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterPropertyWithIdForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest(), clustersetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -89,7 +89,7 @@ func TestServiceExportReconciler_Reconcile_ExistingServiceExport(t *testing.T) { mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // GetService from Cloudmap returns endpoint1 and endpoint2 - mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). + mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). Return(test.GetTestService(), nil) // call to delete the endpoint not present in the k8s cluster mock.EXPECT().DeleteEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, @@ -124,7 +124,7 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { serviceExportObj.Finalizers = []string{ServiceExportFinalizer} fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(serviceExportObj, clusterPropertyWithIdForTest()). + WithObjects(serviceExportObj, clusterIdForTest(), clustersetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). @@ -136,7 +136,7 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // GetService from Cloudmap returns endpoint1 and endpoint2 - mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). + mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). Return(test.GetTestService(), nil) // call to delete the endpoint in the cloudmap mock.EXPECT().DeleteEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, diff --git a/pkg/model/types.go b/pkg/model/types.go index cf514014..2062c97c 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -39,6 +39,8 @@ type Endpoint struct { IP string EndpointPort Port ServicePort Port + ClusterId string + ClustersetId string Attributes map[string]string } @@ -56,7 +58,8 @@ const ( EndpointPortAttr = "AWS_INSTANCE_PORT" EndpointPortNameAttr = "ENDPOINT_PORT_NAME" EndpointProtocolAttr = "ENDPOINT_PROTOCOL" - ClusterIdAttrName = "CLUSTER_ID" + ClusterIdAttr = "CLUSTER_ID" + ClustersetIdAttr = "CLUSTERSET_ID" ServicePortNameAttr = "SERVICE_PORT_NAME" ServicePortAttr = "SERVICE_PORT" ServiceTargetPortAttr = "SERVICE_TARGET_PORT" @@ -74,7 +77,7 @@ func NewEndpointFromInstance(inst *types.HttpInstanceSummary) (endpointPtr *Endp attributes[key] = value } - // Remove and set the IP, Port, Port + // Remove and set the IP, Port, Port, ClusterId if endpoint.IP, err = removeStringAttr(attributes, EndpointIpv4Attr); err != nil { return nil, err } @@ -87,6 +90,14 @@ func NewEndpointFromInstance(inst *types.HttpInstanceSummary) (endpointPtr *Endp return nil, err } + if endpoint.ClusterId, err = clusterIdFromAttr(attributes); err != nil { + return nil, err + } + + if endpoint.ClustersetId, err = clustersetIdFromAttr(attributes); err != nil { + return nil, err + } + // Add the remaining attributes endpoint.Attributes = attributes @@ -124,6 +135,16 @@ func servicePortFromAttr(attributes map[string]string) (port Port, err error) { return port, err } +func clusterIdFromAttr(attributes map[string]string) (clusterId string, err error) { + clusterId, err = removeStringAttr(attributes, ClusterIdAttr) + return clusterId, err +} + +func clustersetIdFromAttr(attributes map[string]string) (clustersetId string, err error) { + clustersetId, err = removeStringAttr(attributes, ClustersetIdAttr) + return clustersetId, err +} + func removeStringAttr(attributes map[string]string, attr string) (string, error) { if value, hasValue := attributes[attr]; hasValue { delete(attributes, attr) @@ -149,6 +170,8 @@ func removeIntAttr(attributes map[string]string, attr string) (int32, error) { func (e *Endpoint) GetCloudMapAttributes() map[string]string { attrs := make(map[string]string) + attrs[ClusterIdAttr] = e.ClusterId + attrs[ClustersetIdAttr] = e.ClustersetId attrs[EndpointIpv4Attr] = e.IP attrs[EndpointPortAttr] = strconv.Itoa(int(e.EndpointPort.Port)) attrs[EndpointProtocolAttr] = e.EndpointPort.Protocol diff --git a/pkg/model/types_test.go b/pkg/model/types_test.go index 019308fa..80820e9d 100644 --- a/pkg/model/types_test.go +++ b/pkg/model/types_test.go @@ -9,6 +9,8 @@ import ( var instId = "my-instance" var ip = "192.168.0.1" +var clusterId = "test-mcs-clusterId" +var clustersetId = "test-mcs-clustersetId" func TestNewEndpointFromInstance(t *testing.T) { tests := []struct { @@ -22,6 +24,8 @@ func TestNewEndpointFromInstance(t *testing.T) { inst: &types.HttpInstanceSummary{ InstanceId: &instId, Attributes: map[string]string{ + ClusterIdAttr: clusterId, + ClustersetIdAttr: clustersetId, EndpointIpv4Attr: ip, EndpointPortAttr: "80", EndpointProtocolAttr: "TCP", @@ -47,6 +51,8 @@ func TestNewEndpointFromInstance(t *testing.T) { TargetPort: "80", Protocol: "TCP", }, + ClusterId: clusterId, + ClustersetId: clustersetId, Attributes: map[string]string{ "custom-attr": "custom-val", }, @@ -92,6 +98,44 @@ func TestNewEndpointFromInstance(t *testing.T) { }, wantErr: true, }, + { + name: "missing clusterid", + inst: &types.HttpInstanceSummary{ + InstanceId: &instId, + Attributes: map[string]string{ + ClustersetIdAttr: clustersetId, + EndpointIpv4Attr: ip, + EndpointPortAttr: "80", + EndpointProtocolAttr: "TCP", + EndpointPortNameAttr: "http", + ServicePortNameAttr: "http", + ServiceProtocolAttr: "TCP", + ServicePortAttr: "65535", + ServiceTargetPortAttr: "80", + "custom-attr": "custom-val", + }, + }, + wantErr: true, + }, + { + name: "missing clustersetid", + inst: &types.HttpInstanceSummary{ + InstanceId: &instId, + Attributes: map[string]string{ + ClusterIdAttr: clusterId, + EndpointIpv4Attr: ip, + EndpointPortAttr: "80", + EndpointProtocolAttr: "TCP", + EndpointPortNameAttr: "http", + ServicePortNameAttr: "http", + ServiceProtocolAttr: "TCP", + ServicePortAttr: "65535", + ServiceTargetPortAttr: "80", + "custom-attr": "custom-val", + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -113,6 +157,8 @@ func TestEndpoint_GetAttributes(t *testing.T) { IP string EndpointPort Port ServicePort Port + ClusterId string + ClustersetId string Attributes map[string]string } tests := []struct { @@ -135,11 +181,15 @@ func TestEndpoint_GetAttributes(t *testing.T) { TargetPort: "80", Protocol: "TCP", }, + ClusterId: clusterId, + ClustersetId: clustersetId, Attributes: map[string]string{ "custom-attr": "custom-val", }, }, want: map[string]string{ + ClusterIdAttr: clusterId, + ClustersetIdAttr: clustersetId, EndpointIpv4Attr: ip, EndpointPortAttr: "80", EndpointProtocolAttr: "TCP", @@ -159,6 +209,8 @@ func TestEndpoint_GetAttributes(t *testing.T) { IP: tt.fields.IP, EndpointPort: tt.fields.EndpointPort, ServicePort: tt.fields.ServicePort, + ClusterId: tt.fields.ClusterId, + ClustersetId: tt.fields.ClustersetId, Attributes: tt.fields.Attributes, } if got := e.GetCloudMapAttributes(); !reflect.DeepEqual(got, tt.want) { diff --git a/test/test-constants.go b/test/test-constants.go index 477de060..cef3e418 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -13,8 +13,8 @@ const ( DnsNsId = "dns-ns-id" SvcName = "svc-name" SvcId = "svc-id" - ClusterIdName = "CLUSTER_ID" - ClusterIdValue = "test_clusterid" + ClusterId = "test-mcs-clusterId" + ClustersetId = "test-mcs-clustersetId" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -83,9 +83,9 @@ func GetTestEndpoint1() *model.Endpoint { TargetPort: PortStr1, Protocol: Protocol1, }, - Attributes: map[string]string{ - ClusterIdName: ClusterIdValue, - }, + ClusterId: ClusterId, + ClustersetId: ClustersetId, + Attributes: make(map[string]string), } } @@ -104,9 +104,9 @@ func GetTestEndpoint2() *model.Endpoint { TargetPort: PortStr2, Protocol: Protocol2, }, - Attributes: map[string]string{ - ClusterIdName: ClusterIdValue, - }, + ClusterId: ClusterId, + ClustersetId: ClustersetId, + Attributes: make(map[string]string), } } @@ -114,6 +114,7 @@ func GetTestEndpoints(count int) (endpts []*model.Endpoint) { // use +3 offset go avoid collision with test endpoint 1 and 2 for i := 3; i < count+3; i++ { e := GetTestEndpoint1() + e.ClusterId = ClusterId e.Id = fmt.Sprintf("tcp-192_168_0_%d-1", i) e.IP = fmt.Sprintf("192.168.0.%d", i) endpts = append(endpts, e) From 17687557f961860390607347d3f911f1a5e6c565 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Tue, 26 Jul 2022 23:00:19 +0000 Subject: [PATCH 27/30] cleanup of get/set of clusterId and clustersetId --- .../multicluster/cloudmap_controller.go | 59 ++++++++++++++++--- .../multicluster/serviceexport_controller.go | 52 ++++++---------- .../serviceexport_controller_test.go | 17 +++++- 3 files changed, 85 insertions(+), 43 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index c0a87455..7d48d159 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -22,6 +22,12 @@ const ( syncPeriod = 2 * time.Second ) +var ( + ClusterId string + ClustersetId string + ClusterPropertiesNeedRefresh = false +) + // CloudMapReconciler reconciles state of Cloud Map services with local ServiceImport objects type CloudMapReconciler struct { Client client.Client @@ -54,13 +60,13 @@ func (r *CloudMapReconciler) Start(ctx context.Context) error { // Reconcile triggers a single reconciliation round func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { - clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} - err := r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) - if err != nil { - r.Log.Error(err, "error getting ClusterProperty for clustersetId") - return err + if ClusterPropertiesNeedRefresh || ClusterId == "" || ClustersetId == "" { + err := r.setClusterPropertyGlobalVariables(ctx) + if err != nil { + r.Log.Error(err, "failed to set cluster properties") + return err + } } - clustersetId := clusterPropertyForClustersetId.Spec.Value namespaces := v1.NamespaceList{} if err := r.Client.List(ctx, &namespaces); err != nil { @@ -69,7 +75,7 @@ func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { } for _, ns := range namespaces.Items { - if err := r.reconcileNamespace(ctx, ns.Name, clustersetId); err != nil { + if err := r.reconcileNamespace(ctx, ns.Name); err != nil { return err } } @@ -77,10 +83,10 @@ func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { return nil } -func (r *CloudMapReconciler) reconcileNamespace(ctx context.Context, namespaceName string, clustersetId string) error { +func (r *CloudMapReconciler) reconcileNamespace(ctx context.Context, namespaceName string) error { r.Log.Debug("syncing namespace", "namespace", namespaceName) - desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName, clustersetId) + desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName, ClustersetId) if err != nil { r.Log.Error(err, "failed to fetch the list Services") return err @@ -310,3 +316,38 @@ func (r *CloudMapReconciler) updateDerivedService(ctx context.Context, svc *v1.S return nil } + +func (r *CloudMapReconciler) setClusterPropertyGlobalVariables(ctx context.Context) error { + var err error + ClusterId, err = r.getClusterId(ctx) + if err != nil { + return err + } + ClustersetId, err = r.getClustersetId(ctx) + if err != nil { + return err + } + ClusterPropertiesNeedRefresh = false + r.Log.Debug("ClusterProperties set", "ClusterId", ClusterId, "ClustersetId", ClustersetId) + return nil +} + +func (r *CloudMapReconciler) getClusterId(ctx context.Context) (string, error) { + clusterPropertyForClusterId := &aboutv1alpha1.ClusterProperty{} + err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterPropertyForClusterId) + if err != nil { + r.Log.Error(err, "error getting ClusterProperty for clusterId") + return "", err + } + return clusterPropertyForClusterId.Spec.Value, nil +} + +func (r *CloudMapReconciler) getClustersetId(ctx context.Context) (string, error) { + clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} + err := r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) + if err != nil { + r.Log.Error(err, "error getting ClusterProperty for clustersetId") + return "", err + } + return clusterPropertyForClustersetId.Spec.Value, nil +} diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 12f2c428..13e7d094 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -72,32 +72,17 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Mark ServiceExport to be deleted, which is indicated by the deletion timestamp being set. isServiceExportMarkedForDelete := serviceExport.GetDeletionTimestamp() != nil - clusterPropertyForClusterId := &aboutv1alpha1.ClusterProperty{} - clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} if !isServiceExportMarkedForDelete { - // Fetch ClusterProperty with name "id.k8s.io" - err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterPropertyForClusterId) - if err != nil { - r.Log.Error(err, "error fetching ClusterProperty with name "+ClusterIdName) - return ctrl.Result{}, err - } else { - r.Log.Debug("ClusterProperty with ClusterId found", "ClusterId", clusterPropertyForClusterId.Spec.Value) + if ClusterId == "" { + return ctrl.Result{}, fmt.Errorf("ClusterProperty %s not found", ClusterIdName) } - - // Fetch ClusterProperty with name "clusterset.k8s.io" - err = r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) - if err != nil { - r.Log.Error(err, "error fetching ClusterProperty with name "+ClustersetIdName) - return ctrl.Result{}, err - } else { - r.Log.Debug("ClusterProperty with ClustersetId found", "ClustersetId", clusterPropertyForClustersetId.Spec.Value) + if ClustersetId == "" { + return ctrl.Result{}, fmt.Errorf("ClusterProperty %s not found", ClustersetIdName) } } service := v1.Service{} namespacedName := types.NamespacedName{Namespace: serviceExport.Namespace, Name: serviceExport.Name} - clusterId := clusterPropertyForClusterId.Spec.Value - clustersetId := clusterPropertyForClustersetId.Spec.Value if err := r.Client.Get(ctx, namespacedName, &service); err != nil { if errors.IsNotFound(err) { r.Log.Info("no Service found, deleting the ServiceExport", @@ -113,13 +98,13 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Check if the service export is marked to be deleted if isServiceExportMarkedForDelete { - return r.handleDelete(ctx, &serviceExport, clustersetId) + return r.handleDelete(ctx, &serviceExport) } - return r.handleUpdate(ctx, &serviceExport, &service, clusterId, clustersetId) + return r.handleUpdate(ctx, &serviceExport, &service) } -func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service, clusterId string, clustersetId string) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, service *v1.Service) (ctrl.Result, error) { // Add the finalizer to the service export if not present, ensures the ServiceExport won't be deleted if !controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { controllerutil.AddFinalizer(serviceExport, ServiceExportFinalizer) @@ -143,14 +128,14 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor } r.Log.Info("updating Cloud Map service", "namespace", service.Namespace, "name", service.Name) - cmService, err := r.createOrGetCloudMapService(ctx, service, clustersetId) + cmService, err := r.createOrGetCloudMapService(ctx, service) if err != nil { r.Log.Error(err, "error fetching Service from Cloud Map", "namespace", service.Namespace, "name", service.Name) return ctrl.Result{}, err } - endpoints, err := r.extractEndpoints(ctx, service, clusterId, clustersetId) + endpoints, err := r.extractEndpoints(ctx, service) if err != nil { r.Log.Error(err, "error extracting Endpoints", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -191,8 +176,8 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor return ctrl.Result{}, nil } -func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context, service *v1.Service, clustersetId string) (*model.Service, error) { - cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name, clustersetId) +func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context, service *v1.Service) (*model.Service, error) { + cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name, ClustersetId) if err != nil { return nil, err } @@ -204,7 +189,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context "namespace", service.Namespace, "name", service.Name) return nil, err } - if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name, clustersetId); err != nil { + if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name, ClustersetId); err != nil { return nil, err } } @@ -212,11 +197,11 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context return cmService, nil } -func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport, clustersetId string) (ctrl.Result, error) { +func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExport *multiclusterv1alpha1.ServiceExport) (ctrl.Result, error) { if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) - cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name, clustersetId) + cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name, ClustersetId) if err != nil { r.Log.Error(err, "error fetching Service from Cloud Map", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -241,7 +226,7 @@ func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExpor return ctrl.Result{}, nil } -func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service, clusterId string, clustersetId string) ([]*model.Endpoint, error) { +func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1.Service) ([]*model.Endpoint, error) { result := make([]*model.Endpoint, 0) endpointSlices := discovery.EndpointSliceList{} @@ -277,8 +262,8 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. IP: IP, EndpointPort: port, ServicePort: servicePortMap[*endpointPort.Name], - ClusterId: clusterId, - ClustersetId: clustersetId, + ClusterId: ClusterId, + ClustersetId: ClustersetId, Attributes: attributes, }) } @@ -323,7 +308,7 @@ func (r *ServiceExportReconciler) endpointSliceEventHandler() handler.MapFunc { } func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc { - // Return reconcile requests for all services + // Return reconcile requests for all service exports return func(object client.Object) []reconcile.Request { serviceExports := &multiclusterv1alpha1.ServiceExportList{} if err := r.Client.List(context.TODO(), serviceExports); err != nil { @@ -338,6 +323,7 @@ func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc Namespace: serviceExport.Namespace, }}) } + ClusterPropertiesNeedRefresh = true return result } } diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index 8becf37d..f8f4f77f 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -35,6 +35,10 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { }). Build() + // set global variables + ClusterId = test.ClusterId + ClustersetId = test.ClustersetId + // create a mock cloudmap service discovery client mockController := gomock.NewController(t) defer mockController.Finish() @@ -83,6 +87,10 @@ func TestServiceExportReconciler_Reconcile_ExistingServiceExport(t *testing.T) { }). Build() + // set global variables + ClusterId = test.ClusterId + ClustersetId = test.ClustersetId + mockController := gomock.NewController(t) defer mockController.Finish() @@ -130,6 +138,10 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { }). Build() + // set global variables + ClusterId = test.ClusterId + ClustersetId = test.ClustersetId + mockController := gomock.NewController(t) defer mockController.Finish() @@ -171,6 +183,9 @@ func TestServiceExportReconciler_Reconcile_NoClusterId(t *testing.T) { }). Build() + ClusterId = "" + ClustersetId = "" + // create a mock cloudmap service discovery client mockController := gomock.NewController(t) defer mockController.Finish() @@ -188,7 +203,7 @@ func TestServiceExportReconciler_Reconcile_NoClusterId(t *testing.T) { // Reconciling should throw an error got, err := reconciler.Reconcile(context.Background(), request) - expectedError := fmt.Errorf("clusterproperties.about.k8s.io \"id.k8s.io\" not found") + expectedError := fmt.Errorf("ClusterProperty id.k8s.io not found") assert.ErrorContains(t, err, expectedError.Error()) assert.Equal(t, ctrl.Result{}, got, "Result should be empty") } From b04b7dbbede445c40925b9f7f96c139f9a2736af Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Fri, 29 Jul 2022 20:32:53 +0000 Subject: [PATCH 28/30] clusterUtils --- .../annotation_for_clusterproperties.yaml | 2 +- integration/janitor/api.go | 3 +- integration/janitor/janitor.go | 10 +-- integration/janitor/janitor_test.go | 6 +- integration/janitor/runner/main.go | 7 +- .../shared/scenarios/export_service.go | 5 +- main.go | 20 +++--- pkg/cloudmap/api.go | 24 ++++--- pkg/cloudmap/api_test.go | 18 +++-- pkg/cloudmap/client.go | 41 ++++++----- pkg/cloudmap/client_test.go | 32 ++++----- pkg/common/cluster.go | 65 +++++++++++++++++ .../multicluster/cloudmap_controller.go | 67 ++++------------- .../multicluster/cloudmap_controller_test.go | 11 +-- .../multicluster/controllers_common_test.go | 23 ------ .../multicluster/serviceexport_controller.go | 48 +++++++------ .../serviceexport_controller_test.go | 72 ++++++++++++------- pkg/controllers/multicluster/suite_test.go | 2 + pkg/model/types.go | 20 ++---- pkg/model/types_test.go | 16 ++--- test/test-constants.go | 33 ++++++++- 21 files changed, 299 insertions(+), 226 deletions(-) create mode 100644 pkg/common/cluster.go diff --git a/config/crd/patches/annotation_for_clusterproperties.yaml b/config/crd/patches/annotation_for_clusterproperties.yaml index 9ceefc79..8dc306ce 100644 --- a/config/crd/patches/annotation_for_clusterproperties.yaml +++ b/config/crd/patches/annotation_for_clusterproperties.yaml @@ -3,5 +3,5 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - api-approved.kubernetes.io: "https://github.com/kubernetes/kubernetes/pull/78458" + api-approved.kubernetes.io: "https://github.com/kubernetes/enhancements/pull/3084" name: clusterproperties.about.k8s.io diff --git a/integration/janitor/api.go b/integration/janitor/api.go index 6e00a400..c66f09cc 100644 --- a/integration/janitor/api.go +++ b/integration/janitor/api.go @@ -4,6 +4,7 @@ import ( "context" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" "github.com/aws/aws-sdk-go-v2/aws" sd "github.com/aws/aws-sdk-go-v2/service/servicediscovery" ) @@ -21,7 +22,7 @@ type serviceDiscoveryJanitorApi struct { func NewServiceDiscoveryJanitorApiFromConfig(cfg *aws.Config) ServiceDiscoveryJanitorApi { return &serviceDiscoveryJanitorApi{ - ServiceDiscoveryApi: cloudmap.NewServiceDiscoveryApiFromConfig(cfg), + ServiceDiscoveryApi: cloudmap.NewServiceDiscoveryApiFromConfig(cfg, common.ClusterUtils{}), janitorFacade: NewSdkJanitorFacadeFromConfig(cfg), } } diff --git a/integration/janitor/janitor.go b/integration/janitor/janitor.go index a117c428..24127bdf 100644 --- a/integration/janitor/janitor.go +++ b/integration/janitor/janitor.go @@ -13,7 +13,7 @@ import ( // CloudMapJanitor handles AWS Cloud Map resource cleanup during integration tests. type CloudMapJanitor interface { // Cleanup removes all instances, services and the namespace from AWS Cloud Map for a given namespace name. - Cleanup(ctx context.Context, nsName string, clustersetId string) + Cleanup(ctx context.Context, nsName string) } type cloudMapJanitor struct { @@ -36,7 +36,7 @@ func NewDefaultJanitor() CloudMapJanitor { } } -func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string, clustersetId string) { +func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string) { fmt.Printf("Cleaning up all test resources in Cloud Map for namespace : %s\n", nsName) nsMap, err := j.sdApi.GetNamespaceMap(ctx) @@ -57,7 +57,7 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string, clusterset for svcName, svcId := range svcIdMap { fmt.Printf("found service to clean: %s\n", svcId) - j.deregisterInstances(ctx, nsName, svcName, svcId, clustersetId) + j.deregisterInstances(ctx, nsName, svcName, svcId) delSvcErr := j.sdApi.DeleteService(ctx, svcId) j.checkOrFail(delSvcErr, "service deleted", "could not cleanup service") @@ -71,8 +71,8 @@ func (j *cloudMapJanitor) Cleanup(ctx context.Context, nsName string, clusterset j.checkOrFail(err, "clean up successful", "could not cleanup namespace") } -func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, nsName string, svcName string, svcId string, clustersetId string) { - insts, err := j.sdApi.DiscoverInstances(ctx, nsName, svcName, clustersetId) +func (j *cloudMapJanitor) deregisterInstances(ctx context.Context, nsName string, svcName string, svcId string) { + insts, err := j.sdApi.DiscoverInstances(ctx, nsName, svcName) j.checkOrFail(err, fmt.Sprintf("service has %d instances to clean", len(insts)), "could not list instances to cleanup") diff --git a/integration/janitor/janitor_test.go b/integration/janitor/janitor_test.go index 0e2bbe3e..196125e4 100644 --- a/integration/janitor/janitor_test.go +++ b/integration/janitor/janitor_test.go @@ -32,7 +32,7 @@ func TestCleanupHappyCase(t *testing.T) { 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, test.ClustersetId). + tj.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). Return([]types.HttpInstanceSummary{{InstanceId: aws.String(test.EndptId1)}}, nil) tj.mockApi.EXPECT().DeregisterInstance(context.TODO(), test.SvcId, test.EndptId1). @@ -46,7 +46,7 @@ func TestCleanupHappyCase(t *testing.T) { tj.mockApi.EXPECT().PollNamespaceOperation(context.TODO(), test.OpId2). Return(test.HttpNsId, nil) - tj.janitor.Cleanup(context.TODO(), test.HttpNsName, test.ClustersetId) + tj.janitor.Cleanup(context.TODO(), test.HttpNsName) assert.False(t, *tj.failed) } @@ -57,7 +57,7 @@ func TestCleanupNothingToClean(t *testing.T) { tj.mockApi.EXPECT().GetNamespaceMap(context.TODO()). Return(map[string]*model.Namespace{}, nil) - tj.janitor.Cleanup(context.TODO(), test.HttpNsName, test.ClustersetId) + tj.janitor.Cleanup(context.TODO(), test.HttpNsName) assert.False(t, *tj.failed) } diff --git a/integration/janitor/runner/main.go b/integration/janitor/runner/main.go index 33a004b9..9711bfed 100644 --- a/integration/janitor/runner/main.go +++ b/integration/janitor/runner/main.go @@ -9,13 +9,12 @@ import ( ) func main() { - if len(os.Args) != 3 { - fmt.Println("Expected single namespace name and clustersetId arguments") + if len(os.Args) != 2 { + fmt.Println("Expected single namespace name argument") os.Exit(1) } j := janitor.NewDefaultJanitor() nsName := os.Args[1] - clustersetId := os.Args[2] - j.Cleanup(context.TODO(), nsName, clustersetId) + j.Cleanup(context.TODO(), nsName) } diff --git a/integration/shared/scenarios/export_service.go b/integration/shared/scenarios/export_service.go index 859ea8fa..8e2dd845 100644 --- a/integration/shared/scenarios/export_service.go +++ b/integration/shared/scenarios/export_service.go @@ -8,6 +8,7 @@ import ( "time" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" multiclustercontrollers "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/controllers/multicluster" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" "github.com/aws/aws-sdk-go-v2/aws" @@ -68,7 +69,7 @@ func NewExportServiceScenario(cfg *aws.Config, nsName string, svcName string, po NsTTL: time.Second, SvcTTL: time.Second, EndptTTL: time.Second, - }), + }, common.ClusterUtils{}), expectedSvc: model.Service{ Namespace: nsName, Name: svcName, @@ -82,7 +83,7 @@ func (e *exportServiceScenario) Run() error { return wait.Poll(defaultScenarioPollInterval, defaultScenarioPollTimeout, func() (done bool, err error) { fmt.Println("Polling service...") - cmSvc, err := e.sdClient.GetService(context.TODO(), e.expectedSvc.Namespace, e.expectedSvc.Name, "") + cmSvc, err := e.sdClient.GetService(context.TODO(), e.expectedSvc.Namespace, e.expectedSvc.Name) if err != nil { return true, err } diff --git a/main.go b/main.go index b483ec82..7f30de81 100644 --- a/main.go +++ b/main.go @@ -87,21 +87,25 @@ func main() { log.Info("Running with AWS region", "AWS_REGION", awsCfg.Region) - serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg) + clusterUtils := common.NewClusterUtils(mgr.GetClient()) + serviceDiscoveryClient := cloudmap.NewDefaultServiceDiscoveryClient(&awsCfg, clusterUtils) + if err = (&multiclustercontrollers.ServiceExportReconciler{ - Client: mgr.GetClient(), - Log: common.NewLogger("controllers", "ServiceExport"), - Scheme: mgr.GetScheme(), - CloudMap: serviceDiscoveryClient, + Client: mgr.GetClient(), + Log: common.NewLogger("controllers", "ServiceExport"), + Scheme: mgr.GetScheme(), + CloudMap: serviceDiscoveryClient, + ClusterUtils: clusterUtils, }).SetupWithManager(mgr); err != nil { log.Error(err, "unable to create controller", "controller", "ServiceExport") os.Exit(1) } cloudMapReconciler := &multiclustercontrollers.CloudMapReconciler{ - Client: mgr.GetClient(), - Cloudmap: serviceDiscoveryClient, - Log: common.NewLogger("controllers", "Cloudmap"), + Client: mgr.GetClient(), + Cloudmap: serviceDiscoveryClient, + Log: common.NewLogger("controllers", "Cloudmap"), + ClusterUtils: clusterUtils, } if err = mgr.Add(cloudMapReconciler); err != nil { diff --git a/pkg/cloudmap/api.go b/pkg/cloudmap/api.go index 5dd235f3..ad33af9e 100644 --- a/pkg/cloudmap/api.go +++ b/pkg/cloudmap/api.go @@ -27,7 +27,7 @@ type ServiceDiscoveryApi interface { 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, clustersetId string) (insts []types.HttpInstanceSummary, err error) + DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error) // ListOperations returns a map of operations to their status matching a list of filters. ListOperations(ctx context.Context, opFilters []types.OperationFilter) (operationStatusMap map[string]types.OperationStatus, err error) @@ -52,15 +52,17 @@ type ServiceDiscoveryApi interface { } type serviceDiscoveryApi struct { - log common.Logger - awsFacade AwsFacade + log common.Logger + awsFacade AwsFacade + clusterUtils common.ClusterUtils } // NewServiceDiscoveryApiFromConfig creates a new AWS Cloud Map API connection manager from an AWS client config. -func NewServiceDiscoveryApiFromConfig(cfg *aws.Config) ServiceDiscoveryApi { +func NewServiceDiscoveryApiFromConfig(cfg *aws.Config, clusterUtils common.ClusterUtils) ServiceDiscoveryApi { return &serviceDiscoveryApi{ - log: common.NewLogger("cloudmap"), - awsFacade: NewAwsFacadeFromConfig(cfg), + log: common.NewLogger("cloudmap"), + awsFacade: NewAwsFacadeFromConfig(cfg), + clusterUtils: clusterUtils, } } @@ -113,14 +115,20 @@ func (sdApi *serviceDiscoveryApi) GetServiceIdMap(ctx context.Context, nsId stri return serviceIdMap, nil } -func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string, clustersetId string) (insts []types.HttpInstanceSummary, err error) { +func (sdApi *serviceDiscoveryApi) DiscoverInstances(ctx context.Context, nsName string, svcName string) (insts []types.HttpInstanceSummary, err error) { + clusterSetId, err := sdApi.clusterUtils.GetClusterSetId(ctx) + if err != nil { + sdApi.log.Error(err, "failed to retrieve clusterSetId") + return nil, err + } + out, err := sdApi.awsFacade.DiscoverInstances(ctx, &sd.DiscoverInstancesInput{ NamespaceName: aws.String(nsName), ServiceName: aws.String(svcName), HealthStatus: types.HealthStatusFilterAll, MaxResults: aws.Int32(1000), QueryParameters: map[string]string{ - model.ClustersetIdAttr: clustersetId, + model.ClusterSetIdAttr: clusterSetId, }, }) diff --git a/pkg/cloudmap/api_test.go b/pkg/cloudmap/api_test.go index 55a256a7..cb90b8e6 100644 --- a/pkg/cloudmap/api_test.go +++ b/pkg/cloudmap/api_test.go @@ -6,6 +6,8 @@ import ( "fmt" "testing" + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" + 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" @@ -16,10 +18,12 @@ import ( "github.com/go-logr/logr/testr" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestNewServiceDiscoveryApi(t *testing.T) { - sdc := NewServiceDiscoveryApiFromConfig(&aws.Config{}) + sdc := NewServiceDiscoveryApiFromConfig(&aws.Config{}, common.ClusterUtils{}) assert.NotNil(t, sdc) } @@ -103,7 +107,7 @@ func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) { HealthStatus: types.HealthStatusFilterAll, MaxResults: aws.Int32(1000), QueryParameters: map[string]string{ - model.ClustersetIdAttr: test.ClustersetId, + model.ClusterSetIdAttr: test.ClustersetId, }, }). Return(&sd.DiscoverInstancesOutput{ @@ -113,7 +117,7 @@ func TestServiceDiscoveryApi_DiscoverInstances_HappyCase(t *testing.T) { }, }, nil) - insts, err := sdApi.DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) + insts, err := sdApi.DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName) assert.Nil(t, err, "No error for happy case") assert.True(t, len(insts) == 2) assert.Equal(t, test.EndptId1, *insts[0].InstanceId) @@ -325,8 +329,12 @@ func TestServiceDiscoveryApi_PollNamespaceOperation_HappyCase(t *testing.T) { } func getServiceDiscoveryApi(t *testing.T, awsFacade *cloudmapMock.MockAwsFacade) ServiceDiscoveryApi { + scheme := runtime.NewScheme() + scheme.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) + fakeClient := fake.NewClientBuilder().WithObjects(test.ClusterIdForTest(), test.ClusterSetIdForTest()).WithScheme(scheme).Build() return &serviceDiscoveryApi{ - log: common.NewLoggerWithLogr(testr.New(t)), - awsFacade: awsFacade, + log: common.NewLoggerWithLogr(testr.New(t)), + awsFacade: awsFacade, + clusterUtils: common.NewClusterUtils(fakeClient), } } diff --git a/pkg/cloudmap/client.go b/pkg/cloudmap/client.go index a9d10042..1fdfa1a8 100644 --- a/pkg/cloudmap/client.go +++ b/pkg/cloudmap/client.go @@ -14,13 +14,13 @@ import ( // multi-cluster service discovery for Kubernetes controller. It maintains local caches for all AWS Cloud Map resources. type ServiceDiscoveryClient interface { // ListServices returns all services and their endpoints for a given namespace. - ListServices(ctx context.Context, namespaceName string, clustersetId string) ([]*model.Service, error) + ListServices(ctx context.Context, namespaceName string) ([]*model.Service, error) // CreateService creates a Cloud Map service resource, and namespace if necessary. CreateService(ctx context.Context, namespaceName string, serviceName string) error // GetService returns a service resource fetched from AWS Cloud Map or nil if not found. - GetService(ctx context.Context, namespaceName string, serviceName string, clustersetId string) (*model.Service, error) + GetService(ctx context.Context, namespaceName string, serviceName string) (*model.Service, error) // RegisterEndpoints registers all endpoints for given service. RegisterEndpoints(ctx context.Context, namespaceName string, serviceName string, endpoints []*model.Endpoint) error @@ -30,37 +30,40 @@ type ServiceDiscoveryClient interface { } type serviceDiscoveryClient struct { - log common.Logger - sdApi ServiceDiscoveryApi - cache ServiceDiscoveryClientCache + log common.Logger + sdApi ServiceDiscoveryApi + cache ServiceDiscoveryClientCache + clusterUtils common.ClusterUtils } // NewDefaultServiceDiscoveryClient creates a new service discovery client for AWS Cloud Map with default resource cache // from a given AWS client config. -func NewDefaultServiceDiscoveryClient(cfg *aws.Config) ServiceDiscoveryClient { +func NewDefaultServiceDiscoveryClient(cfg *aws.Config, clusterUtils common.ClusterUtils) ServiceDiscoveryClient { return &serviceDiscoveryClient{ - log: common.NewLogger("cloudmap"), - sdApi: NewServiceDiscoveryApiFromConfig(cfg), - cache: NewDefaultServiceDiscoveryClientCache(), + log: common.NewLogger("cloudmap"), + sdApi: NewServiceDiscoveryApiFromConfig(cfg, clusterUtils), + cache: NewDefaultServiceDiscoveryClientCache(), + clusterUtils: clusterUtils, } } -func NewServiceDiscoveryClientWithCustomCache(cfg *aws.Config, cacheConfig *SdCacheConfig) ServiceDiscoveryClient { +func NewServiceDiscoveryClientWithCustomCache(cfg *aws.Config, cacheConfig *SdCacheConfig, clusterUtils common.ClusterUtils) ServiceDiscoveryClient { return &serviceDiscoveryClient{ - log: common.NewLogger("cloudmap"), - sdApi: NewServiceDiscoveryApiFromConfig(cfg), - cache: NewServiceDiscoveryClientCache(cacheConfig), + log: common.NewLogger("cloudmap"), + sdApi: NewServiceDiscoveryApiFromConfig(cfg, clusterUtils), + cache: NewServiceDiscoveryClientCache(cacheConfig), + clusterUtils: clusterUtils, } } -func (sdc *serviceDiscoveryClient) ListServices(ctx context.Context, nsName string, clustersetId string) (svcs []*model.Service, err error) { +func (sdc *serviceDiscoveryClient) ListServices(ctx context.Context, nsName string) (svcs []*model.Service, err error) { svcIdMap, err := sdc.getServiceIds(ctx, nsName) if err != nil { return svcs, err } for svcName := range svcIdMap { - endpts, endptsErr := sdc.getEndpoints(ctx, nsName, svcName, clustersetId) + endpts, endptsErr := sdc.getEndpoints(ctx, nsName, svcName) if endptsErr != nil { return svcs, endptsErr } @@ -102,7 +105,7 @@ func (sdc *serviceDiscoveryClient) CreateService(ctx context.Context, nsName str return nil } -func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string, svcName string, clustersetId string) (svc *model.Service, err error) { +func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string, svcName string) (svc *model.Service, err error) { sdc.log.Info("fetching a service", "namespace", nsName, "name", svcName) endpts, cacheHit := sdc.cache.GetEndpoints(nsName, svcName) @@ -123,7 +126,7 @@ func (sdc *serviceDiscoveryClient) GetService(ctx context.Context, nsName string return nil, nil } - endpts, err = sdc.getEndpoints(ctx, nsName, svcName, clustersetId) + endpts, err = sdc.getEndpoints(ctx, nsName, svcName) if err != nil { return nil, err @@ -221,13 +224,13 @@ func (sdc *serviceDiscoveryClient) DeleteEndpoints(ctx context.Context, nsName s return nil } -func (sdc *serviceDiscoveryClient) getEndpoints(ctx context.Context, nsName string, svcName string, clustersetId string) (endpts []*model.Endpoint, err error) { +func (sdc *serviceDiscoveryClient) getEndpoints(ctx context.Context, nsName string, svcName string) (endpts []*model.Endpoint, err error) { endpts, cacheHit := sdc.cache.GetEndpoints(nsName, svcName) if cacheHit { return endpts, nil } - insts, err := sdc.sdApi.DiscoverInstances(ctx, nsName, svcName, clustersetId) + insts, err := sdc.sdApi.DiscoverInstances(ctx, nsName, svcName) if err != nil { return nil, err } diff --git a/pkg/cloudmap/client_test.go b/pkg/cloudmap/client_test.go index 2ed8efd3..6cbbb118 100644 --- a/pkg/cloudmap/client_test.go +++ b/pkg/cloudmap/client_test.go @@ -24,7 +24,7 @@ type testSdClient struct { } func TestNewServiceDiscoveryClient(t *testing.T) { - sdc := NewDefaultServiceDiscoveryClient(&aws.Config{}) + sdc := NewDefaultServiceDiscoveryClient(&aws.Config{}, common.ClusterUtils{}) assert.NotNil(t, sdc) } @@ -42,12 +42,12 @@ func TestServiceDiscoveryClient_ListServices_HappyCase(t *testing.T) { tc.mockCache.EXPECT().CacheServiceIdMap(test.HttpNsName, getServiceIdMapForTest()) tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return(nil, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). Return(getHttpInstanceSummaryForTest(), nil) tc.mockCache.EXPECT().CacheEndpoints(test.HttpNsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) assert.Equal(t, []*model.Service{test.GetTestService()}, svcs) assert.Nil(t, err, "No error for happy case") } @@ -64,7 +64,7 @@ func TestServiceDiscoveryClient_ListServices_HappyCaseCachedResults(t *testing.T tc.mockCache.EXPECT().GetEndpoints(test.DnsNsName, test.SvcName). Return([]*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}, true) - svcs, err := tc.client.ListServices(context.TODO(), test.DnsNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.DnsNsName) assert.Equal(t, []*model.Service{dnsService}, svcs) assert.Nil(t, err, "No error for happy case") } @@ -79,7 +79,7 @@ func TestServiceDiscoveryClient_ListServices_NamespaceError(t *testing.T) { tc.mockCache.EXPECT().GetNamespaceMap().Return(nil, false) tc.mockApi.EXPECT().GetNamespaceMap(context.TODO()).Return(nil, nsErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) assert.Equal(t, nsErr, err) assert.Empty(t, svcs) } @@ -96,7 +96,7 @@ func TestServiceDiscoveryClient_ListServices_ServiceError(t *testing.T) { tc.mockApi.EXPECT().GetServiceIdMap(context.TODO(), test.HttpNsId). Return(nil, svcErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) assert.Equal(t, svcErr, err) assert.Empty(t, svcs) } @@ -109,10 +109,10 @@ func TestServiceDiscoveryClient_ListServices_InstanceError(t *testing.T) { endptErr := errors.New("error listing endpoints") tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return(nil, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). Return([]types.HttpInstanceSummary{}, endptErr) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) assert.Equal(t, endptErr, err) assert.Empty(t, svcs) } @@ -124,7 +124,7 @@ func TestServiceDiscoveryClient_ListServices_NamespaceNotFound(t *testing.T) { tc.mockCache.EXPECT().GetServiceIdMap(test.HttpNsName).Return(nil, false) tc.mockCache.EXPECT().GetNamespaceMap().Return(nil, true) - svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName, test.ClustersetId) + svcs, err := tc.client.ListServices(context.TODO(), test.HttpNsName) assert.Empty(t, svcs) assert.Nil(t, err, "No error for namespace not found") } @@ -254,12 +254,12 @@ func TestServiceDiscoveryClient_GetService_HappyCase(t *testing.T) { tc.mockCache.EXPECT().CacheServiceIdMap(test.HttpNsName, getServiceIdMapForTest()) tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName).Return([]*model.Endpoint{}, false) - tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId). + tc.mockApi.EXPECT().DiscoverInstances(context.TODO(), test.HttpNsName, test.SvcName). Return(getHttpInstanceSummaryForTest(), nil) tc.mockCache.EXPECT().CacheEndpoints(test.HttpNsName, test.SvcName, []*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}) - svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) + svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName) assert.Nil(t, err) assert.Equal(t, test.GetTestService(), svc) } @@ -271,7 +271,7 @@ func TestServiceDiscoveryClient_GetService_CachedValues(t *testing.T) { tc.mockCache.EXPECT().GetEndpoints(test.HttpNsName, test.SvcName). Return([]*model.Endpoint{test.GetTestEndpoint1(), test.GetTestEndpoint2()}, true) - svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName, test.ClustersetId) + svc, err := tc.client.GetService(context.TODO(), test.HttpNsName, test.SvcName) assert.Nil(t, err) assert.Equal(t, test.GetTestService(), svc) } @@ -284,7 +284,7 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { attrs1 := map[string]string{ model.ClusterIdAttr: test.ClusterId, - model.ClustersetIdAttr: test.ClustersetId, + model.ClusterSetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -296,7 +296,7 @@ func TestServiceDiscoveryClient_RegisterEndpoints(t *testing.T) { } attrs2 := map[string]string{ model.ClusterIdAttr: test.ClusterId, - model.ClustersetIdAttr: test.ClustersetId, + model.ClusterSetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, @@ -367,7 +367,7 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { InstanceId: aws.String(test.EndptId1), Attributes: map[string]string{ model.ClusterIdAttr: test.ClusterId, - model.ClustersetIdAttr: test.ClustersetId, + model.ClusterSetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp1, model.EndpointPortAttr: test.PortStr1, model.EndpointPortNameAttr: test.PortName1, @@ -382,7 +382,7 @@ func getHttpInstanceSummaryForTest() []types.HttpInstanceSummary { InstanceId: aws.String(test.EndptId2), Attributes: map[string]string{ model.ClusterIdAttr: test.ClusterId, - model.ClustersetIdAttr: test.ClustersetId, + model.ClusterSetIdAttr: test.ClustersetId, model.EndpointIpv4Attr: test.EndptIp2, model.EndpointPortAttr: test.PortStr2, model.EndpointPortNameAttr: test.PortName2, diff --git a/pkg/common/cluster.go b/pkg/common/cluster.go new file mode 100644 index 00000000..1966a4df --- /dev/null +++ b/pkg/common/cluster.go @@ -0,0 +1,65 @@ +package common + +import ( + "context" + "fmt" + + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + ClusterIdName = "id.k8s.io" + ClusterSetIdName = "clusterset.k8s.io" +) + +// ClusterUtils provides utility functions for working with clusters +type ClusterUtils struct { + client client.Client + clusterId string + clusterSetId string +} + +// constructor +func NewClusterUtils(client client.Client) ClusterUtils { + return ClusterUtils{ + client: client, + } +} + +// retrieve the clusterId from the local field. If not set, retrieve from client +func (r *ClusterUtils) GetClusterId(ctx context.Context) (string, error) { + if r.clusterId != "" { + return r.clusterId, nil + } + clusterPropertyForClusterId := &aboutv1alpha1.ClusterProperty{} + err := r.client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterPropertyForClusterId) + if err != nil { + return "", err + } + if clusterPropertyForClusterId.Spec.Value == "" { + err := fmt.Errorf("ClusterId not found") + return "", err + } + r.clusterId = clusterPropertyForClusterId.Spec.Value + return r.clusterId, nil +} + +// retrieve the clusterSetId from the local field. If not set, retrieve from client +func (r *ClusterUtils) GetClusterSetId(ctx context.Context) (string, error) { + if r.clusterSetId != "" { + return r.clusterSetId, nil + } + clusterPropertyForClusterSetId := &aboutv1alpha1.ClusterProperty{} + err := r.client.Get(ctx, client.ObjectKey{Name: ClusterSetIdName}, clusterPropertyForClusterSetId) + if err != nil { + return "", err + } + if clusterPropertyForClusterSetId.Spec.Value == "" { + err := fmt.Errorf("ClusterSetId not found") + return "", err + } + r.clusterSetId = clusterPropertyForClusterSetId.Spec.Value + return r.clusterSetId, nil +} diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 7d48d159..9d5f821e 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/cloudmap" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" @@ -22,17 +21,12 @@ const ( syncPeriod = 2 * time.Second ) -var ( - ClusterId string - ClustersetId string - ClusterPropertiesNeedRefresh = false -) - // CloudMapReconciler reconciles state of Cloud Map services with local ServiceImport objects type CloudMapReconciler struct { - Client client.Client - Cloudmap cloudmap.ServiceDiscoveryClient - Log common.Logger + Client client.Client + Cloudmap cloudmap.ServiceDiscoveryClient + Log common.Logger + ClusterUtils common.ClusterUtils } // +kubebuilder:rbac:groups="",resources=namespaces,verbs=list;watch @@ -60,12 +54,16 @@ func (r *CloudMapReconciler) Start(ctx context.Context) error { // Reconcile triggers a single reconciliation round func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { - if ClusterPropertiesNeedRefresh || ClusterId == "" || ClustersetId == "" { - err := r.setClusterPropertyGlobalVariables(ctx) - if err != nil { - r.Log.Error(err, "failed to set cluster properties") - return err - } + var err error + clusterId, err = r.ClusterUtils.GetClusterId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterId") + return err + } + clusterSetId, err = r.ClusterUtils.GetClusterSetId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterSetId") + return err } namespaces := v1.NamespaceList{} @@ -86,7 +84,7 @@ func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { func (r *CloudMapReconciler) reconcileNamespace(ctx context.Context, namespaceName string) error { r.Log.Debug("syncing namespace", "namespace", namespaceName) - desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName, ClustersetId) + desiredServices, err := r.Cloudmap.ListServices(ctx, namespaceName) if err != nil { r.Log.Error(err, "failed to fetch the list Services") return err @@ -316,38 +314,3 @@ func (r *CloudMapReconciler) updateDerivedService(ctx context.Context, svc *v1.S return nil } - -func (r *CloudMapReconciler) setClusterPropertyGlobalVariables(ctx context.Context) error { - var err error - ClusterId, err = r.getClusterId(ctx) - if err != nil { - return err - } - ClustersetId, err = r.getClustersetId(ctx) - if err != nil { - return err - } - ClusterPropertiesNeedRefresh = false - r.Log.Debug("ClusterProperties set", "ClusterId", ClusterId, "ClustersetId", ClustersetId) - return nil -} - -func (r *CloudMapReconciler) getClusterId(ctx context.Context) (string, error) { - clusterPropertyForClusterId := &aboutv1alpha1.ClusterProperty{} - err := r.Client.Get(ctx, client.ObjectKey{Name: ClusterIdName}, clusterPropertyForClusterId) - if err != nil { - r.Log.Error(err, "error getting ClusterProperty for clusterId") - return "", err - } - return clusterPropertyForClusterId.Spec.Value, nil -} - -func (r *CloudMapReconciler) getClustersetId(ctx context.Context) (string, error) { - clusterPropertyForClustersetId := &aboutv1alpha1.ClusterProperty{} - err := r.Client.Get(ctx, client.ObjectKey{Name: ClustersetIdName}, clusterPropertyForClustersetId) - if err != nil { - r.Log.Error(err, "error getting ClusterProperty for clustersetId") - return "", err - } - return clusterPropertyForClustersetId.Spec.Value, nil -} diff --git a/pkg/controllers/multicluster/cloudmap_controller_test.go b/pkg/controllers/multicluster/cloudmap_controller_test.go index e113772e..281e4493 100644 --- a/pkg/controllers/multicluster/cloudmap_controller_test.go +++ b/pkg/controllers/multicluster/cloudmap_controller_test.go @@ -31,7 +31,7 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { s.AddKnownTypes(multiclusterv1alpha1.GroupVersion, &multiclusterv1alpha1.ServiceImportList{}, &multiclusterv1alpha1.ServiceImport{}) s.AddKnownTypes(aboutv1alpha1.GroupVersion, &aboutv1alpha1.ClusterProperty{}) - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(objs...).WithObjects(clusterIdForTest(), clustersetIdForTest()).Build() + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(objs...).WithObjects(test.ClusterIdForTest(), test.ClusterSetIdForTest()).Build() // create a mock cloudmap service discovery client mockController := gomock.NewController(t) @@ -39,7 +39,7 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { mockSDClient := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // The service model in the Cloudmap - mockSDClient.EXPECT().ListServices(context.TODO(), test.HttpNsName, test.ClustersetId). + mockSDClient.EXPECT().ListServices(context.TODO(), test.HttpNsName). Return([]*model.Service{test.GetTestServiceWithEndpoint([]*model.Endpoint{test.GetTestEndpoint1()})}, nil) reconciler := getReconciler(t, mockSDClient, fakeClient) @@ -77,8 +77,9 @@ func TestCloudMapReconciler_Reconcile(t *testing.T) { func getReconciler(t *testing.T, mockSDClient *cloudmapMock.MockServiceDiscoveryClient, client client.Client) *CloudMapReconciler { return &CloudMapReconciler{ - Client: client, - Cloudmap: mockSDClient, - Log: common.NewLoggerWithLogr(testr.New(t)), + Client: client, + Cloudmap: mockSDClient, + Log: common.NewLoggerWithLogr(testr.New(t)), + ClusterUtils: common.NewClusterUtils(client), } } diff --git a/pkg/controllers/multicluster/controllers_common_test.go b/pkg/controllers/multicluster/controllers_common_test.go index df045948..2917376a 100644 --- a/pkg/controllers/multicluster/controllers_common_test.go +++ b/pkg/controllers/multicluster/controllers_common_test.go @@ -1,7 +1,6 @@ package controllers import ( - aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" multiclusterv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/multicluster/v1alpha1" "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/test" "github.com/aws/aws-sdk-go-v2/aws" @@ -84,25 +83,3 @@ func endpointSliceWithIpsAndPortsForTest(ips []string, ports []discovery.Endpoin return slice } - -func clusterIdForTest() *aboutv1alpha1.ClusterProperty { - return &aboutv1alpha1.ClusterProperty{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClusterIdName, - }, - Spec: aboutv1alpha1.ClusterPropertySpec{ - Value: test.ClusterId, - }, - } -} - -func clustersetIdForTest() *aboutv1alpha1.ClusterProperty { - return &aboutv1alpha1.ClusterProperty{ - ObjectMeta: metav1.ObjectMeta{ - Name: ClustersetIdName, - }, - Spec: aboutv1alpha1.ClusterPropertySpec{ - Value: test.ClustersetId, - }, - } -} diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 13e7d094..5b9f3ec3 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -29,19 +29,23 @@ import ( ) const ( - ClusterIdName = "id.k8s.io" - ClustersetIdName = "clusterset.k8s.io" K8sVersionAttr = "K8S_CONTROLLER" ServiceExportFinalizer = "multicluster.k8s.aws/service-export-finalizer" EndpointSliceServiceLabel = "kubernetes.io/service-name" ) +var ( + clusterId string + clusterSetId string +) + // ServiceExportReconciler reconciles a ServiceExport object type ServiceExportReconciler struct { - Client client.Client - Log common.Logger - Scheme *runtime.Scheme - CloudMap cloudmap.ServiceDiscoveryClient + Client client.Client + Log common.Logger + Scheme *runtime.Scheme + CloudMap cloudmap.ServiceDiscoveryClient + ClusterUtils common.ClusterUtils } // +kubebuilder:rbac:groups="",resources=services,verbs=get @@ -57,6 +61,18 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques name := req.NamespacedName r.Log.Debug("reconciling ServiceExport", "Namespace", namespace, "Name", name) + var err error + clusterId, err = r.ClusterUtils.GetClusterId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterId") + return ctrl.Result{}, err + } + clusterSetId, err = r.ClusterUtils.GetClusterSetId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterSetId") + return ctrl.Result{}, err + } + serviceExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(ctx, name, &serviceExport); err != nil { if errors.IsNotFound(err) { @@ -72,15 +88,6 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Mark ServiceExport to be deleted, which is indicated by the deletion timestamp being set. isServiceExportMarkedForDelete := serviceExport.GetDeletionTimestamp() != nil - if !isServiceExportMarkedForDelete { - if ClusterId == "" { - return ctrl.Result{}, fmt.Errorf("ClusterProperty %s not found", ClusterIdName) - } - if ClustersetId == "" { - return ctrl.Result{}, fmt.Errorf("ClusterProperty %s not found", ClustersetIdName) - } - } - service := v1.Service{} namespacedName := types.NamespacedName{Namespace: serviceExport.Namespace, Name: serviceExport.Name} if err := r.Client.Get(ctx, namespacedName, &service); err != nil { @@ -177,7 +184,7 @@ func (r *ServiceExportReconciler) handleUpdate(ctx context.Context, serviceExpor } func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context, service *v1.Service) (*model.Service, error) { - cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name, ClustersetId) + cmService, err := r.CloudMap.GetService(ctx, service.Namespace, service.Name) if err != nil { return nil, err } @@ -189,7 +196,7 @@ func (r *ServiceExportReconciler) createOrGetCloudMapService(ctx context.Context "namespace", service.Namespace, "name", service.Name) return nil, err } - if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name, ClustersetId); err != nil { + if cmService, err = r.CloudMap.GetService(ctx, service.Namespace, service.Name); err != nil { return nil, err } } @@ -201,7 +208,7 @@ func (r *ServiceExportReconciler) handleDelete(ctx context.Context, serviceExpor if controllerutil.ContainsFinalizer(serviceExport, ServiceExportFinalizer) { r.Log.Info("removing service export", "namespace", serviceExport.Namespace, "name", serviceExport.Name) - cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name, ClustersetId) + cmService, err := r.CloudMap.GetService(ctx, serviceExport.Namespace, serviceExport.Name) if err != nil { r.Log.Error(err, "error fetching Service from Cloud Map", "namespace", serviceExport.Namespace, "name", serviceExport.Name) @@ -262,8 +269,8 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. IP: IP, EndpointPort: port, ServicePort: servicePortMap[*endpointPort.Name], - ClusterId: ClusterId, - ClustersetId: ClustersetId, + ClusterId: clusterId, + ClusterSetId: clusterSetId, Attributes: attributes, }) } @@ -323,7 +330,6 @@ func (r *ServiceExportReconciler) clusterPropertyEventHandler() handler.MapFunc Namespace: serviceExport.Namespace, }}) } - ClusterPropertiesNeedRefresh = true return result } } diff --git a/pkg/controllers/multicluster/serviceexport_controller_test.go b/pkg/controllers/multicluster/serviceexport_controller_test.go index f8f4f77f..45eaaa16 100644 --- a/pkg/controllers/multicluster/serviceexport_controller_test.go +++ b/pkg/controllers/multicluster/serviceexport_controller_test.go @@ -29,16 +29,12 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest(), clustersetIdForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), test.ClusterIdForTest(), test.ClusterSetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). Build() - // set global variables - ClusterId = test.ClusterId - ClustersetId = test.ClustersetId - // create a mock cloudmap service discovery client mockController := gomock.NewController(t) defer mockController.Finish() @@ -47,8 +43,8 @@ func TestServiceExportReconciler_Reconcile_NewServiceExport(t *testing.T) { // expected interactions with the Cloud Map client // The first get call is expected to return nil, then second call after the creation of service is // supposed to return the value - first := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId).Return(nil, nil) - second := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). + first := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil, nil) + second := mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). Return(&model.Service{Namespace: test.HttpNsName, Name: test.SvcName}, nil) gomock.InOrder(first, second) mock.EXPECT().CreateService(gomock.Any(), test.HttpNsName, test.SvcName).Return(nil).Times(1) @@ -81,23 +77,19 @@ func TestServiceExportReconciler_Reconcile_ExistingServiceExport(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest(), clusterIdForTest(), clustersetIdForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), test.ClusterIdForTest(), test.ClusterSetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). Build() - // set global variables - ClusterId = test.ClusterId - ClustersetId = test.ClustersetId - mockController := gomock.NewController(t) defer mockController.Finish() mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // GetService from Cloudmap returns endpoint1 and endpoint2 - mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). + mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). Return(test.GetTestService(), nil) // call to delete the endpoint not present in the k8s cluster mock.EXPECT().DeleteEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, @@ -132,23 +124,19 @@ func TestServiceExportReconciler_Reconcile_DeleteExistingService(t *testing.T) { serviceExportObj.Finalizers = []string{ServiceExportFinalizer} fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(serviceExportObj, clusterIdForTest(), clustersetIdForTest()). + WithObjects(serviceExportObj, test.ClusterIdForTest(), test.ClusterSetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). Build() - // set global variables - ClusterId = test.ClusterId - ClustersetId = test.ClustersetId - mockController := gomock.NewController(t) defer mockController.Finish() mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) // GetService from Cloudmap returns endpoint1 and endpoint2 - mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName, test.ClustersetId). + mock.EXPECT().GetService(gomock.Any(), test.HttpNsName, test.SvcName). Return(test.GetTestService(), nil) // call to delete the endpoint in the cloudmap mock.EXPECT().DeleteEndpoints(gomock.Any(), test.HttpNsName, test.SvcName, @@ -177,14 +165,43 @@ func TestServiceExportReconciler_Reconcile_NoClusterId(t *testing.T) { // create a fake controller client and add some objects fakeClient := fake.NewClientBuilder(). WithScheme(getServiceExportScheme()). - WithObjects(k8sServiceForTest(), serviceExportForTest()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), test.ClusterSetIdForTest()). WithLists(&discovery.EndpointSliceList{ Items: []discovery.EndpointSlice{*endpointSliceForTest()}, }). Build() - ClusterId = "" - ClustersetId = "" + // create a mock cloudmap service discovery client + mockController := gomock.NewController(t) + defer mockController.Finish() + + mock := cloudmapMock.NewMockServiceDiscoveryClient(mockController) + + reconciler := getServiceExportReconciler(t, mock, fakeClient) + + request := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: test.HttpNsName, + Name: test.SvcName, + }, + } + + // Reconciling should throw an error + got, err := reconciler.Reconcile(context.Background(), request) + expectedError := fmt.Errorf("clusterproperties.about.k8s.io \"id.k8s.io\" not found") + assert.ErrorContains(t, err, expectedError.Error()) + assert.Equal(t, ctrl.Result{}, got, "Result should be empty") +} + +func TestServiceExportReconciler_Reconcile_NoClustersetId(t *testing.T) { + // create a fake controller client and add some objects + fakeClient := fake.NewClientBuilder(). + WithScheme(getServiceExportScheme()). + WithObjects(k8sServiceForTest(), serviceExportForTest(), test.ClusterIdForTest()). + WithLists(&discovery.EndpointSliceList{ + Items: []discovery.EndpointSlice{*endpointSliceForTest()}, + }). + Build() // create a mock cloudmap service discovery client mockController := gomock.NewController(t) @@ -203,7 +220,7 @@ func TestServiceExportReconciler_Reconcile_NoClusterId(t *testing.T) { // Reconciling should throw an error got, err := reconciler.Reconcile(context.Background(), request) - expectedError := fmt.Errorf("ClusterProperty id.k8s.io not found") + expectedError := fmt.Errorf("clusterproperties.about.k8s.io \"clusterset.k8s.io\" not found") assert.ErrorContains(t, err, expectedError.Error()) assert.Equal(t, ctrl.Result{}, got, "Result should be empty") } @@ -219,9 +236,10 @@ func getServiceExportScheme() *runtime.Scheme { func getServiceExportReconciler(t *testing.T, mockClient *cloudmapMock.MockServiceDiscoveryClient, client client.Client) *ServiceExportReconciler { return &ServiceExportReconciler{ - Client: client, - Log: common.NewLoggerWithLogr(testr.New(t)), - Scheme: client.Scheme(), - CloudMap: mockClient, + Client: client, + Log: common.NewLoggerWithLogr(testr.New(t)), + Scheme: client.Scheme(), + CloudMap: mockClient, + ClusterUtils: common.NewClusterUtils(client), } } diff --git a/pkg/controllers/multicluster/suite_test.go b/pkg/controllers/multicluster/suite_test.go index 952f7d0f..6ea09944 100644 --- a/pkg/controllers/multicluster/suite_test.go +++ b/pkg/controllers/multicluster/suite_test.go @@ -38,6 +38,8 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") + // TODO: Add CRDs for about.k8s.io_clusterproperties & add patch + crds := []string{ filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceexports.yaml"), filepath.Join("..", "..", "..", "config", "crd", "bases", "multicluster.x-k8s.io_serviceimports.yaml"), diff --git a/pkg/model/types.go b/pkg/model/types.go index 2062c97c..9daedfc1 100644 --- a/pkg/model/types.go +++ b/pkg/model/types.go @@ -40,7 +40,7 @@ type Endpoint struct { EndpointPort Port ServicePort Port ClusterId string - ClustersetId string + ClusterSetId string Attributes map[string]string } @@ -59,7 +59,7 @@ const ( EndpointPortNameAttr = "ENDPOINT_PORT_NAME" EndpointProtocolAttr = "ENDPOINT_PROTOCOL" ClusterIdAttr = "CLUSTER_ID" - ClustersetIdAttr = "CLUSTERSET_ID" + ClusterSetIdAttr = "CLUSTERSET_ID" ServicePortNameAttr = "SERVICE_PORT_NAME" ServicePortAttr = "SERVICE_PORT" ServiceTargetPortAttr = "SERVICE_TARGET_PORT" @@ -90,11 +90,11 @@ func NewEndpointFromInstance(inst *types.HttpInstanceSummary) (endpointPtr *Endp return nil, err } - if endpoint.ClusterId, err = clusterIdFromAttr(attributes); err != nil { + if endpoint.ClusterId, err = removeStringAttr(attributes, ClusterIdAttr); err != nil { return nil, err } - if endpoint.ClustersetId, err = clustersetIdFromAttr(attributes); err != nil { + if endpoint.ClusterSetId, err = removeStringAttr(attributes, ClusterSetIdAttr); err != nil { return nil, err } @@ -135,16 +135,6 @@ func servicePortFromAttr(attributes map[string]string) (port Port, err error) { return port, err } -func clusterIdFromAttr(attributes map[string]string) (clusterId string, err error) { - clusterId, err = removeStringAttr(attributes, ClusterIdAttr) - return clusterId, err -} - -func clustersetIdFromAttr(attributes map[string]string) (clustersetId string, err error) { - clustersetId, err = removeStringAttr(attributes, ClustersetIdAttr) - return clustersetId, err -} - func removeStringAttr(attributes map[string]string, attr string) (string, error) { if value, hasValue := attributes[attr]; hasValue { delete(attributes, attr) @@ -171,7 +161,7 @@ func (e *Endpoint) GetCloudMapAttributes() map[string]string { attrs := make(map[string]string) attrs[ClusterIdAttr] = e.ClusterId - attrs[ClustersetIdAttr] = e.ClustersetId + attrs[ClusterSetIdAttr] = e.ClusterSetId attrs[EndpointIpv4Attr] = e.IP attrs[EndpointPortAttr] = strconv.Itoa(int(e.EndpointPort.Port)) attrs[EndpointProtocolAttr] = e.EndpointPort.Protocol diff --git a/pkg/model/types_test.go b/pkg/model/types_test.go index 80820e9d..b8352717 100644 --- a/pkg/model/types_test.go +++ b/pkg/model/types_test.go @@ -10,7 +10,7 @@ import ( var instId = "my-instance" var ip = "192.168.0.1" var clusterId = "test-mcs-clusterId" -var clustersetId = "test-mcs-clustersetId" +var clusterSetId = "test-mcs-clusterSetId" func TestNewEndpointFromInstance(t *testing.T) { tests := []struct { @@ -25,7 +25,7 @@ func TestNewEndpointFromInstance(t *testing.T) { InstanceId: &instId, Attributes: map[string]string{ ClusterIdAttr: clusterId, - ClustersetIdAttr: clustersetId, + ClusterSetIdAttr: clusterSetId, EndpointIpv4Attr: ip, EndpointPortAttr: "80", EndpointProtocolAttr: "TCP", @@ -52,7 +52,7 @@ func TestNewEndpointFromInstance(t *testing.T) { Protocol: "TCP", }, ClusterId: clusterId, - ClustersetId: clustersetId, + ClusterSetId: clusterSetId, Attributes: map[string]string{ "custom-attr": "custom-val", }, @@ -103,7 +103,7 @@ func TestNewEndpointFromInstance(t *testing.T) { inst: &types.HttpInstanceSummary{ InstanceId: &instId, Attributes: map[string]string{ - ClustersetIdAttr: clustersetId, + ClusterSetIdAttr: clusterSetId, EndpointIpv4Attr: ip, EndpointPortAttr: "80", EndpointProtocolAttr: "TCP", @@ -158,7 +158,7 @@ func TestEndpoint_GetAttributes(t *testing.T) { EndpointPort Port ServicePort Port ClusterId string - ClustersetId string + ClusterSetId string Attributes map[string]string } tests := []struct { @@ -182,14 +182,14 @@ func TestEndpoint_GetAttributes(t *testing.T) { Protocol: "TCP", }, ClusterId: clusterId, - ClustersetId: clustersetId, + ClusterSetId: clusterSetId, Attributes: map[string]string{ "custom-attr": "custom-val", }, }, want: map[string]string{ ClusterIdAttr: clusterId, - ClustersetIdAttr: clustersetId, + ClusterSetIdAttr: clusterSetId, EndpointIpv4Attr: ip, EndpointPortAttr: "80", EndpointProtocolAttr: "TCP", @@ -210,7 +210,7 @@ func TestEndpoint_GetAttributes(t *testing.T) { EndpointPort: tt.fields.EndpointPort, ServicePort: tt.fields.ServicePort, ClusterId: tt.fields.ClusterId, - ClustersetId: tt.fields.ClustersetId, + ClusterSetId: tt.fields.ClusterSetId, Attributes: tt.fields.Attributes, } if got := e.GetCloudMapAttributes(); !reflect.DeepEqual(got, tt.want) { diff --git a/test/test-constants.go b/test/test-constants.go index cef3e418..a42e1a87 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -3,6 +3,11 @@ package test import ( "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + aboutv1alpha1 "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/apis/about/v1alpha1" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/common" + "github.com/aws/aws-cloud-map-mcs-controller-for-k8s/pkg/model" ) @@ -14,7 +19,7 @@ const ( SvcName = "svc-name" SvcId = "svc-id" ClusterId = "test-mcs-clusterId" - ClustersetId = "test-mcs-clustersetId" + ClustersetId = "test-mcs-clusterSetId" EndptId1 = "tcp-192_168_0_1-1" EndptId2 = "tcp-192_168_0_2-2" EndptIp1 = "192.168.0.1" @@ -84,7 +89,7 @@ func GetTestEndpoint1() *model.Endpoint { Protocol: Protocol1, }, ClusterId: ClusterId, - ClustersetId: ClustersetId, + ClusterSetId: ClustersetId, Attributes: make(map[string]string), } } @@ -105,7 +110,7 @@ func GetTestEndpoint2() *model.Endpoint { Protocol: Protocol2, }, ClusterId: ClusterId, - ClustersetId: ClustersetId, + ClusterSetId: ClustersetId, Attributes: make(map[string]string), } } @@ -121,3 +126,25 @@ func GetTestEndpoints(count int) (endpts []*model.Endpoint) { } return endpts } + +func ClusterIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ClusterIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: ClusterId, + }, + } +} + +func ClusterSetIdForTest() *aboutv1alpha1.ClusterProperty { + return &aboutv1alpha1.ClusterProperty{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.ClusterSetIdName, + }, + Spec: aboutv1alpha1.ClusterPropertySpec{ + Value: ClustersetId, + }, + } +} From 62e4f8d438cae1be24e1f91fbce8ba231c075069 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Fri, 29 Jul 2022 21:17:44 +0000 Subject: [PATCH 29/30] fix merge conflicts --- config/crd/bases/about.k8s.io_clusterproperties.yaml | 8 +------- pkg/model/types_test.go | 6 +++--- test/test-constants.go | 6 ++---- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/config/crd/bases/about.k8s.io_clusterproperties.yaml b/config/crd/bases/about.k8s.io_clusterproperties.yaml index 71b36cf2..45de7c04 100644 --- a/config/crd/bases/about.k8s.io_clusterproperties.yaml +++ b/config/crd/bases/about.k8s.io_clusterproperties.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: clusterproperties.about.k8s.io spec: @@ -57,9 +57,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/pkg/model/types_test.go b/pkg/model/types_test.go index 5f4fd385..2ef9a66c 100644 --- a/pkg/model/types_test.go +++ b/pkg/model/types_test.go @@ -55,7 +55,7 @@ func TestNewEndpointFromInstance(t *testing.T) { }, ClusterId: clusterId, ClusterSetId: clusterSetId, - ServiceType: ServiceType(serviceType), + ServiceType: ServiceType(serviceType), Attributes: map[string]string{ "custom-attr": "custom-val", }, @@ -140,7 +140,7 @@ func TestNewEndpointFromInstance(t *testing.T) { }, wantErr: true, }, - { + { name: "missing service type", inst: &types.HttpInstanceSummary{ InstanceId: &instId, @@ -208,7 +208,7 @@ func TestEndpoint_GetAttributes(t *testing.T) { }, ClusterId: clusterId, ClusterSetId: clusterSetId, - ServiceType: ServiceType(serviceType), + ServiceType: ServiceType(serviceType), Attributes: map[string]string{ "custom-attr": "custom-val", }, diff --git a/test/test-constants.go b/test/test-constants.go index f1bb2ce3..22b0e108 100644 --- a/test/test-constants.go +++ b/test/test-constants.go @@ -91,9 +91,8 @@ func GetTestEndpoint1() *model.Endpoint { }, ClusterId: ClusterId, ClusterSetId: ClustersetId, + ServiceType: model.ClusterSetIPType, Attributes: make(map[string]string), - ServiceType: model.ClusterSetIPType, - Attributes: make(map[string]string), } } @@ -114,9 +113,8 @@ func GetTestEndpoint2() *model.Endpoint { }, ClusterId: ClusterId, ClusterSetId: ClustersetId, + ServiceType: model.ClusterSetIPType, Attributes: make(map[string]string), - ServiceType: model.ClusterSetIPType, - Attributes: make(map[string]string), } } From 1aa5dde3de2d2667704e94db7b2476bd8d5d06b9 Mon Sep 17 00:00:00 2001 From: matthewgoodman13 Date: Fri, 29 Jul 2022 22:26:44 +0000 Subject: [PATCH 30/30] clusterId/setId method called twice --- .../multicluster/cloudmap_controller.go | 5 +++-- .../multicluster/serviceexport_controller.go | 21 ++++++++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/multicluster/cloudmap_controller.go b/pkg/controllers/multicluster/cloudmap_controller.go index 9d5f821e..276cb651 100644 --- a/pkg/controllers/multicluster/cloudmap_controller.go +++ b/pkg/controllers/multicluster/cloudmap_controller.go @@ -55,16 +55,17 @@ func (r *CloudMapReconciler) Start(ctx context.Context) error { // Reconcile triggers a single reconciliation round func (r *CloudMapReconciler) Reconcile(ctx context.Context) error { var err error - clusterId, err = r.ClusterUtils.GetClusterId(ctx) + clusterId, err := r.ClusterUtils.GetClusterId(ctx) if err != nil { r.Log.Error(err, "unable to retrieve clusterId") return err } - clusterSetId, err = r.ClusterUtils.GetClusterSetId(ctx) + clusterSetId, err := r.ClusterUtils.GetClusterSetId(ctx) if err != nil { r.Log.Error(err, "unable to retrieve clusterSetId") return err } + r.Log.Debug("ClusterId and ClusterSetId found", "ClusterId", clusterId, "ClusterSetId", clusterSetId) namespaces := v1.NamespaceList{} if err := r.Client.List(ctx, &namespaces); err != nil { diff --git a/pkg/controllers/multicluster/serviceexport_controller.go b/pkg/controllers/multicluster/serviceexport_controller.go index 79bd7baa..535e4e98 100644 --- a/pkg/controllers/multicluster/serviceexport_controller.go +++ b/pkg/controllers/multicluster/serviceexport_controller.go @@ -34,11 +34,6 @@ const ( EndpointSliceServiceLabel = "kubernetes.io/service-name" ) -var ( - clusterId string - clusterSetId string -) - // ServiceExportReconciler reconciles a ServiceExport object type ServiceExportReconciler struct { Client client.Client @@ -62,16 +57,17 @@ func (r *ServiceExportReconciler) Reconcile(ctx context.Context, req ctrl.Reques r.Log.Debug("reconciling ServiceExport", "Namespace", namespace, "Name", name) var err error - clusterId, err = r.ClusterUtils.GetClusterId(ctx) + clusterId, err := r.ClusterUtils.GetClusterId(ctx) if err != nil { r.Log.Error(err, "unable to retrieve clusterId") return ctrl.Result{}, err } - clusterSetId, err = r.ClusterUtils.GetClusterSetId(ctx) + clusterSetId, err := r.ClusterUtils.GetClusterSetId(ctx) if err != nil { r.Log.Error(err, "unable to retrieve clusterSetId") return ctrl.Result{}, err } + r.Log.Debug("ClusterId and ClusterSetId found", "ClusterId", clusterId, "ClusterSetId", clusterSetId) serviceExport := multiclusterv1alpha1.ServiceExport{} if err := r.Client.Get(ctx, name, &serviceExport); err != nil { @@ -251,6 +247,17 @@ func (r *ServiceExportReconciler) extractEndpoints(ctx context.Context, svc *v1. servicePortMap[svcPort.Name] = ServicePortToPort(svcPort) } + clusterId, err := r.ClusterUtils.GetClusterId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterId") + return nil, err + } + clusterSetId, err := r.ClusterUtils.GetClusterSetId(ctx) + if err != nil { + r.Log.Error(err, "unable to retrieve clusterSetId") + return nil, err + } + for _, slice := range endpointSlices.Items { if slice.AddressType != discovery.AddressTypeIPv4 { return nil, fmt.Errorf("unsupported address type %s for service %s", slice.AddressType, svc.Name)