Check Deployment ObservedGeneration to set DeploymentReadyCondition

DeploymentReadyCondition is something that depends on the number of Replicas, but it should also be influenced by the ObservedGeneration. We need to make sure that the readiness is set for the last CR version, and this patch adds the additional check that we're rolling out to the other operators as well. ReadyCount is assigned only if we're looking at the last CR, and we check and update the DeploymentReadyCondition only in this case. Signed-off-by: Francesco Pantano <[email protected]>
openstack-k8s-operators · Apr 16, 2024 · b7d6102 · b7d6102
1 parent 1bc90c7
commit b7d6102
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 22 deletions.
diff --git a/controllers/neutronapi_controller.go b/controllers/neutronapi_controller.go
@@ -1009,34 +1009,38 @@ func (r *NeutronAPIReconciler) reconcileNormal(ctx context.Context, instance *ne
 		return ctrlResult, nil
 	}
 
-	instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
+	if depl.GetDeployment().Generation <= depl.GetDeployment().Status.ObservedGeneration {
+		instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
 
-	// verify if network attachment matches expectations
-	networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
+		// verify if network attachment matches expectations
+		networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 
-	instance.Status.NetworkAttachments = networkAttachmentStatus
-	if networkReady {
-		instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage)
-	} else {
-		err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments)
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.NetworkAttachmentsReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.NetworkAttachmentsReadyErrorMessage,
-			err.Error()))
+		instance.Status.NetworkAttachments = networkAttachmentStatus
+		if networkReady {
+			instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage)
+		} else {
+			err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments)
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.NetworkAttachmentsReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.NetworkAttachmentsReadyErrorMessage,
+				err.Error()))
 
-		return ctrl.Result{}, err
-	}
+			return ctrl.Result{}, err
+		}
 
-	if instance.Status.ReadyCount > 0 {
-		instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
+		if instance.Status.ReadyCount == *instance.Spec.Replicas {
+			instance.Status.Conditions.MarkTrue(
+				condition.DeploymentReadyCondition,
+				condition.DeploymentReadyMessage,
+			)
+		}
 	}
 	// create Deployment - end
-
 	if instance.Status.ReadyCount > 0 {
 		// remove finalizers from unused MariaDBAccount records
 		err = mariadbv1.DeleteUnusedMariaDBAccountFinalizers(

diff --git a/test/functional/neutronapi_controller_test.go b/test/functional/neutronapi_controller_test.go
@@ -915,6 +915,7 @@ var _ = Describe("NeutronAPI controller", func() {
 				Name:      "neutron",
 			}
 			depl := th.GetDeployment(deplName)
+			th.SimulateDeploymentReadyWithPods(deplName, map[string][]string{})
 
 			expectedAnnotation, err := json.Marshal(
 				[]networkv1.NetworkSelectionElement{