Fix requeue handling and switch to making timestamp based on finishTi…

…me of restoreJob.
k8ssandra · Jan 8, 2024 · ec3db10 · ec3db10
1 parent b28e8d1
commit ec3db10
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 29 deletions.
diff --git a/controllers/medusa/medusarestorejob_controller.go b/controllers/medusa/medusarestorejob_controller.go
@@ -178,7 +178,7 @@ func (r *MedusaRestoreJobReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	request.Log.Info("The restore operation is complete for DC", "CassandraDatacenter", request.Datacenter.Name)
 
-	recRes := medusa.RefreshSecrets(request.Datacenter, ctx, r.Client, request.Log, r.DefaultDelay)
+	recRes := medusa.RefreshSecrets(request.Datacenter, ctx, r.Client, request.Log, r.DefaultDelay, request.RestoreJob.Status.FinishTime)
 	switch {
 	case recRes.IsError():
 		return ctrl.Result{RequeueAfter: r.DefaultDelay}, recRes.GetError()

diff --git a/pkg/medusa/refresh_secrets.go b/pkg/medusa/refresh_secrets.go
@@ -11,6 +11,7 @@ import (
 	"github.com/k8ssandra/k8ssandra-operator/pkg/reconciliation"
 	"github.com/k8ssandra/k8ssandra-operator/pkg/result"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -19,7 +20,7 @@ const (
 	maxRetries = 5
 )
 
-func RefreshSecrets(dc *cassdcapi.CassandraDatacenter, ctx context.Context, client client.Client, logger logr.Logger, requeueDelay time.Duration) result.ReconcileResult {
+func RefreshSecrets(dc *cassdcapi.CassandraDatacenter, ctx context.Context, client client.Client, logger logr.Logger, requeueDelay time.Duration, restoreTimestamp metav1.Time) result.ReconcileResult {
 	logger.Info(fmt.Sprintf("Restore complete for DC %#v, Refreshing secrets", dc.ObjectMeta))
 	userSecrets := []string{}
 	for _, user := range dc.Spec.Users {
@@ -32,37 +33,34 @@ func RefreshSecrets(dc *cassdcapi.CassandraDatacenter, ctx context.Context, clie
 	}
 	logger.Info(fmt.Sprintf("refreshing user secrets for %v", userSecrets))
 	//  Both Reaper and medusa secrets go into the userSecrets, so they don't need special handling.
-	timestamp := time.Now().String()
+	requeues := 0
 	for _, i := range userSecrets {
 		secret := &corev1.Secret{}
 		// We need to do our own retries here instead of delegating it back up to the reconciler, because of
 		// the nature (time based) of the annotation we're adding. Otherwise we never complete because the
 		// object on the server never matches the desired object with the new time.
-		retries := 0
-	InnerRetryLoop:
-		for retries <= maxRetries {
-			err := client.Get(ctx, types.NamespacedName{Name: i, Namespace: dc.Namespace}, secret)
-			if err != nil {
-				logger.Error(err, fmt.Sprintf("Failed to get secret %s", i))
-				return result.Error(err)
-			}
-			if secret.ObjectMeta.Annotations == nil {
-				secret.ObjectMeta.Annotations = make(map[string]string)
-			}
-			secret.ObjectMeta.Annotations[k8ssandraapi.RefreshAnnotation] = timestamp
-			recRes := reconciliation.ReconcileObject(ctx, client, requeueDelay, *secret)
-			switch {
-			case recRes.IsError():
-				return recRes
-			case recRes.IsRequeue():
-				retries++
-				continue
-			case recRes.IsDone():
-				break InnerRetryLoop
-			}
+		err := client.Get(ctx, types.NamespacedName{Name: i, Namespace: dc.Namespace}, secret)
+
+		if err != nil {
+			logger.Error(err, fmt.Sprintf("Failed to get secret %s", i))
+			return result.Error(err)
+		}
+		if secret.ObjectMeta.Annotations == nil {
+			secret.ObjectMeta.Annotations = make(map[string]string)
+		}
+		secret.ObjectMeta.Annotations[k8ssandraapi.RefreshAnnotation] = restoreTimestamp.String()
+		recRes := reconciliation.ReconcileObject(ctx, client, requeueDelay, *secret)
+		switch {
+		case recRes.IsError():
+			return recRes
+		case recRes.IsRequeue():
+			requeues++
+			continue
+		case recRes.IsDone():
+			continue
 		}
-		if retries == maxRetries {
-			return result.Error(fmt.Errorf("failed to refresh secret %s after %d retries", i, maxRetries))
+		if requeues > 0 {
+			return result.RequeueSoon(requeueDelay)
 		}
 	}
 	return result.Done()

diff --git a/pkg/medusa/refresh_secrets_test.go b/pkg/medusa/refresh_secrets_test.go
@@ -28,7 +28,8 @@ func TestRefreshSecrets_defaultSUSecret(t *testing.T) {
 	for _, i := range secrets {
 		assert.NoError(t, fakeClient.Create(context.Background(), &i))
 	}
-	recRes := RefreshSecrets(&cassDC, context.Background(), fakeClient, logr.Logger{}, 0)
+
+	recRes := RefreshSecrets(&cassDC, context.Background(), fakeClient, logr.Logger{}, 0, metav1.Now())
 	assert.True(t, recRes.IsDone())
 	suSecret := &corev1.Secret{}
 	assert.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: "test-cluster-superuser", Namespace: "test"}, suSecret))
@@ -57,7 +58,7 @@ func TestRefreshSecrets_customSecrets(t *testing.T) {
 		assert.NoError(t, fakeClient.Create(context.Background(), &i))
 	}
 
-	recRes := RefreshSecrets(&cassDC, context.Background(), fakeClient, logr.Logger{}, 0)
+	recRes := RefreshSecrets(&cassDC, context.Background(), fakeClient, logr.Logger{}, 0, metav1.Now())
 	assert.True(t, recRes.IsDone())
 	suSecret := &corev1.Secret{}
 	assert.NoError(t, fakeClient.Get(context.Background(), types.NamespacedName{Name: "cass-custom-superuser", Namespace: "test"}, suSecret))