Update the ReadyCondition for GlanceAPI and mirror to the top level CR

This patch updates the Ready condition by removing it from the defer
function and put it at the end of the reconcile loop. In addition,
before marking the GlanceAPI Deployment Ready, it checks that the
number of replicas is equals to the ReadyCount. If the deployment is
still in progress, of this condition is not met, it's mirrored to
the top-level CR that shows at which point the Deployment is blocked.

Signed-off-by: Francesco Pantano <fpantano@redhat.com>

api/v1beta1/glance_types.go

@@ -206,7 +206,11 @@ func init() {
 	SchemeBuilder.Register(&Glance{}, &GlanceList{})
 }
 
-// IsReady - returns true if Glance is reconciled successfully
+// IsReady - returns true if the underlying GlanceAPI is reconciled successfully
+// and set the ReadyCondition to True: it is mirrored to the top-level CR, so
+// the purpose of this function is to let the openstack-operator to gather the
+// Status of the entire Glance Deployment (intended as a single entity) from a
+// single place
 func (instance Glance) IsReady() bool {
 	return instance.Status.Conditions.IsTrue(condition.ReadyCondition)
 }

controllers/glance_controller.go

@@ -699,8 +699,14 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance
 		return ctrlResult, err
 	}
 	instance.Status.Conditions.MarkTrue(condition.CronJobReadyCondition, condition.CronJobReadyMessage)
-
 	// create CronJob - end
+
+	// We reached the end of the Reconcile, update the Ready condition based on
+	// the sub conditions
+	if instance.Status.Conditions.AllSubConditionIsTrue() {
+		instance.Status.Conditions.MarkTrue(
+			condition.ReadyCondition, condition.ReadyMessage)
+	}
 	return ctrl.Result{}, nil
 }
 
@@ -873,6 +879,10 @@ func (r *GlanceReconciler) apiDeploymentCreateOrUpdate(
 	apiSpec.GlanceAPITemplate.StorageRequest = instance.Spec.StorageRequest
 	apiSpec.GlanceAPITemplate.StorageClass = instance.Spec.StorageClass
 	apiSpec.MemcachedInstance = memcached.Name
+	// Make sure to inject the ContainerImage passed by the OpenStackVersions
+	// resource to all the underlying instances and rollout a new StatefulSet
+	// if it has been changed
+	apiSpec.GlanceAPITemplate.ContainerImage = instance.Spec.ContainerImage
 
 	// We select which glanceAPI should register the keystoneEndpoint by using
 	// an API selector defined in the main glance CR; if it matches with the

controllers/glanceapi_controller.go

@@ -134,18 +134,8 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	// Always patch the instance status when exiting this function so we can persist any changes.
 	defer func() {
-		// update the Ready condition based on the sub conditions
-		if instance.Status.Conditions.AllSubConditionIsTrue() {
-			instance.Status.Conditions.MarkTrue(
-				condition.ReadyCondition, condition.ReadyMessage)
-		} else {
-			// something is not ready so reset the Ready condition
-			instance.Status.Conditions.MarkUnknown(
-				condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage)
-			// and recalculate it based on the state of the rest of the conditions
-			instance.Status.Conditions.Set(
-				instance.Status.Conditions.Mirror(condition.ReadyCondition))
-		}
+		// Always mirror the Condition from the sub level CRs
+		instance.Status.Conditions.Set(instance.Status.Conditions.Mirror(condition.ReadyCondition))
 		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
 		err := helper.PatchInstance(ctx, instance)
 		if err != nil {
@@ -835,8 +825,10 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
 
 		return ctrl.Result{}, err
 	}
-
-	if instance.Status.ReadyCount > 0 || *instance.Spec.Replicas == 0 {
+	// Mark the Deployment as Ready only if the number of Replicas is equals
+	// to the Deployed instances (ReadyCount), but mark it as True is Replicas
+	// is zero
+	if instance.Status.ReadyCount == *instance.Spec.Replicas || *instance.Spec.Replicas == 0 {
 		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
 	} else {
 		instance.Status.Conditions.Set(condition.FalseCondition(
@@ -889,8 +881,14 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla
 		condition.CronJobReadyCondition,
 		condition.CronJobReadyMessage,
 	)
-
 	// create ImageCache cronJobs - end
+
+	// We reached the end of the Reconcile, update the Ready condition based on
+	// the sub conditions
+	if instance.Status.Conditions.AllSubConditionIsTrue() {
+		instance.Status.Conditions.MarkTrue(
+			condition.ReadyCondition, condition.ReadyMessage)
+	}
 	r.Log.Info(fmt.Sprintf("Reconciled Service '%s' successfully", instance.Name))
 	return ctrl.Result{}, nil
 }

docs/dev/design-decisions.md

@@ -426,20 +426,31 @@ Conditions represent a critical aspect for reconciliation because they allow:
 3. identify the status of the underlying GlanceAPIs and report their healhy to
    the upper level CR
 
-The document [docs/conditions]() describes the meaning of the k8s-operators
-shared conditions.
-Conditions are re-evaluated when a new Reconciliation loop starts, and if one of
-them is different from what has been previously registered, an update is performed,
+The document
+[docs/conditions](https://github.com/openstack-k8s-operators/docs/blob/main/conditions.md)
+describes the meaning of the k8s-operators shared `Conditions`.
+Conditions are re-evaluated when a new `Reconcile` loop starts, and if one of
+them is different from what was previously registered, an update is performed,
 and it allows to give immediate feedback to the user.
 Conditions are initially marked as `Unknown`, while the general `ReadyCondition`
-is marked as `False` because we assume that the Deployment is in progress but
+is marked as `False` because we assume that the `Deployment` is in progress but
 not `Ready` at the same time, until the entire set of conditions are evaluated.
-Conditions can be seen as a checklist or steps within the reconciliation loop,
-and while the loop proceed to the end, each of them is evaluated.
-If a particular component with an associated condition is not Ready, an `error`
+`Conditions` can be seen as a checklist or steps within the reconcile loop, and
+while the loop proceed to the end, each of them is evaluated.
+If a particular component with an associated condition is not `Ready`, an `error`
 is returned from the operator, and its failing condition is marked as `False`
 with an appropriate error message.
 If the end of the loop is reached, it means we passed through all the steps and
-the Conditions are marked to `True`: it is possible, at this point, to mark the
+the `Conditions` are marked to `True`: it is possible, at this point, to mark the
 overall `ReadyCondition` to `Status=True` as well and `Mirror` the result to the
 top-level CR.
+`Conditions` are always Mirrored using the defer function, so that we always
+have an information to the `ReadyCondition` that reflects the point where the
+loop is exiting/failing.
+The general `IsReady` function is used as a wrapper for the `ReadyCondition`
+boolean, and it can be used to get the status of the `instance`. In general
+in the glance-operator we make the following assumptions:
+- A Glance/GlanceAPI is considered `Ready` if all the subconditions are verified
+- A Deployment (intended as the `StatefulSet` that represents the GlanceAPIs) is
+  considered `Ready` if the number of `Replicas` specified in the `Glance` CR
+  spec is **equal** to the number of available instances (`ReadyCount`).