diff --git a/api/bases/ironic.openstack.org_ironicapis.yaml b/api/bases/ironic.openstack.org_ironicapis.yaml index db40c32d..f101971e 100644 --- a/api/bases/ironic.openstack.org_ironicapis.yaml +++ b/api/bases/ironic.openstack.org_ironicapis.yaml @@ -464,6 +464,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic API instances format: int32 diff --git a/api/bases/ironic.openstack.org_ironicconductors.yaml b/api/bases/ironic.openstack.org_ironicconductors.yaml index 2d70c025..c7d14c72 100644 --- a/api/bases/ironic.openstack.org_ironicconductors.yaml +++ b/api/bases/ironic.openstack.org_ironicconductors.yaml @@ -319,6 +319,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic Conductor instances format: int32 diff --git a/api/bases/ironic.openstack.org_ironicinspectors.yaml b/api/bases/ironic.openstack.org_ironicinspectors.yaml index 5fec5cfe..7cf528dd 100644 --- a/api/bases/ironic.openstack.org_ironicinspectors.yaml +++ b/api/bases/ironic.openstack.org_ironicinspectors.yaml @@ -517,6 +517,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of Ironic Inspector instances format: int32 diff --git a/api/bases/ironic.openstack.org_ironicneutronagents.yaml b/api/bases/ironic.openstack.org_ironicneutronagents.yaml index 87c2af79..30d2a5f1 100644 --- a/api/bases/ironic.openstack.org_ironicneutronagents.yaml +++ b/api/bases/ironic.openstack.org_ironicneutronagents.yaml @@ -220,6 +220,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic Conductor instances format: int32 diff --git a/api/bases/ironic.openstack.org_ironics.yaml b/api/bases/ironic.openstack.org_ironics.yaml index e19ce8ea..79e7b220 100644 --- a/api/bases/ironic.openstack.org_ironics.yaml +++ b/api/bases/ironic.openstack.org_ironics.yaml @@ -1149,7 +1149,7 @@ spec: 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 + injected by the openstack-operator in the top-level CR (e.g. the ContainerImage) format: int64 type: integer diff --git a/api/v1beta1/ironic_types.go b/api/v1beta1/ironic_types.go index 9c165dd2..3680cbab 100644 --- a/api/v1beta1/ironic_types.go +++ b/api/v1beta1/ironic_types.go @@ -239,7 +239,7 @@ type IronicStatus struct { // 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) + // the openstack-operator in the top-level CR (e.g. the ContainerImage) ObservedGeneration int64 `json:"observedGeneration,omitempty"` } diff --git a/api/v1beta1/ironicapi_types.go b/api/v1beta1/ironicapi_types.go index 62a7cbed..3d3db133 100644 --- a/api/v1beta1/ironicapi_types.go +++ b/api/v1beta1/ironicapi_types.go @@ -123,6 +123,12 @@ type IronicAPIStatus struct { // NetworkAttachments status of the deployment pods NetworkAttachments map[string][]string `json:"networkAttachments,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 openstack-operator in the top-level CR (e.g. the ContainerImage) + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1beta1/ironicconductor_types.go b/api/v1beta1/ironicconductor_types.go index f1623bf8..d4e5827b 100644 --- a/api/v1beta1/ironicconductor_types.go +++ b/api/v1beta1/ironicconductor_types.go @@ -134,6 +134,12 @@ type IronicConductorStatus struct { // NetworkAttachments status of the deployment pods NetworkAttachments map[string][]string `json:"networkAttachments,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 openstack-operator in the top-level CR (e.g. the ContainerImage) + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1beta1/ironicinspector_types.go b/api/v1beta1/ironicinspector_types.go index fb970263..9c81f82b 100644 --- a/api/v1beta1/ironicinspector_types.go +++ b/api/v1beta1/ironicinspector_types.go @@ -190,6 +190,12 @@ type IronicInspectorStatus struct { // NetworkAttachments status of the deployment pods NetworkAttachments map[string][]string `json:"networkAttachments,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 openstack-operator in the top-level CR (e.g. the ContainerImage) + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/v1beta1/ironicneutronagent_types.go b/api/v1beta1/ironicneutronagent_types.go index 49447bc7..534a0eb1 100644 --- a/api/v1beta1/ironicneutronagent_types.go +++ b/api/v1beta1/ironicneutronagent_types.go @@ -76,6 +76,12 @@ type IronicNeutronAgentStatus 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 openstack-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/ironic.openstack.org_ironicapis.yaml b/config/crd/bases/ironic.openstack.org_ironicapis.yaml index db40c32d..f101971e 100644 --- a/config/crd/bases/ironic.openstack.org_ironicapis.yaml +++ b/config/crd/bases/ironic.openstack.org_ironicapis.yaml @@ -464,6 +464,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic API instances format: int32 diff --git a/config/crd/bases/ironic.openstack.org_ironicconductors.yaml b/config/crd/bases/ironic.openstack.org_ironicconductors.yaml index 2d70c025..c7d14c72 100644 --- a/config/crd/bases/ironic.openstack.org_ironicconductors.yaml +++ b/config/crd/bases/ironic.openstack.org_ironicconductors.yaml @@ -319,6 +319,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic Conductor instances format: int32 diff --git a/config/crd/bases/ironic.openstack.org_ironicinspectors.yaml b/config/crd/bases/ironic.openstack.org_ironicinspectors.yaml index 5fec5cfe..7cf528dd 100644 --- a/config/crd/bases/ironic.openstack.org_ironicinspectors.yaml +++ b/config/crd/bases/ironic.openstack.org_ironicinspectors.yaml @@ -517,6 +517,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of Ironic Inspector instances format: int32 diff --git a/config/crd/bases/ironic.openstack.org_ironicneutronagents.yaml b/config/crd/bases/ironic.openstack.org_ironicneutronagents.yaml index 87c2af79..30d2a5f1 100644 --- a/config/crd/bases/ironic.openstack.org_ironicneutronagents.yaml +++ b/config/crd/bases/ironic.openstack.org_ironicneutronagents.yaml @@ -220,6 +220,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 openstack-operator in the top-level CR (e.g. the + ContainerImage) + format: int64 + type: integer readyCount: description: ReadyCount of ironic Conductor instances format: int32 diff --git a/config/crd/bases/ironic.openstack.org_ironics.yaml b/config/crd/bases/ironic.openstack.org_ironics.yaml index e19ce8ea..79e7b220 100644 --- a/config/crd/bases/ironic.openstack.org_ironics.yaml +++ b/config/crd/bases/ironic.openstack.org_ironics.yaml @@ -1149,7 +1149,7 @@ spec: 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 + injected by the openstack-operator in the top-level CR (e.g. the ContainerImage) format: int64 type: integer diff --git a/controllers/ironic_controller.go b/controllers/ironic_controller.go index 79eebad5..9febbac3 100644 --- a/controllers/ironic_controller.go +++ b/controllers/ironic_controller.go @@ -184,6 +184,7 @@ func (r *IronicReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ) instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation // If we're not deleting this and the service object doesn't have our finalizer, add it. if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { @@ -457,19 +458,40 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic err.Error())) return ctrl.Result{}, err } - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicConductor.Name, string(op))) - } - // Mirror IronicConductor status' ReadyCount to this parent CR - condGrp := conductorSpec.ConductorGroup - if conductorSpec.ConductorGroup == "" { - condGrp = ironicv1.ConductorGroupNull + + // Check the observed Generation and mirror the condition from the + // underlying resource reconciliation + cndObsGen, err := r.checkIronicConductorGeneration(instance) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + ironicv1.IronicConductorReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + ironicv1.IronicConductorReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err } - instance.Status.IronicConductorReadyCount[condGrp] = ironicConductor.Status.ReadyCount - // Mirror IronicConductor's condition status - c := ironicConductor.Status.Conditions.Mirror(ironicv1.IronicConductorReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + if !cndObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + ironicv1.IronicConductorReadyCondition, + condition.InitReason, + ironicv1.IronicConductorReadyInitMessage, + )) + } else { + // Mirror IronicConductor status' ReadyCount to this parent CR + condGrp := conductorSpec.ConductorGroup + if conductorSpec.ConductorGroup == "" { + condGrp = ironicv1.ConductorGroupNull + } + instance.Status.IronicConductorReadyCount[condGrp] = ironicConductor.Status.ReadyCount + // Mirror IronicConductor's condition status + c := ironicConductor.Status.Conditions.Mirror(ironicv1.IronicConductorReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicConductor.Name, string(op))) + } } } @@ -484,20 +506,42 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic err.Error())) return ctrl.Result{}, err } - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicAPI.Name, string(op))) - } - // Mirror IronicAPI status' APIEndpoints and ReadyCount to this parent CR - for k, v := range ironicAPI.Status.APIEndpoints { - instance.Status.APIEndpoints[k] = v + // Check the observed Generation and mirror the condition from the + // underlying resource reconciliation + apiObsGen, err := r.checkIronicAPIGeneration(instance) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + ironicv1.IronicAPIReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + ironicv1.IronicAPIReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err } - instance.Status.IronicAPIReadyCount = ironicAPI.Status.ReadyCount + // Only mirror the underlying condition if the observedGeneration is + // the last seen + if !apiObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + ironicv1.IronicAPIReadyCondition, + condition.InitReason, + ironicv1.IronicAPIReadyInitMessage, + )) + } else { + // Mirror IronicAPI status' APIEndpoints and ReadyCount to this parent CR + for k, v := range ironicAPI.Status.APIEndpoints { + instance.Status.APIEndpoints[k] = v + } + instance.Status.IronicAPIReadyCount = ironicAPI.Status.ReadyCount - // Mirror IronicAPI's condition status - c := ironicAPI.Status.Conditions.Mirror(ironicv1.IronicAPIReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + // Mirror IronicAPI's condition status + c := ironicAPI.Status.Conditions.Mirror(ironicv1.IronicAPIReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicAPI.Name, string(op))) + } } // deploy ironic-inspector @@ -513,20 +557,43 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic err.Error())) return ctrl.Result{}, err } - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicInspector.Name, string(op))) - } - // Mirror IronicInspector status APIEndpoints and ReadyCount to this parent CR - for k, v := range ironicInspector.Status.APIEndpoints { - instance.Status.APIEndpoints[k] = v + // Check the observed Generation and mirror the condition from the + // underlying resource reconciliation + nspObsGen, err := r.checkIronicInspectorGeneration(instance) + if err != nil { + instance.Status.Conditions.Set( + condition.FalseCondition( + ironicv1.IronicInspectorReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + ironicv1.IronicInspectorReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err } - instance.Status.InspectorReadyCount = ironicInspector.Status.ReadyCount + // Only mirror the underlying condition if the observedGeneration is + // the last seen + if !nspObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + ironicv1.IronicInspectorReadyCondition, + condition.InitReason, + ironicv1.IronicInspectorReadyInitMessage, + )) + } else { + // Mirror IronicInspector status APIEndpoints and ReadyCount to this parent CR + for k, v := range ironicInspector.Status.APIEndpoints { + instance.Status.APIEndpoints[k] = v + } + instance.Status.InspectorReadyCount = ironicInspector.Status.ReadyCount - // Mirror IronicInspector's condition status - c = ironicInspector.Status.Conditions.Mirror(ironicv1.IronicInspectorReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + // Mirror IronicInspector's condition status + c := ironicInspector.Status.Conditions.Mirror(ironicv1.IronicInspectorReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicInspector.Name, string(op))) + } } } else { err := r.inspectorDeploymentDelete(ctx, instance) @@ -557,15 +624,39 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic err.Error())) return ctrl.Result{}, err } - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicNeutronAgent.Name, string(op))) + + // Check the observed Generation and mirror the condition from the + // underlying resource reconciliation + agObsGen, err := r.checkNeutronAgentGeneration(instance) + if err != nil { + instance.Status.Conditions.Set( + condition.FalseCondition( + ironicv1.IronicNeutronAgentReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + ironicv1.IronicNeutronAgentReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err } - // Mirror IronicNeutronAgent status ReadyCount to this parent CR - instance.Status.IronicNeutronAgentReadyCount = ironicNeutronAgent.Status.ReadyCount - // Mirror IronicNeutronAgent's condition status - c = ironicNeutronAgent.Status.Conditions.Mirror(ironicv1.IronicNeutronAgentReadyCondition) - if c != nil { - instance.Status.Conditions.Set(c) + // Only mirror the underlying condition if the observedGeneration is + // the last seen + if !agObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + ironicv1.IronicNeutronAgentReadyCondition, + condition.InitReason, + ironicv1.IronicNeutronAgentReadyInitMessage, + )) + } else { + // Mirror IronicNeutronAgent status ReadyCount to this parent CR + instance.Status.IronicNeutronAgentReadyCount = ironicNeutronAgent.Status.ReadyCount + // Mirror IronicNeutronAgent's condition status + c := ironicNeutronAgent.Status.Conditions.Mirror(ironicv1.IronicNeutronAgentReadyCondition) + if c != nil { + instance.Status.Conditions.Set(c) + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", ironicNeutronAgent.Name, string(op))) + } } } else { err := r.ironicNeutronAgentDeploymentDelete(ctx, instance) @@ -590,7 +681,6 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic // We reached the end of the Reconcile, update the Ready condition based on // the sub conditions - instance.Status.ObservedGeneration = instance.Generation if instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) @@ -1100,3 +1190,87 @@ func (r *IronicReconciler) ensureDB( instance.Status.Conditions.MarkTrue(condition.DBReadyCondition, condition.DBReadyMessage) return db, ctrlResult, nil } + +// checkIronicAPIGeneration - +func (r *IronicReconciler) checkIronicAPIGeneration( + instance *ironicv1.Ironic, +) (bool, error) { + Log := r.GetLogger(context.Background()) + api := &ironicv1.IronicAPIList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), api, listOpts...); err != nil { + Log.Error(err, "Unable to retrieve IronicAPI CR %w") + return false, err + } + for _, item := range api.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} + +// checkIronicConductorGeneration - +func (r *IronicReconciler) checkIronicConductorGeneration( + instance *ironicv1.Ironic, +) (bool, error) { + Log := r.GetLogger(context.Background()) + cnd := &ironicv1.IronicConductorList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), cnd, listOpts...); err != nil { + Log.Error(err, "Unable to retrieve IronicConductor CR %w") + return false, err + } + for _, item := range cnd.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} + +// checkIronicInspectorGeneration - +func (r *IronicReconciler) checkIronicInspectorGeneration( + instance *ironicv1.Ironic, +) (bool, error) { + Log := r.GetLogger(context.Background()) + nsp := &ironicv1.IronicInspectorList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), nsp, listOpts...); err != nil { + Log.Error(err, "Unable to retrieve IronicInspector CR %w") + return false, err + } + for _, item := range nsp.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} + +// checkNeutronAgentGeneration - +func (r *IronicReconciler) checkNeutronAgentGeneration( + instance *ironicv1.Ironic, +) (bool, error) { + Log := r.GetLogger(context.Background()) + ag := &ironicv1.IronicNeutronAgentList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), ag, listOpts...); err != nil { + Log.Error(err, "Unable to retrieve IronicNeutronAgent CR %w") + return false, err + } + for _, item := range ag.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} diff --git a/controllers/ironicapi_controller.go b/controllers/ironicapi_controller.go index 6f4f6943..8a37dac3 100644 --- a/controllers/ironicapi_controller.go +++ b/controllers/ironicapi_controller.go @@ -182,6 +182,7 @@ func (r *IronicAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation // If we're not deleting this and the service object doesn't have our finalizer, add it. if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { @@ -860,31 +861,34 @@ func (r *IronicAPIReconciler) reconcileNormal(ctx context.Context, instance *iro condition.DeploymentReadyRunningMessage)) return ctrlResult, nil } - 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 - } + // Only check readiness if controller sees the last version of the CR + if depl.GetDeployment().Generation == depl.GetDeployment().Status.ObservedGeneration { + instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas - 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())) + // 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 + } - 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())) + 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 diff --git a/controllers/ironicconductor_controller.go b/controllers/ironicconductor_controller.go index c97994af..7bd8923d 100644 --- a/controllers/ironicconductor_controller.go +++ b/controllers/ironicconductor_controller.go @@ -169,6 +169,7 @@ func (r *IronicConductorReconciler) Reconcile(ctx context.Context, req ctrl.Requ ) instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation // If we're not deleting this and the service object doesn't have our finalizer, add it. if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { @@ -699,31 +700,33 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc return ctrlResult, nil } - instance.Status.ReadyCount = ss.GetStatefulSet().Status.ReadyReplicas + // Only check readiness if controller sees the last version of the CR + if ss.GetStatefulSet().Generation == ss.GetStatefulSet().Status.ObservedGeneration { + instance.Status.ReadyCount = ss.GetStatefulSet().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 - } - - 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())) + // 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 + } - 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())) + 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) + } } // We reached the end of the Reconcile, update the Ready condition based on diff --git a/controllers/ironicinspector_controller.go b/controllers/ironicinspector_controller.go index 3cd5b70b..4d7a3645 100644 --- a/controllers/ironicinspector_controller.go +++ b/controllers/ironicinspector_controller.go @@ -235,6 +235,7 @@ func (r *IronicInspectorReconciler) Reconcile( ) } instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation // If we're not deleting this and the service object doesn't have our finalizer, add it. if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { @@ -651,35 +652,37 @@ func (r *IronicInspectorReconciler) reconcileStatefulSet( return ctrlResult, nil } - instance.Status.ReadyCount = ss.GetStatefulSet().Status.ReadyReplicas + // Only check readiness if controller sees the last version of the CR + if ss.GetStatefulSet().Generation == ss.GetStatefulSet().Status.ObservedGeneration { + instance.Status.ReadyCount = ss.GetStatefulSet().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 - } - - 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())) + // 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 + } - 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())) - if instance.Status.ReadyCount > 0 { - instance.Status.Conditions.MarkTrue( - condition.DeploymentReadyCondition, - condition.DeploymentReadyMessage) - } else { - return ctrl.Result{RequeueAfter: time.Second * 10}, nil + return ctrl.Result{}, err + } + if instance.Status.ReadyCount == *instance.Spec.Replicas { + instance.Status.Conditions.MarkTrue( + condition.DeploymentReadyCondition, + condition.DeploymentReadyMessage) + } else { + return ctrl.Result{RequeueAfter: time.Second * 10}, nil + } } // create Statefulset - end diff --git a/controllers/ironicneutronagent_controller.go b/controllers/ironicneutronagent_controller.go index a34a5eca..59632ed4 100644 --- a/controllers/ironicneutronagent_controller.go +++ b/controllers/ironicneutronagent_controller.go @@ -190,6 +190,7 @@ func (r *IronicNeutronAgentReconciler) Reconcile( ) instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation // If we're not deleting this and the service object doesn't have our finalizer, add it. if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { @@ -480,10 +481,14 @@ func (r *IronicNeutronAgentReconciler) reconcileDeployment( condition.DeploymentReadyRunningMessage)) return ctrlResult, nil } - instance.Status.ReadyCount = deployment.GetDeployment().Status.ReadyReplicas - if instance.Status.ReadyCount > 0 { - instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + // Only check ReadyCount if controller sees the last version of the CR + if deployment.GetDeployment().Generation == deployment.GetDeployment().Status.ObservedGeneration { + instance.Status.ReadyCount = deployment.GetDeployment().Status.ReadyReplicas + + if instance.Status.ReadyCount == *instance.Spec.Replicas { + instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + } } return ctrl.Result{}, nil diff --git a/tests/functional/ironicapi_controller_test.go b/tests/functional/ironicapi_controller_test.go index ec030187..fd2ab535 100644 --- a/tests/functional/ironicapi_controller_test.go +++ b/tests/functional/ironicapi_controller_test.go @@ -167,6 +167,7 @@ var _ = Describe("IronicAPI controller", func() { ContainSubstring("[client]\nssl=0")) }) It("Sets NetworkAttachmentsReady", func() { + th.SimulateDeploymentReplicaReady(ironicNames.IronicName) th.ExpectCondition( ironicNames.APIName, ConditionGetterFunc(IronicAPIConditionGetter), diff --git a/tests/functional/ironicconductor_controller_test.go b/tests/functional/ironicconductor_controller_test.go index 24321c41..0853593b 100644 --- a/tests/functional/ironicconductor_controller_test.go +++ b/tests/functional/ironicconductor_controller_test.go @@ -152,6 +152,7 @@ var _ = Describe("IronicConductor controller", func() { ContainSubstring("[client]\nssl=0")) }) It("Sets NetworkAttachmentsReady", func() { + th.SimulateStatefulSetReplicaReady(ironicNames.ConductorName) th.ExpectCondition( ironicNames.ConductorName, ConditionGetterFunc(IronicConductorConditionGetter),