Add ObservedGeneration to the sub Resources

This patch does a few things: 1. it adds observedGeneration to the sub custom resources 2. it proposes to bump the observedGeneration at the beginning of the reconciliation loop (knative/serving#4937) 3. it checks, at the top level, if the ObservedGeneration matches with the metadata.generation assigned to the subCR(s) 4. before marking the DeploymentReadyCondition as True, the ObservedGeneration is compared with the Generation of the Deployment Signed-off-by: Francesco Pantano <[email protected]>
fmount · Apr 15, 2024 · 96e8e7c · 96e8e7c
1 parent 4de3d73
commit 96e8e7c
Show file tree

Hide file tree

Showing 12 changed files with 205 additions and 24 deletions.
diff --git a/api/bases/swift.openstack.org_swiftproxies.yaml b/api/bases/swift.openstack.org_swiftproxies.yaml
@@ -379,6 +379,14 @@ spec:
                   type: array
                 description: NetworkAttachments status of the deployment pods
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
               readyCount:
                 description: ReadyCount of SwiftProxy instances
                 format: int32

diff --git a/api/bases/swift.openstack.org_swiftrings.yaml b/api/bases/swift.openstack.org_swiftrings.yaml
@@ -131,6 +131,14 @@ spec:
                   type: string
                 description: Map of hashes to track e.g. job status
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

diff --git a/api/bases/swift.openstack.org_swiftstorages.yaml b/api/bases/swift.openstack.org_swiftstorages.yaml
@@ -161,6 +161,14 @@ spec:
                   type: array
                 description: NetworkAttachments status of the deployment pods
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
               readyCount:
                 description: ReadyCount of SwiftStorage instances
                 format: int32

diff --git a/api/v1beta1/swiftproxy_types.go b/api/v1beta1/swiftproxy_types.go
@@ -126,6 +126,12 @@ type SwiftProxyStatus struct {
 
 	// TransportURLSecret - Secret containing RabbitMQ transportURL
 	TransportURLSecret string `json:"transportURLSecret,omitempty"`
+
+	// ObservedGeneration - the most recent generation observed for this
+	// service. If the observed generation is less than the spec generation,
+	// then the controller has not processed the latest changes injected by
+	// the opentack-operator in the top-level CR (e.g. the ContainerImage)
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1beta1/swiftring_types.go b/api/v1beta1/swiftring_types.go
@@ -76,6 +76,12 @@ type SwiftRingStatus struct {
 
 	// Map of hashes to track e.g. job status
 	Hash map[string]string `json:"hash,omitempty"`
+
+	// ObservedGeneration - the most recent generation observed for this
+	// service. If the observed generation is less than the spec generation,
+	// then the controller has not processed the latest changes injected by
+	// the opentack-operator in the top-level CR (e.g. the ContainerImage)
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1beta1/swiftstorage_types.go b/api/v1beta1/swiftstorage_types.go
@@ -94,6 +94,12 @@ type SwiftStorageStatus struct {
 
 	// Map of hashes to track e.g. job status
 	Hash map[string]string `json:"hash,omitempty"`
+
+	// ObservedGeneration - the most recent generation observed for this
+	// service. If the observed generation is less than the spec generation,
+	// then the controller has not processed the latest changes injected by
+	// the opentack-operator in the top-level CR (e.g. the ContainerImage)
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/config/crd/bases/swift.openstack.org_swiftproxies.yaml b/config/crd/bases/swift.openstack.org_swiftproxies.yaml
@@ -379,6 +379,14 @@ spec:
                   type: array
                 description: NetworkAttachments status of the deployment pods
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
               readyCount:
                 description: ReadyCount of SwiftProxy instances
                 format: int32

diff --git a/config/crd/bases/swift.openstack.org_swiftrings.yaml b/config/crd/bases/swift.openstack.org_swiftrings.yaml
@@ -131,6 +131,14 @@ spec:
                   type: string
                 description: Map of hashes to track e.g. job status
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

diff --git a/config/crd/bases/swift.openstack.org_swiftstorages.yaml b/config/crd/bases/swift.openstack.org_swiftstorages.yaml
@@ -161,6 +161,14 @@ spec:
                   type: array
                 description: NetworkAttachments status of the deployment pods
                 type: object
+              observedGeneration:
+                description: ObservedGeneration - the most recent generation observed
+                  for this service. If the observed generation is less than the spec
+                  generation, then the controller has not processed the latest changes
+                  injected by the opentack-operator in the top-level CR (e.g. the
+                  ContainerImage)
+                format: int64
+                type: integer
               readyCount:
                 description: ReadyCount of SwiftStorage instances
                 format: int32

diff --git a/controllers/swift_controller.go b/controllers/swift_controller.go
@@ -281,14 +281,32 @@ func (r *SwiftReconciler) reconcileNormal(ctx context.Context, instance *swiftv1
 			err.Error()))
 		return ctrl.Result{}, err
 	}
-	if op != controllerutil.OperationResultNone {
-		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
+	// make sure the controller is watching the last generation of the subCR
+	stg, err := r.checkSwiftStorageGeneration(instance)
+	if err != nil {
+		instance.Status.Conditions.Set(condition.FalseCondition(
+			swiftv1.SwiftStorageReadyCondition,
+			condition.ErrorReason,
+			condition.SeverityWarning,
+			swiftv1.SwiftStorageReadyErrorMessage,
+			err.Error()))
+		return ctrl.Result{}, err
 	}
-
-	// Mirror SwiftStorage's condition status
-	c := swiftStorage.Status.Conditions.Mirror(swiftv1.SwiftStorageReadyCondition)
-	if c != nil {
-		instance.Status.Conditions.Set(c)
+	if !stg {
+		instance.Status.Conditions.Set(condition.UnknownCondition(
+			swiftv1.SwiftStorageReadyCondition,
+			condition.InitReason,
+			swiftv1.SwiftStorageReadyInitMessage,
+		))
+	} else {
+		// Mirror SwiftStorage's condition status
+		c := swiftStorage.Status.Conditions.Mirror(swiftv1.SwiftStorageReadyCondition)
+		if c != nil {
+			instance.Status.Conditions.Set(c)
+		}
+	}
+	if op != controllerutil.OperationResultNone && stg {
+		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
 	}
 
 	// create or update Swift rings
@@ -302,14 +320,33 @@ func (r *SwiftReconciler) reconcileNormal(ctx context.Context, instance *swiftv1
 			err.Error()))
 		return ctrl.Result{}, err
 	}
-	if op != controllerutil.OperationResultNone {
-		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
-	}
 
-	// Mirror SwiftRing's condition status
-	c = swiftRing.Status.Conditions.Mirror(swiftv1.SwiftRingReadyCondition)
-	if c != nil {
-		instance.Status.Conditions.Set(c)
+	// make sure the controller is watching the last generation of the subCR
+	ring, err := r.checkSwiftRingGeneration(instance)
+	if err != nil {
+		instance.Status.Conditions.Set(condition.FalseCondition(
+			swiftv1.SwiftRingReadyCondition,
+			condition.ErrorReason,
+			condition.SeverityWarning,
+			swiftv1.SwiftRingReadyErrorMessage,
+			err.Error()))
+		return ctrl.Result{}, err
+	}
+	if !ring {
+		instance.Status.Conditions.Set(condition.UnknownCondition(
+			swiftv1.SwiftRingReadyCondition,
+			condition.InitReason,
+			swiftv1.SwiftRingReadyInitMessage,
+		))
+	} else {
+		// Mirror SwiftRing's condition status
+		c := swiftRing.Status.Conditions.Mirror(swiftv1.SwiftRingReadyCondition)
+		if c != nil {
+			instance.Status.Conditions.Set(c)
+		}
+	}
+	if op != controllerutil.OperationResultNone && ring {
+		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
 	}
 
 	// create or update Swift proxy
@@ -323,16 +360,32 @@ func (r *SwiftReconciler) reconcileNormal(ctx context.Context, instance *swiftv1
 			err.Error()))
 		return ctrl.Result{}, err
 	}
-	if op != controllerutil.OperationResultNone {
-		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
+	sst, err := r.checkSwiftProxyGeneration(instance)
+	if err != nil {
+		instance.Status.Conditions.Set(condition.FalseCondition(
+			swiftv1.SwiftProxyReadyCondition,
+			condition.ErrorReason,
+			condition.SeverityWarning,
+			swiftv1.SwiftProxyReadyErrorMessage,
+			err.Error()))
+		return ctrl.Result{}, err
 	}
-
-	// Mirror SwiftProxy's condition status
-	c = swiftProxy.Status.Conditions.Mirror(swiftv1.SwiftProxyReadyCondition)
-	if c != nil {
-		instance.Status.Conditions.Set(c)
+	if !sst {
+		instance.Status.Conditions.Set(condition.UnknownCondition(
+			swiftv1.SwiftProxyReadyCondition,
+			condition.InitReason,
+			swiftv1.SwiftProxyReadyInitMessage,
+		))
+	} else {
+		// Mirror SwiftProxy's condition status
+		c := swiftProxy.Status.Conditions.Mirror(swiftv1.SwiftProxyReadyCondition)
+		if c != nil {
+			instance.Status.Conditions.Set(c)
+		}
+	}
+	if op != controllerutil.OperationResultNone && sst {
+		r.Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
 	}
-
 	r.Log.Info(fmt.Sprintf("Reconciled Service '%s' successfully", instance.Name))
 
 	// We reached the end of the Reconcile, update the Ready condition based on
@@ -474,3 +527,63 @@ func (r *SwiftReconciler) proxyCreateOrUpdate(ctx context.Context, instance *swi
 
 	return deployment, op, err
 }
+
+// checkSwiftProxyGeneration -
+func (r *SwiftReconciler) checkSwiftProxyGeneration(
+	instance *swiftv1.Swift,
+) (bool, error) {
+	proxy := &swiftv1.SwiftProxyList{}
+	listOpts := []client.ListOption{
+		client.InNamespace(instance.Namespace),
+	}
+	if err := r.Client.List(context.Background(), proxy, listOpts...); err != nil {
+		r.Log.Error(err, "Unable to retrieve SwiftProxy %w")
+		return false, err
+	}
+	for _, item := range proxy.Items {
+		if item.Generation != item.Status.ObservedGeneration {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+// checkSwiftStorageGeneration -
+func (r *SwiftReconciler) checkSwiftStorageGeneration(
+	instance *swiftv1.Swift,
+) (bool, error) {
+	sst := &swiftv1.SwiftStorageList{}
+	listOpts := []client.ListOption{
+		client.InNamespace(instance.Namespace),
+	}
+	if err := r.Client.List(context.Background(), sst, listOpts...); err != nil {
+		r.Log.Error(err, "Unable to retrieve SwiftStorage %w")
+		return false, err
+	}
+	for _, item := range sst.Items {
+		if item.Generation != item.Status.ObservedGeneration {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+// checkSwiftRingGeneration -
+func (r *SwiftReconciler) checkSwiftRingGeneration(
+	instance *swiftv1.Swift,
+) (bool, error) {
+	rings := &swiftv1.SwiftRingList{}
+	listOpts := []client.ListOption{
+		client.InNamespace(instance.Namespace),
+	}
+	if err := r.Client.List(context.Background(), rings, listOpts...); err != nil {
+		r.Log.Error(err, "Unable to retrieve SwiftRing %w")
+		return false, err
+	}
+	for _, item := range rings.Items {
+		if item.Generation != item.Status.ObservedGeneration {
+			return false, nil
+		}
+	}
+	return true, nil
+}
diff --git a/controllers/swiftproxy_controller.go b/controllers/swiftproxy_controller.go
@@ -654,7 +654,8 @@ func (r *SwiftProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
 
-	if instance.Status.ReadyCount == *instance.Spec.Replicas {
+	if instance.Status.ReadyCount == *instance.Spec.Replicas &&
+		depl.GetDeployment().Generation == depl.GetDeployment().Status.ObservedGeneration {
 
 		// verify if network attachment matches expectations
 		networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount)

diff --git a/controllers/swiftstorage_controller.go b/controllers/swiftstorage_controller.go
@@ -313,7 +313,8 @@ func (r *SwiftStorageReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	instance.Status.ReadyCount = sset.GetStatefulSet().Status.ReadyReplicas
-	if instance.Status.ReadyCount == *instance.Spec.Replicas {
+	if instance.Status.ReadyCount == *instance.Spec.Replicas &&
+		sset.GetStatefulSet().Generation == sset.GetStatefulSet().Status.ObservedGeneration {
 		networkReady, networkAttachmentStatus, err := networkattachment.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, serviceLabels, instance.Status.ReadyCount)
 		if err != nil {
 			return ctrl.Result{}, err