Debug and fix e2e tests in CI (opendatahub-io#667)

* Update opendatahub label

* Update Codeflare manifests path

* Move creation of default DSC

* Trigger delete configmap after finalizer removal

* Revert changes in deploy

Revert changes made in deploy.go. Dashboard resources fail to cleanup with updated changes

components/codeflare/codeflare.go

@@ -4,7 +4,6 @@ package codeflare
 
 import (
 	"fmt"
-
 	"path/filepath"
 
 	dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
@@ -34,7 +33,7 @@ func (c *CodeFlare) OverrideManifests(_ string) error {
 			return err
 		}
 		// If overlay is defined, update paths
-		defaultKustomizePath := "base"
+		defaultKustomizePath := "manifests"
 		if manifestConfig.SourcePath != "" {
 			defaultKustomizePath = manifestConfig.SourcePath
 		}
@@ -60,7 +59,6 @@ func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 	if err != nil {
 		return err
 	}
-
 	if enabled {
 		// Download manifests and update paths
 		if err = c.OverrideManifests(string(platform)); err != nil {
@@ -75,9 +73,9 @@ func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 		}
 
 		if found, err := deploy.OperatorExists(cli, dependentOperator); err != nil {
-			return err
-		} else if !found {
-			return fmt.Errorf("operator %s not found. Please install the operator before enabling %s component",
+			return fmt.Errorf("operator exists throws error %v", err)
+		} else if found {
+			return fmt.Errorf("operator %s  found. Please uninstall the operator before enabling %s component",
 				dependentOperator, ComponentName)
 		}
 
@@ -90,8 +88,13 @@ func (c *CodeFlare) ReconcileComponent(cli client.Client, owner metav1.Object, d
 	}
 
 	// Deploy Codeflare
-	err = deploy.DeployManifestsFromPath(cli, owner, CodeflarePath, dscispec.ApplicationsNamespace, c.GetComponentName(), enabled)
-	return err
+	if err := deploy.DeployManifestsFromPath(cli, owner,
+		CodeflarePath,
+		dscispec.ApplicationsNamespace,
+		ComponentName, enabled); err != nil {
+		return err
+	}
+	return nil
 }
 
 func (c *CodeFlare) DeepCopyInto(target *CodeFlare) {

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -107,15 +107,6 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 
 	instance := &instances.Items[0]
 
-	if instance.GetDeletionTimestamp() != nil {
-		r.Log.Info("DataScienceCluster instance is deleted.", "name", instance.Name)
-		if upgrade.HasDeleteConfigMap(r.Client) {
-			// if delete configmap exists, requeue the request to handle operator uninstall
-			return reconcile.Result{Requeue: true}, nil
-		}
-		return ctrl.Result{}, nil
-	}
-
 	var err error
 
 	// Verify a valid DSCInitialization instance is created
@@ -175,7 +166,10 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 				return ctrl.Result{}, err
 			}
 		}
-
+		if upgrade.HasDeleteConfigMap(r.Client) {
+			// if delete configmap exists, requeue the request to handle operator uninstall
+			return reconcile.Result{Requeue: true}, nil
+		}
 		return ctrl.Result{}, nil
 	}
 

controllers/dscinitialization/dscinitialization_controller.go

@@ -29,8 +29,6 @@ import (
 	dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
-
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	netv1 "k8s.io/api/networking/v1"
@@ -139,13 +137,6 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return reconcile.Result{}, err
 	}
 
-	// Apply update from legacy operator
-	// TODO: Update upgrade logic to get components through KfDef
-	if err = upgrade.UpdateFromLegacyVersion(r.Client, platform); err != nil {
-		r.Log.Error(err, "unable to update from legacy operator version")
-		return reconcile.Result{}, err
-	}
-
 	// Apply Rhods specific configs
 	if platform == deploy.ManagedRhods || platform == deploy.SelfManagedRhods {
 		// Apply osd specific permissions

main.go

@@ -26,6 +26,8 @@ import (
 	datascienceclustercontrollers "github.com/opendatahub-io/opendatahub-operator/v2/controllers/datasciencecluster"
 	dscicontr "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/pkg/deploy"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
 	addonv1alpha1 "github.com/openshift/addon-operator/apis/addons/v1alpha1"
 	ocv1 "github.com/openshift/api/oauth/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -45,8 +47,8 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"os"
-	client2 "sigs.k8s.io/controller-runtime/pkg/client"
-
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
@@ -167,7 +169,7 @@ func main() {
 	_, disableDSCConfig := os.LookupEnv("DISABLE_DSC_CONFIG")
 	if !disableDSCConfig {
 		// Create DSCInitialization CR if it's not present
-		client := mgr.GetClient()
+		c := mgr.GetClient()
 		releaseDscInitialization := &dsci.DSCInitialization{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "DSCInitialization",
@@ -184,7 +186,7 @@ func main() {
 				},
 			},
 		}
-		err = client.Create(context.TODO(), releaseDscInitialization)
+		err = c.Create(context.TODO(), releaseDscInitialization)
 		switch {
 		case err == nil:
 			setupLog.Info("created DscInitialization resource")
@@ -195,8 +197,8 @@ func main() {
 			if err != nil {
 				setupLog.Error(err, "failed to get DscInitialization custom resource data")
 			}
-			err = client.Patch(context.TODO(), releaseDscInitialization, client2.RawPatch(types.ApplyPatchType, data),
-				client2.ForceOwnership, client2.FieldOwner("opendatahub-operator"))
+			err = c.Patch(context.TODO(), releaseDscInitialization, client.RawPatch(types.ApplyPatchType, data),
+				client.ForceOwnership, client.FieldOwner("opendatahub-operator"))
 			if err != nil {
 				setupLog.Error(err, "failed to update DscInitialization custom resource")
 			}
@@ -206,6 +208,31 @@ func main() {
 		}
 	}
 
+	// Create new uncached client to run initial setup
+	setupCfg, err := config.GetConfig()
+	if err != nil {
+		setupLog.Error(err, "error getting config for setup")
+		os.Exit(1)
+	}
+
+	setupClient, err := client.New(setupCfg, client.Options{Scheme: scheme})
+	if err != nil {
+		setupLog.Error(err, "error getting client for setup")
+		os.Exit(1)
+	}
+	// Get operator platform
+	platform, err := deploy.GetPlatform(setupClient)
+	if err != nil {
+		setupLog.Error(err, "error getting client for setup")
+		os.Exit(1)
+	}
+
+	// Apply update from legacy operator
+	if err = upgrade.UpdateFromLegacyVersion(setupClient, platform); err != nil {
+		setupLog.Error(err, "unable to update from legacy operator version")
+		os.Exit(1)
+	}
+
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)

pkg/upgrade/upgrade.go

@@ -139,7 +139,7 @@ func HasDeleteConfigMap(c client.Client) bool {
 
 // createDefaultDSC creates a default instance of DSC.
 // Note: When the platform is not Managed, and a DSC instance already exists, the function doesn't re-create/update the resource.
-func createDefaultDSC(cli client.Client, platform deploy.Platform) error {
+func CreateDefaultDSC(cli client.Client, platform deploy.Platform) error {
 	releaseDataScienceCluster := &dsc.DataScienceCluster{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "DataScienceCluster",
@@ -204,7 +204,7 @@ func createDefaultDSC(cli client.Client, platform deploy.Platform) error {
 func UpdateFromLegacyVersion(cli client.Client, platform deploy.Platform) error {
 	// If platform is Managed, remove Kfdefs and create default dsc
 	if platform == deploy.ManagedRhods {
-		err := createDefaultDSC(cli, platform)
+		err := CreateDefaultDSC(cli, platform)
 		if err != nil {
 			return err
 		}
@@ -216,37 +216,21 @@ func UpdateFromLegacyVersion(cli client.Client, platform deploy.Platform) error
 		return nil
 	}
 
-	// If KfDef CRD is not found, we see it as a cluster not pre-installed v1 operator	// Check if kfdef are deployed
-	kfdefCrd := &apiextv1.CustomResourceDefinition{}
-	err := cli.Get(context.TODO(), client.ObjectKey{Name: "kfdefs.kfdef.apps.kubeflow.org"}, kfdefCrd)
-	if err != nil {
-		if apierrs.IsNotFound(err) {
-			// If no Crd found, return, since its a new Installation
-			return nil
-		} else {
-			return fmt.Errorf("error retrieving kfdef CRD : %v", err)
+	if platform == deploy.SelfManagedRhods {
+		kfDefList, err := getKfDefInstances(cli)
+		if err != nil {
+			return fmt.Errorf("error getting kfdef instances: %v", err)
 		}
-	}
 
-	// If KfDef Instances found, and no DSC instances are found in Self-managed, that means this is an upgrade path from
-	// legacy version. Create a default DSC instance
-	kfDefList := &kfdefv1.KfDefList{}
-	err = cli.List(context.TODO(), kfDefList)
-	if err != nil {
-		if apierrs.IsNotFound(err) {
-			// If no KfDefs, do nothing and return
-			return nil
-		} else {
-			return fmt.Errorf("error getting list of kfdefs: %v", err)
-		}
-	}
-	if len(kfDefList.Items) > 0 {
-		err := createDefaultDSC(cli, platform)
-		if err != nil {
-			return err
+		if len(kfDefList.Items) > 0 {
+			err := CreateDefaultDSC(cli, platform)
+			if err != nil {
+				return err
+			}
 		}
+		return err
 	}
-	return err
+	return nil
 }
 
 func GetOperatorNamespace() (string, error) {
@@ -261,28 +245,12 @@ func GetOperatorNamespace() (string, error) {
 
 func RemoveKfDefInstances(cli client.Client, platform deploy.Platform) error {
 	// Check if kfdef are deployed
-	kfdefCrd := &apiextv1.CustomResourceDefinition{}
-
-	err := cli.Get(context.TODO(), client.ObjectKey{Name: "kfdefs.kfdef.apps.kubeflow.org"}, kfdefCrd)
+	expectedKfDefList, err := getKfDefInstances(cli)
 	if err != nil {
-		if apierrs.IsNotFound(err) {
-			// If no Crd found, return, since its a new Installation
-			return nil
-		} else {
-			return fmt.Errorf("error retrieving kfdef CRD : %v", err)
-		}
-	} else {
-		expectedKfDefList := &kfdefv1.KfDefList{}
-		err := cli.List(context.TODO(), expectedKfDefList)
-		if err != nil {
-			if apierrs.IsNotFound(err) {
-				// If no KfDefs, do nothing and return
-				return nil
-			} else {
-				return fmt.Errorf("error getting list of kfdefs: %v", err)
-			}
-		}
-		// Delete kfdefs
+		return err
+	}
+	// Delete kfdefs
+	if len(expectedKfDefList.Items) > 0 {
 		for _, kfdef := range expectedKfDefList.Items {
 			// Remove finalizer
 			updatedKfDef := &kfdef
@@ -297,6 +265,7 @@ func RemoveKfDefInstances(cli client.Client, platform deploy.Platform) error {
 			}
 		}
 	}
+
 	return nil
 }
 
@@ -350,3 +319,31 @@ func getClusterServiceVersion(cfg *rest.Config, watchNameSpace string) (*ofapi.C
 	}
 	return nil, nil
 }
+
+func getKfDefInstances(c client.Client) (*kfdefv1.KfDefList, error) {
+	// If KfDef CRD is not found, we see it as a cluster not pre-installed v1 operator	// Check if kfdef are deployed
+	kfdefCrd := &apiextv1.CustomResourceDefinition{}
+	err := c.Get(context.TODO(), client.ObjectKey{Name: "kfdefs.kfdef.apps.kubeflow.org"}, kfdefCrd)
+	if err != nil {
+		if apierrs.IsNotFound(err) {
+			// If no Crd found, return, since its a new Installation
+			return nil, nil
+		} else {
+			return nil, fmt.Errorf("error retrieving kfdef CRD : %v", err)
+		}
+	}
+
+	// If KfDef Instances found, and no DSC instances are found in Self-managed, that means this is an upgrade path from
+	// legacy version. Create a default DSC instance
+	kfDefList := &kfdefv1.KfDefList{}
+	err = c.List(context.TODO(), kfDefList)
+	if err != nil {
+		if apierrs.IsNotFound(err) {
+			// If no KfDefs, do nothing and return
+			return nil, nil
+		} else {
+			return nil, fmt.Errorf("error getting list of kfdefs: %v", err)
+		}
+	}
+	return kfDefList, nil
+}

tests/e2e/dsc_creation_test.go

@@ -194,9 +194,7 @@ func (tc *testContext) testAllApplicationCreation(t *testing.T) error {
 		if tc.testDsc.Spec.Components.CodeFlare.ManagementState == operatorv1.Managed {
 			if err != nil {
 				// dependent operator error, as expected
-				if strings.Contains(err.Error(), "Please install the operator before enabling component") {
-					t.Logf("expected error: %v", err.Error())
-				} else {
+				{
 					require.NoError(t, err, "error validating application %v when enabled", tc.testDsc.Spec.Components.CodeFlare.GetComponentName())
 				}
 			}

tests/e2e/dsc_deletion_test.go

@@ -57,7 +57,7 @@ func (tc *testContext) testApplicationDeletion(component components.ComponentInt
 
 	if err := wait.PollUntilContextTimeout(tc.ctx, tc.resourceRetryInterval, tc.resourceCreationTimeout, false, func(ctx context.Context) (done bool, err error) {
 		appList, err := tc.kubeClient.AppsV1().Deployments(tc.applicationsNamespace).List(ctx, metav1.ListOptions{
-			LabelSelector: "app.kubernetes.io/part-of=" + component.GetComponentName(),
+			LabelSelector: "app.opendatahub.io/" + component.GetComponentName(),
 		})
 		if err != nil {
 			log.Printf("error listing component deployments :%v. Trying again...", err)