[ACM-7638] Remove legacy prometheuesrule and servicemonitor from open…

…shift-monit… (stolostron#1156) * remove legacy prometheuesrule and servicemonitor from openshift-monitoring ns Signed-off-by: Disaiah Bennett <[email protected]> * updated test cases for methods func Signed-off-by: Disaiah Bennett <[email protected]> --------- Signed-off-by: Disaiah Bennett <[email protected]>
zlayne · Oct 4, 2023 · f685867 · f685867
1 parent a231cc2
commit f685867
Show file tree

Hide file tree

Showing 11 changed files with 220 additions and 39 deletions.
diff --git a/.github/workflows/regenerate-bundles-2.7.yml b/.github/workflows/regenerate-bundles-2.7.yml
@@ -35,6 +35,7 @@ jobs:
 
       # Runs a set of commands using the runners shell
       - name: Regenerate Operator Bundles
+        continue-on-error: true
         id: generate_bundle
         run: |
           make regenerate-operator-bundles
@@ -43,9 +44,9 @@ jobs:
         id: job_status
         run: |
           if [[ ${{ steps.generate_bundle.outcome }} == 'failure' ]]; then
-            echo "::set-output name=status::failure"
+            echo "status=failure" >> $GITHUB_OUTPUT
           else
-            echo "::set-output name=status::success"
+            echo "status=success" >> $GITHUB_OUTPUT
           fi
 
       - name: Send Slack Message on Failure

diff --git a/.github/workflows/regenerate-bundles-2.8.yml b/.github/workflows/regenerate-bundles-2.8.yml
@@ -35,6 +35,7 @@ jobs:
 
       # Runs a set of commands using the runners shell
       - name: Regenerate Operator Bundles
+        continue-on-error: true
         id: generate_bundle
         run: |
           make regenerate-operator-bundles
@@ -43,9 +44,9 @@ jobs:
         id: job_status
         run: |
           if [[ ${{ steps.generate_bundle.outcome }} == 'failure' ]]; then
-            echo "::set-output name=status::failure"
+            echo "status=failure" >> $GITHUB_OUTPUT
           else
-            echo "::set-output name=status::success"
+            echo "status=success" >> $GITHUB_OUTPUT
           fi
 
       - name: Send Slack Message on Failure

diff --git a/.github/workflows/regenerate-bundles.yml b/.github/workflows/regenerate-bundles.yml
@@ -34,6 +34,7 @@ jobs:
 
       # Runs a set of commands using the runners shell
       - name: Regenerate Operator Bundles
+        continue-on-error: true
         id: generate_bundle
         run: |
           make regenerate-operator-bundles
@@ -42,9 +43,9 @@ jobs:
         id: job_status
         run: |
           if [[ ${{ steps.generate_bundle.outcome }} == 'failure' ]]; then
-            echo "::set-output name=status::failure"
+            echo "status=failure" >> $GITHUB_OUTPUT
           else
-            echo "::set-output name=status::success"
+            echo "status=success" >> $GITHUB_OUTPUT
           fi
 
       - name: Send Slack Message on Failure

diff --git a/.github/workflows/update-pregenerated-charts.yml b/.github/workflows/update-pregenerated-charts.yml
@@ -34,6 +34,7 @@ jobs:
 
       # Runs a set of commands using the runners shell
       - name: Regenerate Operator Bundles
+        continue-on-error: true
         id: generate_bundle
         run: |
           make copy-charts
@@ -42,9 +43,9 @@ jobs:
         id: job_status
         run: |
           if [[ ${{ steps.generate_bundle.outcome }} == 'failure' ]]; then
-            echo "::set-output name=status::failure"
+            echo "status=failure" >> $GITHUB_OUTPUT
           else
-            echo "::set-output name=status::success"
+            echo "status=success" >> $GITHUB_OUTPUT
           fi
 
       - name: Send Slack Message on Failure

diff --git a/api/v1/multiclusterhub_methods.go b/api/v1/multiclusterhub_methods.go
@@ -113,10 +113,27 @@ var MCEComponents = []string{
 	MCEServerFoundation,
 }
 
-// clusterManagementAddOns is a map that associates certain component names with their corresponding add-ons.
-var clusterManagementAddOns = map[string]string{
+var LegacyPrometheusKind = []string{"PrometheusRule", "ServiceMonitor"}
+
+// MCHPrometheusRules is a map that associates certain component names with their corresponding prometheus rules.
+var MCHPrometheusRules = map[string]string{
+	Console: "acm-console-prometheus-rules",
+	GRC:     "ocm-grc-policy-propagator-metrics",
+	// Add other components here when PrometheusRules is required.
+}
+
+// MCHServiceMonitors is a map that associates certain component names with their corresponding service monitors.
+var MCHServiceMonitors = map[string]string{
+	Console:  "console-monitor",
+	GRC:      "ocm-grc-policy-propagator-metrics",
+	Insights: "acm-insights",
+	// Add other components here when ServiceMonitors is required.
+}
+
+// ClusterManagementAddOns is a map that associates certain component names with their corresponding add-ons.
+var ClusterManagementAddOns = map[string]string{
 	SubmarinerAddon: "submariner",
-	// Add other components here when clusterManagementAddOns is required.
+	// Add other components here when ClusterManagementAddOns is required.
 }
 
 /*
@@ -168,13 +185,39 @@ func GetDefaultHostedComponents() []string {
 
 // GetClusterManagementAddonName returns the name of the ClusterManagementAddOn based on the provided component name.
 func GetClusterManagementAddonName(component string) (string, error) {
-	if val, ok := clusterManagementAddOns[component]; !ok {
+	if val, ok := ClusterManagementAddOns[component]; !ok {
 		return val, fmt.Errorf("failed to find ClusterManagementAddOn name for: %s component", component)
 	} else {
 		return val, nil
 	}
 }
 
+/*
+GetLegacyPrometheusKind returns a list of legacy kind resources that are required to be removed before updating to
+ACM 2.9 and later.
+*/
+func GetLegacyPrometheusKind() []string {
+	return LegacyPrometheusKind
+}
+
+// GetPrometheusRulesName returns the name of the PrometheusRules based on the provided component name.
+func GetPrometheusRulesName(component string) (string, error) {
+	if val, ok := MCHPrometheusRules[component]; !ok {
+		return val, fmt.Errorf("failed to find PrometheusRules name for: %s component", component)
+	} else {
+		return val, nil
+	}
+}
+
+// GetServiceMonitorName returns the name of the ServiceMonitors based on the provided component name.
+func GetServiceMonitorName(component string) (string, error) {
+	if val, ok := MCHServiceMonitors[component]; !ok {
+		return val, fmt.Errorf("failed to find ServiceMonitors name for: %s component", component)
+	} else {
+		return val, nil
+	}
+}
+
 /*
 ComponentPresent checks if a specific component is present based on the provided component name in the
 MultiClusterHub struct.

diff --git a/api/v1/multiclusterhub_methods_test.go b/api/v1/multiclusterhub_methods_test.go
@@ -3,6 +3,8 @@ package v1
 import (
 	"reflect"
 	"testing"
+
+	"k8s.io/utils/strings/slices"
 )
 
 func TestMultiClusterHub_Prune(t *testing.T) {
@@ -187,3 +189,102 @@ func TestGetClusterManagementAddonName(t *testing.T) {
 		})
 	}
 }
+
+func TestGetLegacyPrometheusKind(t *testing.T) {
+	tests := []struct {
+		name  string
+		kind  string
+		want  int
+		want2 []string
+	}{
+		{
+			name:  "legacy Prometheus Configuration Kind",
+			kind:  "PrometheusRule",
+			want:  2,
+			want2: LegacyPrometheusKind,
+		},
+		{
+			name:  "legacy Prometheus Configuration Kind",
+			kind:  "ServiceMonitor",
+			want:  2,
+			want2: LegacyPrometheusKind,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetLegacyPrometheusKind()
+			if len(got) == 0 {
+				t.Errorf("GetLegacyPrometheusKind() = %v, want: %v", len(got), tt.want)
+			}
+
+			if ok := slices.Contains(got, tt.kind); !ok {
+				t.Errorf("GetLegacyPrometheusKind() = %v, want: %v", got, tt.want2)
+			}
+		})
+	}
+}
+
+func TestGetPrometheusRulesName(t *testing.T) {
+	tests := []struct {
+		name      string
+		component string
+		want      string
+	}{
+		{
+			name:      "console PrometheusRule",
+			component: Console,
+			want:      MCHPrometheusRules[Console],
+		},
+		{
+			name:      "unknown PrometheusRule",
+			component: "unknown",
+			want:      MCHPrometheusRules["unknown"],
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetPrometheusRulesName(tt.component)
+			if err != nil && tt.component != "unknown" {
+				t.Errorf("GetPrometheusRulesName(%v) = %v, want: %v", tt.component, err.Error(), tt.want)
+			}
+
+			if got != tt.want {
+				t.Errorf("GetPrometheusRulesName(%v) = %v, want: %v", tt.component, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGetServiceMonitorName(t *testing.T) {
+	tests := []struct {
+		name      string
+		component string
+		want      string
+	}{
+		{
+			name:      "console ServiceMonitor",
+			component: Console,
+			want:      MCHServiceMonitors[Console],
+		},
+		{
+			name:      "unknown ServiceMonitor",
+			component: "unknown",
+			want:      MCHServiceMonitors["unknown"],
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetServiceMonitorName(tt.component)
+			if err != nil && tt.component != "unknown" {
+				t.Errorf("GetServiceMonitorName(%v) = %v, want: %v", tt.component, err.Error(), tt.want)
+			}
+
+			if got != tt.want {
+				t.Errorf("GetServiceMonitorName(%v) = %v, want: %v", tt.component, got, tt.want)
+			}
+		})
+	}
+}
diff --git a/controllers/multiclusterhub_controller.go b/controllers/multiclusterhub_controller.go
@@ -313,9 +313,8 @@ func (r *MultiClusterHubReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return result, err
 	}
 
-	err = r.removeLegacyGRCPrometheusConfig(ctx)
-	if err != nil {
-		return ctrl.Result{}, err
+	for _, kind := range operatorv1.GetLegacyPrometheusKind() {
+		err = r.removeLegacyPrometheusConfigurations(ctx, "openshift-monitoring", kind)
 	}
 
 	// Install CRDs

diff --git a/controllers/multiclusterhub_controller_test.go b/controllers/multiclusterhub_controller_test.go
@@ -928,9 +928,14 @@ var _ = Describe("MultiClusterHub controller", func() {
 			err = k8sClient.Create(context.TODO(), sm)
 			Expect(err).To(BeNil())
 
+			legacyResourceKind := operatorv1.GetLegacyPrometheusKind()
+			ns := "openshift-monitoring"
+
 			By("Running the cleanup of the legacy Prometheus configuration")
-			err = reconciler.removeLegacyGRCPrometheusConfig(context.TODO())
-			Expect(err).To(BeNil())
+			for _, kind := range legacyResourceKind {
+				err = reconciler.removeLegacyPrometheusConfigurations(context.TODO(), ns, kind)
+				Expect(err).To(BeNil())
+			}
 
 			By("Verifying that the legacy GRC PrometheusRule is deleted")
 			err = k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(pr), pr)
@@ -941,8 +946,10 @@ var _ = Describe("MultiClusterHub controller", func() {
 			Expect(errors.IsNotFound(err)).To(BeTrue())
 
 			By("Running the cleanup of the legacy Prometheus configuration again should do nothing")
-			err = reconciler.removeLegacyGRCPrometheusConfig(context.TODO())
-			Expect(err).To(BeNil())
+			for _, kind := range legacyResourceKind {
+				err = reconciler.removeLegacyPrometheusConfigurations(context.TODO(), ns, kind)
+				Expect(err).To(BeNil())
+			}
 		})
 	})
 

diff --git a/controllers/uninstall.go b/controllers/uninstall.go
@@ -274,36 +274,63 @@ func (r *MultiClusterHubReconciler) uninstall(m *operatorsv1.MultiClusterHub, u
 	return false, nil
 }
 
-// removeLegacyGRCPrometheusConfig will remove the GRC PrometheusRule and ServiceMonitor in the openshift-monitoring
-// namespace. This configuration should be in the controller namespace instead.
-func (r *MultiClusterHubReconciler) removeLegacyGRCPrometheusConfig(ctx context.Context) error {
-	for _, kind := range []string{"PrometheusRule", "ServiceMonitor"} {
-		obj := &unstructured.Unstructured{}
-		obj.SetGroupVersionKind(schema.GroupVersionKind{
-			Group:   "monitoring.coreos.com",
-			Kind:    kind,
-			Version: "v1",
-		})
-		obj.SetName("ocm-grc-policy-propagator-metrics")
-		obj.SetNamespace("openshift-monitoring")
-
-		err := r.Client.Delete(ctx, obj)
+/*
+removeLegacyPrometheusConfigurations will remove the specified kind of configuration
+(PrometheusRule or ServiceMonitor) in the target namespace. This configuration should be in the controller namespace
+instead.
+*/
+func (r *MultiClusterHubReconciler) removeLegacyPrometheusConfigurations(ctx context.Context,
+	targetNamespace string, kind string) error {
+	obj := &unstructured.Unstructured{}
+	obj.SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "monitoring.coreos.com",
+		Kind:    kind,
+		Version: "v1",
+	})
+
+	var configType string
+	switch kind {
+	case "PrometheusRule":
+		configType = "PrometheusRule"
+
+	case "ServiceMonitor":
+		configType = "ServiceMonitor"
+
+	default:
+		return fmt.Errorf("Unsupported kind detected when trying to remove legacy configuration: %s", kind)
+	}
+
+	for _, c := range operatorsv1.MCHComponents {
+		res, err := func() (string, error) {
+			if configType == "PrometheusRule" {
+				return operatorsv1.GetPrometheusRulesName(c)
+			}
+
+			return operatorsv1.GetServiceMonitorName(c)
+		}()
+
+		if err != nil {
+			continue
+		}
+
+		obj.SetName(res)
+		obj.SetNamespace(targetNamespace)
+
+		err = r.Client.Delete(ctx, obj)
 		if err != nil {
 			if !errors.IsNotFound(err) && !apimeta.IsNoMatchError(err) {
 				r.Log.Error(
 					err,
-					"Error while deleting the legacy GRC Prometheus configuration",
+					fmt.Sprintf("Error while deleting the legacy %s configuration", configType),
 					"kind", kind,
 					"name", obj.GetName(),
 				)
-
 				return err
 			}
 		} else {
-			r.Log.Info("Deleted the legacy GRC Prometheus configuration", "kind", kind, "name", obj.GetName())
+			r.Log.Info(fmt.Sprintf("Deleted the legacy %s configuration: %s", configType, obj.GetName()))
 		}
 	}
-
 	return nil
 }
 

diff --git a/pkg/templates/charts/toggle/console/templates/console-servicemonitor.yaml b/pkg/templates/charts/toggle/console/templates/console-servicemonitor.yaml
@@ -2,7 +2,7 @@ apiVersion: monitoring.coreos.com/v1
 kind: ServiceMonitor
 metadata:
   name: console-monitor
-  namespace: openshift-monitoring
+  namespace: {{ .Values.global.namespace }}
 spec:
   endpoints:
     - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token