Disable cluster monitoring for Velero and remove ServiceMonitor from the Velero deployment. (#1081)

First change to remove cluster monitoring of OADP that will be replaced by the user workload
monitoring (UWM).

Related Issues:
  #769
  https://issues.redhat.com/browse/OADP-1887
  https://issues.redhat.com/browse/OADP-661

The openshift-adp-velero-metrics-svc is left to easy process of enabling UWM.

Once UWM is enabled, it will require setting up ServiceMonitor and configuring alerts
or dashboards that are crucial for a particular use-case.

Enablement of user workload monitoring with additional documentation is not part
of this PR to easilly re-add the cluster monitoring in the future by reverting this change.

Signed-off-by: Michal Pryc <[email protected]>

Author: mpryc

bundle/manifests/oadp-operator.clusterserviceversion.yaml

@@ -155,7 +155,6 @@ metadata:
     description: OADP (OpenShift API for Data Protection) operator sets up and installs
       Data Protection Applications on the OpenShift platform.
     olm.skipRange: '>=0.0.0 <99.0.0'
-    operatorframework.io/cluster-monitoring: "true"
     operatorframework.io/suggested-namespace: openshift-adp
     operators.openshift.io/infrastructure-features: '["Disconnected"]'
     operators.openshift.io/valid-subscription: '["OpenShift Container Platform", "OpenShift

bundle/manifests/oadp-operator_monitoring.coreos.com_v1_prometheusrule.yaml

@@ -3,7 +3,6 @@ kind: Namespace
 metadata:
   labels:
     control-plane: controller-manager
-    openshift.io/cluster-monitoring: "true"
   name: system
 ---
 apiVersion: apps/v1

config/manager/manager.yaml

@@ -10,7 +10,6 @@ metadata:
     description: OADP (OpenShift API for Data Protection) operator sets up and installs
       Data Protection Applications on the OpenShift platform.
     olm.skipRange: '>=0.0.0 <99.0.0'
-    operatorframework.io/cluster-monitoring: "true"
     operatorframework.io/suggested-namespace: openshift-adp
     operators.openshift.io/infrastructure-features: '["Disconnected"]'
     operators.openshift.io/valid-subscription: '["OpenShift Container Platform", "OpenShift

config/manifests/bases/oadp-operator.clusterserviceversion.yaml

@@ -6,7 +6,6 @@ resources:
 - ../samples
 - ../scorecard
 - ../velero
-- ../prometheus
 
 # [WEBHOOK] To enable webhooks, uncomment all the sections with [WEBHOOK] prefix.
 # Do NOT uncomment sections with prefix [CERTMANAGER], as OLM does not support cert-manager.

config/manifests/kustomization.yaml

@@ -102,7 +102,6 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 		r.ReconcileVolumeSnapshotLocations,
 		r.ReconcileVeleroDeployment,
 		r.ReconcileResticDaemonset,
-		r.ReconcileVeleroServiceMonitor,
 		r.ReconcileVeleroMetricsSVC,
 		r.ReconcileDataMoverController,
 		r.ReconcileDataMoverResticSecret,

config/prometheus/kustomization.yaml

@@ -5,105 +5,12 @@ import (
 
 	"github.com/go-logr/logr"
 	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
-	monitor "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
-func (r *DPAReconciler) ReconcileVeleroServiceMonitor(log logr.Logger) (bool, error) {
-
-	dpa := oadpv1alpha1.DataProtectionApplication{}
-	if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil {
-		return false, err
-	}
-
-	serviceMonitor := &monitor.ServiceMonitor{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "openshift-adp-velero-metrics-sm",
-			Namespace: r.NamespacedName.Namespace,
-		},
-	}
-
-	op, err := controllerutil.CreateOrPatch(r.Context, r.Client, serviceMonitor, func() error {
-
-		if serviceMonitor.ObjectMeta.CreationTimestamp.IsZero() {
-			serviceMonitor.Spec.Selector = metav1.LabelSelector{
-				MatchLabels: getDpaAppLabels(&dpa),
-			}
-		}
-
-		// update service monitor
-		return r.buildVeleroServiceMonitor(serviceMonitor, &dpa)
-	})
-
-	if err != nil {
-		return false, err
-	}
-
-	//TODO: Review service monitor status and report errors and conditions
-
-	if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated {
-		// Trigger event to indicate service monitor was created or updated
-		r.EventRecorder.Event(serviceMonitor,
-			corev1.EventTypeNormal,
-			"VeleroServiceMonitorReconciled",
-			fmt.Sprintf("performed %s on dpa service monitor %s/%s", op, serviceMonitor.Namespace, serviceMonitor.Name),
-		)
-	}
-	return true, nil
-}
-
-func (r *DPAReconciler) buildVeleroServiceMonitor(serviceMonitor *monitor.ServiceMonitor, dpa *oadpv1alpha1.DataProtectionApplication) error {
-
-	if dpa == nil {
-		return fmt.Errorf("dpa CR cannot be nil")
-	}
-
-	if serviceMonitor == nil {
-		return fmt.Errorf("service monitor cannot be nil")
-	}
-
-	// Setting controller owner reference on the service monitor
-	err := controllerutil.SetControllerReference(dpa, serviceMonitor, r.Scheme)
-	if err != nil {
-		return err
-	}
-
-	serviceMonitor.Spec.Selector = metav1.LabelSelector{
-		MatchLabels: getDpaAppLabels(dpa),
-	}
-
-	serviceMonitor.Labels = getDpaAppLabels(dpa)
-
-	serviceMonitor.Spec.Endpoints = []monitor.Endpoint{
-		{
-			Interval: "30s",
-			Port:     "monitoring",
-			MetricRelabelConfigs: []*monitor.RelabelConfig{
-				{
-					Action: "keep",
-					Regex:  ("velero_backup_total|velero_restore_total"),
-					SourceLabels: []string{
-						"__name__",
-					},
-				},
-			},
-		},
-	}
-
-	//serviceMonitor.Spec.JobLabel = "app"
-
-	serviceMonitor.Spec.NamespaceSelector = monitor.NamespaceSelector{
-		MatchNames: []string{
-			dpa.Namespace,
-		},
-	}
-
-	return nil
-}
-
 func (r *DPAReconciler) ReconcileVeleroMetricsSVC(log logr.Logger) (bool, error) {
 	dpa := oadpv1alpha1.DataProtectionApplication{}
 	if err := r.Get(r.Context, r.NamespacedName, &dpa); err != nil {

config/prometheus/monitor.yaml

@@ -56,146 +56,6 @@ func getFakeClientFromObjectsForMonitor(objs ...client.Object) (client.WithWatch
 	return fake.NewClientBuilder().WithScheme(schemeForFakeClient).WithObjects(objs...).Build(), nil
 }
 
-func TestDPAReconciler_buildVeleroServiceMonitor(t *testing.T) {
-	tests := []struct {
-		name                     string
-		serviceMonitor           *monitor.ServiceMonitor
-		dpa                      *oadpv1alpha1.DataProtectionApplication
-		wantErr                  bool
-		wantVeleroServiceMonitor *monitor.ServiceMonitor
-	}{
-		{
-			name: "given appropriate dpa valid service monitor is built",
-			serviceMonitor: &monitor.ServiceMonitor{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "openshift-adp-velero-metrics-sm",
-					Namespace: "test-ns",
-				},
-				Spec: monitor.ServiceMonitorSpec{
-					Selector: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"app.kubernetes.io/name":       common.Velero,
-							"app.kubernetes.io/instance":   "test-dpa",
-							"app.kubernetes.io/managed-by": common.OADPOperator,
-							"app.kubernetes.io/component":  Server,
-							oadpv1alpha1.OadpOperatorLabel: "True",
-						},
-					},
-				},
-			},
-			dpa: &oadpv1alpha1.DataProtectionApplication{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-dpa",
-					Namespace: "test-ns",
-				},
-				Spec: oadpv1alpha1.DataProtectionApplicationSpec{
-					Configuration: &oadpv1alpha1.ApplicationConfig{
-						Velero: &oadpv1alpha1.VeleroConfig{
-							DefaultPlugins: []oadpv1alpha1.DefaultPlugin{
-								oadpv1alpha1.DefaultPluginAWS,
-							},
-						},
-					},
-					BackupLocations: []oadpv1alpha1.BackupLocation{
-						{
-							CloudStorage: &oadpv1alpha1.CloudStorageLocation{
-								CloudStorageRef: corev1.LocalObjectReference{
-									Name: "bucket-123",
-								},
-								Config: map[string]string{},
-								Credential: &corev1.SecretKeySelector{
-									LocalObjectReference: corev1.LocalObjectReference{
-										Name: "cloud-credentials",
-									},
-									Key: "creds",
-								},
-								Default:          false,
-								BackupSyncPeriod: &metav1.Duration{},
-							},
-						},
-					},
-				},
-			},
-			wantErr: false,
-			wantVeleroServiceMonitor: &monitor.ServiceMonitor{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "openshift-adp-velero-metrics-sm",
-					Namespace: "test-ns",
-					Labels: map[string]string{
-						"app.kubernetes.io/name":       common.Velero,
-						"app.kubernetes.io/instance":   "test-dpa",
-						"app.kubernetes.io/managed-by": common.OADPOperator,
-						"app.kubernetes.io/component":  Server,
-						oadpv1alpha1.OadpOperatorLabel: "True",
-					},
-				},
-				Spec: monitor.ServiceMonitorSpec{
-					Selector: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"app.kubernetes.io/name":       common.Velero,
-							"app.kubernetes.io/instance":   "test-dpa",
-							"app.kubernetes.io/managed-by": common.OADPOperator,
-							"app.kubernetes.io/component":  Server,
-							oadpv1alpha1.OadpOperatorLabel: "True",
-						},
-					},
-					Endpoints: []monitor.Endpoint{
-						{
-							Interval: "30s",
-							Port:     "monitoring",
-							MetricRelabelConfigs: []*monitor.RelabelConfig{
-								{
-									Action: "keep",
-									Regex:  ("velero_backup_total|velero_restore_total"),
-									SourceLabels: []string{
-										"__name__",
-									},
-								},
-							},
-						},
-					},
-					NamespaceSelector: monitor.NamespaceSelector{
-						MatchNames: []string{
-							"test-ns",
-						},
-					},
-				},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			fakeClient, err := getFakeClientFromObjectsForMonitor(tt.serviceMonitor, tt.dpa)
-			if err != nil {
-				t.Errorf("error in creating fake client, likely programmer error")
-			}
-			r := &DPAReconciler{
-				Client:  fakeClient,
-				Scheme:  fakeClient.Scheme(),
-				Log:     logr.Discard(),
-				Context: newContextForTest(tt.name),
-				NamespacedName: types.NamespacedName{
-					Namespace: tt.serviceMonitor.Namespace,
-					Name:      tt.serviceMonitor.Name,
-				},
-				EventRecorder: record.NewFakeRecorder(10),
-			}
-
-			err = r.buildVeleroServiceMonitor(tt.serviceMonitor, tt.dpa)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("buildVeleroServiceMonitor() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(tt.wantVeleroServiceMonitor.Labels, tt.serviceMonitor.Labels) {
-				t.Errorf("expected velero service monitor labels to be %#v, got %#v", tt.wantVeleroServiceMonitor.Labels, tt.serviceMonitor.Labels)
-			}
-			if !reflect.DeepEqual(tt.wantVeleroServiceMonitor.Spec, tt.serviceMonitor.Spec) {
-				t.Errorf("expected velero service monitor spec to be %#v, got %#v", tt.wantVeleroServiceMonitor.Spec, tt.serviceMonitor.Spec)
-			}
-		})
-	}
-}
-
 func TestDPAReconciler_updateVeleroMetricsSVC(t *testing.T) {
 	tests := []struct {
 		name                string
@@ -300,10 +160,6 @@ func TestDPAReconciler_updateVeleroMetricsSVC(t *testing.T) {
 			}
 
 			err = r.updateVeleroMetricsSVC(tt.svc, tt.dpa)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("buildVeleroServiceMonitor() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
 			if !reflect.DeepEqual(tt.wantVeleroMtricsSVC.Labels, tt.svc.Labels) {
 				t.Errorf("expected velero metrics svc labels to be %#v, got %#v", tt.wantVeleroMtricsSVC.Labels, tt.svc.Labels)
 			}

config/prometheus/oadp-metrics-recording-rule.yaml

@@ -101,7 +101,4 @@ for f in bundle/manifests/oadp-operator.clusterserviceversion.yaml
 if [[ "$f" =~ .*clusterserviceversion.* ]] && ! grep -q infrastructure-features ${f}; then
   sed -i '/^spec:/i\ \ \ \ operators.openshift.io/infrastructure-features: \x27[\"Disconnected\"]\x27'                                                               ${f}
 fi
-if [[ "$f" =~ .*clusterserviceversion.* ]] && ! grep -q cluster-monitoring ${f}; then
-  sed -i '/^spec:/i\ \ \ \ operatorframework.io/cluster-monitoring: "true"'                                                                                          ${f}
-fi
 done