[release-1.14] On metric token refresh, also delete the ServiceMonitor (#3790)

controllers/alerts/metrics_test.go

@@ -10,11 +10,11 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gstruct"
-
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/ptr"
@@ -122,6 +122,43 @@ var _ = Describe("alert tests", func() {
 			err := r.Reconcile(req, false)
 			Expect(err).To(MatchError(fakeError))
 		})
+
+		It("should recreate Secret, and delete ServiceMonitor, if token is changed", func(ctx context.Context) {
+			cl := commontestutils.InitClient([]client.Object{ns})
+			r := NewMonitoringReconciler(ci, cl, ee, commontestutils.GetScheme())
+
+			Expect(r.Reconcile(req, false)).To(Succeed())
+
+			// Secret
+			sec := &corev1.Secret{}
+			Expect(cl.Get(ctx, client.ObjectKey{Namespace: commontestutils.Namespace, Name: secretName}, sec)).To(Succeed())
+			Expect(sec.StringData).To(HaveKey("token"))
+
+			origToken := sec.StringData["token"]
+			sec.StringData["token"] = "some-wrong-token"
+			Expect(cl.Update(ctx, sec)).To(Succeed())
+
+			newSec := &corev1.Secret{}
+			Expect(cl.Get(ctx, client.ObjectKey{Namespace: commontestutils.Namespace, Name: secretName}, newSec)).To(Succeed())
+			Expect(newSec.StringData).To(HaveKey("token"))
+			Expect(newSec.StringData["token"]).To(Equal("some-wrong-token"))
+
+			// Reconcile should delete the old secret and create a new one
+			Expect(r.Reconcile(req, false)).To(Succeed())
+
+			newSec = &corev1.Secret{}
+			Expect(cl.Get(ctx, client.ObjectKey{Namespace: commontestutils.Namespace, Name: secretName}, newSec)).To(Succeed())
+			Expect(newSec.StringData).To(HaveKey("token"))
+			Expect(newSec.StringData["token"]).To(Equal(origToken))
+
+			newSM := &monitoringv1.ServiceMonitor{}
+			Expect(cl.Get(ctx, client.ObjectKey{Namespace: commontestutils.Namespace, Name: serviceName}, newSM)).To(MatchError(apierrors.IsNotFound, "not found error"))
+
+			Expect(r.Reconcile(req, false)).To(Succeed())
+			newSM = &monitoringv1.ServiceMonitor{}
+			Expect(cl.Get(ctx, client.ObjectKey{Namespace: commontestutils.Namespace, Name: serviceName}, newSM)).To(Succeed())
+		})
+
 	})
 
 	Context("test PrometheusRule", func() {
@@ -345,7 +382,7 @@ var _ = Describe("alert tests", func() {
 
 		It("should use the desired runbook URL template when its ENV Variable is set", func() {
 			desiredRunbookURLTemplate := "desired/runbookURL/template/%s"
-			os.Setenv(runbookURLTemplateEnv, desiredRunbookURLTemplate)
+			Expect(os.Setenv(runbookURLTemplateEnv, desiredRunbookURLTemplate)).To(Succeed())
 
 			err := rules.SetupRules()
 			Expect(err).ToNot(HaveOccurred())

controllers/alerts/reconciler.go

@@ -63,14 +63,16 @@ func NewMonitoringReconciler(ci hcoutil.ClusterInfo, cl client.Client, ee hcouti
 		logger.Error(err, "failed to create the 'PrometheusRule' reconciler")
 	}
 
+	refresh := NewRefresher()
+
 	return &MonitoringReconciler{
 		reconcilers: []MetricReconciler{
 			alertRuleReconciler,
 			newRoleReconciler(namespace, owner),
 			newRoleBindingReconciler(namespace, owner, ci),
 			newMetricServiceReconciler(namespace, owner),
-			newSecretReconciler(namespace, owner),
-			newServiceMonitorReconciler(namespace, owner),
+			NewSecretReconciler(namespace, owner, secretName, newSecret, refresh),
+			newServiceMonitorReconciler(namespace, owner, refresh),
 		},
 		scheme:       scheme,
 		client:       cl,

controllers/alerts/refresher.go

@@ -0,0 +1,46 @@
+package alerts
+
+import (
+	"sync"
+)
+
+type Refresher interface {
+	setShouldRefresh()
+	refresh(f func() error) error
+}
+
+type refresher struct {
+	*sync.Mutex
+	upToDate bool
+}
+
+func NewRefresher() Refresher {
+	return &refresher{
+		Mutex:    &sync.Mutex{},
+		upToDate: true,
+	}
+}
+
+func (r *refresher) setShouldRefresh() {
+	r.Lock()
+	defer r.Unlock()
+
+	r.upToDate = false
+}
+
+func (r *refresher) refresh(f func() error) error {
+	r.Lock()
+	defer r.Unlock()
+
+	if r.upToDate {
+		return nil
+	}
+
+	if err := f(); err != nil {
+		return err
+	}
+
+	r.upToDate = true
+
+	return nil
+}

controllers/alerts/secret.go

@@ -2,6 +2,7 @@ package alerts
 
 import (
 	"context"
+	"sync"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -17,43 +18,71 @@ const (
 	secretName = "hco-bearer-auth"
 )
 
-type secretReconciler struct {
-	theSecret *corev1.Secret
+type CreateSecretFunc func(namespace string, owner metav1.OwnerReference, token string) *corev1.Secret
+type SecretReconciler struct {
+	theSecret    *corev1.Secret
+	lock         *sync.RWMutex
+	createSecret CreateSecretFunc
+	secretName   string
+	refresher    Refresher
 }
 
-func newSecretReconciler(namespace string, owner metav1.OwnerReference) *secretReconciler {
+func NewSecretReconciler(namespace string, owner metav1.OwnerReference, secretName string, createSecret CreateSecretFunc, rfr Refresher) *SecretReconciler {
 	token, err := authorization.CreateToken()
 	if err != nil {
 		logger.Error(err, "failed to create bearer token")
 		return nil
 	}
 
-	return &secretReconciler{
-		theSecret: NewSecret(namespace, owner, token),
+	return &SecretReconciler{
+		theSecret:    createSecret(namespace, owner, token),
+		lock:         &sync.RWMutex{},
+		createSecret: createSecret,
+		secretName:   secretName,
+		refresher:    rfr,
 	}
 }
 
-func (r *secretReconciler) Kind() string {
+func (r *SecretReconciler) Kind() string {
 	return "Secret"
 }
 
-func (r *secretReconciler) ResourceName() string {
-	return secretName
+func (r *SecretReconciler) ResourceName() string {
+	return r.secretName
 }
 
-func (r *secretReconciler) GetFullResource() client.Object {
+func (r *SecretReconciler) GetFullResource() client.Object {
+	r.lock.RLock()
+	defer r.lock.RUnlock()
+
 	return r.theSecret.DeepCopy()
 }
 
-func (r *secretReconciler) EmptyObject() client.Object {
+func (r *SecretReconciler) refreshToken() error {
+	r.lock.Lock()
+	defer r.lock.Unlock()
+
+	token, err := authorization.CreateToken()
+	if err != nil {
+		return err
+	}
+
+	r.theSecret = r.createSecret(r.theSecret.Namespace, r.theSecret.OwnerReferences[0], token)
+
+	r.refresher.setShouldRefresh()
+
+	return nil
+}
+
+func (r *SecretReconciler) EmptyObject() client.Object {
 	return &corev1.Secret{}
 }
 
 // UpdateExistingResource checks if the secret already exists and has the correct token.
 // If it does, it returns nil. If the secret exists but the token is incorrect, it deletes the old secret
 // and creates a new one with the updated token. If the secret does not exist, it creates a new one.
 // It deletes the old secret to force Prometheus to reload the configuration.
-func (r *secretReconciler) UpdateExistingResource(ctx context.Context, cl client.Client, resource client.Object, logger logr.Logger) (client.Object, bool, error) {
+func (r *SecretReconciler) UpdateExistingResource(ctx context.Context, cl client.Client, resource client.Object, logger logr.Logger) (client.Object, bool, error) {
 	found := resource.(*corev1.Secret)
 
 	token, err := authorization.CreateToken()
@@ -66,24 +95,29 @@ func (r *secretReconciler) UpdateExistingResource(ctx context.Context, cl client
 		return nil, false, nil
 	}
 
-	// If the the token is incorrect, delete the old secret and create a new one
-	if err := cl.Delete(ctx, found); err != nil {
+	// If the token is incorrect, delete the old secret and create a new one
+	if err = cl.Delete(ctx, found); err != nil {
 		if !errors.IsNotFound(err) {
 			logger.Error(err, "failed to delete old secret")
 			return nil, false, err
 		}
 	}
 
-	newSecret := NewSecret(r.theSecret.Namespace, r.theSecret.OwnerReferences[0], token)
-	if err := cl.Create(ctx, newSecret); err != nil {
+	if err = r.refreshToken(); err != nil {
+		logger.Error(err, "failed to refresh token")
+		return nil, false, err
+	}
+
+	sec := r.GetFullResource()
+	if err = cl.Create(ctx, sec); err != nil {
 		logger.Error(err, "failed to create new secret")
 		return nil, false, err
 	}
 
-	return newSecret, true, nil
+	return sec, true, nil
 }
 
-func NewSecret(namespace string, owner metav1.OwnerReference, token string) *corev1.Secret {
+func newSecret(namespace string, owner metav1.OwnerReference, token string) *corev1.Secret {
 	return &corev1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "v1",

controllers/alerts/serviceMonitor.go

@@ -8,37 +8,53 @@ import (
 	monitoringv1 "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/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
 )
 
-type serviceMonitorReconciler struct {
+type ServiceMonitorReconciler struct {
+	refresher         Refresher
 	theServiceMonitor *monitoringv1.ServiceMonitor
 }
 
-func newServiceMonitorReconciler(namespace string, owner metav1.OwnerReference) *serviceMonitorReconciler {
-	return &serviceMonitorReconciler{theServiceMonitor: NewServiceMonitor(namespace, owner)}
+func CreateServiceMonitorReconciler(serviceMonitor *monitoringv1.ServiceMonitor, rfr Refresher) *ServiceMonitorReconciler {
+	return &ServiceMonitorReconciler{
+		theServiceMonitor: serviceMonitor,
+		refresher:         rfr,
+	}
+}
+
+func newServiceMonitorReconciler(namespace string, owner metav1.OwnerReference, rfr Refresher) *ServiceMonitorReconciler {
+	return CreateServiceMonitorReconciler(NewServiceMonitor(namespace, owner), rfr)
 }
 
-func (r serviceMonitorReconciler) Kind() string {
+func (r ServiceMonitorReconciler) Kind() string {
 	return monitoringv1.ServiceMonitorsKind
 }
 
-func (r serviceMonitorReconciler) ResourceName() string {
-	return serviceName
+func (r ServiceMonitorReconciler) ResourceName() string {
+	return r.theServiceMonitor.Name
 }
 
-func (r serviceMonitorReconciler) GetFullResource() client.Object {
+func (r ServiceMonitorReconciler) GetFullResource() client.Object {
 	return r.theServiceMonitor.DeepCopy()
 }
 
-func (r serviceMonitorReconciler) EmptyObject() client.Object {
+func (r ServiceMonitorReconciler) EmptyObject() client.Object {
 	return &monitoringv1.ServiceMonitor{}
 }
 
-func (r serviceMonitorReconciler) UpdateExistingResource(ctx context.Context, cl client.Client, resource client.Object, logger logr.Logger) (client.Object, bool, error) {
+func (r ServiceMonitorReconciler) UpdateExistingResource(ctx context.Context, cl client.Client, resource client.Object, logger logr.Logger) (client.Object, bool, error) {
 	found := resource.(*monitoringv1.ServiceMonitor)
+
+	if err := r.refresher.refresh(func() error {
+		return r.deleteServiceMonitor(ctx, cl, found)
+	}); err != nil {
+		return nil, false, err
+	}
+
 	modified := false
 	if !reflect.DeepEqual(found.Spec, r.theServiceMonitor.Spec) {
 		r.theServiceMonitor.Spec.DeepCopyInto(&found.Spec)
@@ -58,23 +74,35 @@ func (r serviceMonitorReconciler) UpdateExistingResource(ctx context.Context, cl
 	return found, modified, nil
 }
 
+func (r ServiceMonitorReconciler) deleteServiceMonitor(ctx context.Context, cl client.Client, found *monitoringv1.ServiceMonitor) error {
+	return cl.Delete(ctx, found)
+}
+
 func NewServiceMonitor(namespace string, owner metav1.OwnerReference) *monitoringv1.ServiceMonitor {
 	labels := hcoutil.GetLabels(hcoutil.HyperConvergedName, hcoutil.AppComponentMonitoring)
 	spec := monitoringv1.ServiceMonitorSpec{
 		Selector: metav1.LabelSelector{
 			MatchLabels: labels,
 		},
-		Endpoints: []monitoringv1.Endpoint{{
-			Port: operatorPortName,
-			Authorization: &monitoringv1.SafeAuthorization{
-				Credentials: &corev1.SecretKeySelector{
-					LocalObjectReference: corev1.LocalObjectReference{
-						Name: secretName,
+		Endpoints: []monitoringv1.Endpoint{
+			{
+				Port:   operatorPortName,
+				Scheme: "https",
+				Authorization: &monitoringv1.SafeAuthorization{
+					Credentials: &corev1.SecretKeySelector{
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: secretName,
+						},
+						Key: "token",
+					},
+				},
+				TLSConfig: &monitoringv1.TLSConfig{
+					SafeTLSConfig: monitoringv1.SafeTLSConfig{
+						InsecureSkipVerify: ptr.To(true),
 					},
-					Key: "token",
 				},
 			},
-		}},
+		},
 	}
 
 	return &monitoringv1.ServiceMonitor{