Merge pull request #617 from securesign/fix-sbj-not-running-after-upg…

…rade

SBJ | Make sure SBJ reconciles after upgrade

internal/controller/securesign/actions/constants.go

@@ -7,7 +7,6 @@ const (
 	RekorCondition           = "RekorAvailable"
 	TrillianCondition        = "TrillianAvailable"
 	CTlogCondition           = "CTlogAvailable"
-	SBJCondition             = "SBJCondition"
 	SegmentBackupCronJobName = "segment-backup-nightly-metrics"
 	SegmentBackupJobName     = "segment-backup-installation"
 	SegmentRBACName          = "rhtas-segment-backup-job"

internal/controller/securesign/actions/initialize_status.go

@@ -10,6 +10,10 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var conditions = []string{
+	constants.Ready, TrillianCondition, FulcioCondition, RekorCondition, CTlogCondition, TufCondition, TSACondition, MetricsCondition,
+}
+
 func NewInitializeStatusAction() action.Action[*rhtasv1alpha1.Securesign] {
 	return &initializeStatus{}
 }
@@ -23,16 +27,23 @@ func (i initializeStatus) Name() string {
 }
 
 func (i initializeStatus) CanHandle(_ context.Context, instance *rhtasv1alpha1.Securesign) bool {
-	return meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) == nil
+	for _, condition := range conditions {
+		if c := meta.FindStatusCondition(instance.Status.Conditions, condition); c == nil {
+			return true
+		}
+	}
+	return false
 }
 
 func (i initializeStatus) Handle(ctx context.Context, instance *rhtasv1alpha1.Securesign) *action.Result {
-	for _, conditionType := range []string{constants.Ready, TrillianCondition, FulcioCondition, RekorCondition, CTlogCondition, TufCondition, TSACondition, SBJCondition} {
-		meta.SetStatusCondition(&instance.Status.Conditions, v1.Condition{
-			Type:   conditionType,
-			Status: v1.ConditionUnknown,
-			Reason: constants.Pending,
-		})
+	for _, conditionType := range conditions {
+		if c := meta.FindStatusCondition(instance.Status.Conditions, conditionType); c == nil {
+			meta.SetStatusCondition(&instance.Status.Conditions, v1.Condition{
+				Type:   conditionType,
+				Status: v1.ConditionUnknown,
+				Reason: constants.Pending,
+			})
+		}
 	}
 	return i.StatusUpdate(ctx, instance)
 }

internal/controller/securesign/actions/segment_backup_cronjob.go

@@ -35,6 +35,10 @@ func (i segmentBackupCronJob) Name() string {
 	return "segment-backup-nightly-metrics"
 }
 func (i segmentBackupCronJob) CanHandle(_ context.Context, instance *rhtasv1alpha1.Securesign) bool {
+	c := meta.FindStatusCondition(instance.Status.Conditions, MetricsCondition)
+	if c == nil || c.Reason == constants.Ready {
+		return false
+	}
 	val, found := instance.Annotations[annotations.Metrics]
 	if !found {
 		return true
@@ -115,24 +119,24 @@ func (i segmentBackupCronJob) Handle(ctx context.Context, instance *rhtasv1alpha
 			Reason:  constants.Failure,
 			Message: err.Error(),
 		})
-		meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
-			Type:    constants.Ready,
-			Status:  metav1.ConditionFalse,
-			Reason:  constants.Failure,
-			Message: err.Error(),
-		})
 		return i.FailedWithStatusUpdate(ctx, fmt.Errorf("could not create segment backup cron job: %w", err), instance)
 	}
 
 	if updated {
 		meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
-			Type:    constants.Ready,
+			Type:    MetricsCondition,
 			Status:  metav1.ConditionFalse,
 			Reason:  constants.Creating,
-			Message: "Segment backup Cron Job created",
+			Message: "Segment backup Cron Job creating",
 		})
 		return i.StatusUpdate(ctx, instance)
-	} else {
-		return i.Continue()
 	}
+
+	meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+		Type:    MetricsCondition,
+		Status:  metav1.ConditionTrue,
+		Reason:  constants.Ready,
+		Message: "Segment backup Cron Job created",
+	})
+	return i.Continue()
 }

internal/controller/securesign/actions/segment_backup_job.go

@@ -32,12 +32,13 @@ type segmentBackupJob struct {
 }
 
 func (i segmentBackupJob) Name() string {
-	return "segment-backup-installation"
+	return SegmentBackupJobName
 }
 
 func (i segmentBackupJob) CanHandle(_ context.Context, instance *rhtasv1alpha1.Securesign) bool {
-	if c := meta.FindStatusCondition(instance.Status.Conditions, SBJCondition); c != nil {
-		return c.Status != metav1.ConditionTrue
+	c := meta.FindStatusCondition(instance.Status.Conditions, MetricsCondition)
+	if c == nil || c.Reason == constants.Ready {
+		return false
 	}
 
 	val, found := instance.Annotations[annotations.Metrics]
@@ -92,18 +93,15 @@ func (i segmentBackupJob) Handle(ctx context.Context, instance *rhtasv1alpha1.Se
 	if err = ctrl.SetControllerReference(instance, job, i.Client.Scheme()); err != nil {
 		return i.Failed(fmt.Errorf("could not set controller reference for Job: %w", err))
 	}
-
 	_, err = i.Ensure(ctx, job)
 	if err != nil {
-		return i.Failed(fmt.Errorf("failed to Ensure the job: %w", err))
+		meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+			Type:    MetricsCondition,
+			Status:  metav1.ConditionFalse,
+			Reason:  constants.Creating,
+			Message: err.Error(),
+		})
+		return i.StatusUpdate(ctx, instance)
 	}
-
-	meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
-		Type:    SBJCondition,
-		Status:  metav1.ConditionTrue,
-		Reason:  constants.Ready,
-		Message: "Segment Backup Job Created",
-	})
-
-	return i.StatusUpdate(ctx, instance)
+	return i.Continue()
 }

internal/controller/securesign/actions/rbac.go → internal/controller/securesign/actions/segment_rbac.go

@@ -24,7 +24,7 @@ const (
 	OpenshiftMonitoringNS  = "openshift-monitoring"
 )
 
-func NewRBACAction() action.Action[*rhtasv1alpha1.Securesign] {
+func NewSBJRBACAction() action.Action[*rhtasv1alpha1.Securesign] {
 	return &rbacAction{}
 }
 
@@ -37,6 +37,10 @@ func (i rbacAction) Name() string {
 }
 
 func (i rbacAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.Securesign) bool {
+	c := meta.FindStatusCondition(instance.Status.Conditions, MetricsCondition)
+	if c == nil || c.Reason == constants.Ready {
+		return false
+	}
 	val, found := instance.Annotations[annotations.Metrics]
 	if !found {
 		return true
@@ -227,5 +231,12 @@ func (i rbacAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Securesi
 		return i.FailedWithStatusUpdate(ctx, fmt.Errorf("could not create openshift-console ClusterRoleBinding for SBJ: %w", err), instance)
 	}
 
+	meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+		Type:    MetricsCondition,
+		Status:  metav1.ConditionTrue,
+		Reason:  constants.Creating,
+		Message: "Segment Backup Job Creating",
+	})
+
 	return i.Continue()
 }

internal/controller/securesign/securesign_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 
 	v12 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/operator-framework/operator-lib/predicate"
@@ -126,22 +127,28 @@ func (r *SecuresignReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	acs := []action.Action[*rhtasv1alpha1.Securesign]{
 		actions.NewInitializeStatusAction(),
+		actions.NewSBJRBACAction(),
+		actions.NewSegmentBackupJobAction(),
+		actions.NewSegmentBackupCronJobAction(),
 		actions.NewTrillianAction(),
 		actions.NewFulcioAction(),
 		actions.NewRekorAction(),
 		actions.NewCtlogAction(),
 		actions.NewTufAction(),
 		actions.NewTsaAction(),
-		actions.NewRBACAction(),
-		actions.NewSegmentBackupJobAction(),
-		actions.NewSegmentBackupCronJobAction(),
 		actions.NewUpdateStatusAction(),
 	}
 
 	for _, a := range acs {
 		a.InjectClient(r.Client)
 		a.InjectLogger(log.WithName(a.Name()))
 
+		if a.Name() == actions.SegmentBackupJobName {
+			if c := meta.FindStatusCondition(instance.GetConditions(), actions.MetricsCondition); c != nil && c.Reason == constants.Creating {
+				continue
+			}
+		}
+
 		if a.CanHandle(ctx, target) {
 			result := a.Handle(ctx, target)
 			if result != nil {