From 8418dcbff922c6a75406bb44349cc80b723364ef Mon Sep 17 00:00:00 2001 From: rabi Date: Mon, 10 Jun 2024 10:21:41 +0530 Subject: [PATCH] Cleanup/fix how we manage DeploymentReadyCondition - Don't use deployemnt.Deployed as there could be more than one nodeset in a deployment. - Check NodeSetDeploymentReadyCondition for the nodeset rather than Ready condition for the deployment. - Break when a failed deployment for a nodeset is encountered. Continuing can overwrite the nodeset deployment conditions. Signed-off-by: rabi --- .../openstackdataplanenodeset_controller.go | 99 +++++++++---------- .../00-assert.yaml | 4 +- 2 files changed, 50 insertions(+), 53 deletions(-) diff --git a/controllers/openstackdataplanenodeset_controller.go b/controllers/openstackdataplanenodeset_controller.go index 8a75f2bbd..85ad9855d 100644 --- a/controllers/openstackdataplanenodeset_controller.go +++ b/controllers/openstackdataplanenodeset_controller.go @@ -396,41 +396,32 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req Log.Error(err, "Unable to get deployed OpenStackDataPlaneDeployments.") return ctrl.Result{}, err } - deployErrMsg := "" - if isDeploymentFailed { - deployErrMsg = err.Error() - } - - if isDeploymentRunning { - Log.Info("Deployment still running...", "instance", instance) - return ctrl.Result{}, nil + if !isDeploymentRunning && !isDeploymentFailed { + // Generate NodeSet Inventory + _, err = deployment.GenerateNodeSetInventory(ctx, helper, instance, + allIPSets, dnsDetails.ServerAddresses, containerImages) + if err != nil { + errorMsg := fmt.Sprintf("Unable to generate inventory for %s", instance.Name) + util.LogErrorForObject(helper, err, errorMsg, instance) + instance.Status.Conditions.MarkFalse( + dataplanev1.SetupReadyCondition, + condition.ErrorReason, + condition.SeverityError, + dataplanev1.DataPlaneNodeSetErrorMessage, + errorMsg) + return ctrl.Result{}, err + } } - - // Generate NodeSet Inventory - _, err = deployment.GenerateNodeSetInventory(ctx, helper, instance, - allIPSets, dnsDetails.ServerAddresses, containerImages) - if err != nil { - errorMsg := fmt.Sprintf("Unable to generate inventory for %s", instance.Name) - util.LogErrorForObject(helper, err, errorMsg, instance) - instance.Status.Conditions.MarkFalse( - dataplanev1.SetupReadyCondition, - condition.ErrorReason, - condition.SeverityError, - dataplanev1.DataPlaneNodeSetErrorMessage, - errorMsg) - return ctrl.Result{}, err - } - // all setup tasks complete, mark SetupReadyCondition True instance.Status.Conditions.MarkTrue(dataplanev1.SetupReadyCondition, condition.ReadyMessage) // Set DeploymentReadyCondition to False if it was unknown. // Handles the case where the NodeSet is created, but not yet deployed. if instance.Status.Conditions.IsUnknown(condition.DeploymentReadyCondition) { - Log.Info("Set DeploymentReadyCondition false") + Log.Info("Set NodeSet DeploymentReadyCondition false") instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition, - condition.NotRequestedReason, condition.SeverityInfo, + condition.RequestedReason, condition.SeverityInfo, condition.DeploymentReadyInitMessage) } @@ -439,23 +430,23 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) } else if isDeploymentRunning { + Log.Info("Deployment still running...", "instance", instance) Log.Info("Set NodeSet DeploymentReadyCondition false") instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition, condition.RequestedReason, condition.SeverityInfo, condition.DeploymentReadyRunningMessage) } else if isDeploymentFailed { Log.Info("Set NodeSet DeploymentReadyCondition false") + deployErrorMsg := "" + if err != nil { + deployErrorMsg = err.Error() + } instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition, condition.ErrorReason, condition.SeverityError, condition.DeploymentReadyErrorMessage, - deployErrMsg) - err = fmt.Errorf(deployErrMsg) - } else { - Log.Info("Set NodeSet DeploymentReadyCondition false") - instance.Status.Conditions.MarkFalse(condition.DeploymentReadyCondition, - condition.RequestedReason, condition.SeverityInfo, - condition.DeploymentReadyInitMessage) + deployErrorMsg) } + return ctrl.Result{}, err } @@ -473,9 +464,9 @@ func checkDeployment(helper *helper.Helper, return false, false, false, err } - isDeploymentReady := false - isDeploymentRunning := false - isDeploymentFailed := false + var isDeploymentReady bool + var isDeploymentRunning bool + var isDeploymentFailed bool // Sort deployments from oldest to newest by the LastTransitionTime of // their DeploymentReadyCondition @@ -496,9 +487,28 @@ func checkDeployment(helper *helper.Helper, } if slices.Contains( deployment.Spec.NodeSets, instance.Name) { + + // Reset the vars for every deployment isDeploymentReady = false - if deployment.Status.Deployed { + isDeploymentRunning = false + deploymentConditions := deployment.Status.NodeSetConditions[instance.Name] + if instance.Status.DeploymentStatuses == nil { + instance.Status.DeploymentStatuses = make(map[string]condition.Conditions) + } + instance.Status.DeploymentStatuses[deployment.Name] = deploymentConditions + deploymentCondition := deploymentConditions.Get(dataplanev1.NodeSetDeploymentReadyCondition) + if condition.IsError(deploymentCondition) { + msg := strings.Replace(deploymentCondition.Message, strings.Split(condition.DeploymentReadyErrorMessage, "%")[0], "", -1) + err = fmt.Errorf(msg) + isDeploymentFailed = true + break + } else if deploymentConditions.IsFalse(dataplanev1.NodeSetDeploymentReadyCondition) { + isDeploymentRunning = true + } else if deploymentConditions.IsTrue(dataplanev1.NodeSetDeploymentReadyCondition) { isDeploymentReady = true + } + + if isDeploymentReady { for k, v := range deployment.Status.ConfigMapHashes { instance.Status.ConfigMapHashes[k] = v } @@ -511,20 +521,7 @@ func checkDeployment(helper *helper.Helper, instance.Status.DeployedConfigHash = deployment.Status.NodeSetHashes[instance.Name] instance.Status.DeployedVersion = deployment.Status.DeployedVersion } - deploymentConditions := deployment.Status.NodeSetConditions[instance.Name] - if instance.Status.DeploymentStatuses == nil { - instance.Status.DeploymentStatuses = make(map[string]condition.Conditions) - } - instance.Status.DeploymentStatuses[deployment.Name] = deploymentConditions - deploymentCondition := deploymentConditions.Get(dataplanev1.NodeSetDeploymentReadyCondition) - if condition.IsError(deploymentCondition) { - msg := strings.Replace(deploymentCondition.Message, strings.Split(condition.DeploymentReadyErrorMessage, "%")[0], "", -1) - err = fmt.Errorf(msg) - isDeploymentFailed = true - } else if deployment.Status.Conditions.IsFalse(condition.ReadyCondition) { - isDeploymentRunning = true - isDeploymentReady = false - } + } } diff --git a/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml b/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml index e06165732..efce9b8b1 100644 --- a/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml +++ b/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml @@ -16,12 +16,12 @@ spec: status: observedGeneration: 1 conditions: - - message: Deployment not started + - message: Deployment in progress reason: Requested severity: Info status: "False" type: Ready - - message: Deployment not started + - message: Deployment in progress reason: Requested severity: Info status: "False"