From b1bb237d743f4a3ac6c16b3c1c2fde6f24219ab9 Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Fri, 25 Oct 2024 10:21:00 +0200 Subject: [PATCH] Fix dashboard internal api tests --- .../components/dashboard/dashboard_support.go | 4 +- .../reconciler/component_reconciler.go | 23 +++++-- tests/e2e/dashboard_test.go | 68 +++++++++++++------ 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/controllers/components/dashboard/dashboard_support.go b/controllers/components/dashboard/dashboard_support.go index 91461cc7c13..2d207e72b65 100644 --- a/controllers/components/dashboard/dashboard_support.go +++ b/controllers/components/dashboard/dashboard_support.go @@ -81,9 +81,9 @@ func computeKustomizeVariable(ctx context.Context, cli client.Client, platform c func computeComponentName() string { release := cluster.GetRelease() - name := ComponentNameDownstream + name := ComponentNameUpstream if release.Name == cluster.SelfManagedRhods || release.Name == cluster.ManagedRhods { - name = ComponentNameUpstream + name = ComponentNameDownstream } return name diff --git a/pkg/controller/reconciler/component_reconciler.go b/pkg/controller/reconciler/component_reconciler.go index 9a9cbbcfe4f..311c16cd9a9 100644 --- a/pkg/controller/reconciler/component_reconciler.go +++ b/pkg/controller/reconciler/component_reconciler.go @@ -6,6 +6,8 @@ import ( "fmt" "reflect" + k8serr "k8s.io/apimachinery/pkg/api/errors" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -166,17 +168,24 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } } - // update status - err = r.Client.ApplyStatus( + // + // update status with standard update mechanism as the SSA one seems causing + // a weird issue on some openshift releases: + // + // failed to create typed patch object (...): .status.url: field not declared in schema + // + err = r.Client.Status().Update( ctx, rr.Instance, - client.FieldOwner(rr.Instance.GetName()), - client.ForceOwnership, ) - if err != nil { + switch { + case err == nil: + return ctrl.Result{}, nil + case k8serr.IsConflict(err): + l.Info("conflict detected while updating status, retrying") + return ctrl.Result{Requeue: true}, nil + default: return ctrl.Result{}, err } - - return ctrl.Result{}, nil } diff --git a/tests/e2e/dashboard_test.go b/tests/e2e/dashboard_test.go index b328cbf77e1..e6891582783 100644 --- a/tests/e2e/dashboard_test.go +++ b/tests/e2e/dashboard_test.go @@ -14,6 +14,7 @@ import ( k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,7 +48,7 @@ func dashboardTestSuite(t *testing.T) { }) t.Run("Validate Ownerrefrences exist", func(t *testing.T) { - err = dashboardCtx.testOwnerrefrences() + err = dashboardCtx.testOwnerReferences() require.NoError(t, err, "error getting all Dashboard's Ownerrefrences") }) @@ -71,20 +72,35 @@ func dashboardTestSuite(t *testing.T) { } func (tc *DashboardTestCtx) testDashboardCreation() error { - existingDashboardList := &componentsv1.DashboardList{} + if tc.testCtx.testDsc.Spec.Components.Dashboard.ManagementState != operatorv1.Managed { + return nil + } - if tc.testCtx.testDsc.Spec.Components.Dashboard.ManagementState == operatorv1.Managed { - err := tc.testCtx.customClient.List(tc.testCtx.ctx, existingDashboardList) - if err == nil { - if len(existingDashboardList.Items) == 1 { - tc.testDashboardInstance = existingDashboardList.Items[0] - return nil - } else { - return fmt.Errorf("unexpected Dashboard CR instances. Expected 1 , "+ - "Found %v instance", len(existingDashboardList.Items)) - } + err := tc.testCtx.wait(func(ctx context.Context) (bool, error) { + existingDashboardList := &componentsv1.DashboardList{} + + err := tc.testCtx.customClient.List(ctx, existingDashboardList) + if err != nil { + return false, err + } + + switch { + case len(existingDashboardList.Items) == 1: + tc.testDashboardInstance = existingDashboardList.Items[0] + return true, nil + + case len(existingDashboardList.Items) > 1: + return false, fmt.Errorf( + "unexpected Dashboard CR instances. Expected 1 , Found %v instance", len(existingDashboardList.Items)) + default: + return false, nil } + }) + + if err != nil { + return fmt.Errorf("unable to find Dashboard CR instance: %w", err) } + return nil } @@ -98,7 +114,11 @@ func (tc *DashboardTestCtx) validateDashboard() error { return nil } -func (tc *DashboardTestCtx) testOwnerrefrences() error { +func (tc *DashboardTestCtx) testOwnerReferences() error { + if len(tc.testDashboardInstance.OwnerReferences) != 1 { + return errors.New("expected ownerreferences to be non empty") + } + // Test Dashboard CR ownerref if tc.testDashboardInstance.OwnerReferences[0].Kind != "DataScienceCluster" { return fmt.Errorf("expected ownerreference not found. Got ownereferrence: %v", @@ -124,8 +144,10 @@ func (tc *DashboardTestCtx) testOwnerrefrences() error { // Verify Dashboard instance is in Ready phase when dashboard deployments are up and running. func (tc *DashboardTestCtx) validateDashboardReady() error { - // wait for 2 mins which is on the safe side, normally it should get ready once all components are ready - err := tc.testCtx.wait(func(ctx context.Context) (bool, error) { + // the dashboard deployment may take quite a long time to get ready as the related readiness + // probes have an initial delay time of 30 sec and each check is performed with a delay of + // 30 sec. + err := wait.PollUntilContextTimeout(tc.testCtx.ctx, generalRetryInterval, componentReadyTimeout, true, func(ctx context.Context) (bool, error) { key := types.NamespacedName{Name: tc.testDashboardInstance.Name} dashboard := &componentsv1.Dashboard{} @@ -147,7 +169,7 @@ func (tc *DashboardTestCtx) testUpdateOnDashboardResources() error { // Test Updating Dashboard Replicas appDeployments, err := tc.testCtx.kubeClient.AppsV1().Deployments(tc.testCtx.applicationsNamespace).List(tc.testCtx.ctx, metav1.ListOptions{ - LabelSelector: labels.ODH.Component("dashboard"), + LabelSelector: labels.ComponentManagedBy + "=" + tc.testDashboardInstance.Name, }) if err != nil { return err @@ -238,13 +260,15 @@ func (tc *DashboardTestCtx) testUpdateDashboardComponentDisabled() error { return fmt.Errorf("error after retry %w", err) } - // Verify dashboard CR is deleted - dashboard := &componentsv1.Dashboard{} - err = tc.testCtx.customClient.Get(tc.testCtx.ctx, client.ObjectKey{Name: tc.testDashboardInstance.Name}, dashboard) - if err == nil { + err = tc.testCtx.wait(func(ctx context.Context) (bool, error) { + // Verify dashboard CR is deleted + dashboard := &componentsv1.Dashboard{} + err = tc.testCtx.customClient.Get(ctx, client.ObjectKey{Name: tc.testDashboardInstance.Name}, dashboard) + return k8serr.IsNotFound(err), nil + }) + + if err != nil { return fmt.Errorf("component %v is disabled, should not get the Dashboard CR %v", "dashboard", tc.testDashboardInstance.Name) - } else if !k8serr.IsNotFound(err) { - return err } // Sleep for 20 seconds to allow the operator to reconcile