diff --git a/api/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml b/api/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml index 403b8ecd..b210f58d 100644 --- a/api/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml +++ b/api/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml @@ -304,6 +304,14 @@ spec: type: array description: NetworkAttachment 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 Octavia Amphora Controllers format: int32 diff --git a/api/bases/octavia.openstack.org_octaviaapis.yaml b/api/bases/octavia.openstack.org_octaviaapis.yaml index 2743a14b..8fee8a5a 100644 --- a/api/bases/octavia.openstack.org_octaviaapis.yaml +++ b/api/bases/octavia.openstack.org_octaviaapis.yaml @@ -459,6 +459,14 @@ spec: type: array description: NetworkAttachment 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 octavia API instances format: int32 diff --git a/api/v1beta1/amphoracontroller_types.go b/api/v1beta1/amphoracontroller_types.go index 367948b5..af8c8b8a 100644 --- a/api/v1beta1/amphoracontroller_types.go +++ b/api/v1beta1/amphoracontroller_types.go @@ -161,6 +161,12 @@ type OctaviaAmphoraControllerStatus struct { // NetworkAttachment 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 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/octaviaapi_types.go b/api/v1beta1/octaviaapi_types.go index c0ef3879..b5a5520e 100644 --- a/api/v1beta1/octaviaapi_types.go +++ b/api/v1beta1/octaviaapi_types.go @@ -163,6 +163,12 @@ type OctaviaAPIStatus struct { // NetworkAttachment 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 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/octavia.openstack.org_octaviaamphoracontrollers.yaml b/config/crd/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml index 403b8ecd..b210f58d 100644 --- a/config/crd/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml +++ b/config/crd/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml @@ -304,6 +304,14 @@ spec: type: array description: NetworkAttachment 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 Octavia Amphora Controllers format: int32 diff --git a/config/crd/bases/octavia.openstack.org_octaviaapis.yaml b/config/crd/bases/octavia.openstack.org_octaviaapis.yaml index 2743a14b..8fee8a5a 100644 --- a/config/crd/bases/octavia.openstack.org_octaviaapis.yaml +++ b/config/crd/bases/octavia.openstack.org_octaviaapis.yaml @@ -459,6 +459,14 @@ spec: type: array description: NetworkAttachment 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 octavia API instances format: int32 diff --git a/controllers/amphoracontroller_controller.go b/controllers/amphoracontroller_controller.go index 2fe402fc..f1f2d10f 100644 --- a/controllers/amphoracontroller_controller.go +++ b/controllers/amphoracontroller_controller.go @@ -162,6 +162,7 @@ func (r *OctaviaAmphoraControllerReconciler) Reconcile(ctx context.Context, req ) instance.Status.Conditions.Init(&cl) + instance.Status.ObservedGeneration = instance.Generation if isNewInstance { if err := r.Status().Update(ctx, instance); err != nil { @@ -463,40 +464,42 @@ func (r *OctaviaAmphoraControllerReconciler) reconcileNormal(ctx context.Context return ctrlResult, nil } - // 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 - } + if dset.GetDaemonSet().Generation == dset.GetDaemonSet().Status.ObservedGeneration { + instance.Status.DesiredNumberScheduled = dset.GetDaemonSet().Status.DesiredNumberScheduled + // TODO(gthiemonge) change for NumberReady? + instance.Status.ReadyCount = dset.GetDaemonSet().Status.NumberReady - 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{RequeueAfter: time.Duration(1) * time.Second}, nil - } + 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.DesiredNumberScheduled = dset.GetDaemonSet().Status.DesiredNumberScheduled - // TODO(gthiemonge) change for NumberReady? - instance.Status.ReadyCount = dset.GetDaemonSet().Status.NumberReady - if instance.Status.ReadyCount == instance.Status.DesiredNumberScheduled { - instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) - } + return ctrl.Result{RequeueAfter: time.Duration(1) * time.Second}, nil + } + if instance.Status.ReadyCount == instance.Status.DesiredNumberScheduled { + instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + } + } // create DaemonSet - end // We reached the end of the Reconcile, update the Ready condition based on diff --git a/controllers/octavia_controller.go b/controllers/octavia_controller.go index d2b44bf1..83ade46a 100644 --- a/controllers/octavia_controller.go +++ b/controllers/octavia_controller.go @@ -196,6 +196,7 @@ func (r *OctaviaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re ) 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 { @@ -609,19 +610,34 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav err.Error())) return ctrl.Result{}, err } - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op))) + // Check the underlying OctaviaAPI condition according to the + // ObservedGeneration + apiObsGen, err := r.checkOctaviaAPIGeneration(instance) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + octaviav1.OctaviaAPIReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + octaviav1.OctaviaAPIReadyErrorMessage, + err.Error())) + return ctrlResult, nil } - - // Mirror OctaviaAPI status' ReadyCount to this parent CR - // TODO(beagles): We need to have a way to aggregate conditions from the other services into this - // - instance.Status.OctaviaAPIReadyCount = octaviaAPI.Status.ReadyCount - conditionStatus := octaviaAPI.Status.Conditions.Mirror(octaviav1.OctaviaAPIReadyCondition) - if conditionStatus != nil { - instance.Status.Conditions.Set(conditionStatus) + if !apiObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + octaviav1.OctaviaAPIReadyCondition, + condition.InitReason, + octaviav1.OctaviaAPIReadyInitMessage, + )) } else { - instance.Status.Conditions.MarkTrue(octaviav1.OctaviaAPIReadyCondition, condition.DeploymentReadyMessage) + // Mirror OctaviaAPI status' ReadyCount to this parent CR + instance.Status.OctaviaAPIReadyCount = octaviaAPI.Status.ReadyCount + conditionStatus := octaviaAPI.Status.Conditions.Mirror(octaviav1.OctaviaAPIReadyCondition) + if conditionStatus != nil { + instance.Status.Conditions.Set(conditionStatus) + } + } + if op != controllerutil.OperationResultNone && apiObsGen { + Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op))) } // ------------------------------------------------------------------------------------------------------------ @@ -654,17 +670,34 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav err.Error())) return ctrl.Result{}, err } - - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment of OctaviaHealthManager for %s successfully reconciled - operation: %s", instance.Name, string(op))) + // Even if we trigger three deployments, the Amphora subCR is only one, no + // need to call this functions three times in the same reconciliation loop + ampObsGen, err := r.checkAmphoraGeneration(instance) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + amphoraControllerReadyCondition(octaviav1.HealthManager), + condition.ErrorReason, + condition.SeverityWarning, + amphoraControllerErrorMessage(octaviav1.HealthManager), + err.Error())) + return ctrlResult, nil } - - instance.Status.OctaviaHealthManagerReadyCount = octaviaHealthManager.Status.ReadyCount - conditionStatus = octaviaHealthManager.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.HealthManager)) - if conditionStatus != nil { - instance.Status.Conditions.Set(conditionStatus) + if !ampObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + amphoraControllerReadyCondition(octaviav1.HealthManager), + condition.InitReason, + amphoraControllerErrorMessage(octaviav1.HealthManager), + )) } else { - instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.HealthManager), condition.DeploymentReadyMessage) + instance.Status.OctaviaHealthManagerReadyCount = octaviaHealthManager.Status.ReadyCount + conditionStatus := octaviaHealthManager.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.HealthManager)) + if conditionStatus != nil { + instance.Status.Conditions.Set(conditionStatus) + } + } + + if op != controllerutil.OperationResultNone && ampObsGen { + Log.Info(fmt.Sprintf("Deployment of OctaviaHealthManager for %s successfully reconciled - operation: %s", instance.Name, string(op))) } // @@ -687,17 +720,22 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav err.Error())) return ctrl.Result{}, err } - - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment of OctaviaHousekeeping for %s successfully reconciled - operation: %s", instance.Name, string(op))) + if !ampObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + amphoraControllerReadyCondition(octaviav1.Housekeeping), + condition.InitReason, + amphoraControllerErrorMessage(octaviav1.Housekeeping), + )) + } else { + instance.Status.OctaviaHousekeepingReadyCount = octaviaHousekeeping.Status.ReadyCount + conditionStatus := octaviaHousekeeping.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Housekeeping)) + if conditionStatus != nil { + instance.Status.Conditions.Set(conditionStatus) + } } - instance.Status.OctaviaHousekeepingReadyCount = octaviaHousekeeping.Status.ReadyCount - conditionStatus = octaviaHousekeeping.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Housekeeping)) - if conditionStatus != nil { - instance.Status.Conditions.Set(conditionStatus) - } else { - instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.Housekeeping), condition.DeploymentReadyMessage) + if op != controllerutil.OperationResultNone && ampObsGen { + Log.Info(fmt.Sprintf("Deployment of OctaviaHousekeeping for %s successfully reconciled - operation: %s", instance.Name, string(op))) } octaviaWorker, op, err := r.amphoraControllerDaemonSetCreateOrUpdate(instance, networkInfo, @@ -711,17 +749,21 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav err.Error())) return ctrl.Result{}, err } - - if op != controllerutil.OperationResultNone { - Log.Info(fmt.Sprintf("Deployment of OctaviaWorker for %s successfully reconciled - operation: %s", instance.Name, string(op))) - } - - instance.Status.OctaviaWorkerReadyCount = octaviaWorker.Status.ReadyCount - conditionStatus = octaviaWorker.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Worker)) - if conditionStatus != nil { - instance.Status.Conditions.Set(conditionStatus) + if !ampObsGen { + instance.Status.Conditions.Set(condition.UnknownCondition( + amphoraControllerReadyCondition(octaviav1.Worker), + condition.InitReason, + amphoraControllerErrorMessage(octaviav1.Worker), + )) } else { - instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.Worker), condition.DeploymentReadyMessage) + instance.Status.OctaviaWorkerReadyCount = octaviaWorker.Status.ReadyCount + conditionStatus := octaviaWorker.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Worker)) + if conditionStatus != nil { + instance.Status.Conditions.Set(conditionStatus) + } + } + if op != controllerutil.OperationResultNone && ampObsGen { + Log.Info(fmt.Sprintf("Deployment of OctaviaWorker for %s successfully reconciled - operation: %s", instance.Name, string(op))) } // remove finalizers from unused MariaDBAccount records @@ -751,8 +793,6 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav // create Deployment - end - // Update the lastObserved generation before evaluating conditions - instance.Status.ObservedGeneration = instance.Generation // We reached the end of the Reconcile, update the Ready condition based on // the sub conditions if instance.Status.Conditions.AllSubConditionIsTrue() { @@ -1355,3 +1395,43 @@ func amphoraControllerErrorMessage(role string) string { } return condMap[role] } + +// checkOctaviaAPIGeneration - +func (r *OctaviaReconciler) checkOctaviaAPIGeneration( + instance *octaviav1.Octavia, +) (bool, error) { + api := &octaviav1.OctaviaAPIList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), api, listOpts...); err != nil { + r.Log.Error(err, "Unable to retrieve OctaviaAPI %w") + return false, err + } + for _, item := range api.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} + +// checkAmphoraGeneration - +func (r *OctaviaReconciler) checkAmphoraGeneration( + instance *octaviav1.Octavia, +) (bool, error) { + amph := &octaviav1.OctaviaAmphoraControllerList{} + listOpts := []client.ListOption{ + client.InNamespace(instance.Namespace), + } + if err := r.Client.List(context.Background(), amph, listOpts...); err != nil { + r.Log.Error(err, "Unable to retrieve OctaviaAPI %w") + return false, err + } + for _, item := range amph.Items { + if item.Generation != item.Status.ObservedGeneration { + return false, nil + } + } + return true, nil +} diff --git a/controllers/octaviaapi_controller.go b/controllers/octaviaapi_controller.go index 14b114a0..52ac1670 100644 --- a/controllers/octaviaapi_controller.go +++ b/controllers/octaviaapi_controller.go @@ -175,6 +175,7 @@ func (r *OctaviaAPIReconciler) 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 { @@ -778,41 +779,43 @@ func (r *OctaviaAPIReconciler) reconcileNormal(ctx context.Context, instance *oc return ctrlResult, nil } - // verify if network attachment matches expectations - networkReady := false - networkAttachmentStatus := map[string][]string{} - if *instance.Spec.Replicas > 0 { - networkReady, networkAttachmentStatus, err = nad.VerifyNetworkStatusFromAnnotation( - ctx, - helper, - instance.Spec.NetworkAttachments, - serviceLabels, - instance.Status.ReadyCount, - ) - if err != nil { - return ctrl.Result{}, err + if depl.GetDeployment().Generation == depl.GetDeployment().Status.ObservedGeneration { + instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas + // verify if network attachment matches expectations + networkReady := false + networkAttachmentStatus := map[string][]string{} + if *instance.Spec.Replicas > 0 { + networkReady, networkAttachmentStatus, err = nad.VerifyNetworkStatusFromAnnotation( + ctx, + helper, + instance.Spec.NetworkAttachments, + serviceLabels, + instance.Status.ReadyCount, + ) + if err != nil { + return ctrl.Result{}, err + } + } else { + networkReady = true } - } else { - networkReady = true - } - 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 - } - instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas - if instance.Status.ReadyCount > 0 { - instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + return ctrl.Result{}, err + } + if instance.Status.ReadyCount == *instance.Spec.Replicas { + instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + } } // create Deployment - end