From e776e0aedc3e530de7bcb197c2703c7da9784c31 Mon Sep 17 00:00:00 2001 From: biswassri Date: Mon, 9 Dec 2024 20:13:45 -0500 Subject: [PATCH 01/12] Migration uninstall configmap from DSC to setup controller --- .../setupcontroller/setup_controller.go | 77 +++++++++++++++++++ main.go | 8 ++ 2 files changed, 85 insertions(+) create mode 100644 controllers/setupcontroller/setup_controller.go diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go new file mode 100644 index 00000000000..518274c0e5d --- /dev/null +++ b/controllers/setupcontroller/setup_controller.go @@ -0,0 +1,77 @@ +package setupcontroller + +import ( + "context" + "fmt" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" + k8serr "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + finalizerName = "datasciencecluster.opendatahub.io/finalizer" +) + +type SetupControllerReconciler struct { + *odhClient.Client +} + +func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logf.FromContext(ctx).WithName("DataScienceCluster") + log.Info("Reconciling setup controller", "Request.Name", req.Name) + currentOperatorRelease := cluster.GetRelease() + platform := currentOperatorRelease.Name + instances := &dscv1.DataScienceClusterList{} + + // Check if the delete ConfigMap exists + if !upgrade.HasDeleteConfigMap(ctx, r.Client) { + log.Info("No delete ConfigMap found, skipping reconciliation") + return reconcile.Result{}, nil + } + + // Fetch the DataScienceCluster instance + if len(instances.Items) == 0 { + log.Info("No DataScienceCluster resources found") + if upgrade.HasDeleteConfigMap(ctx, r.Client) { + if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, platform); uninstallErr != nil { + return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr) + } + } + } + instance := &instances.Items[0] + if upgrade.HasDeleteConfigMap(ctx, r.Client) { + if controllerutil.ContainsFinalizer(instance, finalizerName) { + if controllerutil.RemoveFinalizer(instance, finalizerName) { + if err := r.Update(ctx, instance); err != nil { + log.Info("Error to remove DSC finalizer", "error", err) + return ctrl.Result{}, err + } + log.Info("Removed finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName) + + } + } + // Delete the DataScienceCluster instance + if err := r.Client.Delete(ctx, instance, []client.DeleteOption{}...); err != nil { + if !k8serr.IsNotFound(err) { + log.Error(err, "Failed to delete DataScienceCluster instance", "name", instance.Name) + return reconcile.Result{}, err + } + } + log.Info("DataScienceCluster instance deleted successfully, should be requeuing [?]") + return reconcile.Result{Requeue: true}, nil + } + return ctrl.Result{}, nil +} + +func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&dscv1.DataScienceCluster{}). + Complete(r) +} diff --git a/main.go b/main.go index c1ea5274e2c..fcbe3a0a1a0 100644 --- a/main.go +++ b/main.go @@ -69,6 +69,7 @@ import ( dscctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/datasciencecluster" dscictrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/dscinitialization" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/secretgenerator" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/setupcontroller" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/webhook" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" @@ -301,6 +302,13 @@ func main() { //nolint:funlen,maintidx os.Exit(1) } + if err = (&setupcontroller.SetupControllerReconciler{ + Client: oc, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "SetupController") + os.Exit(1) + } + if err = (&secretgenerator.SecretGeneratorReconciler{ Client: oc, Scheme: mgr.GetScheme(), From 71c1cb720a1fa0e7bcc104381aeac57e30e5738a Mon Sep 17 00:00:00 2001 From: biswassri Date: Fri, 13 Dec 2024 01:47:03 -0500 Subject: [PATCH 02/12] Updating after rebase --- .../datasciencecluster_controller.go | 35 ++-------- .../setupcontroller/setup_controller.go | 70 ++++++++----------- 2 files changed, 34 insertions(+), 71 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index da9351f97d1..c97a2d022e4 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -79,24 +79,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R instance := &dscv1.DataScienceCluster{} err := r.Client.Get(ctx, req.NamespacedName, instance) - - switch { - case k8serr.IsNotFound(err): - // Request object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. - // For additional cleanup logic use operatorUninstall function. - // Return and don't requeue - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); uninstallErr != nil { - return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr) - } - } - - return ctrl.Result{}, nil - case err != nil: - return ctrl.Result{}, err - } - + //Removed operator uninstall to the setup operator // We don't need finalizer anymore, remove it if present to handle the // upgrade case if controllerutil.RemoveFinalizer(instance, finalizerName) { @@ -106,17 +89,9 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R } // If DSC CR exist and deletion CM exist - // delete DSC CR and let reconcile requeue - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - err := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) - if err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - return ctrl.Result{}, nil - } - - if !instance.ObjectMeta.DeletionTimestamp.IsZero() { + // delete DSC CR and let reconcile requeue - Moved to setup controller + //No longer needed as setup is watches the HasDeleteConfigMap + /*if !instance.ObjectMeta.DeletionTimestamp.IsZero() { log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name) if upgrade.HasDeleteConfigMap(ctx, r.Client) { @@ -124,7 +99,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R } return ctrl.Result{}, nil - } + }*/ // validate pre-requisites if err := r.validate(ctx, instance); err != nil { diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index 518274c0e5d..1749397dca5 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -3,20 +3,18 @@ package setupcontroller import ( "context" "fmt" - dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" - odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" + + corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" -) -const ( - finalizerName = "datasciencecluster.opendatahub.io/finalizer" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" ) type SetupControllerReconciler struct { @@ -26,52 +24,42 @@ type SetupControllerReconciler struct { func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("DataScienceCluster") log.Info("Reconciling setup controller", "Request.Name", req.Name) - currentOperatorRelease := cluster.GetRelease() - platform := currentOperatorRelease.Name - instances := &dscv1.DataScienceClusterList{} - // Check if the delete ConfigMap exists - if !upgrade.HasDeleteConfigMap(ctx, r.Client) { - log.Info("No delete ConfigMap found, skipping reconciliation") - return reconcile.Result{}, nil - } + instance := &dscv1.DataScienceCluster{} + err := r.Client.Get(ctx, req.NamespacedName, instance) - // Fetch the DataScienceCluster instance - if len(instances.Items) == 0 { - log.Info("No DataScienceCluster resources found") + switch { + case k8serr.IsNotFound(err): + // Request object not found, could have been deleted after reconcile request. + // Owned objects are automatically garbage collected. + // For additional cleanup logic use operatorUninstall function. + // Return and don't requeue if upgrade.HasDeleteConfigMap(ctx, r.Client) { - if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, platform); uninstallErr != nil { + if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); uninstallErr != nil { return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr) } } + + return ctrl.Result{}, nil + case err != nil: + return ctrl.Result{}, err } - instance := &instances.Items[0] - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - if controllerutil.ContainsFinalizer(instance, finalizerName) { - if controllerutil.RemoveFinalizer(instance, finalizerName) { - if err := r.Update(ctx, instance); err != nil { - log.Info("Error to remove DSC finalizer", "error", err) - return ctrl.Result{}, err - } - log.Info("Removed finalizer for DataScienceCluster", "name", instance.Name, "finalizer", finalizerName) - } - } - // Delete the DataScienceCluster instance - if err := r.Client.Delete(ctx, instance, []client.DeleteOption{}...); err != nil { - if !k8serr.IsNotFound(err) { - log.Error(err, "Failed to delete DataScienceCluster instance", "name", instance.Name) - return reconcile.Result{}, err - } + //upgrade + if upgrade.HasDeleteConfigMap(ctx, r.Client) { + err := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) + if err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) } - log.Info("DataScienceCluster instance deleted successfully, should be requeuing [?]") - return reconcile.Result{Requeue: true}, nil + + return ctrl.Result{}, nil } + return ctrl.Result{}, nil } func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&dscv1.DataScienceCluster{}). + For(&corev1.ConfigMap{}). Complete(r) } From 9fb3f158377201093df6949b95157bfe73002edb Mon Sep 17 00:00:00 2001 From: biswassri Date: Fri, 13 Dec 2024 02:05:56 -0500 Subject: [PATCH 03/12] Fix lint + add comments --- .../datasciencecluster_controller.go | 15 ++++++--------- controllers/setupcontroller/setup_controller.go | 9 +++++---- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index c97a2d022e4..008a3dea748 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -28,7 +28,6 @@ import ( corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" @@ -78,20 +77,18 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) instance := &dscv1.DataScienceCluster{} - err := r.Client.Get(ctx, req.NamespacedName, instance) - //Removed operator uninstall to the setup operator + + // Removed operator uninstall to the setup operator // We don't need finalizer anymore, remove it if present to handle the // upgrade case + if controllerutil.RemoveFinalizer(instance, finalizerName) { if err := r.Client.Update(ctx, instance); err != nil { return ctrl.Result{}, err } } - // If DSC CR exist and deletion CM exist - // delete DSC CR and let reconcile requeue - Moved to setup controller - //No longer needed as setup is watches the HasDeleteConfigMap - /*if !instance.ObjectMeta.DeletionTimestamp.IsZero() { + if !instance.ObjectMeta.DeletionTimestamp.IsZero() { log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name) if upgrade.HasDeleteConfigMap(ctx, r.Client) { @@ -99,7 +96,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R } return ctrl.Result{}, nil - }*/ + } // validate pre-requisites if err := r.validate(ctx, instance); err != nil { @@ -118,7 +115,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R return strings.Compare(string(a.Type), string(b.Type)) }) - err = r.Client.ApplyStatus(ctx, instance, client.FieldOwner(fieldOwner), client.ForceOwnership) + err := r.Client.ApplyStatus(ctx, instance, client.FieldOwner(fieldOwner), client.ForceOwnership) switch { case err == nil: return ctrl.Result{}, nil diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index 1749397dca5..89afbf688a9 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -45,11 +45,12 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } - //upgrade + // If DSC CR exist and deletion CM exist + // delete DSC CR if upgrade.HasDeleteConfigMap(ctx, r.Client) { - err := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) - if err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + deleteErr := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) + if deleteErr != nil { + return ctrl.Result{}, fmt.Errorf("error while deleting DSC CR: %w", deleteErr) } return ctrl.Result{}, nil From 5c6d14e725ab0936103970afb0d6007df0085e56 Mon Sep 17 00:00:00 2001 From: biswassri Date: Fri, 13 Dec 2024 10:43:24 -0500 Subject: [PATCH 04/12] Adding predicate to watch configMap --- Makefile | 2 +- .../datasciencecluster_controller.go | 47 ------------- .../setupcontroller/setup_controller.go | 69 +++++++++++++++++-- 3 files changed, 66 insertions(+), 52 deletions(-) diff --git a/Makefile b/Makefile index ecfae782af3..3a3ff108a61 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ IMG ?= $(IMAGE_TAG_BASE):$(IMG_TAG) # You can use it as an arg. (E.g make bundle-build BUNDLE_IMG=/:) BUNDLE_IMG ?= $(IMAGE_TAG_BASE)-bundle:v$(VERSION) -IMAGE_BUILDER ?= podman +IMAGE_BUILDER ?= docker OPERATOR_NAMESPACE ?= opendatahub-operator-system DEFAULT_MANIFESTS_PATH ?= opt/manifests CGO_ENABLED ?= 1 diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 008a3dea748..6d119241136 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -35,9 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" @@ -51,7 +49,6 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/dependent" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" ) // DataScienceClusterReconciler reconciles a DataScienceCluster object. @@ -299,53 +296,9 @@ func (r *DataScienceClusterReconciler) SetupWithManager(_ context.Context, mgr c Watches( &dsciv1.DSCInitialization{}, handlers.Fn(r.watchDataScienceClusters)). - Watches( - &corev1.ConfigMap{}, - handlers.Fn(r.watchDataScienceClusters), - builder.WithPredicates(r.filterDeleteConfigMap())). Complete(r) } -func (r *DataScienceClusterReconciler) filterDeleteConfigMap() predicate.Funcs { - filter := func(obj client.Object) bool { - cm, ok := obj.(*corev1.ConfigMap) - if !ok { - return false - } - - // Trigger reconcile function when uninstall configmap is created - operatorNs, err := cluster.GetOperatorNamespace() - if err != nil { - return false - } - - if cm.Namespace != operatorNs { - return false - } - - if cm.Labels[upgrade.DeleteConfigMapLabel] != "true" { - return false - } - - return true - } - - return predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { - return filter(e.Object) - }, - UpdateFunc: func(e event.UpdateEvent) bool { - return filter(e.ObjectNew) - }, - DeleteFunc: func(e event.DeleteEvent) bool { - return false - }, - GenericFunc: func(e event.GenericEvent) bool { - return false - }, - } -} - func (r *DataScienceClusterReconciler) watchDataScienceClusters(ctx context.Context, _ client.Object) []reconcile.Request { instanceList := &dscv1.DataScienceClusterList{} err := r.Client.List(ctx, instanceList) diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index 89afbf688a9..b064ebcc811 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -8,8 +8,12 @@ import ( k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" @@ -21,9 +25,17 @@ type SetupControllerReconciler struct { *odhClient.Client } +const ( + finalizerName = "datasciencecluster.opendatahub.io/finalizer" +) + func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := logf.FromContext(ctx).WithName("DataScienceCluster") - log.Info("Reconciling setup controller", "Request.Name", req.Name) + log := logf.FromContext(ctx).WithName("SetupContoller") + log.Info("Reconciling setup controller", "Request.Name", req.Name) // log.V(1).Info(...) + + if !upgrade.HasDeleteConfigMap(ctx, r.Client) { + return ctrl.Result{}, nil + } instance := &dscv1.DataScienceCluster{} err := r.Client.Get(ctx, req.NamespacedName, instance) @@ -36,6 +48,7 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ // Return and don't requeue if upgrade.HasDeleteConfigMap(ctx, r.Client) { if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); uninstallErr != nil { + log.Info("TRYING TO OperatorUninstall", "InstanceName", instance.Name) return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr) } } @@ -45,10 +58,18 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } + if controllerutil.RemoveFinalizer(instance, finalizerName) { + if err := r.Client.Update(ctx, instance); err != nil { + return ctrl.Result{}, err + } + } + // If DSC CR exist and deletion CM exist - // delete DSC CR + // delete DSC CR first then Operator Uninstall + if upgrade.HasDeleteConfigMap(ctx, r.Client) { deleteErr := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) + log.Info("TRYING TO DELETE", "InstanceName", instance.Name) if deleteErr != nil { return ctrl.Result{}, fmt.Errorf("error while deleting DSC CR: %w", deleteErr) } @@ -61,6 +82,46 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&corev1.ConfigMap{}). + For(&corev1.ConfigMap{}, builder.WithPredicates(r.filterDeleteConfigMap())). Complete(r) } + +func (r *SetupControllerReconciler) filterDeleteConfigMap() predicate.Funcs { + filter := func(obj client.Object) bool { + cm, ok := obj.(*corev1.ConfigMap) + if !ok { + return false + } + + // Trigger reconcile function when uninstall configmap is created + operatorNs, err := cluster.GetOperatorNamespace() + if err != nil { + return false + } + + if cm.Namespace != operatorNs { + return false + } + + if cm.Labels[upgrade.DeleteConfigMapLabel] != "true" { + return false + } + + return true + } + + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return filter(e.Object) + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return filter(e.ObjectNew) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + } +} From bed238f520fa21657ff4261220d74b5d34cf325f Mon Sep 17 00:00:00 2001 From: biswassri Date: Mon, 16 Dec 2024 01:03:36 -0500 Subject: [PATCH 05/12] Changes to debug --- .../datasciencecluster_controller.go | 9 ++-- .../setupcontroller/setup_controller.go | 49 ++----------------- pkg/upgrade/uninstallation.go | 33 ++++++++++++- 3 files changed, 40 insertions(+), 51 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 6d119241136..89e66e80187 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -49,6 +49,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/dependent" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" ) // DataScienceClusterReconciler reconciles a DataScienceCluster object. @@ -72,12 +73,12 @@ const ( func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("DataScienceCluster") log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) - instance := &dscv1.DataScienceCluster{} - // Removed operator uninstall to the setup operator - // We don't need finalizer anymore, remove it if present to handle the - // upgrade case + if err := r.Client.Get(ctx, req.NamespacedName, instance); err != nil { + log := logf.Log.WithValues("namespace", req.NamespacedName.Namespace, "name", req.NamespacedName.Name) + log.Error(err, "Failed to get instance") + } if controllerutil.RemoveFinalizer(instance, finalizerName) { if err := r.Client.Update(ctx, instance); err != nil { diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index b064ebcc811..b4ae5d1b0a0 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -5,17 +5,13 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - k8serr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" - dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" @@ -25,10 +21,6 @@ type SetupControllerReconciler struct { *odhClient.Client } -const ( - finalizerName = "datasciencecluster.opendatahub.io/finalizer" -) - func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("SetupContoller") log.Info("Reconciling setup controller", "Request.Name", req.Name) // log.V(1).Info(...) @@ -37,47 +29,12 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - instance := &dscv1.DataScienceCluster{} - err := r.Client.Get(ctx, req.NamespacedName, instance) - - switch { - case k8serr.IsNotFound(err): - // Request object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. - // For additional cleanup logic use operatorUninstall function. - // Return and don't requeue - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - if uninstallErr := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); uninstallErr != nil { - log.Info("TRYING TO OperatorUninstall", "InstanceName", instance.Name) - return ctrl.Result{}, fmt.Errorf("error while operator uninstall: %w", uninstallErr) - } - } - - return ctrl.Result{}, nil - case err != nil: - return ctrl.Result{}, err - } - - if controllerutil.RemoveFinalizer(instance, finalizerName) { - if err := r.Client.Update(ctx, instance); err != nil { - return ctrl.Result{}, err - } - } - - // If DSC CR exist and deletion CM exist - // delete DSC CR first then Operator Uninstall - - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - deleteErr := r.Client.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)) - log.Info("TRYING TO DELETE", "InstanceName", instance.Name) - if deleteErr != nil { - return ctrl.Result{}, fmt.Errorf("error while deleting DSC CR: %w", deleteErr) - } - - return ctrl.Result{}, nil + if err := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name, req); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to uninstall setup controller: %w", err) } return ctrl.Result{}, nil + } func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/pkg/upgrade/uninstallation.go b/pkg/upgrade/uninstallation.go index 4701866285a..beb577a23bf 100644 --- a/pkg/upgrade/uninstallation.go +++ b/pkg/upgrade/uninstallation.go @@ -5,6 +5,10 @@ import ( "fmt" "time" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/hashicorp/go-multierror" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" @@ -25,8 +29,21 @@ const ( // OperatorUninstall deletes all the externally generated resources. // This includes DSCI, namespace created by operator (but not workbench or MR's), subscription and CSV. -func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform) error { +func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform, req ctrl.Request) error { log := logf.FromContext(ctx) + instance := &dscv1.DataScienceCluster{} + err := cli.Get(ctx, req.NamespacedName, instance) + if err != nil { + return fmt.Errorf("getting DataScienceCluster %s/%s failed: %w", req.Namespace, req.Name, err) + } + // if DSC exists then delete DSC + if !k8serr.IsNotFound(err) { + if deleteError := removeDSC(ctx, cli, req); deleteError != nil { + return fmt.Errorf("error deleting DSC : %w", deleteError) + } + } + + // If DSC doesn't continue deleting DSCI and other resources if err := removeDSCInitialization(ctx, cli); err != nil { return err } @@ -102,6 +119,20 @@ func removeDSCInitialization(ctx context.Context, cli client.Client) error { return multiErr.ErrorOrNil() } +func removeDSC(ctx context.Context, cli client.Client, req ctrl.Request) error { + instance := &dscv1.DataScienceCluster{} + if err := cli.Get(ctx, req.NamespacedName, instance); err != nil { + return err + } + + var multiErr *multierror.Error + if deleteErr := cli.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)); !k8serr.IsNotFound(deleteErr) { + multiErr = multierror.Append(multiErr, deleteErr) + } + + return multiErr.ErrorOrNil() +} + // HasDeleteConfigMap returns true if delete configMap is added to the operator namespace by managed-tenants repo. // It returns false in all other cases. func HasDeleteConfigMap(ctx context.Context, c client.Client) bool { From 4b116f33115f22c72f6adea9c6ef6739bc7045e9 Mon Sep 17 00:00:00 2001 From: biswassri Date: Mon, 16 Dec 2024 09:14:08 -0500 Subject: [PATCH 06/12] Deletion update + rbac change --- Makefile | 2 +- ...atahub-operator.clusterserviceversion.yaml | 3 +- config/rbac/role.yaml | 1 + .../datasciencecluster/kubebuilder_rbac.go | 2 +- .../setupcontroller/setup_controller.go | 5 ++- pkg/upgrade/uninstallation.go | 36 ++++++------------- 6 files changed, 18 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 3a3ff108a61..ecfae782af3 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ IMG ?= $(IMAGE_TAG_BASE):$(IMG_TAG) # You can use it as an arg. (E.g make bundle-build BUNDLE_IMG=/:) BUNDLE_IMG ?= $(IMAGE_TAG_BASE)-bundle:v$(VERSION) -IMAGE_BUILDER ?= docker +IMAGE_BUILDER ?= podman OPERATOR_NAMESPACE ?= opendatahub-operator-system DEFAULT_MANIFESTS_PATH ?= opt/manifests CGO_ENABLED ?= 1 diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 752e41221e2..45f22c8640c 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -106,7 +106,7 @@ metadata: categories: AI/Machine Learning, Big Data certified: "False" containerImage: quay.io/opendatahub/opendatahub-operator:v2.21.0 - createdAt: "2024-11-22T19:16:14Z" + createdAt: "2024-12-16T13:35:31Z" olm.skipRange: '>=1.0.0 <2.21.0' operators.operatorframework.io/builder: operator-sdk-v1.31.0 operators.operatorframework.io/internal-objects: '["featuretrackers.features.opendatahub.io", @@ -649,6 +649,7 @@ spec: verbs: - create - delete + - deletecollection - get - list - patch diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e259f476bad..ae000c13f5b 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -389,6 +389,7 @@ rules: verbs: - create - delete + - deletecollection - get - list - patch diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go index e9a31405cc4..3cfcaf14c0f 100644 --- a/controllers/datasciencecluster/kubebuilder_rbac.go +++ b/controllers/datasciencecluster/kubebuilder_rbac.go @@ -2,7 +2,7 @@ package datasciencecluster // +kubebuilder:rbac:groups="datasciencecluster.opendatahub.io",resources=datascienceclusters/status,verbs=get;update;patch // +kubebuilder:rbac:groups="datasciencecluster.opendatahub.io",resources=datascienceclusters/finalizers,verbs=update;patch -// +kubebuilder:rbac:groups="datasciencecluster.opendatahub.io",resources=datascienceclusters,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="datasciencecluster.opendatahub.io",resources=datascienceclusters,verbs=get;list;watch;create;update;patch;delete;deletecollection // +kubebuilder:rbac:groups="authentication.k8s.io",resources=tokenreviews,verbs=create;get // +kubebuilder:rbac:groups="authorization.k8s.io",resources=subjectaccessreviews,verbs=create;get diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index b4ae5d1b0a0..aa8dd470109 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -22,19 +22,18 @@ type SetupControllerReconciler struct { } func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := logf.FromContext(ctx).WithName("SetupContoller") + log := logf.FromContext(ctx).WithName("SetupController") log.Info("Reconciling setup controller", "Request.Name", req.Name) // log.V(1).Info(...) if !upgrade.HasDeleteConfigMap(ctx, r.Client) { return ctrl.Result{}, nil } - if err := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name, req); err != nil { + if err := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); err != nil { return ctrl.Result{}, fmt.Errorf("failed to uninstall setup controller: %w", err) } return ctrl.Result{}, nil - } func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/pkg/upgrade/uninstallation.go b/pkg/upgrade/uninstallation.go index beb577a23bf..5c2a174781d 100644 --- a/pkg/upgrade/uninstallation.go +++ b/pkg/upgrade/uninstallation.go @@ -5,17 +5,15 @@ import ( "fmt" "time" - dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/hashicorp/go-multierror" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" @@ -29,20 +27,12 @@ const ( // OperatorUninstall deletes all the externally generated resources. // This includes DSCI, namespace created by operator (but not workbench or MR's), subscription and CSV. -func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform, req ctrl.Request) error { +func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform) error { log := logf.FromContext(ctx) - instance := &dscv1.DataScienceCluster{} - err := cli.Get(ctx, req.NamespacedName, instance) - if err != nil { - return fmt.Errorf("getting DataScienceCluster %s/%s failed: %w", req.Namespace, req.Name, err) - } - // if DSC exists then delete DSC - if !k8serr.IsNotFound(err) { - if deleteError := removeDSC(ctx, cli, req); deleteError != nil { - return fmt.Errorf("error deleting DSC : %w", deleteError) - } - } + if deleteError := removeDSC(ctx, cli); deleteError != nil { + return fmt.Errorf("error deleting DSC : %w", deleteError) + } // If DSC doesn't continue deleting DSCI and other resources if err := removeDSCInitialization(ctx, cli); err != nil { return err @@ -111,7 +101,7 @@ func removeDSCInitialization(ctx context.Context, cli client.Client) error { var multiErr *multierror.Error for _, dsciInstance := range instanceList.Items { - if err := cli.Delete(ctx, &dsciInstance); !k8serr.IsNotFound(err) { + if err := cli.Delete(ctx, &dsciInstance); !k8serr.IsNotFound(err) { // DeleteAll multiErr = multierror.Append(multiErr, err) } } @@ -119,18 +109,14 @@ func removeDSCInitialization(ctx context.Context, cli client.Client) error { return multiErr.ErrorOrNil() } -func removeDSC(ctx context.Context, cli client.Client, req ctrl.Request) error { +func removeDSC(ctx context.Context, cli client.Client) error { instance := &dscv1.DataScienceCluster{} - if err := cli.Get(ctx, req.NamespacedName, instance); err != nil { - return err - } - var multiErr *multierror.Error - if deleteErr := cli.Delete(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)); !k8serr.IsNotFound(deleteErr) { - multiErr = multierror.Append(multiErr, deleteErr) + if deleteErr := cli.DeleteAllOf(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)); deleteErr != nil { + return fmt.Errorf("failure deleting DSC: %w", deleteErr) } - return multiErr.ErrorOrNil() + return nil } // HasDeleteConfigMap returns true if delete configMap is added to the operator namespace by managed-tenants repo. From 8668a62e529702999e604184a993409e6cc6b953 Mon Sep 17 00:00:00 2001 From: biswassri Date: Mon, 16 Dec 2024 09:45:12 -0500 Subject: [PATCH 07/12] Adding IsNotFound() check in DSC_controller --- .../datasciencecluster/datasciencecluster_controller.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 89e66e80187..2542dc76f63 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -75,11 +75,9 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) instance := &dscv1.DataScienceCluster{} - if err := r.Client.Get(ctx, req.NamespacedName, instance); err != nil { - log := logf.Log.WithValues("namespace", req.NamespacedName.Namespace, "name", req.NamespacedName.Name) - log.Error(err, "Failed to get instance") + if err := r.Client.Get(ctx, req.NamespacedName, instance); k8serr.IsNotFound(err) { + return ctrl.Result{}, err } - if controllerutil.RemoveFinalizer(instance, finalizerName) { if err := r.Client.Update(ctx, instance); err != nil { return ctrl.Result{}, err From bd85515b4e63e0c4c54d2dc387dc588de7ca1753 Mon Sep 17 00:00:00 2001 From: biswassri Date: Mon, 16 Dec 2024 13:13:59 -0500 Subject: [PATCH 08/12] Changing k8serr.IsNotFound error from if to switch logic --- .../datasciencecluster/datasciencecluster_controller.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index 2542dc76f63..bfa3fe2376c 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -74,10 +74,15 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R log := logf.FromContext(ctx).WithName("DataScienceCluster") log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name) instance := &dscv1.DataScienceCluster{} + err := r.Client.Get(ctx, req.NamespacedName, instance) - if err := r.Client.Get(ctx, req.NamespacedName, instance); k8serr.IsNotFound(err) { + switch { + case k8serr.IsNotFound(err): + return ctrl.Result{}, nil + case err != nil: return ctrl.Result{}, err } + if controllerutil.RemoveFinalizer(instance, finalizerName) { if err := r.Client.Update(ctx, instance); err != nil { return ctrl.Result{}, err @@ -111,7 +116,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R return strings.Compare(string(a.Type), string(b.Type)) }) - err := r.Client.ApplyStatus(ctx, instance, client.FieldOwner(fieldOwner), client.ForceOwnership) + err = r.Client.ApplyStatus(ctx, instance, client.FieldOwner(fieldOwner), client.ForceOwnership) switch { case err == nil: return ctrl.Result{}, nil From 52b15cd148dd48f7079a7889b80eb09555b638a3 Mon Sep 17 00:00:00 2001 From: biswassri Date: Tue, 17 Dec 2024 03:00:34 -0500 Subject: [PATCH 09/12] Changes from review --- .../setupcontroller/setup_controller.go | 18 +++++++++--------- pkg/upgrade/uninstallation.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index aa8dd470109..a9c85d5bd02 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -23,7 +23,7 @@ type SetupControllerReconciler struct { func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx).WithName("SetupController") - log.Info("Reconciling setup controller", "Request.Name", req.Name) // log.V(1).Info(...) + log.Info("Reconciling setup controller") if !upgrade.HasDeleteConfigMap(ctx, r.Client) { return ctrl.Result{}, nil @@ -37,21 +37,21 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ } func (r *SetupControllerReconciler) SetupWithManager(mgr ctrl.Manager) error { + operatorNs, err := cluster.GetOperatorNamespace() + + if err != nil { + return fmt.Errorf("failed to get operator namespace: %w", err) + } return ctrl.NewControllerManagedBy(mgr). - For(&corev1.ConfigMap{}, builder.WithPredicates(r.filterDeleteConfigMap())). + For(&corev1.ConfigMap{}, builder.WithPredicates(r.filterDeleteConfigMap(operatorNs))). Complete(r) } -func (r *SetupControllerReconciler) filterDeleteConfigMap() predicate.Funcs { +func (r *SetupControllerReconciler) filterDeleteConfigMap(operatorNs string) predicate.Funcs { filter := func(obj client.Object) bool { cm, ok := obj.(*corev1.ConfigMap) - if !ok { - return false - } - // Trigger reconcile function when uninstall configmap is created - operatorNs, err := cluster.GetOperatorNamespace() - if err != nil { + if !ok { return false } diff --git a/pkg/upgrade/uninstallation.go b/pkg/upgrade/uninstallation.go index 5c2a174781d..7da53109d88 100644 --- a/pkg/upgrade/uninstallation.go +++ b/pkg/upgrade/uninstallation.go @@ -31,7 +31,7 @@ func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster. log := logf.FromContext(ctx) if deleteError := removeDSC(ctx, cli); deleteError != nil { - return fmt.Errorf("error deleting DSC : %w", deleteError) + return deleteError } // If DSC doesn't continue deleting DSCI and other resources if err := removeDSCInitialization(ctx, cli); err != nil { From 749c1c2a2f4b0e90839e0097399ed3ecd2c2f8d4 Mon Sep 17 00:00:00 2001 From: biswassri Date: Tue, 17 Dec 2024 09:29:37 -0500 Subject: [PATCH 10/12] Removing HasDeleteConfig from DSC_controller --- .../datasciencecluster/datasciencecluster_controller.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index bfa3fe2376c..73cd9f911ae 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -49,7 +49,6 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/dependent" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" ) // DataScienceClusterReconciler reconciles a DataScienceCluster object. @@ -91,11 +90,6 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R if !instance.ObjectMeta.DeletionTimestamp.IsZero() { log.Info("Finalization DataScienceCluster start deleting instance", "name", instance.Name) - - if upgrade.HasDeleteConfigMap(ctx, r.Client) { - return ctrl.Result{Requeue: true}, nil - } - return ctrl.Result{}, nil } From 37263e6b644523bc7ece14422306a87590a46253 Mon Sep 17 00:00:00 2001 From: biswassri Date: Wed, 18 Dec 2024 09:46:47 -0500 Subject: [PATCH 11/12] Remove comment + change deleteErr name --- pkg/upgrade/uninstallation.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/upgrade/uninstallation.go b/pkg/upgrade/uninstallation.go index 7da53109d88..aef43edc053 100644 --- a/pkg/upgrade/uninstallation.go +++ b/pkg/upgrade/uninstallation.go @@ -30,10 +30,10 @@ const ( func OperatorUninstall(ctx context.Context, cli client.Client, platform cluster.Platform) error { log := logf.FromContext(ctx) - if deleteError := removeDSC(ctx, cli); deleteError != nil { - return deleteError + if err := removeDSC(ctx, cli); err != nil { + return err } - // If DSC doesn't continue deleting DSCI and other resources + if err := removeDSCInitialization(ctx, cli); err != nil { return err } @@ -101,7 +101,7 @@ func removeDSCInitialization(ctx context.Context, cli client.Client) error { var multiErr *multierror.Error for _, dsciInstance := range instanceList.Items { - if err := cli.Delete(ctx, &dsciInstance); !k8serr.IsNotFound(err) { // DeleteAll + if err := cli.Delete(ctx, &dsciInstance); !k8serr.IsNotFound(err) { multiErr = multierror.Append(multiErr, err) } } @@ -112,8 +112,8 @@ func removeDSCInitialization(ctx context.Context, cli client.Client) error { func removeDSC(ctx context.Context, cli client.Client) error { instance := &dscv1.DataScienceCluster{} - if deleteErr := cli.DeleteAllOf(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)); deleteErr != nil { - return fmt.Errorf("failure deleting DSC: %w", deleteErr) + if err := cli.DeleteAllOf(ctx, instance, client.PropagationPolicy(metav1.DeletePropagationForeground)); err != nil { + return fmt.Errorf("failure deleting DSC: %w", err) } return nil From b2af7e2748a1d4eac157c6857d1857be48577d50 Mon Sep 17 00:00:00 2001 From: biswassri Date: Wed, 18 Dec 2024 11:46:28 -0500 Subject: [PATCH 12/12] Updating error message --- controllers/setupcontroller/setup_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/setupcontroller/setup_controller.go b/controllers/setupcontroller/setup_controller.go index a9c85d5bd02..767e7e1b30a 100644 --- a/controllers/setupcontroller/setup_controller.go +++ b/controllers/setupcontroller/setup_controller.go @@ -30,7 +30,7 @@ func (r *SetupControllerReconciler) Reconcile(ctx context.Context, req ctrl.Requ } if err := upgrade.OperatorUninstall(ctx, r.Client, cluster.GetRelease().Name); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to uninstall setup controller: %w", err) + return ctrl.Result{}, fmt.Errorf("operator uninstall failed : %w", err) } return ctrl.Result{}, nil