Repository: Azure/ARO-RP
Pull request #1564 (2021-06-29 14:01:14)

fix/ change purger logic

### Which issue this PR addresses:
This now allows dev clusters to be created with a proper ManagedBy field. Purger determined if production cluster by `ManagedBy == nil && *ManagedBy == ""`, and now we will look for a tag `purge: true`. If that tag exists it is a devCluster and the purger will attempt to purge it.

### Test plan for issue:

I've verified it creates new clusters with `purge:true` tag, but will need to confirm purger cleaning up resources after their TTL expires. 

### Is there any documentation that needs to be updated for this PR?

tech debt clean up, n/a.


bryanro92: After this is merged in, https://github.com/Azure/ARO-RP/pull/1541 will begin passing e2e tests
troy0820: Looks like a timeout flake 
/azp run e2e 
troy0820: /azp run e2e 
azure-pipelines[bot]: <samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>
bryanro92: /azp run e2e


azure-pipelines[bot]: <samp>
Azure Pipelines successfully started running 1 pipeline(s).<br>

</samp>

Patch:
diff --git a/docs/deploy-development-rp.md b/docs/deploy-development-rp.md
index f02a9d558..a2abac2ff 100644
--- a/docs/deploy-development-rp.md
+++ b/docs/deploy-development-rp.md
@@ -134,12 +134,6 @@
      ```bash
      journalctl _COMM=aro -o json --since "15 min ago" -f | jq -r 'select (.COMPONENT != null and (.COMPONENT | contains("access"))|not) | .MESSAGE'
      ```
-     
-   * Perform AdminUpdate on a dev cluster
-   
-```bash
-export CLUSTER=cluster
-curl -X PATCH -k "https://localhost:8443/subscriptions/$AZURE_SUBSCRIPTION_ID/resourceGroups/$RESOURCEGROUP/providers/Microsoft.RedHatOpenShift/openShiftClusters/$CLUSTER?api-version=admin" --header "Content-Type: application/json" -d "{}"
 
 ## Debugging
 
diff --git a/hack/clean/clean.go b/hack/clean/clean.go
index facbaf7b8..e407099dc 100644
--- a/hack/clean/clean.go
+++ b/hack/clean/clean.go
@@ -93,11 +93,17 @@ func run(ctx context.Context, log *logrus.Entry) error {
 	}
 
 	shouldDelete := func(resourceGroup mgmtfeatures.ResourceGroup, log *logrus.Entry) bool {
+		//assume its a prod cluster, dev clusters will have purge tag
+		devCluster := false
+		if resourceGroup.Tags != nil {
+			_, devCluster = resourceGroup.Tags["purge"]
+		}
+
 		// don't mess with clusters in RGs managed by a production RP. Although
 		// the production deny assignment will prevent us from breaking most
 		// things, that does not include us potentially detaching the cluster's
 		// NSG from the vnet, thus breaking inbound access to the cluster.
-		if resourceGroup.ManagedBy != nil && *resourceGroup.ManagedBy != "" {
+		if !devCluster && resourceGroup.ManagedBy != nil && *resourceGroup.ManagedBy != "" {
 			return false
 		}
 
diff --git a/pkg/cluster/delete.go b/pkg/cluster/delete.go
index 737cb524b..b73fed41e 100644
--- a/pkg/cluster/delete.go
+++ b/pkg/cluster/delete.go
@@ -259,21 +259,9 @@ func (m *manager) deleteRoleDefinition(ctx context.Context) error {
 
 func (m *manager) Delete(ctx context.Context) error {
 	resourceGroup := stringutils.LastTokenByte(m.doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')
-	//In edge case of CRG not being managedBy ARO, we have a different delete path
-	//we will assume normal case and set rgManagedByARO to true CRG is managedby ARO
-	rgManagedByARO := true
-	rg, err := m.resourceGroups.Get(ctx, resourceGroup)
-	if err != nil {
-		m.log.Warnf("failed to get resourceGroup %s", err)
-	} else if !m.env.IsLocalDevelopmentMode() {
-		if rg.ManagedBy == nil || *rg.ManagedBy == "" || !strings.EqualFold(*rg.ManagedBy, m.doc.OpenShiftCluster.ID) {
-			rgManagedByARO = false
-			m.log.Infof("cluster resource group not managed by aro %s", *rg.Name)
-		}
-	}
 
 	m.log.Printf("deleting dns")
-	err = m.dns.Delete(ctx, m.doc.OpenShiftCluster)
+	err := m.dns.Delete(ctx, m.doc.OpenShiftCluster)
 	if err != nil {
 		return err
 	}
@@ -296,27 +284,25 @@ func (m *manager) Delete(ctx context.Context) error {
 		return err
 	}
 
-	// only delete if managedByARO
-	if rgManagedByARO {
-		m.log.Printf("deleting resources")
-		err = m.deleteResources(ctx)
-		if err != nil {
-			return err
-		}
+	m.log.Printf("deleting resources")
+	err = m.deleteResources(ctx)
+	if err != nil {
+		return err
+	}
 
-		m.log.Printf("deleting resource group %s", resourceGroup)
-		err = m.resourceGroups.DeleteAndWait(ctx, resourceGroup)
-		if detailedErr, ok := err.(autorest.DetailedError); ok &&
-			(detailedErr.StatusCode == http.StatusForbidden || detailedErr.StatusCode == http.StatusNotFound) {
-			err = nil
-		}
-		if azureerrors.HasAuthorizationFailedError(err) {
-			err = nil
-		}
-		if err != nil {
-			return err
-		}
+	m.log.Printf("deleting resource group %s", resourceGroup)
+	err = m.resourceGroups.DeleteAndWait(ctx, resourceGroup)
+	if detailedErr, ok := err.(autorest.DetailedError); ok &&
+		(detailedErr.StatusCode == http.StatusForbidden || detailedErr.StatusCode == http.StatusNotFound) {
+		err = nil
 	}
+	if azureerrors.HasAuthorizationFailedError(err) {
+		err = nil
+	}
+	if err != nil {
+		return err
+	}
+
 	if !m.env.FeatureIsSet(env.FeatureDisableSignedCertificates) {
 		managedDomain, err := dns.ManagedDomain(m.env, m.doc.OpenShiftCluster.Properties.ClusterProfile.Domain)
 		if err != nil {
diff --git a/pkg/cluster/deploystorage.go b/pkg/cluster/deploystorage.go
index 0b8670ae2..98030cdf7 100644
--- a/pkg/cluster/deploystorage.go
+++ b/pkg/cluster/deploystorage.go
@@ -72,9 +72,15 @@ func (m *manager) ensureResourceGroup(ctx context.Context) error {
 		ManagedBy: to.StringPtr(m.doc.OpenShiftCluster.ID),
 	}
 	if m.env.IsLocalDevelopmentMode() {
-		// TODO: ideally this should be removed, but removing it causes the
-		// purge script to ignore RGs in CI E2E.  Fix that at the same time.
-		group.ManagedBy = nil
+		// grab tags so we do not accidently remove them on createOrUpdate, set purge tag to true for dev clusters
+		rg, err := m.resourceGroups.Get(ctx, resourceGroup)
+		if err == nil {
+			group.Tags = rg.Tags
+		}
+		if group.Tags == nil {
+			group.Tags = map[string]*string{}
+		}
+		group.Tags["purge"] = to.StringPtr("true")
 	}
 
 	// According to https://stackoverflow.microsoft.com/a/245391/62320,
diff --git a/pkg/operator/controllers/monitoring/monitoring_controller.go b/pkg/operator/controllers/monitoring/monitoring_controller.go
index 50acbf788..c2d6bd84a 100644
--- a/pkg/operator/controllers/monitoring/monitoring_controller.go
+++ b/pkg/operator/controllers/monitoring/monitoring_controller.go
@@ -5,7 +5,7 @@ package monitoring
 
 import (
 	"context"
-	"encoding/json"
+	"reflect"
 
 	"github.com/ghodss/yaml"
 	"github.com/sirupsen/logrus"
@@ -29,10 +29,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/operator/controllers"
 )
 
-var (
-	monitoringName   = types.NamespacedName{Name: "cluster-monitoring-config", Namespace: "openshift-monitoring"}
-	prometheusLabels = "app=prometheus,prometheus=k8s"
-)
+var monitoringName = types.NamespacedName{Name: "cluster-monitoring-config", Namespace: "openshift-monitoring"}
 
 // Config represents cluster monitoring stack configuration.
 // Reconciler reconciles retention and storage settings,
@@ -41,15 +38,21 @@ type Config struct {
 	api.MissingFields
 	PrometheusK8s struct {
 		api.MissingFields
-		Retention           string           `json:"retention,omitempty"`
-		VolumeClaimTemplate *json.RawMessage `json:"volumeClaimTemplate,omitempty"`
+		Retention           string `json:"retention,omitempty"`
+		VolumeClaimTemplate struct {
+			api.MissingFields
+		} `json:"volumeClaimTemplate,omitempty"`
 	} `json:"prometheusK8s,omitempty"`
 	AlertManagerMain struct {
 		api.MissingFields
-		VolumeClaimTemplate *json.RawMessage `json:"volumeClaimTemplate,omitempty"`
+		VolumeClaimTemplate struct {
+			api.MissingFields
+		} `json:"volumeClaimTemplate,omitempty"`
 	} `json:"alertmanagerMain,omitempty"`
 }
 
+var defaultConfig = `prometheusK8s: {}`
+
 type Reconciler struct {
 	arocli        aroclient.Interface
 	kubernetescli kubernetes.Interface
@@ -67,39 +70,10 @@ func NewReconciler(log *logrus.Entry, kubernetescli kubernetes.Interface, arocli
 }
 
 func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
-	for _, f := range []func(context.Context, ctrl.Request) (ctrl.Result, error){
-		r.reconcileConfiguration,
-		r.reconcilePVC, // TODO(mj): This should be removed once we don't have PVC anymore
-	} {
-		result, err := f(ctx, request)
-		if err != nil {
-			return result, err
-		}
-	}
-	return reconcile.Result{}, nil
-}
-
-func (r *Reconciler) reconcilePVC(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
-	pvcList, err := r.kubernetescli.CoreV1().PersistentVolumeClaims(monitoringName.Namespace).List(ctx, metav1.ListOptions{LabelSelector: prometheusLabels})
-	if err != nil {
-		return reconcile.Result{}, err
-	}
-
-	for _, pvc := range pvcList.Items {
-		err = r.kubernetescli.CoreV1().PersistentVolumeClaims(monitoringName.Namespace).Delete(ctx, pvc.Name, metav1.DeleteOptions{})
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-	}
-	return reconcile.Result{}, nil
-}
-
-func (r *Reconciler) reconcileConfiguration(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
 	cm, isCreate, err := r.monitoringConfigMap(ctx)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-
 	if cm.Data == nil {
 		cm.Data = map[string]string{}
 	}
@@ -116,20 +90,19 @@ func (r *Reconciler) reconcileConfiguration(ctx context.Context, request ctrl.Re
 	}
 
 	changed := false
-
-	// Nil out the fields we don't want set
-	if configData.AlertManagerMain.VolumeClaimTemplate != nil {
-		configData.AlertManagerMain.VolumeClaimTemplate = nil
-		changed = true
-	}
-
+	// we are disabling persistence. We use omitempty on the struct to
+	// clean the fields
 	if configData.PrometheusK8s.Retention != "" {
 		configData.PrometheusK8s.Retention = ""
 		changed = true
 	}
+	if !reflect.DeepEqual(configData.PrometheusK8s.VolumeClaimTemplate, struct{ api.MissingFields }{}) {
+		configData.PrometheusK8s.VolumeClaimTemplate = struct{ api.MissingFields }{}
+		changed = true
+	}
 
-	if configData.PrometheusK8s.VolumeClaimTemplate != nil {
-		configData.PrometheusK8s.VolumeClaimTemplate = nil
+	if !reflect.DeepEqual(configData.AlertManagerMain.VolumeClaimTemplate, struct{ api.MissingFields }{}) {
+		configData.AlertManagerMain.VolumeClaimTemplate = struct{ api.MissingFields }{}
 		changed = true
 	}
 
@@ -167,7 +140,9 @@ func (r *Reconciler) monitoringConfigMap(ctx context.Context) (*corev1.ConfigMap
 				Name:      monitoringName.Name,
 				Namespace: monitoringName.Namespace,
 			},
-			Data: nil,
+			Data: map[string]string{
+				"config.yaml": defaultConfig,
+			},
 		}, true, nil
 	}
 	if err != nil {
diff --git a/pkg/operator/controllers/workaround/cleanprompvc.go b/pkg/operator/controllers/workaround/cleanprompvc.go
new file mode 100644
index 000000000..747f966d3
--- /dev/null
+++ b/pkg/operator/controllers/workaround/cleanprompvc.go
@@ -0,0 +1,100 @@
+package workaround
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+// Clean the remaining PVCs in openshift-monitoring namespace.
+// These PVCs with labels: app=prometheus,prometheus=k8s are left
+// behind after switching back to use emptydir as persistent storage
+// for prometheus by disabling featureflag in monitoing controller.
+// This workaround is in effect for all clusters set to
+// have non-persistent prometheus.
+// The cleanup loop removes only up to 2 PVCs as this is the
+// production configuration at the time of the workaround release.
+
+import (
+	"context"
+	"reflect"
+
+	"github.com/ghodss/yaml"
+	"github.com/sirupsen/logrus"
+	"github.com/ugorji/go/codec"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/util/retry"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/operator/controllers/monitoring"
+	"github.com/Azure/ARO-RP/pkg/util/version"
+)
+
+const (
+	prometheusLabels    = "app=prometheus,prometheus=k8s"
+	monitoringName      = "cluster-monitoring-config"
+	monitoringNamespace = "openshift-monitoring"
+)
+
+type cleanPromPVC struct {
+	log *logrus.Entry
+	cli kubernetes.Interface
+}
+
+func NewCleanFromPVCWorkaround(log *logrus.Entry, cli kubernetes.Interface) Workaround {
+	return &cleanPromPVC{
+		log: log,
+		cli: cli,
+	}
+}
+
+func (*cleanPromPVC) Name() string {
+	return "Clean prometheus PVC after disabling persistency"
+}
+
+func (c *cleanPromPVC) IsRequired(clusterVersion *version.Version) bool {
+	cm, err := c.cli.CoreV1().ConfigMaps(monitoringNamespace).Get(context.Background(), monitoringName, metav1.GetOptions{})
+	if err != nil {
+		return false
+	}
+
+	configDataJSON, err := yaml.YAMLToJSON([]byte(cm.Data["config.yaml"]))
+	if err != nil {
+		return false
+	}
+	var configData monitoring.Config
+	handle := new(codec.JsonHandle)
+	err = codec.NewDecoderBytes(configDataJSON, handle).Decode(&configData)
+	if err != nil {
+		return false
+	}
+
+	if configData.PrometheusK8s.Retention == "" && reflect.DeepEqual(configData.PrometheusK8s.VolumeClaimTemplate, struct{ api.MissingFields }{}) {
+		return true
+	}
+
+	return false
+}
+
+func (c *cleanPromPVC) Ensure(ctx context.Context) error {
+
+	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		pvcList, err := c.cli.CoreV1().PersistentVolumeClaims(monitoringNamespace).List(ctx, metav1.ListOptions{LabelSelector: prometheusLabels})
+		if err != nil {
+			return err
+		}
+
+		for _, pvc := range pvcList.Items {
+			err = c.cli.CoreV1().PersistentVolumeClaims(monitoringNamespace).Delete(ctx, pvc.Name, metav1.DeleteOptions{})
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+
+	return err
+}
+
+func (c *cleanPromPVC) Remove(ctx context.Context) error {
+	return nil
+}
diff --git a/pkg/operator/controllers/workaround/workaround_controller.go b/pkg/operator/controllers/workaround/workaround_controller.go
index b009f0472..1e80c0108 100644
--- a/pkg/operator/controllers/workaround/workaround_controller.go
+++ b/pkg/operator/controllers/workaround/workaround_controller.go
@@ -44,7 +44,7 @@ func NewReconciler(log *logrus.Entry, kubernetescli kubernetes.Interface, config
 		configcli:     configcli,
 		arocli:        arocli,
 		restConfig:    restConfig,
-		workarounds:   []Workaround{NewSystemReserved(log, mcocli, dh), NewIfReload(log, kubernetescli)},
+		workarounds:   []Workaround{NewSystemReserved(log, mcocli, dh), NewIfReload(log, kubernetescli), NewCleanFromPVCWorkaround(log, kubernetescli)},
 		log:           log,
 	}
 }


Test patch:
diff --git a/pkg/operator/controllers/monitoring/monitoring_controller_test.go b/pkg/operator/controllers/monitoring/monitoring_controller_test.go
index 79d837e95..f237f0e4d 100644
--- a/pkg/operator/controllers/monitoring/monitoring_controller_test.go
+++ b/pkg/operator/controllers/monitoring/monitoring_controller_test.go
@@ -5,7 +5,6 @@ package monitoring
 
 import (
 	"context"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -13,11 +12,8 @@ import (
 	"github.com/ugorji/go/codec"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	ctrl "sigs.k8s.io/controller-runtime"
-
-	"github.com/Azure/ARO-RP/pkg/util/cmp"
 )
 
 var cmMetadata = metav1.ObjectMeta{Name: "cluster-monitoring-config", Namespace: "openshift-monitoring"}
@@ -25,33 +21,61 @@ var cmMetadata = metav1.ObjectMeta{Name: "cluster-monitoring-config", Namespace:
 func TestReconcileMonitoringConfig(t *testing.T) {
 	log := logrus.NewEntry(logrus.StandardLogger())
 	type test struct {
-		name          string
-		kubernetescli kubernetes.Interface
-		wantConfig    string
+		name         string
+		setConfigMap func() *Reconciler
+		wantConfig   string
 	}
 
 	for _, tt := range []*test{
 		{
-			name:          "ConfigMap does not exist - enable",
-			kubernetescli: fake.NewSimpleClientset(),
-			wantConfig:    `{}`,
+			name: "ConfigMap does not exist - enable",
+			setConfigMap: func() *Reconciler {
+				return &Reconciler{
+					kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{}),
+					log:           log,
+					jsonHandle:    new(codec.JsonHandle),
+				}
+			},
+			wantConfig: `
+{}`,
+		},
+		{
+			name: "ConfigMap does not have data",
+			setConfigMap: func() *Reconciler {
+				return &Reconciler{
+					kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
+						ObjectMeta: cmMetadata,
+					}),
+					log:        log,
+					jsonHandle: new(codec.JsonHandle),
+				}
+			},
+			wantConfig: ``,
 		},
 		{
 			name: "empty config.yaml",
-			kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
-				ObjectMeta: cmMetadata,
-				Data: map[string]string{
-					"config.yaml": ``,
-				},
-			}),
+			setConfigMap: func() *Reconciler {
+				return &Reconciler{
+					kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
+						ObjectMeta: cmMetadata,
+						Data: map[string]string{
+							"config.yaml": ``,
+						},
+					}),
+					log:        log,
+					jsonHandle: new(codec.JsonHandle),
+				}
+			},
 			wantConfig: ``,
 		},
 		{
 			name: "settings restored to default and extra fields are preserved",
-			kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
-				ObjectMeta: cmMetadata,
-				Data: map[string]string{
-					"config.yaml": `
+			setConfigMap: func() *Reconciler {
+				return &Reconciler{
+					kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
+						ObjectMeta: cmMetadata,
+						Data: map[string]string{
+							"config.yaml": `
 prometheusK8s:
   extraField: prometheus
   retention: 1d
@@ -76,51 +100,39 @@ alertmanagerMain:
         storageClassName: snail-mail
         volumeMode: Filesystem
 `,
-				},
-			}),
+						},
+					}),
+					log:        log,
+					jsonHandle: new(codec.JsonHandle),
+				}
+			},
 			wantConfig: `
 alertmanagerMain:
   extraField: yeet
 prometheusK8s:
   extraField: prometheus
-`,
-		},
-		{
-			name: "empty volumeClaimTemplate struct is cleared out",
-			kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
-				ObjectMeta: cmMetadata,
-				Data: map[string]string{
-					"config.yaml": `
-alertmanagerMain:
-  volumeClaimTemplate: {}
-  extraField: alertmanager
-prometheusK8s:
-  volumeClaimTemplate: {}
-  bugs: not-here
-`,
-				},
-			}),
-			wantConfig: `
-alertmanagerMain:
-  extraField: alertmanager
-prometheusK8s:
-  bugs: not-here
 `,
 		},
 		{
 			name: "other monitoring components are configured",
-			kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
-				ObjectMeta: cmMetadata,
-				Data: map[string]string{
-					"config.yaml": `
+			setConfigMap: func() *Reconciler {
+				return &Reconciler{
+					kubernetescli: fake.NewSimpleClientset(&corev1.ConfigMap{
+						ObjectMeta: cmMetadata,
+						Data: map[string]string{
+							"config.yaml": `
 alertmanagerMain:
   nodeSelector:
     foo: bar
 somethingElse:
   configured: true
 `,
-				},
-			}),
+						},
+					}),
+					log:        log,
+					jsonHandle: new(codec.JsonHandle),
+				}
+			},
 			wantConfig: `
 alertmanagerMain:
   nodeSelector:
@@ -131,12 +143,8 @@ somethingElse:
 		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
-			r := &Reconciler{
-				kubernetescli: tt.kubernetescli,
-				log:           log,
-				jsonHandle:    new(codec.JsonHandle),
-			}
 			ctx := context.Background()
+			r := tt.setConfigMap()
 			request := ctrl.Request{}
 			request.Name = "cluster-monitoring-config"
 			request.Namespace = "openshift-monitoring"
@@ -157,98 +165,3 @@ somethingElse:
 		})
 	}
 }
-
-func TestReconcilePVC(t *testing.T) {
-	log := logrus.NewEntry(logrus.StandardLogger())
-	tests := []struct {
-		name          string
-		kubernetescli kubernetes.Interface
-		want          []corev1.PersistentVolumeClaim
-	}{
-		{
-			name: "Should delete the prometheus PVCs",
-			kubernetescli: fake.NewSimpleClientset(&corev1.PersistentVolumeClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "prometheus-k8s-db-prometheus-k8s-0",
-					Namespace: "openshift-monitoring",
-					Labels: map[string]string{
-						"app":        "prometheus",
-						"prometheus": "k8s",
-					},
-				},
-			},
-				&corev1.PersistentVolumeClaim{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      "prometheus-k8s-db-prometheus-k8s-1",
-						Namespace: "openshift-monitoring",
-						Labels: map[string]string{
-							"app":        "prometheus",
-							"prometheus": "k8s",
-						},
-					},
-				}),
-			want: nil,
-		},
-		{
-			name: "Should preserve 1 pvc",
-			kubernetescli: fake.NewSimpleClientset(&corev1.PersistentVolumeClaim{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "prometheus-k8s-db-prometheus-k8s-0",
-					Namespace: "openshift-monitoring",
-					Labels: map[string]string{
-						"app":        "prometheus",
-						"prometheus": "k8s",
-					},
-				},
-			},
-				&corev1.PersistentVolumeClaim{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      "random-pvc",
-						Namespace: "openshift-monitoring",
-						Labels: map[string]string{
-							"app": "random",
-						},
-					},
-				}),
-			want: []corev1.PersistentVolumeClaim{
-				{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      "random-pvc",
-						Namespace: "openshift-monitoring",
-						Labels: map[string]string{
-							"app": "random",
-						},
-					},
-				},
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			ctx := context.Background()
-			r := &Reconciler{
-				log:           log,
-				kubernetescli: tt.kubernetescli,
-				jsonHandle:    new(codec.JsonHandle),
-			}
-			request := ctrl.Request{}
-			request.Name = "cluster-monitoring-config"
-			request.Namespace = "openshift-monitoring"
-
-			_, err := r.Reconcile(ctx, request)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			pvcList, err := r.kubernetescli.CoreV1().PersistentVolumeClaims(monitoringName.Namespace).List(context.Background(), metav1.ListOptions{})
-			if err != nil {
-				t.Fatalf("Unexpected error during list of PVCs: %v", err)
-			}
-
-			if !reflect.DeepEqual(pvcList.Items, tt.want) {
-				t.Error(cmp.Diff(pvcList.Items, tt.want))
-			}
-		})
-	}
-}
diff --git a/pkg/operator/controllers/workaround/cleanprompvc_test.go b/pkg/operator/controllers/workaround/cleanprompvc_test.go
new file mode 100644
index 000000000..028e7dc90
--- /dev/null
+++ b/pkg/operator/controllers/workaround/cleanprompvc_test.go
@@ -0,0 +1,150 @@
+package workaround
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+
+	utillog "github.com/Azure/ARO-RP/pkg/util/log"
+)
+
+func TestCleanPromPVCEnsure(t *testing.T) {
+
+	pvc0 := corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "prometheus-k8s-db-prometheus-k8s-00",
+			Namespace: "openshift-monitoring",
+			Labels: map[string]string{
+				"app":        "prometheus",
+				"prometheus": "k8s",
+			},
+		},
+	}
+
+	pvc1 := corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "prometheus-k8s-db-prometheus-k8s-0",
+			Namespace: "openshift-monitoring",
+			Labels: map[string]string{
+				"app":        "prometheus",
+				"prometheus": "k8s",
+			},
+		},
+	}
+
+	pvc2 := corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "prometheus-k8s-db-prometheus-k8s-1",
+			Namespace: "openshift-monitoring",
+			Labels: map[string]string{
+				"app":        "prometheus",
+				"prometheus": "k8s",
+			},
+		},
+	}
+
+	pvc3 := corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "dummy",
+			Namespace: "openshift-monitoring",
+		},
+	}
+
+	tests := []struct {
+		name       string
+		cli        *fake.Clientset
+		wantPVCNum int
+		wantErr    error
+	}{
+		{
+			name:       "Should delete the prometheus PVCs",
+			cli:        fake.NewSimpleClientset(&pvc1, &pvc2, &pvc3),
+			wantPVCNum: 1,
+			wantErr:    nil,
+		},
+		{
+			name:       "Should not delete the prometheus PVCs, too many items",
+			cli:        fake.NewSimpleClientset(&pvc1, &pvc2, &pvc3, &pvc0),
+			wantPVCNum: 1,
+			wantErr:    nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			w := NewCleanFromPVCWorkaround(utillog.GetLogger(), tt.cli)
+			err := w.Ensure(context.Background())
+			if err != tt.wantErr {
+				t.Fatalf("Unexpected error\nwant: %v\ngot: %v", tt.wantErr, err)
+			}
+
+			pvcList, err := tt.cli.CoreV1().PersistentVolumeClaims(monitoringNamespace).List(context.Background(), metav1.ListOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error during list of PVCs: %v", err)
+			}
+			if len(pvcList.Items) != tt.wantPVCNum {
+				t.Fatalf("Unexpected number of PVCs\nwant: %d\ngot: %d", tt.wantPVCNum, len(pvcList.Items))
+			}
+		})
+	}
+}
+
+func TestCleanPromPVCIsRequired(t *testing.T) {
+	newKubernetesCli := func(config string) *fake.Clientset {
+		configMap := corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      monitoringName,
+				Namespace: monitoringNamespace,
+			},
+			Data: make(map[string]string),
+		}
+
+		configMap.Data["config.yaml"] = config
+
+		return fake.NewSimpleClientset(&configMap)
+	}
+
+	tests := []struct {
+		name         string
+		kcli         *fake.Clientset
+		wantRequired bool
+	}{
+		{
+			name: "Should not be required, persistent set true",
+			kcli: newKubernetesCli(`prometheusK8s:
+  retention: 15d
+  volumeClaimTemplate:
+    spec:
+      resources:
+        requests:
+          storage: 100Gi
+            `),
+			wantRequired: false,
+		},
+		{
+			name: "Should be required, persistent set to false",
+			kcli: newKubernetesCli(`prometheusK8s:
+  retention: ""
+  volumeClaimTemplate: {}
+`),
+			wantRequired: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			w := NewCleanFromPVCWorkaround(utillog.GetLogger(), tt.kcli)
+			required := w.IsRequired(nil)
+			if required != tt.wantRequired {
+				t.Fatalf("Unexpected workaroud required result\nwant: %t\ngot: %t", tt.wantRequired, required)
+			}
+
+		})
+	}
+}
diff --git a/test/e2e/operator.go b/test/e2e/operator.go
index c443110ee..5add00407 100644
--- a/test/e2e/operator.go
+++ b/test/e2e/operator.go
@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/util/retry"
 
+	"github.com/Azure/ARO-RP/pkg/api"
 	arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/monitoring"
 	"github.com/Azure/ARO-RP/pkg/util/ready"
@@ -268,10 +269,8 @@ var _ = Describe("ARO Operator - Cluster Monitoring ConfigMap", func() {
 			log.Warn(err)
 		}
 
-		Expect(configData.PrometheusK8s.Retention).To(BeEmpty())
-		Expect(configData.PrometheusK8s.VolumeClaimTemplate).To(BeNil())
-		Expect(configData.AlertManagerMain.VolumeClaimTemplate).To(BeNil())
-
+		Expect(configData.PrometheusK8s.Retention).To(Equal(""))
+		Expect(configData.PrometheusK8s.VolumeClaimTemplate).To(Equal(struct{ api.MissingFields }{}))
 	})
 
 	Specify("cluster monitoring configmap should be restored if deleted", func() {