From b7d61021f5d5685c637aaf75b1e74399feebfa8a Mon Sep 17 00:00:00 2001
From: Francesco Pantano <fpantano@redhat.com>
Date: Fri, 12 Apr 2024 10:42:22 +0200
Subject: [PATCH] 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 <fpantano@redhat.com>
---
 controllers/neutronapi_controller.go          | 48 ++++++++++---------
 test/functional/neutronapi_controller_test.go |  1 +
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/controllers/neutronapi_controller.go b/controllers/neutronapi_controller.go
index 8819711b..9cc09327 100644
--- 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
index ce30c34e..7395fa95 100644
--- 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{