From acd37a912019c88096d06993d34ba1ccf19721d4 Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Tue, 24 Oct 2023 23:29:11 -0500 Subject: [PATCH] use tags service for managedcluster tags --- azure/scope/managedcontrolplane.go | 12 ++++++++ azure/services/managedclusters/spec.go | 30 ++++--------------- azure/services/managedclusters/spec_test.go | 4 +++ .../azuremanagedcontrolplane_reconciler.go | 6 ++++ test/e2e/aks_tags.go | 23 +++++++------- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 332b7f0a8b1..0782694c065 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -764,6 +764,18 @@ func (s *ManagedControlPlaneScope) SetAnnotation(key, value string) { s.ControlPlane.Annotations[key] = value } +// TagsSpecs returns the tag specs for the ManagedControlPlane. +func (s *ManagedControlPlaneScope) TagsSpecs() []azure.TagsSpec { + specs := []azure.TagsSpec{ + { + Scope: azure.ManagedClusterID(s.SubscriptionID(), s.ResourceGroup(), s.ManagedClusterSpec().ResourceRef().Name), + Tags: s.AdditionalTags(), + Annotation: azure.ManagedClusterTagsLastAppliedAnnotation, + }, + } + return specs +} + // AvailabilityStatusResource refers to the AzureManagedControlPlane. func (s *ManagedControlPlaneScope) AvailabilityStatusResource() conditions.Setter { return s.ControlPlane diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 2c323254de1..f3ba08f7e25 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -31,7 +31,6 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/aso" "sigs.k8s.io/cluster-api-provider-azure/util/tele" "sigs.k8s.io/cluster-api/util/secret" ) @@ -308,7 +307,7 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai ClusterName: s.ClusterName, Name: ptr.To(s.Name), Role: ptr.To(infrav1.CommonRole), - // Additional tags managed separately + Additional: s.Tags, }), }, } @@ -513,6 +512,11 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing *asocontai } } + if managedCluster.Status.Tags != nil { + // tags managed separately + spec.Tags = nil + } + return managedCluster, nil } @@ -588,25 +592,3 @@ func (s *ManagedClusterSpec) WasManaged(resource *asocontainerservicev1.ManagedC // CAPZ has never supported BYO managed clusters. return true } - -var _ aso.TagsGetterSetter[*asocontainerservicev1.ManagedCluster] = (*ManagedClusterSpec)(nil) - -// GetAdditionalTags implements aso.TagsGetterSetter. -func (s *ManagedClusterSpec) GetAdditionalTags() infrav1.Tags { - return s.Tags -} - -// GetDesiredTags implements aso.TagsGetterSetter. -func (*ManagedClusterSpec) GetDesiredTags(resource *asocontainerservicev1.ManagedCluster) infrav1.Tags { - return resource.Spec.Tags -} - -// GetActualTags implements aso.TagsGetterSetter. -func (*ManagedClusterSpec) GetActualTags(resource *asocontainerservicev1.ManagedCluster) infrav1.Tags { - return resource.Status.Tags -} - -// SetTags implements aso.TagsGetterSetter. -func (*ManagedClusterSpec) SetTags(resource *asocontainerservicev1.ManagedCluster, tags infrav1.Tags) { - resource.Spec.Tags = tags -} diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index b84a157ff08..0448cc0e658 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -511,6 +511,10 @@ func getExistingCluster() *asocontainerservicev1.ManagedCluster { // only nil vs. non-nil matters here mc.Status.AgentPoolProfiles = []asocontainerservicev1.ManagedClusterAgentPoolProfile_STATUS{} + // tags managed separately + mc.Spec.Tags = nil + mc.Status.Tags = map[string]string{} + return mc } diff --git a/controllers/azuremanagedcontrolplane_reconciler.go b/controllers/azuremanagedcontrolplane_reconciler.go index 1c13e01ab05..fa303cb7ad2 100644 --- a/controllers/azuremanagedcontrolplane_reconciler.go +++ b/controllers/azuremanagedcontrolplane_reconciler.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure/services/privateendpoints" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourcehealth" "sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/tags" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" "sigs.k8s.io/cluster-api-provider-azure/util/tele" "sigs.k8s.io/cluster-api/util/secret" @@ -56,6 +57,10 @@ func newAzureManagedControlPlaneReconciler(scope *scope.ManagedControlPlaneScope if err != nil { return nil, err } + tagsSvc, err := tags.New(scope) + if err != nil { + return nil, err + } virtualNetworksSvc, err := virtualnetworks.New(scope) if err != nil { return nil, err @@ -69,6 +74,7 @@ func newAzureManagedControlPlaneReconciler(scope *scope.ManagedControlPlaneScope subnetsSvc, managedclusters.New(scope), privateEndpointsSvc, + tagsSvc, resourceHealthSvc, }, }, nil diff --git a/test/e2e/aks_tags.go b/test/e2e/aks_tags.go index 474db7061fd..5fedf1cbcfc 100644 --- a/test/e2e/aks_tags.go +++ b/test/e2e/aks_tags.go @@ -98,11 +98,16 @@ func AKSAdditionalTagsSpec(ctx context.Context, inputGetter func() AKSAdditional } } + By("Deleting all tags for control plane") + expectedTags = nil var initialTags infrav1.Tags Eventually(func(g Gomega) { g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) initialTags = infraControlPlane.Spec.AdditionalTags + infraControlPlane.Spec.AdditionalTags = expectedTags + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags, input.WaitForUpdate...).Should(Succeed()) By("Creating tags for control plane") expectedTags = infrav1.Tags{ @@ -127,16 +132,14 @@ func AKSAdditionalTagsSpec(ctx context.Context, inputGetter func() AKSAdditional }, inputGetter().WaitForUpdate...).Should(Succeed()) Eventually(checkTags, input.WaitForUpdate...).Should(Succeed()) - if initialTags != nil { - By("Restoring initial tags for control plane") - expectedTags = initialTags - Eventually(func(g Gomega) { - g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) - infraControlPlane.Spec.AdditionalTags = expectedTags - g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) - }, inputGetter().WaitForUpdate...).Should(Succeed()) - Eventually(checkTags, input.WaitForUpdate...).Should(Succeed()) - } + By("Restoring initial tags for control plane") + expectedTags = initialTags + Eventually(func(g Gomega) { + g.Expect(mgmtClient.Get(ctx, client.ObjectKeyFromObject(infraControlPlane), infraControlPlane)).To(Succeed()) + infraControlPlane.Spec.AdditionalTags = expectedTags + g.Expect(mgmtClient.Update(ctx, infraControlPlane)).To(Succeed()) + }, inputGetter().WaitForUpdate...).Should(Succeed()) + Eventually(checkTags, input.WaitForUpdate...).Should(Succeed()) }() for _, mp := range input.MachinePools {