From 1ea21ad5e33ee0dbf7a8cc70ba212a737684c7b3 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 26 Nov 2024 09:39:11 +0100 Subject: [PATCH] update: DSC status and remove component CR's case - delete component CR when it is either set to Removed or not set in DSC - remove old status updates, only update when component CR create successful or failed Signed-off-by: Wen Zhou --- README.md | 2 +- .../datasciencecluster_controller.go | 38 ++++++++----------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 801328202f3..d9bb63b6f8b 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,7 @@ Example commands to run test suite for the dashboard `component` only, with the make run-nowebhook ``` ```shell -make e2e-test -e OPERATOR_NAMESPACE= -e E2E_TEST_FLAGS="--test-operator-controller=false --test-webhook=false --test-component=dashboard,trustyai" +make e2e-test -e OPERATOR_NAMESPACE= -e E2E_TEST_FLAGS="--test-operator-controller=false --test-webhook=false --test-component=dashboard" ``` diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 749b2bcd853..5bdaf8cfadd 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -277,37 +277,20 @@ func (r *DataScienceClusterReconciler) ReconcileComponent( r.Log.Info("Starting reconciliation of component: " + componentName) - enabled := component.GetManagementState(instance) == operatorv1.Managed componentCR := component.NewCRObject(instance) err := r.apply(ctx, instance, componentCR) if err != nil { - return instance, err - } - - _, isExistStatus := instance.Status.InstalledComponents[componentName] - if !isExistStatus { - message := "Component is disabled" - if enabled { - message = "Component is enabled" - } - var err error - instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { - status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileInit, message, corev1.ConditionUnknown) - }) - if err != nil { - return instance, fmt.Errorf("failed to update DataScienceCluster conditions before first time reconciling %s: %w", componentName, err) - } - } - - if err != nil { - r.Log.Error(err, "Failed to reconcile component: "+componentName) - instance = r.reportError(err, instance, fmt.Sprintf("failed to reconcile %s on DataScienceCluster", componentName)) + r.Log.Error(err, "Failed to reconciled component CR: "+componentName) + instance = r.reportError(err, instance, fmt.Sprintf("failed to reconciled %s by DSC", componentName)) instance, _ = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileFailed, fmt.Sprintf("Component reconciliation failed: %v", err), corev1.ConditionFalse) }) return instance, err } + enabled := component.GetManagementState(instance) == operatorv1.Managed + + // TODO: check component status before update DSC status to successful .GetStatus().Phase == "Ready" r.Log.Info("component reconciled successfully: " + componentName) instance, err = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { if saved.Status.InstalledComponents == nil { @@ -324,6 +307,15 @@ func (r *DataScienceClusterReconciler) ReconcileComponent( return instance, fmt.Errorf("failed to update DataScienceCluster status after reconciling %s: %w", componentName, err) } + // TODO: report failed component status .GetStatus().Phase == "NotReady" not only creation + if err != nil { + r.Log.Error(err, "Failed to reconcile component: "+componentName) + instance = r.reportError(err, instance, fmt.Sprintf("failed to reconcile %s", componentName)) + instance, _ = status.UpdateWithRetry(ctx, r.Client, instance, func(saved *dscv1.DataScienceCluster) { + status.SetComponentCondition(&saved.Status.Conditions, componentName, status.ReconcileFailed, fmt.Sprintf("Component reconciliation failed: %v", err), corev1.ConditionFalse) + }) + return instance, err + } return instance, nil } @@ -360,7 +352,7 @@ func (r *DataScienceClusterReconciler) apply(ctx context.Context, dsc *dscv1.Dat } managementStateAnn, exists := obj.GetAnnotations()[annotations.ManagementStateAnnotation] - if exists && managementStateAnn == string(operatorv1.Removed) { + if exists && (managementStateAnn == string(operatorv1.Removed) || managementStateAnn == "Unknown") { err := r.Client.Delete(ctx, obj) if k8serr.IsNotFound(err) { return nil