Added permissions to resources for metrics exporter

controllers/storagecluster/exporter.go

@@ -21,7 +21,8 @@ import (
 
 const (
 	metricsExporterName     = "ocs-metrics-exporter"
-	metricsExporterRoleName = "ocs-metrics-svc"
+	prometheusRoleName      = "ocs-metrics-svc"
+	metricsExporterRoleName = metricsExporterName
 	portMetrics             = "metrics"
 	portExporter            = "exporter"
 	metricsPath             = "/metrics"
@@ -579,11 +580,6 @@ const expectedPrometheusK8RoleJSON = `
 			"apiGroups":[""],
 			"resources":["services","endpoints","pods"],
 			"verbs":["get","list","watch"]
-		},
-		{
-			"apiGroups":[""],
-			"resources":["persistentvolumeclaims","pods","configmaps","secrets"],
-			"verbs":["get","list","watch"]
 		}
 	]
 }
@@ -597,7 +593,7 @@ const expectedMetricExporterRoleJSON = `
 	"rules":[
 		{
 			"apiGroups":[""],
-			"resources":["secrets","configmaps"],
+			"resources":["secrets","configmaps","persistentvolumeclaims","pods"],
 			"verbs":["get","list","watch"]
 		},
 		{
@@ -626,61 +622,45 @@ const expectedMetricExporterRoleJSON = `
 
 func createMetricsExporterRoles(ctx context.Context,
 	r *StorageClusterReconciler, instance *ocsv1.StorageCluster) error {
-	// create/update prometheus server needed roles
-	var expectedRole = new(rbacv1.Role)
-	err := json.Unmarshal([]byte(expectedPrometheusK8RoleJSON), expectedRole)
-	if err != nil {
-		r.Log.Error(err, "an unexpected error occurred while unmarshalling prometheus role")
-		return err
-	}
-	currentRole := new(rbacv1.Role)
-	expectedRole.Name, currentRole.Name = metricsExporterRoleName, metricsExporterRoleName
-	expectedRole.Namespace, currentRole.Namespace = instance.Namespace, instance.Namespace
-	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRole, func() error {
-		expectedRulesLen := len(expectedRole.Rules)
-		if len(currentRole.Rules) != expectedRulesLen {
-			currentRole.Rules = make([]rbacv1.PolicyRule, expectedRulesLen)
+	var jsonRoles = map[string]string{
+		prometheusRoleName:      expectedPrometheusK8RoleJSON,
+		metricsExporterRoleName: expectedMetricExporterRoleJSON,
+	}
+
+	for roleName, jsonRole := range jsonRoles {
+		// create expected roles
+		var expectedRole = new(rbacv1.Role)
+		err := json.Unmarshal([]byte(jsonRole), expectedRole)
+		if err != nil {
+			r.Log.Error(err,
+				"an unexpected error occurred while unmarshalling following JSON role",
+				"JSONRoleName", roleName,
+				"JSONRole", jsonRole)
+			return err
 		}
-		copy(currentRole.Rules, expectedRole.Rules)
-		currentRole.OwnerReferences = []metav1.OwnerReference{{
-			APIVersion: instance.APIVersion,
-			Kind:       instance.Kind,
-			Name:       instance.Name,
-			UID:        instance.UID,
-		}}
-		return nil
-	})
-	if err != nil {
-		r.Log.Error(err, "failed to create/update prometheus roles")
-		return err
-	}
-
-	// create/update metrics exporter roles
-	expectedRole = new(rbacv1.Role)
-	err = json.Unmarshal([]byte(expectedMetricExporterRoleJSON), expectedRole)
-	if err != nil {
-		r.Log.Error(err, "an unexpected error occurred while unmarshalling metrics exporter role")
-		return err
-	}
-	currentRole = new(rbacv1.Role)
-	expectedRole.Name, currentRole.Name = metricsExporterName, metricsExporterName
-	expectedRole.Namespace, currentRole.Namespace = instance.Namespace, instance.Namespace
-	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRole, func() error {
-		expectedRulesLen := len(expectedRole.Rules)
-		if len(currentRole.Rules) != expectedRulesLen {
-			currentRole.Rules = make([]rbacv1.PolicyRule, expectedRulesLen)
+		currentRole := new(rbacv1.Role)
+		expectedRole.Name, currentRole.Name = roleName, roleName
+		expectedRole.Namespace, currentRole.Namespace = instance.Namespace, instance.Namespace
+		_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRole, func() error {
+			expectedRulesLen := len(expectedRole.Rules)
+			if len(currentRole.Rules) != expectedRulesLen {
+				currentRole.Rules = make([]rbacv1.PolicyRule, expectedRulesLen)
+			}
+			copy(currentRole.Rules, expectedRole.Rules)
+			currentRole.OwnerReferences = []metav1.OwnerReference{{
+				APIVersion: instance.APIVersion,
+				Kind:       instance.Kind,
+				Name:       instance.Name,
+				UID:        instance.UID,
+			}}
+			return nil
+		})
+		if err != nil {
+			r.Log.Error(err, "failed to create/update exporter role", "RoleName", roleName)
+			return err
 		}
-		copy(currentRole.Rules, expectedRole.Rules)
-		currentRole.OwnerReferences = []metav1.OwnerReference{{
-			APIVersion: instance.APIVersion,
-			Kind:       instance.Kind,
-			Name:       instance.Name,
-			UID:        instance.UID,
-		}}
-		return nil
-	})
-
-	return err
+	}
+	return nil
 }
 
 const expectedPrometheusK8RoleBindingJSON = `
@@ -692,14 +672,7 @@ const expectedPrometheusK8RoleBindingJSON = `
 		"apiGroup":"rbac.authorization.k8s.io",
 		"kind":"Role",
 		"name":"ocs-metrics-svc"
-	},
-	"subjects":[
-		{
-			"kind":"ServiceAccount",
-			"name":"prometheus-k8s",
-			"namespace":"openshift-monitoring"
-		}
-	]
+	}
 }`
 
 // expectedMetricsExporterRoleBindingJSON rolebindings for metrics exporter
@@ -718,73 +691,80 @@ const expectedMetricsExporterRoleBindingJSON = `
 
 func createMetricsExporterRolebindings(ctx context.Context,
 	r *StorageClusterReconciler, instance *ocsv1.StorageCluster) error {
-	// rolebinding for prometheus
-	var expectedRoleBinding = new(rbacv1.RoleBinding)
-	err := json.Unmarshal([]byte(expectedPrometheusK8RoleBindingJSON), expectedRoleBinding)
-	if err != nil {
-		r.Log.Error(err,
-			"an unexpected error occurred while unmarshalling prometheus rolebinding")
-		return err
+	var roleBindings = []string{
+		expectedPrometheusK8RoleBindingJSON, expectedMetricsExporterRoleBindingJSON,
 	}
-	currentRoleBinding := new(rbacv1.RoleBinding)
-	expectedRoleBinding.Name, currentRoleBinding.Name = metricsExporterRoleName, metricsExporterRoleName
-	expectedRoleBinding.Namespace, currentRoleBinding.Namespace = instance.Namespace, instance.Namespace
-	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRoleBinding, func() error {
-		currentRoleBinding.RoleRef.APIGroup = expectedRoleBinding.RoleRef.APIGroup
-		currentRoleBinding.RoleRef.Kind = expectedRoleBinding.RoleRef.Kind
-		currentRoleBinding.RoleRef.Name = expectedRoleBinding.RoleRef.Name
-		expectedSubjectsLen := len(expectedRoleBinding.Subjects)
-		if len(currentRoleBinding.Subjects) != expectedSubjectsLen {
-			currentRoleBinding.Subjects = make([]rbacv1.Subject, expectedSubjectsLen)
-		}
-		copy(currentRoleBinding.Subjects, expectedRoleBinding.Subjects)
-		currentRoleBinding.OwnerReferences = []metav1.OwnerReference{{
-			APIVersion: instance.APIVersion,
-			Kind:       instance.Kind,
-			Name:       instance.Name,
-			UID:        instance.UID,
-		}}
-		return nil
-	})
-	if err != nil {
-		r.Log.Error(err, "error while create/update prometheus rolebindings")
-		return err
+	var roleBindingNames = []string{
+		// rolebindings have the same names as the roles
+		prometheusRoleName, metricsExporterRoleName,
 	}
-
-	// rolebinding for metrics exporter
-	expectedRoleBinding = new(rbacv1.RoleBinding)
-	err = json.Unmarshal([]byte(expectedMetricsExporterRoleBindingJSON), expectedRoleBinding)
-	if err != nil {
-		r.Log.Error(err,
-			"an unexpected error occurred while unmarshalling metrics exporter rolebinding")
-		return err
+	var roleBindingSubjects = []rbacv1.Subject{
+		// subject for prometheus-k8 rolebinding
+		{
+			Kind:      "ServiceAccount",
+			Name:      "prometheus-k8s",
+			Namespace: "openshift-monitoring",
+		},
+		// subject for metrics exporter rolebinding
+		{
+			Kind:      "ServiceAccount",
+			Name:      metricsExporterName,
+			Namespace: instance.Namespace,
+		},
 	}
-	currentRoleBinding = new(rbacv1.RoleBinding)
-	expectedRoleBinding.Name, currentRoleBinding.Name = metricsExporterName, metricsExporterName
-	expectedRoleBinding.Namespace, currentRoleBinding.Namespace = instance.Namespace, instance.Namespace
-
-	expectedRoleBinding.Subjects = make([]rbacv1.Subject, 1)
-	expectedRoleBinding.Subjects[0].Kind = "ServiceAccount"
-	expectedRoleBinding.Subjects[0].Name = metricsExporterName
-	expectedRoleBinding.Subjects[0].Namespace = instance.Namespace
-
-	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRoleBinding, func() error {
-		currentRoleBinding.RoleRef.APIGroup = expectedRoleBinding.RoleRef.APIGroup
-		currentRoleBinding.RoleRef.Kind = expectedRoleBinding.RoleRef.Kind
-		currentRoleBinding.RoleRef.Name = expectedRoleBinding.RoleRef.Name
-		expectedSubjectsLen := len(expectedRoleBinding.Subjects)
-		if len(currentRoleBinding.Subjects) != expectedSubjectsLen {
+
+	for rbIndx, roleBinding := range roleBindings {
+		var roleBindingName = roleBindingNames[rbIndx]
+		var roleBindingSubject = roleBindingSubjects[rbIndx]
+
+		var expectedRoleBinding = new(rbacv1.RoleBinding)
+		err := json.Unmarshal([]byte(roleBinding), expectedRoleBinding)
+		if err != nil {
+			r.Log.Error(err,
+				"an unexpected error occurred while unmarshalling rolebinding",
+				"RoleBindingName", roleBindingName,
+				"RoleBindingJSON", roleBinding)
+			return err
+		}
+		currentRoleBinding := new(rbacv1.RoleBinding)
+		// name
+		expectedRoleBinding.Name = roleBindingName
+		currentRoleBinding.Name = roleBindingName
+		// namespace
+		expectedRoleBinding.Namespace = instance.Namespace
+		currentRoleBinding.Namespace = instance.Namespace
+		// expected role reference name
+		// PS: we use the same name for both roles and rolebindings
+		expectedRoleBinding.RoleRef.Name = roleBindingName
+		// expected subjects
+		expectedRoleBinding.Subjects = []rbacv1.Subject{roleBindingSubject}
+
+		_, err = controllerutil.CreateOrUpdate(ctx, r.Client, currentRoleBinding, func() error {
+			// add expected role reference
+			currentRoleBinding.RoleRef = expectedRoleBinding.RoleRef
+
+			// add expected subjects
+			expectedSubjectsLen := len(expectedRoleBinding.Subjects)
 			currentRoleBinding.Subjects = make([]rbacv1.Subject, expectedSubjectsLen)
+			copy(currentRoleBinding.Subjects, expectedRoleBinding.Subjects)
+
+			// add owner references
+			currentRoleBinding.OwnerReferences = []metav1.OwnerReference{{
+				APIVersion: instance.APIVersion,
+				Kind:       instance.Kind,
+				Name:       instance.Name,
+				UID:        instance.UID,
+			}}
+
+			return nil
+		})
+		if err != nil {
+			r.Log.Error(err,
+				"error while create/update metrics exporter rolebinding",
+				"RoleBindingName", roleBindingName)
+			return err
 		}
-		copy(currentRoleBinding.Subjects, expectedRoleBinding.Subjects)
-		currentRoleBinding.OwnerReferences = []metav1.OwnerReference{{
-			APIVersion: instance.APIVersion,
-			Kind:       instance.Kind,
-			Name:       instance.Name,
-			UID:        instance.UID,
-		}}
-		return nil
-	})
+	}
 
-	return err
+	return nil
 }

metrics/internal/cache/pv.go

@@ -94,8 +94,14 @@ func appendIfNotExists(slice []string, value string) []string {
 
 // Add inserts to the PersistentVolumeStore.
 func (p *PersistentVolumeStore) Add(obj interface{}) error {
-	pv, ok := obj.(*corev1.PersistentVolume)
-	if !ok {
+	var pv *corev1.PersistentVolume
+	// obj can be of PV type or a pointer to it
+	switch pvFromObj := obj.(type) {
+	case *corev1.PersistentVolume:
+		pv = pvFromObj
+	case corev1.PersistentVolume:
+		pv = &pvFromObj
+	default:
 		return fmt.Errorf("unexpected object of type %T", obj)
 	}
 

metrics/internal/collectors/ceph-block-pool.go

@@ -133,7 +133,7 @@ func (c *CephBlockPoolCollector) collectMirroringImageHealth(cephBlockPools []*c
 				cephBlockPool.Name,
 				cephBlockPool.Namespace)
 		default:
-			klog.Errorf("Invalid image health for pool %s. Must be OK, UNKNOWN, WARNING or ERROR", cephBlockPool.Name)
+			klog.Errorf("Invalid image health, %q, for pool %s. Must be OK, UNKNOWN, WARNING or ERROR.", imageHealth, cephBlockPool.Name)
 		}
 	}
 }