From 9eba871cb618194da8f73e7e49b54de2b0302301 Mon Sep 17 00:00:00 2001 From: yiannistri <8741709+yiannistri@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:25:36 +0100 Subject: [PATCH] fix: Disable monitoring addon when spec.Monitoring is false (cherry picked from commit 146ee4ddd822631cd0d222a910a2d33813175826) --- controller/aks-cluster-config-handler.go | 54 ++++++++++++------- controller/aks-cluster-config-handler_test.go | 17 ++++++ pkg/aks/create.go | 9 +++- pkg/aks/create_test.go | 13 ++++- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/controller/aks-cluster-config-handler.go b/controller/aks-cluster-config-handler.go index 23529d45..4f6cce43 100644 --- a/controller/aks-cluster-config-handler.go +++ b/controller/aks-cluster-config-handler.go @@ -676,24 +676,31 @@ func (h *Handler) buildUpstreamClusterState(ctx context.Context, credentials *ak if addonProfile["omsAgent"] != nil { upstreamSpec.Monitoring = addonProfile["omsAgent"].Enabled - if len(addonProfile["omsAgent"].Config) == 0 { - return nil, fmt.Errorf("cannot set OMS Agent configuration retrieved from Azure") - } - logAnalyticsWorkspaceResourceID := addonProfile["omsAgent"].Config["logAnalyticsWorkspaceResourceID"] + // If the monitoring addon is enabled, attempt to populate the Log Analytics + // workspace name and group from the Azure OMS Agent configuration. + if aks.Bool(addonProfile["omsAgent"].Enabled) && addonProfile["omsAgent"].Config != nil { + if len(addonProfile["omsAgent"].Config) == 0 { + return nil, fmt.Errorf("cannot set OMS Agent configuration retrieved from Azure") + } + logAnalyticsWorkspaceResourceID := addonProfile["omsAgent"].Config["logAnalyticsWorkspaceResourceID"] - group := matchWorkspaceGroup.FindStringSubmatch(aks.String(logAnalyticsWorkspaceResourceID)) - if group == nil { - return nil, fmt.Errorf("OMS Agent configuration workspace group was not found") - } - logAnalyticsWorkspaceGroup := group[1] - upstreamSpec.LogAnalyticsWorkspaceGroup = to.Ptr(logAnalyticsWorkspaceGroup) + group := matchWorkspaceGroup.FindStringSubmatch(aks.String(logAnalyticsWorkspaceResourceID)) + if group == nil { + return nil, fmt.Errorf("OMS Agent configuration workspace group was not found") + } + logAnalyticsWorkspaceGroup := group[1] + upstreamSpec.LogAnalyticsWorkspaceGroup = to.Ptr(logAnalyticsWorkspaceGroup) - name := matchWorkspaceName.FindStringSubmatch(aks.String(logAnalyticsWorkspaceResourceID)) - if name == nil { - return nil, fmt.Errorf("OMS Agent configuration workspace name was not found") + name := matchWorkspaceName.FindStringSubmatch(aks.String(logAnalyticsWorkspaceResourceID)) + if name == nil { + return nil, fmt.Errorf("OMS Agent configuration workspace name was not found") + } + logAnalyticsWorkspaceName := name[1] + upstreamSpec.LogAnalyticsWorkspaceName = to.Ptr(logAnalyticsWorkspaceName) + } else if addonProfile["omsAgent"].Enabled != nil && !aks.Bool(addonProfile["omsAgent"].Enabled) { + upstreamSpec.LogAnalyticsWorkspaceGroup = nil + upstreamSpec.LogAnalyticsWorkspaceName = nil } - logAnalyticsWorkspaceName := name[1] - upstreamSpec.LogAnalyticsWorkspaceName = to.Ptr(logAnalyticsWorkspaceName) } // set API server access profile @@ -791,10 +798,19 @@ func (h *Handler) updateUpstreamClusterState(ctx context.Context, config *aksv1. if config.Spec.Monitoring != nil { if aks.Bool(config.Spec.Monitoring) != aks.Bool(upstreamSpec.Monitoring) { logrus.Infof("Updating monitoring addon for cluster [%s]", config.Spec.ClusterName) - updateAksCluster = true - importedClusterSpec.Monitoring = config.Spec.Monitoring - importedClusterSpec.LogAnalyticsWorkspaceGroup = config.Spec.LogAnalyticsWorkspaceGroup - importedClusterSpec.LogAnalyticsWorkspaceName = config.Spec.LogAnalyticsWorkspaceName + if config.Spec.Monitoring != nil && !aks.Bool(config.Spec.Monitoring) { + logrus.Debugf("Disabling monitoring addon for cluster [%s]", config.Spec.ClusterName) + updateAksCluster = true + importedClusterSpec.Monitoring = config.Spec.Monitoring + importedClusterSpec.LogAnalyticsWorkspaceGroup = nil + importedClusterSpec.LogAnalyticsWorkspaceName = nil + } else if config.Spec.Monitoring != nil && aks.Bool(config.Spec.Monitoring) { + logrus.Debugf("Enabling monitoring addon for cluster [%s]", config.Spec.ClusterName) + updateAksCluster = true + importedClusterSpec.Monitoring = config.Spec.Monitoring + importedClusterSpec.LogAnalyticsWorkspaceGroup = config.Spec.LogAnalyticsWorkspaceGroup + importedClusterSpec.LogAnalyticsWorkspaceName = config.Spec.LogAnalyticsWorkspaceName + } } } diff --git a/controller/aks-cluster-config-handler_test.go b/controller/aks-cluster-config-handler_test.go index b8034ac2..651f6f5d 100644 --- a/controller/aks-cluster-config-handler_test.go +++ b/controller/aks-cluster-config-handler_test.go @@ -937,4 +937,21 @@ var _ = Describe("buildUpstreamClusterState", func() { _, err := handler.buildUpstreamClusterState(ctx, credentials, &aksConfig.Spec) Expect(err).ToNot(HaveOccurred()) }) + + It("should succeed even if the monitoring addon is disabled", func() { + clusterState.Properties.AddonProfiles["omsAgent"] = &armcontainerservice.ManagedClusterAddonProfile{ + Enabled: to.Ptr(false), + Config: nil, + } + clusterClientMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), nil).Return( + armcontainerservice.ManagedClustersClientGetResponse{ + ManagedCluster: *clusterState, + }, nil) + + upstreamSpec, err := handler.buildUpstreamClusterState(ctx, credentials, &aksConfig.Spec) + Expect(err).ToNot(HaveOccurred()) + Expect(*upstreamSpec.Monitoring).To(BeFalse()) + Expect(upstreamSpec.LogAnalyticsWorkspaceGroup).To(BeNil()) + Expect(upstreamSpec.LogAnalyticsWorkspaceGroup).To(BeNil()) + }) }) diff --git a/pkg/aks/create.go b/pkg/aks/create.go index ed2880a5..6cdb8da6 100644 --- a/pkg/aks/create.go +++ b/pkg/aks/create.go @@ -224,7 +224,8 @@ func createManagedCluster(ctx context.Context, cred *Credentials, workplacesClie } // Get monitoring from config spec - if Bool(spec.Monitoring) { + if spec.Monitoring != nil && Bool(spec.Monitoring) { + logrus.Debug("Monitoring is enabled") managedCluster.Properties.AddonProfiles["omsAgent"] = &armcontainerservice.ManagedClusterAddonProfile{ Enabled: spec.Monitoring, } @@ -243,6 +244,12 @@ func createManagedCluster(ctx context.Context, cred *Credentials, workplacesClie managedCluster.Properties.AddonProfiles["omsAgent"].Config = map[string]*string{ "logAnalyticsWorkspaceResourceID": to.Ptr(logAnalyticsWorkspaceResourceID), } + } else if spec.Monitoring != nil && !Bool(spec.Monitoring) { + logrus.Debug("Monitoring is disabled") + managedCluster.Properties.AddonProfiles["omsAgent"] = &armcontainerservice.ManagedClusterAddonProfile{ + Enabled: spec.Monitoring, + Config: nil, + } } if phase != "updating" && phase != "active" { diff --git a/pkg/aks/create_test.go b/pkg/aks/create_test.go index 70e790ae..b8292c85 100644 --- a/pkg/aks/create_test.go +++ b/pkg/aks/create_test.go @@ -334,7 +334,18 @@ var _ = Describe("newManagedCluster", func() { managedCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "test-phase") Expect(err).ToNot(HaveOccurred()) - Expect(managedCluster.Properties.AddonProfiles).ToNot(HaveKey("omsagent")) + Expect(managedCluster.Properties.AddonProfiles).ToNot(HaveKey("omsAgent")) + }) + + It("should successfully create managed cluster with monitoring disabled", func() { + workplacesClientMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(armoperationalinsights.WorkspacesClientGetResponse{}, nil).Times(0) + flag := false + clusterSpec.Monitoring = &flag + managedCluster, err := createManagedCluster(ctx, cred, workplacesClientMock, clusterSpec, "test-phase") + Expect(err).ToNot(HaveOccurred()) + + Expect(managedCluster.Properties.AddonProfiles).To(HaveKey("omsAgent")) + Expect(*managedCluster.Properties.AddonProfiles["omsAgent"].Enabled).To(BeFalse()) }) It("should successfully create managed cluster when phase is set to active", func() {