move status calculation into its own controller

Rather than have each controller responsible for triggering a status calculation, move that responsiblity into its own controller.

Store the upcoming list of Secrets as a shared constant/variable (for use by both the Actuator's Upgradeable() and the status controller's Secret watch list).
Remove GetUpcomingSecrets() from the Actuator interface.
Stop setting default ClusterOperator status condition values from the Actuator's Upgradeable() API call (it is already set in the current status calculation codepath).
Update controllers to register themselves to the new status controller for status calculation callbacks.
Convert previous pkg/util/status into the new status controller (now in pkg/operator/status).
Remove hack of sending fake CredentialsRequests just to trigger a status sync from credentialsrequest controller (new status calculator now will watch and react appropriately).
Update credentialsrequest test cases to only check for the setting of non-default conditions.
Update credentialsrequest test cases to no longer need to register and deregister status callbacks.
Move credentialsrequest status test cases to new status controller.
Update secretannotator actuators to no longer trigger a status sync.

Note: for situations where the status controller cannot watch for certain conditions that should cause a status recalculation, the controller can annotate the CCO's operator config object to add a timestamp (which will in turn trigger a status calc).

Important changes:
Ensure the managementState field is set on our CCO operator config (otherwise we run into problems trying to annotate the operator config). The operator config should not allow an empty string for the managementState field, but it absolutely is not set at all (which shows up as being set to the empty string after doing a Get() of the operator config). Detect this, and assume that the empty string means a managementState of "Managed", and send back "Managed" on Update().

pkg/aws/actuator/actuator.go

@@ -1199,17 +1199,19 @@ func isAWSCredentials(providerSpec *runtime.RawExtension) (bool, error) {
 	return isAWS, nil
 }
 
+// Upgradeable returns a ClusterOperator status condition for the upgradeable type
+// if the system is considered not upgradeable. Otherwise, return nil as the default
+// value is for things to be upgradeable.
 func (a *AWSActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
 	upgradeableCondition := &configv1.ClusterOperatorStatusCondition{
-		Status: configv1.ConditionTrue,
-		Type:   configv1.OperatorUpgradeable,
+		Type: configv1.OperatorUpgradeable,
 	}
 	toRelease := "4.7"
 	if mode == operatorv1.CloudCredentialsModeManual {
 		// Check for the existence of credentials we know are coming in the future. If any do not exist, then we consider
 		// ourselves upgradable=false and inform the user why.
 		missingSecrets := []types.NamespacedName{}
-		for _, nsSecret := range a.GetUpcomingCredSecrets() {
+		for _, nsSecret := range a.getUpcomingCredSecrets() {
 
 			secLog := log.WithField("secret", nsSecret).WithField("toRelease", toRelease)
 			secLog.Debug("checking that secrets exist for manual mode cluster upgrade")
@@ -1240,6 +1242,7 @@ func (a *AWSActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv
 				"Cannot upgrade manual mode cluster to %s due to missing secret(s): %v Please see the Manually Creating IAM for AWS OpenShift documentation.",
 				toRelease,
 				missingSecrets)
+			return upgradeableCondition
 		}
 	} else {
 		// If in mint or passthrough, make sure the root cred secret exists, if not it must be restored prior to upgrade.
@@ -1268,15 +1271,14 @@ func (a *AWSActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv
 		}
 	}
 
-	return upgradeableCondition
+	// Only return non-default conditions as the status controller will set defaults.
+	return nil
 }
 
-func (a *AWSActuator) GetUpcomingCredSecrets() []types.NamespacedName {
+func (a *AWSActuator) getUpcomingCredSecrets() []types.NamespacedName {
 	// For unit testing only, otherwise we want these to be a static list.
 	if a.testUpcomingSecrets != nil {
 		return a.testUpcomingSecrets
 	}
-	return []types.NamespacedName{
-		// Add new 4.7 credentials here, but none are known yet.
-	}
+	return constants.AWSUpcomingSecrets
 }

pkg/aws/actuator/actuator_test.go

@@ -362,8 +362,13 @@ func TestUpgradeable(t *testing.T) {
 				a.testUpcomingSecrets = test.overrideUpcomingSecrets
 			}
 			cond := a.Upgradeable(test.mode)
-			assert.Equal(t, test.expectedStatus, cond.Status)
-			assert.Equal(t, test.expectedReason, cond.Reason)
+
+			if test.expectedStatus == configv1.ConditionTrue {
+				assert.Nil(t, cond, "expect no condition when state is upgradable")
+			} else {
+				assert.Equal(t, test.expectedStatus, cond.Status)
+				assert.Equal(t, test.expectedReason, cond.Reason)
+			}
 		})
 	}
 }

pkg/azure/actuator.go

@@ -656,14 +656,9 @@ func (a *Actuator) getLogger(cr *minterv1.CredentialsRequest) log.FieldLogger {
 	})
 }
 
+// Upgradeable returns a ClusterOperator status condition for the upgradeable type
+// if the system is considered not upgradeable. Otherwise, return nil as the default
+// value is for things to be upgradeable.
 func (a *Actuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
-	upgradeableCondition := &configv1.ClusterOperatorStatusCondition{
-		Status: configv1.ConditionTrue,
-		Type:   configv1.OperatorUpgradeable,
-	}
-	return upgradeableCondition
-}
-
-func (a *Actuator) GetUpcomingCredSecrets() []types.NamespacedName {
-	return []types.NamespacedName{}
+	return nil
 }

pkg/gcp/actuator/actuator.go

@@ -772,6 +772,9 @@ func checkServicesEnabled(gcpClient ccgcp.Client, permList []string, logger log.
 	return serviceAPIsEnabled, nil
 }
 
+// Upgradeable returns a ClusterOperator status condition for the upgradeable type
+// if the system is considered not upgradeable. Otherwise, return nil as the default
+// value is for things to be upgradeable.
 func (a *Actuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
 	upgradeableCondition := &configv1.ClusterOperatorStatusCondition{
 		Status: configv1.ConditionTrue,

pkg/openstack/actuator.go

@@ -273,14 +273,9 @@ func (a *OpenStackActuator) getLogger(cr *minterv1.CredentialsRequest) log.Field
 	})
 }
 
+// Upgradeable returns a ClusterOperator status condition for the upgradeable type
+// if the system is considered not upgradeable. Otherwise, return nil as the default
+// value is for things to be upgradeable.
 func (a *OpenStackActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
-	upgradeableCondition := &configv1.ClusterOperatorStatusCondition{
-		Status: configv1.ConditionTrue,
-		Type:   configv1.OperatorUpgradeable,
-	}
-	return upgradeableCondition
-}
-
-func (a *OpenStackActuator) GetUpcomingCredSecrets() []types.NamespacedName {
-	return []types.NamespacedName{}
+	return nil
 }

pkg/operator/awspodidentity/controller.go

@@ -7,13 +7,8 @@ import (
 	"strings"
 	"time"
 
-	configv1 "github.com/openshift/api/config/v1"
-	"github.com/openshift/library-go/pkg/operator/events"
-	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
-	"github.com/openshift/library-go/pkg/operator/resource/resourcehelper"
-	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
-	"github.com/openshift/library-go/pkg/operator/resource/resourceread"
 	log "github.com/sirupsen/logrus"
+
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -23,6 +18,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	admissionregistrationclientv1beta1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
+
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -32,9 +28,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/library-go/pkg/operator/events"
+	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
+	"github.com/openshift/library-go/pkg/operator/resource/resourcehelper"
+	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
+	"github.com/openshift/library-go/pkg/operator/resource/resourceread"
+
 	"github.com/openshift/cloud-credential-operator/pkg/assets/v410_00_assets"
 	"github.com/openshift/cloud-credential-operator/pkg/operator/platform"
-	"github.com/openshift/cloud-credential-operator/pkg/util/clusteroperator"
+	"github.com/openshift/cloud-credential-operator/pkg/operator/status"
 )
 
 const (
@@ -210,7 +213,7 @@ func Add(mgr manager.Manager, kubeconfig string) error {
 		}
 	}
 
-	clusteroperator.AddStatusHandler(r)
+	status.AddHandler(controllerName, r)
 	mgr.Add(&awsPodIdentityController{reconciler: r, cache: cache, logger: logger})
 
 	return nil
@@ -357,7 +360,7 @@ func reportUpdateEvent(recorder events.Recorder, obj runtime.Object, originalErr
 	}
 }
 
-var _ clusteroperator.StatusHandler = &staticResourceReconciler{}
+var _ status.Handler = &staticResourceReconciler{}
 
 func (r *staticResourceReconciler) GetConditions(logger log.FieldLogger) ([]configv1.ClusterOperatorStatusCondition, error) {
 	return r.conditions, nil

pkg/operator/constants/constants.go

@@ -1,5 +1,7 @@
 package constants
 
+import "k8s.io/apimachinery/pkg/types"
+
 // CredentialsMode enumerates the possible modes of operation for CCO
 type CredentialsMode string
 
@@ -91,6 +93,12 @@ const (
 	// CloudCredOperatorConfig is the name of the credentialsrequest.operator.openshift.io CR holding CCO's config
 	CloudCredOperatorConfig = "cluster"
 
+	// CloudCredOperatorConfigTimestampAnnotation is the annotation controllers can update to trigger a status sync.
+	CloudCredOperatorConfigTimestampAnnotation = "cloudcredential.operator.openshift.io/statussync"
+
+	// CloudCredClusterOperatorName is the name of the CCO's ClusterOperator object
+	CloudCredClusterOperatorName = "cloud-credential"
+
 	// CloudCredSecretNamespace is where the cloud credentials can be found
 	CloudCredSecretNamespace = "kube-system"
 
@@ -118,4 +126,19 @@ var (
 		ModeDegraded,
 		ModeUnknown,
 	}
+
+	// Add known new credentials for next version upgrade
+
+	// AWSUpcomingSecrets contains the list of known new AWS credential secrets for the next version of OpenShift
+	AWSUpcomingSecrets = []types.NamespacedName{}
+	// AzureUpcomingSecrets contains the list of known new Azure credential secrets for the next version of OpenShift
+	AzureUpcomingSecrets = []types.NamespacedName{}
+	// GCPUpcomingSecrets contains the list of known new GCP credential secrets for the next version of OpenShift
+	GCPUpcomingSecrets = []types.NamespacedName{}
+	// OpenStackUpcomingSecrets contains the list of known new OpenStack credential secrets for the next version of OpenShift
+	OpenStackUpcomingSecrets = []types.NamespacedName{}
+	// OvirtUpcomingSecrets contains the list of known new oVirt credential secrets for the next version of OpenShift
+	OvirtUpcomingSecrets = []types.NamespacedName{}
+	// VsphereUpcomingSecrets contains the list of known new vSphere credential secrets for the next version of OpenShift
+	VsphereUpcomingSecrets = []types.NamespacedName{}
 )

pkg/operator/controller.go

@@ -29,6 +29,7 @@ import (
 	"github.com/openshift/cloud-credential-operator/pkg/operator/oidcdiscoveryendpoint"
 	"github.com/openshift/cloud-credential-operator/pkg/operator/platform"
 	"github.com/openshift/cloud-credential-operator/pkg/operator/secretannotator"
+	"github.com/openshift/cloud-credential-operator/pkg/operator/status"
 	"github.com/openshift/cloud-credential-operator/pkg/ovirt"
 	"github.com/openshift/cloud-credential-operator/pkg/util"
 	vsphereactuator "github.com/openshift/cloud-credential-operator/pkg/vsphere/actuator"
@@ -51,6 +52,7 @@ func init() {
 	AddToManagerFuncs = append(AddToManagerFuncs, secretannotator.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, awspodidentity.Add)
 	AddToManagerFuncs = append(AddToManagerFuncs, oidcdiscoveryendpoint.Add)
+	AddToManagerFuncs = append(AddToManagerFuncs, status.Add)
 	AddToManagerWithActuatorFuncs = append(AddToManagerWithActuatorFuncs, credentialsrequest.AddWithActuator)
 }
 

pkg/operator/credentialsrequest/actuator/actuator.go

@@ -43,9 +43,6 @@ type Actuator interface {
 	// Upgradeable returns a ClusterOperator Upgradeable condition to indicate whether or not this cluster can
 	// be safely upgraded to the next "minor" (4.y) Openshift release.
 	Upgradeable(operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition
-	// GetUpcomingCredSecrets returns a slice of NamespacedNames for secrets holding credentials that we know are coming in the next OpenShift release.
-	// Used to pre-check if Upgradeable condition should be true or false for manual mode deployments of the credentials operator.
-	GetUpcomingCredSecrets() []types.NamespacedName
 }
 
 type DummyActuator struct {
@@ -80,10 +77,6 @@ func (a *DummyActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *confi
 	return upgradeableCondition
 }
 
-func (a *DummyActuator) GetUpcomingCredSecrets() []types.NamespacedName {
-	return []types.NamespacedName{}
-}
-
 type ActuatorError struct {
 	ErrReason minterv1.CredentialsRequestConditionType
 	Message   string