Merge pull request #637 from yiannistri/584-disable-monitoring-main

fix: Disable monitoring addon when spec.Monitoring is false
rancher · Aug 16, 2024 · bc0a243 · bc0a243
2 parents 40dd028 + 146ee4d
commit bc0a243
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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() {