diff --git a/.gitignore b/.gitignore index 788554a201bc..59a4a6e79a86 100644 --- a/.gitignore +++ b/.gitignore @@ -57,6 +57,12 @@ opt/manifests/ cover.out +# Coverage HTML reports +*_coverage.html +*_coverage.out +coverage.html +coverage.out +overall_coverage.out config/manager/kustomization.yaml config/rhoai/manager/kustomization.yaml @@ -69,6 +75,8 @@ local.mk # Ignore catalog related files /catalog/ +# Ignore backup files +*.backup # Ignore generated files config/crd/bases config/crd/external diff --git a/Makefile b/Makefile index de2e3fc5bea5..44035a9620a1 100644 --- a/Makefile +++ b/Makefile @@ -556,7 +556,18 @@ unit-test: envtest ginkgo # directly use ginkgo since the framework is not compa --coverprofile=cover.out \ --succinct \ $(TEST_SRC) -CLEANFILES += cover.out +CLEANFILES += cover.out coverage.html + +.PHONY: test-coverage +test-coverage: ## Generate HTML coverage report + $(MAKE) unit-test + @if [ ! -f cover.out ] || [ ! -s cover.out ]; then \ + echo "Warning: cover.out is missing or empty. Skipping coverage report generation."; \ + echo "Make sure unit tests are configured to generate coverage data."; \ + exit 0; \ + fi + go tool cover -html=cover.out -o coverage.html + @echo "Coverage report generated: coverage.html" # Pattern rule to generate .rules.yaml from PrometheusRule templates # This finds the corresponding *-prometheusrules.tmpl.yaml in the same directory diff --git a/internal/controller/components/dashboard/dashboard.go b/internal/controller/components/dashboard/dashboard.go index eb99a22a9686..c4d8434bbb79 100644 --- a/internal/controller/components/dashboard/dashboard.go +++ b/internal/controller/components/dashboard/dashboard.go @@ -22,6 +22,11 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" ) +const ( + // AnacondaSecretName is the name of the anaconda access secret. + AnacondaSecretName = "anaconda-ce-access" //nolint:gosec // This is a Kubernetes secret name, not a credential +) + type componentHandler struct{} func init() { //nolint:gochecknoinits @@ -33,14 +38,17 @@ func (s *componentHandler) GetName() string { } func (s *componentHandler) Init(platform common.Platform) error { - mi := defaultManifestInfo(platform) + mi, err := DefaultManifestInfo(platform) + if err != nil { + return err + } - if err := odhdeploy.ApplyParams(mi.String(), "params.env", imagesMap); err != nil { + if err := odhdeploy.ApplyParams(mi.String(), "params.env", ImagesMap); err != nil { return fmt.Errorf("failed to update images on path %s: %w", mi, err) } - extra := bffManifestsPath() - if err := odhdeploy.ApplyParams(extra.String(), "params.env", imagesMap); err != nil { + extra := BffManifestsPath() + if err := odhdeploy.ApplyParams(extra.String(), "params.env", ImagesMap); err != nil { return fmt.Errorf("failed to update modular-architecture images on path %s: %w", extra, err) } @@ -72,6 +80,10 @@ func (s *componentHandler) IsEnabled(dsc *dscv2.DataScienceCluster) bool { func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.ReconciliationRequest) (metav1.ConditionStatus, error) { cs := metav1.ConditionUnknown + if rr.Client == nil { + return cs, errors.New("client is nil") + } + c := componentApi.Dashboard{} c.Name = componentApi.DashboardInstanceName @@ -89,8 +101,6 @@ func (s *componentHandler) UpdateDSCStatus(ctx context.Context, rr *types.Reconc dsc.Status.Components.Dashboard.ManagementState = ms dsc.Status.Components.Dashboard.DashboardCommonStatus = nil - rr.Conditions.MarkFalse(ReadyConditionType) - if s.IsEnabled(dsc) { dsc.Status.Components.Dashboard.DashboardCommonStatus = c.Status.DashboardCommonStatus.DeepCopy() diff --git a/internal/controller/components/dashboard/dashboard_controller.go b/internal/controller/components/dashboard/dashboard_controller.go index f856b821bda2..c62d8d030250 100644 --- a/internal/controller/components/dashboard/dashboard_controller.go +++ b/internal/controller/components/dashboard/dashboard_controller.go @@ -18,6 +18,7 @@ package dashboard import ( "context" + "errors" "fmt" consolev1 "github.com/openshift/api/console/v1" @@ -46,7 +47,11 @@ import ( // NewComponentReconciler creates a ComponentReconciler for the Dashboard API. func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error { - componentName := computeComponentName() + if mgr == nil { + return errors.New("could not create the dashboard controller: manager cannot be nil") + } + + componentName := ComputeComponentName() _, err := reconciler.ReconcilerFor(mgr, &componentApi.Dashboard{}). // operands - owned @@ -95,9 +100,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. GenericFunc: func(tge event.TypedGenericEvent[client.Object]) bool { return false }, DeleteFunc: func(tde event.TypedDeleteEvent[client.Object]) bool { return false }, }), reconciler.Dynamic(reconciler.CrdExists(gvk.DashboardHardwareProfile))). - WithAction(initialize). + WithAction(Initialize). WithAction(setKustomizedParams). - WithAction(configureDependencies). + WithAction(ConfigureDependencies). + WithAction(CustomizeResources). WithAction(kustomize.NewAction( // Those are the default labels added by the legacy deploy method // and should be preserved as the original plugin were affecting @@ -112,15 +118,15 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. )). WithAction(deploy.NewAction()). WithAction(deployments.NewAction()). - WithAction(reconcileHardwareProfiles). - WithAction(updateStatus). + WithAction(ReconcileHardwareProfiles). + WithAction(UpdateStatus). // must be the final action WithAction(gc.NewAction( gc.WithUnremovables(gvk.OdhDashboardConfig), )). // declares the list of additional, controller specific conditions that are // contributing to the controller readiness status - WithConditions(conditionTypes...). + WithConditions(ConditionTypes...). Build(ctx) if err != nil { diff --git a/internal/controller/components/dashboard/dashboard_controller_actions.go b/internal/controller/components/dashboard/dashboard_controller_actions.go index 3d3d7a04df64..9d82f3fb1bf4 100644 --- a/internal/controller/components/dashboard/dashboard_controller_actions.go +++ b/internal/controller/components/dashboard/dashboard_controller_actions.go @@ -26,6 +26,7 @@ import ( odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" ) @@ -53,14 +54,22 @@ type DashboardHardwareProfileList struct { Items []DashboardHardwareProfile `json:"items"` } -func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - rr.Manifests = []odhtypes.ManifestInfo{defaultManifestInfo(rr.Release.Name)} +func Initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + if rr == nil { + return errors.New("nil ReconciliationRequest") + } + + manifestInfo, err := DefaultManifestInfo(rr.Release.Name) + if err != nil { + return err + } + rr.Manifests = []odhtypes.ManifestInfo{manifestInfo} return nil } func setKustomizedParams(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - extraParamsMap, err := computeKustomizeVariable(ctx, rr.Client, rr.Release.Name) + extraParamsMap, err := ComputeKustomizeVariable(ctx, rr.Client, rr.Release.Name) if err != nil { return fmt.Errorf("failed to set variable for url, section-title etc: %w", err) } @@ -71,18 +80,23 @@ func setKustomizedParams(ctx context.Context, rr *odhtypes.ReconciliationRequest return nil } -func configureDependencies(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { +func ConfigureDependencies(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { if rr.Release.Name == cluster.OpenDataHub { return nil } + // Check for nil client + if rr.Client == nil { + return errors.New("client cannot be nil") + } + // Fetch application namespace from DSCI. appNamespace, err := cluster.ApplicationNamespace(ctx, rr.Client) if err != nil { return err } - err = rr.AddResources(&corev1.Secret{ + anacondaSecret := &corev1.Secret{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Secret", @@ -92,8 +106,14 @@ func configureDependencies(ctx context.Context, rr *odhtypes.ReconciliationReque Namespace: appNamespace, }, Type: corev1.SecretTypeOpaque, - }) + } + // Check if the resource already exists to avoid duplicates + if resourceExists(rr.Resources, anacondaSecret) { + return nil + } + + err = rr.AddResources(anacondaSecret) if err != nil { return fmt.Errorf("failed to create access-secret for anaconda: %w", err) } @@ -101,10 +121,26 @@ func configureDependencies(ctx context.Context, rr *odhtypes.ReconciliationReque return nil } -func updateStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { +func UpdateStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + if rr == nil { + return errors.New("reconciliation request is nil") + } + + if rr.Instance == nil { + return errors.New("instance is nil") + } + + if rr.Client == nil { + return errors.New("client is nil") + } + + if rr.Instance == nil { + return errors.New("instance is nil") + } + d, ok := rr.Instance.(*componentApi.Dashboard) if !ok { - return errors.New("instance is not of type *odhTypes.Dashboard") + return errors.New("instance is not of type *componentApi.Dashboard") } // Fetch application namespace from DSCI. @@ -130,13 +166,20 @@ func updateStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error d.Status.URL = "" if len(rl.Items) == 1 { - d.Status.URL = resources.IngressHost(rl.Items[0]) + host := resources.IngressHost(rl.Items[0]) + if host != "" { + d.Status.URL = "https://" + host + } } return nil } -func reconcileHardwareProfiles(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { +func ReconcileHardwareProfiles(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + if rr.Client == nil { + return errors.New("client is nil") + } + // If the dashboard HWP CRD doesn't exist, skip any migration logic dashHwpCRDExists, err := cluster.HasCRD(ctx, rr.Client, gvk.DashboardHardwareProfile) if err != nil { @@ -156,38 +199,47 @@ func reconcileHardwareProfiles(ctx context.Context, rr *odhtypes.ReconciliationR logger := log.FromContext(ctx) for _, hwprofile := range dashboardHardwareProfiles.Items { - var dashboardHardwareProfile DashboardHardwareProfile - - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(hwprofile.Object, &dashboardHardwareProfile); err != nil { - return fmt.Errorf("failed to convert dashboard hardware profile: %w", err) + if err := ProcessHardwareProfile(ctx, rr, logger, hwprofile); err != nil { + return err } + } + return nil +} - infraHWP := &infrav1.HardwareProfile{} - err := rr.Client.Get(ctx, client.ObjectKey{ - Name: dashboardHardwareProfile.Name, - Namespace: dashboardHardwareProfile.Namespace, - }, infraHWP) +// ProcessHardwareProfile processes a single dashboard hardware profile. +func ProcessHardwareProfile(ctx context.Context, rr *odhtypes.ReconciliationRequest, logger logr.Logger, hwprofile unstructured.Unstructured) error { + var dashboardHardwareProfile DashboardHardwareProfile - if k8serr.IsNotFound(err) { - if err = createInfraHWP(ctx, rr, logger, &dashboardHardwareProfile); err != nil { - return fmt.Errorf("failed to create infrastructure hardware profile: %w", err) - } - continue - } + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(hwprofile.Object, &dashboardHardwareProfile); err != nil { + return fmt.Errorf("failed to convert dashboard hardware profile: %w", err) + } - if err != nil { - return fmt.Errorf("failed to get infrastructure hardware profile: %w", err) - } + infraHWP := &infrav1.HardwareProfile{} + err := rr.Client.Get(ctx, client.ObjectKey{ + Name: dashboardHardwareProfile.Name, + Namespace: dashboardHardwareProfile.Namespace, + }, infraHWP) - err = updateInfraHWP(ctx, rr, logger, &dashboardHardwareProfile, infraHWP) - if err != nil { - return fmt.Errorf("failed to update existing infrastructure hardware profile: %w", err) + if k8serr.IsNotFound(err) { + if err = CreateInfraHWP(ctx, rr, logger, &dashboardHardwareProfile); err != nil { + return fmt.Errorf("failed to create infrastructure hardware profile: %w", err) } + return nil + } + + if err != nil { + return fmt.Errorf("failed to get infrastructure hardware profile: %w", err) + } + + err = UpdateInfraHWP(ctx, rr, logger, &dashboardHardwareProfile, infraHWP) + if err != nil { + return fmt.Errorf("failed to update existing infrastructure hardware profile: %w", err) } + return nil } -func createInfraHWP(ctx context.Context, rr *odhtypes.ReconciliationRequest, logger logr.Logger, dashboardhwp *DashboardHardwareProfile) error { +func CreateInfraHWP(ctx context.Context, rr *odhtypes.ReconciliationRequest, logger logr.Logger, dashboardhwp *DashboardHardwareProfile) error { annotations := make(map[string]string) maps.Copy(annotations, dashboardhwp.Annotations) @@ -222,7 +274,7 @@ func createInfraHWP(ctx context.Context, rr *odhtypes.ReconciliationRequest, log return nil } -func updateInfraHWP( +func UpdateInfraHWP( ctx context.Context, rr *odhtypes.ReconciliationRequest, logger logr.Logger, dashboardhwp *DashboardHardwareProfile, infrahwp *infrav1.HardwareProfile) error { if infrahwp.Annotations == nil { infrahwp.Annotations = make(map[string]string) @@ -251,3 +303,53 @@ func updateInfraHWP( logger.Info("successfully updated infrastructure hardware profile", "name", infrahwp.GetName()) return nil } + +func CustomizeResources(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { + if rr == nil { + return errors.New("reconciliation request is nil") + } + + // Add the opendatahub.io/managed annotation to OdhDashboardConfig resources + for i := range rr.Resources { + resource := &rr.Resources[i] + + // Check if this is an OdhDashboardConfig resource + if resource.GetObjectKind().GroupVersionKind() == gvk.OdhDashboardConfig { + // Get current annotations + currentAnnotations := resource.GetAnnotations() + if currentAnnotations == nil { + currentAnnotations = make(map[string]string) + } + + // Set the managed annotation to false for OdhDashboardConfig resources + // This indicates that the resource should not be reconciled by the operator + currentAnnotations[annotations.ManagedByODHOperator] = "false" + + // Set the annotations back + resource.SetAnnotations(currentAnnotations) + } + } + + return nil +} + +// resourceExists checks if a resource with the same name, namespace, and kind already exists in the Resources slice. +func resourceExists(resources []unstructured.Unstructured, obj client.Object) bool { + if obj == nil { + return false + } + + objName := obj.GetName() + objNamespace := obj.GetNamespace() + objKind := obj.GetObjectKind().GroupVersionKind().Kind + + for _, resource := range resources { + if resource.GetName() == objName && + resource.GetNamespace() == objNamespace && + resource.GetKind() == objKind { + return true + } + } + + return false +} diff --git a/internal/controller/components/dashboard/dashboard_controller_actions_test.go b/internal/controller/components/dashboard/dashboard_controller_actions_test.go deleted file mode 100644 index cf115fea0b3b..000000000000 --- a/internal/controller/components/dashboard/dashboard_controller_actions_test.go +++ /dev/null @@ -1,149 +0,0 @@ -//nolint:testpackage -package dashboard - -import ( - "testing" - - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - - infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/api/infrastructure/v1" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/scheme" - - . "github.com/onsi/gomega" -) - -func TestMigrateHardwareProfiles(t *testing.T) { - ctx := t.Context() - g := NewWithT(t) - - fakeSchema, err := scheme.New() - g.Expect(err).ShouldNot(HaveOccurred()) - - dashboardHardwareProfileListGVK := schema.GroupVersionKind{ - Group: "dashboard.opendatahub.io", - Version: "v1alpha1", - Kind: "HardwareProfileList", - } - - fakeSchema.AddKnownTypeWithName(gvk.DashboardHardwareProfile, &unstructured.Unstructured{}) - fakeSchema.AddKnownTypeWithName(dashboardHardwareProfileListGVK, &unstructured.UnstructuredList{}) - fakeSchema.AddKnownTypeWithName(gvk.HardwareProfile, &infrav1.HardwareProfile{}) - fakeSchema.AddKnownTypeWithName(gvk.HardwareProfile.GroupVersion().WithKind("HardwareProfileList"), &infrav1.HardwareProfileList{}) - - // Create a CRD for Dashboard HardwareProfile to make HasCRD check pass - dashboardHWPCRD := &apiextensionsv1.CustomResourceDefinition{ - ObjectMeta: v1.ObjectMeta{ - Name: "hardwareprofiles.dashboard.opendatahub.io", - }, - Status: apiextensionsv1.CustomResourceDefinitionStatus{ - StoredVersions: []string{gvk.DashboardHardwareProfile.Version}, - }, - } - - mockDashboardHardwareProfile := &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "dashboard.opendatahub.io/v1alpha1", - "kind": "HardwareProfile", - "metadata": map[string]any{ - "name": "test-name", - "namespace": "test-namespace", - }, - "spec": map[string]any{ - "displayName": "Test Display Name", - "enabled": true, - "description": "Test Description", - "tolerations": []any{}, - "nodeSelector": map[string]any{}, - "identifiers": []any{}, - }, - }, - } - - cli, err := fakeclient.New( - fakeclient.WithObjects(mockDashboardHardwareProfile, dashboardHWPCRD), - fakeclient.WithScheme(fakeSchema), - ) - g.Expect(err).ShouldNot(HaveOccurred()) - rr := &types.ReconciliationRequest{ - Client: cli, - } - - err = reconcileHardwareProfiles(ctx, rr) - g.Expect(err).ShouldNot(HaveOccurred()) - - var createdInfraHWProfile infrav1.HardwareProfile - err = cli.Get(ctx, client.ObjectKey{ - Name: "test-name", - Namespace: "test-namespace", - }, &createdInfraHWProfile) - - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(createdInfraHWProfile.Name).Should(Equal("test-name")) - g.Expect(createdInfraHWProfile.Namespace).Should(Equal("test-namespace")) - g.Expect(createdInfraHWProfile.Spec.SchedulingSpec.SchedulingType).Should(Equal(infrav1.NodeScheduling)) - g.Expect(createdInfraHWProfile.GetAnnotations()["opendatahub.io/display-name"]).Should(Equal("Test Display Name")) - g.Expect(createdInfraHWProfile.GetAnnotations()["opendatahub.io/description"]).Should(Equal("Test Description")) - g.Expect(createdInfraHWProfile.GetAnnotations()["opendatahub.io/disabled"]).Should(Equal("false")) -} - -func TestCreateInfraHardwareProfile(t *testing.T) { - ctx := t.Context() - g := NewWithT(t) - - fakeSchema, err := scheme.New() - g.Expect(err).ShouldNot(HaveOccurred()) - - fakeSchema.AddKnownTypeWithName(gvk.HardwareProfile, &infrav1.HardwareProfile{}) - fakeSchema.AddKnownTypeWithName(gvk.HardwareProfile.GroupVersion().WithKind("HardwareProfileList"), &infrav1.HardwareProfileList{}) - cli, err := fakeclient.New( - fakeclient.WithObjects(), - fakeclient.WithScheme(fakeSchema), - ) - g.Expect(err).ShouldNot(HaveOccurred()) - - rr := &types.ReconciliationRequest{ - Client: cli, - } - - logger := log.FromContext(ctx) - - mockDashboardHardwareProfile := &DashboardHardwareProfile{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-name", - Namespace: "test-namespace", - }, - Spec: DashboardHardwareProfileSpec{ - DisplayName: "Test Display Name", - Enabled: true, - Description: "Test Description", - Tolerations: nil, - NodeSelector: nil, - Identifiers: nil, - }, - } - - var receivedHardwareProfile infrav1.HardwareProfile - - err = createInfraHWP(ctx, rr, logger, mockDashboardHardwareProfile) - g.Expect(err).ShouldNot(HaveOccurred()) - - err = cli.Get(ctx, client.ObjectKey{ - Name: "test-name", - Namespace: "test-namespace", - }, &receivedHardwareProfile) - g.Expect(err).ShouldNot(HaveOccurred()) - - g.Expect(receivedHardwareProfile.Name).Should(Equal("test-name")) - g.Expect(receivedHardwareProfile.Namespace).Should(Equal("test-namespace")) - g.Expect(receivedHardwareProfile.GetAnnotations()["opendatahub.io/display-name"]).Should(Equal("Test Display Name")) - g.Expect(receivedHardwareProfile.GetAnnotations()["opendatahub.io/description"]).Should(Equal("Test Description")) - g.Expect(receivedHardwareProfile.GetAnnotations()["opendatahub.io/disabled"]).Should(Equal("false")) -} diff --git a/internal/controller/components/dashboard/dashboard_controller_component_handler_test.go b/internal/controller/components/dashboard/dashboard_controller_component_handler_test.go new file mode 100644 index 000000000000..38150986f209 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_component_handler_test.go @@ -0,0 +1,318 @@ +// This file contains tests for dashboard component handler methods. +// These tests verify the core component handler interface methods. +package dashboard_test + +import ( + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/api/components/v1alpha1" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/api/datasciencecluster/v1" + dscv2 "github.com/opendatahub-io/opendatahub-operator/v2/api/datasciencecluster/v2" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/status" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + + . "github.com/onsi/gomega" +) + +const ( + creatingFakeClientMsg = "creating fake client" +) + +// createComponentTestScheme creates a scheme with the necessary types for testing. +func createComponentTestScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + _ = componentApi.AddToScheme(scheme) + _ = dscv2.AddToScheme(scheme) + return scheme +} + +func TestComponentHandlerGetName(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + handler := getDashboardHandler() + name := handler.GetName() + + g.Expect(name).Should(Equal(componentApi.DashboardComponentName)) +} + +func TestComponentHandlerNewCRObject(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + state operatorv1.ManagementState + validate func(t *testing.T, cr *componentApi.Dashboard) + }{ + { + name: "ValidDSCWithManagedState", + state: operatorv1.Managed, + validate: func(t *testing.T, cr *componentApi.Dashboard) { + t.Helper() + g := NewWithT(t) + g.Expect(cr.Name).Should(Equal(componentApi.DashboardInstanceName)) + g.Expect(cr.Kind).Should(Equal(componentApi.DashboardKind)) + g.Expect(cr.APIVersion).Should(Equal(componentApi.GroupVersion.String())) + g.Expect(cr.Annotations).Should(HaveKeyWithValue(annotations.ManagementStateAnnotation, "Managed")) + }, + }, + { + name: "ValidDSCWithUnmanagedState", + state: operatorv1.Unmanaged, + validate: func(t *testing.T, cr *componentApi.Dashboard) { + t.Helper() + g := NewWithT(t) + g.Expect(cr.Name).Should(Equal(componentApi.DashboardInstanceName)) + g.Expect(cr.Annotations).Should(HaveKeyWithValue(annotations.ManagementStateAnnotation, "Unmanaged")) + }, + }, + { + name: "ValidDSCWithRemovedState", + state: operatorv1.Removed, + validate: func(t *testing.T, cr *componentApi.Dashboard) { + t.Helper() + g := NewWithT(t) + g.Expect(cr.Name).Should(Equal(componentApi.DashboardInstanceName)) + g.Expect(cr.Annotations).Should(HaveKeyWithValue(annotations.ManagementStateAnnotation, "Removed")) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + handler := getDashboardHandler() + dsc := CreateDSCWithDashboard(tc.state) + + cr := handler.NewCRObject(dsc) + + g := NewWithT(t) + g.Expect(cr).ShouldNot(BeNil()) + if tc.validate != nil { + if dashboard, ok := cr.(*componentApi.Dashboard); ok { + tc.validate(t, dashboard) + } + } + }) + } +} + +func TestComponentHandlerIsEnabled(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + state operatorv1.ManagementState + expected bool + }{ + { + name: "ManagedState", + state: operatorv1.Managed, + expected: true, + }, + { + name: "UnmanagedState", + state: operatorv1.Unmanaged, + expected: false, + }, + { + name: "RemovedState", + state: operatorv1.Removed, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + handler := getDashboardHandler() + dsc := CreateDSCWithDashboard(tc.state) + + result := handler.IsEnabled(dsc) + + g := NewWithT(t) + g.Expect(result).Should(Equal(tc.expected)) + }) + } +} + +func TestComponentHandlerUpdateDSCStatus(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + setupRR func(t *testing.T) *odhtypes.ReconciliationRequest + expectError bool + expectedStatus metav1.ConditionStatus + validateResult func(t *testing.T, dsc *dscv1.DataScienceCluster, status metav1.ConditionStatus) + }{ + { + name: "NilClient", + setupRR: setupNilClientRR, + expectError: true, + expectedStatus: metav1.ConditionUnknown, + }, + { + name: "DashboardCRExistsAndEnabled", + setupRR: setupDashboardExistsRR, + expectError: false, + expectedStatus: metav1.ConditionTrue, + validateResult: validateDashboardExists, + }, + { + name: "DashboardCRNotExists", + setupRR: setupDashboardNotExistsRR, + expectError: false, + expectedStatus: metav1.ConditionFalse, + validateResult: validateDashboardNotExists, + }, + { + name: "DashboardDisabled", + setupRR: setupDashboardDisabledRR, + expectError: false, + expectedStatus: metav1.ConditionUnknown, + validateResult: validateDashboardDisabled, + }, + { + name: "InvalidInstanceType", + setupRR: setupInvalidInstanceRR, + expectError: true, + expectedStatus: metav1.ConditionUnknown, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + handler := getDashboardHandler() + rr := tc.setupRR(t) + + status, err := handler.UpdateDSCStatus(t.Context(), rr) + + g := NewWithT(t) + if tc.expectError { + g.Expect(err).Should(HaveOccurred()) + } else { + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(status).Should(Equal(tc.expectedStatus)) + } + + if tc.validateResult != nil && !tc.expectError { + dsc, ok := rr.Instance.(*dscv1.DataScienceCluster) + if ok { + tc.validateResult(t, dsc, status) + } + } + }) + } +} + +// Helper functions to reduce cognitive complexity. +func setupNilClientRR(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + return &odhtypes.ReconciliationRequest{ + Client: nil, + Instance: &dscv1.DataScienceCluster{}, + } +} + +func setupDashboardExistsRR(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli, err := fakeclient.New(fakeclient.WithScheme(createComponentTestScheme())) + require.NoError(t, err, creatingFakeClientMsg) + dashboard := &componentApi.Dashboard{ + ObjectMeta: metav1.ObjectMeta{ + Name: componentApi.DashboardInstanceName, + // Dashboard CR is cluster-scoped, so no namespace + }, + Status: componentApi.DashboardStatus{ + Status: common.Status{ + Conditions: []common.Condition{ + { + Type: status.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: "ComponentReady", + }, + }, + }, + DashboardCommonStatus: componentApi.DashboardCommonStatus{ + URL: "https://odh-dashboard-test-namespace.apps.example.com", + }, + }, + } + require.NoError(t, cli.Create(t.Context(), dashboard), "creating dashboard") + + dsc := CreateDSCWithDashboard(operatorv1.Managed) + + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: &conditions.Manager{}, + } +} + +func setupDashboardNotExistsRR(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli, err := fakeclient.New(fakeclient.WithScheme(createComponentTestScheme())) + require.NoError(t, err, creatingFakeClientMsg) + dsc := CreateDSCWithDashboard(operatorv1.Managed) + + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: &conditions.Manager{}, + } +} + +func setupDashboardDisabledRR(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli, err := fakeclient.New(fakeclient.WithScheme(createComponentTestScheme())) + require.NoError(t, err, creatingFakeClientMsg) + dsc := CreateDSCWithDashboard(operatorv1.Unmanaged) + + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: &conditions.Manager{}, + } +} + +func setupInvalidInstanceRR(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli, err := fakeclient.New(fakeclient.WithScheme(createComponentTestScheme())) + require.NoError(t, err, creatingFakeClientMsg) + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: CreateTestDashboard(), // Wrong type + } +} + +func validateDashboardExists(t *testing.T, dsc *dscv1.DataScienceCluster, status metav1.ConditionStatus) { + t.Helper() + g := NewWithT(t) + g.Expect(dsc.Status.InstalledComponents[dashboard.LegacyComponentNameUpstream]).Should(BeTrue()) + g.Expect(dsc.Status.Components.Dashboard.DashboardCommonStatus).ShouldNot(BeNil()) + expectedURL := "https://odh-dashboard-test-namespace.apps.example.com" + g.Expect(dsc.Status.Components.Dashboard.DashboardCommonStatus.URL).To(Equal(expectedURL)) +} + +func validateDashboardNotExists(t *testing.T, dsc *dscv1.DataScienceCluster, status metav1.ConditionStatus) { + t.Helper() + g := NewWithT(t) + g.Expect(dsc.Status.InstalledComponents[dashboard.LegacyComponentNameUpstream]).Should(BeFalse()) + g.Expect(status).Should(Equal(metav1.ConditionFalse)) +} + +func validateDashboardDisabled(t *testing.T, dsc *dscv1.DataScienceCluster, status metav1.ConditionStatus) { + t.Helper() + g := NewWithT(t) + g.Expect(dsc.Status.InstalledComponents[dashboard.LegacyComponentNameUpstream]).Should(BeFalse()) + g.Expect(status).Should(Equal(metav1.ConditionUnknown)) +} diff --git a/internal/controller/components/dashboard/dashboard_controller_customize_test.go b/internal/controller/components/dashboard/dashboard_controller_customize_test.go new file mode 100644 index 000000000000..054aec02351c --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_customize_test.go @@ -0,0 +1,217 @@ +package dashboard_test + +import ( + "fmt" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + dashboardctrl "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + + . "github.com/onsi/gomega" +) + +const ( + testConfigName = "test-config" +) + +// testScheme is a shared scheme for testing, initialized once. +var testScheme = createTestScheme() + +// createTestScheme creates a new scheme and adds corev1 types to it. +// It panics if AddToScheme fails to ensure test setup failures are visible. +func createTestScheme() *runtime.Scheme { + s := runtime.NewScheme() + if err := corev1.AddToScheme(s); err != nil { + panic(fmt.Sprintf("Failed to add corev1 to scheme: %v", err)) + } + return s +} + +func TestCustomizeResources(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + expectResourcesNotEmpty bool + setupRR func(t *testing.T) *odhtypes.ReconciliationRequest + validate func(t *testing.T, rr *odhtypes.ReconciliationRequest) + }{ + { + name: "WithOdhDashboardConfig", + expectResourcesNotEmpty: true, + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + g := NewWithT(t) + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create a resource with OdhDashboardConfig GVK + odhDashboardConfig := &unstructured.Unstructured{} + odhDashboardConfig.SetGroupVersionKind(gvk.OdhDashboardConfig) + odhDashboardConfig.SetName(testConfigName) + odhDashboardConfig.SetNamespace(TestNamespace) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + rr.Resources = []unstructured.Unstructured{*odhDashboardConfig} + return rr + }, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + // Check that the annotation was set + g.Expect(rr.Resources[0].GetAnnotations()).Should(HaveKey(annotations.ManagedByODHOperator)) + g.Expect(rr.Resources[0].GetAnnotations()[annotations.ManagedByODHOperator]).Should(Equal("false")) + }, + }, + { + name: "WithoutOdhDashboardConfig", + expectResourcesNotEmpty: true, + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + g := NewWithT(t) + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create a resource without OdhDashboardConfig GVK + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: testConfigName, + Namespace: TestNamespace, + }, + } + configMap.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + rr.Resources = []unstructured.Unstructured{*unstructuredFromObject(t, configMap)} + return rr + }, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + // Check that no annotation was set + g.Expect(rr.Resources[0].GetAnnotations()).ShouldNot(HaveKey(annotations.ManagedByODHOperator)) + }, + }, + { + name: "EmptyResources", + expectResourcesNotEmpty: false, + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + g := NewWithT(t) + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + rr.Resources = []unstructured.Unstructured{} + return rr + }, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + + // Assert that resources slice is empty + g.Expect(rr.Resources).Should(BeEmpty(), "Resources should be empty") + + // Verify no annotations were added to any resources (since there are none) + // This ensures the function handles empty resources gracefully + }, + }, + { + name: "MultipleResources", + expectResourcesNotEmpty: true, + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + g := NewWithT(t) + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create multiple resources, one with OdhDashboardConfig GVK + odhDashboardConfig := &unstructured.Unstructured{} + odhDashboardConfig.SetGroupVersionKind(gvk.OdhDashboardConfig) + odhDashboardConfig.SetName(testConfigName) + odhDashboardConfig.SetNamespace(TestNamespace) + + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: TestNamespace, + }, + } + configMap.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + rr.Resources = []unstructured.Unstructured{ + *unstructuredFromObject(t, configMap), + *odhDashboardConfig, + } + return rr + }, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).Should(HaveLen(2)) + + for _, resource := range rr.Resources { + if resource.GetObjectKind().GroupVersionKind() == gvk.OdhDashboardConfig && resource.GetName() == testConfigName { + g.Expect(resource.GetAnnotations()).Should(HaveKey(annotations.ManagedByODHOperator)) + g.Expect(resource.GetAnnotations()[annotations.ManagedByODHOperator]).Should(Equal("false")) + } else { + g.Expect(resource.GetAnnotations()).ShouldNot(HaveKey(annotations.ManagedByODHOperator)) + } + } + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + rr := tc.setupRR(t) + g.Expect(rr.Client).ShouldNot(BeNil(), "Client should not be nil") + if tc.expectResourcesNotEmpty { + g.Expect(rr.Resources).ShouldNot(BeEmpty(), "Resources should not be empty") + } + + ctx := t.Context() + err := dashboardctrl.CustomizeResources(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + + tc.validate(t, rr) + }) + } +} + +// Helper function to convert any object to unstructured. +func unstructuredFromObject(t *testing.T, obj client.Object) *unstructured.Unstructured { + t.Helper() + + unstructuredObj, err := resources.ObjectToUnstructured(testScheme, obj) + if err != nil { + t.Fatalf("ObjectToUnstructured failed for object %+v: %v", obj, err) + } + return unstructuredObj +} diff --git a/internal/controller/components/dashboard/dashboard_controller_dependencies_test.go b/internal/controller/components/dashboard/dashboard_controller_dependencies_test.go new file mode 100644 index 000000000000..f677776e2921 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_dependencies_test.go @@ -0,0 +1,288 @@ +// This file contains tests for dashboard controller dependencies functionality. +// These tests verify the dashboard.ConfigureDependencies function and related dependency logic. +package dashboard_test + +import ( + "context" + "strings" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + + . "github.com/onsi/gomega" +) + +const resourcesNotEmptyMsg = "Resources slice should not be empty" + +// createTestRR creates a reconciliation request with the specified namespace. +func createTestRR(namespace string) func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + return func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + // Create DSCI resource + dsci := CreateTestDSCI(namespace) + _ = cli.Create(ctx, dsci) // Ignore error - test will catch it later if needed + + dashboardInstance := CreateTestDashboard() + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + Release: common.Release{Name: cluster.SelfManagedRhoai}, + } + } +} + +func TestConfigureDependenciesBasicCases(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + setupRR func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest + expectError bool + expectPanic bool + errorContains string + validateResult func(t *testing.T, rr *odhtypes.ReconciliationRequest) + }{ + { + name: "OpenDataHub", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + dashboardInstance := CreateTestDashboard() + return CreateTestReconciliationRequest(cli, dashboardInstance, common.Release{Name: cluster.OpenDataHub}) + }, + expectError: false, + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).Should(BeEmpty()) + }, + }, + { + name: "SelfManagedRhoai", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + _ = cli.Create(ctx, dsci) // Ignore error - test will catch it later if needed + + dashboardInstance := CreateTestDashboard() + return CreateTestReconciliationRequest(cli, dashboardInstance, common.Release{Name: cluster.SelfManagedRhoai}) + }, + expectError: false, + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).ShouldNot(BeEmpty(), resourcesNotEmptyMsg) + secret := rr.Resources[0] + g.Expect(secret.GetKind()).Should(Equal("Secret")) + g.Expect(secret.GetName()).Should(Equal(dashboard.AnacondaSecretName)) + g.Expect(secret.GetNamespace()).Should(Equal(TestNamespace)) + }, + }, + { + name: "ManagedRhoai", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + _ = cli.Create(ctx, dsci) // Ignore error - test will catch it later if needed + + dashboardInstance := CreateTestDashboard() + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + Release: common.Release{Name: cluster.ManagedRhoai}, + } + }, + expectError: false, + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).ShouldNot(BeEmpty(), resourcesNotEmptyMsg) + g.Expect(rr.Resources[0].GetName()).Should(Equal(dashboard.AnacondaSecretName)) + g.Expect(rr.Resources[0].GetNamespace()).Should(Equal(TestNamespace)) + }, + }, + { + name: "WithEmptyNamespace", + // Empty namespace + setupRR: createTestRR(""), + expectError: false, + errorContains: "", + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).Should(HaveLen(1)) + g.Expect(rr.Resources).ShouldNot(BeEmpty(), resourcesNotEmptyMsg) + g.Expect(rr.Resources[0].GetName()).Should(Equal(dashboard.AnacondaSecretName)) + g.Expect(rr.Resources[0].GetNamespace()).Should(Equal("")) + }, + }, + { + name: "SecretProperties", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + _ = cli.Create(ctx, dsci) // Ignore error - test will catch it later if needed + + dashboardInstance := CreateTestDashboard() + return CreateTestReconciliationRequest(cli, dashboardInstance, common.Release{Name: cluster.SelfManagedRhoai}) + }, + expectError: false, + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).ShouldNot(BeEmpty(), resourcesNotEmptyMsg) + validateSecretProperties(t, &rr.Resources[0], dashboard.AnacondaSecretName, TestNamespace) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + cli := CreateTestClient(t) + rr := tc.setupRR(cli, ctx) + runDependencyTest(t, ctx, tc, rr) + }) + } +} + +func TestConfigureDependenciesErrorCases(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + setupRR func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest + expectError bool + expectPanic bool + errorContains string + validateResult func(t *testing.T, rr *odhtypes.ReconciliationRequest) + }{ + { + name: "NilDSCI", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + dashboardInstance := CreateTestDashboard() + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + Release: common.Release{Name: cluster.SelfManagedRhoai}, + } + }, + expectError: true, + errorContains: "DSCI not found", + }, + { + name: "SpecialCharactersInNamespace", + setupRR: createTestRR("test-namespace-with-special-chars!@#$%"), + expectError: false, + errorContains: "", + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).Should(HaveLen(1)) + g.Expect(rr.Resources[0].GetName()).Should(Equal(dashboard.AnacondaSecretName)) + g.Expect(rr.Resources[0].GetNamespace()).Should(Equal("test-namespace-with-special-chars!@#$%")) + }, + }, + { + name: "LongNamespace", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + // Create DSCI resource + dsci := CreateTestDSCI(strings.Repeat("a", 1000)) + _ = cli.Create(ctx, dsci) // Ignore error - test will catch it later if needed + + dashboardInstance := CreateTestDashboard() + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + Release: common.Release{Name: cluster.SelfManagedRhoai}, + } + }, + expectError: false, + errorContains: "", + validateResult: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + g.Expect(rr.Resources).Should(HaveLen(1)) + g.Expect(rr.Resources).ShouldNot(BeEmpty(), resourcesNotEmptyMsg) + g.Expect(rr.Resources[0].GetName()).Should(Equal(dashboard.AnacondaSecretName)) + g.Expect(rr.Resources[0].GetNamespace()).Should(Equal(strings.Repeat("a", 1000))) + }, + }, + { + name: "NilClient", + setupRR: func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest { + dashboardInstance := CreateTestDashboard() + return &odhtypes.ReconciliationRequest{ + Client: nil, // Nil client + Instance: dashboardInstance, + Release: common.Release{Name: cluster.SelfManagedRhoai}, + } + }, + expectError: true, + errorContains: "client cannot be nil", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + cli := CreateTestClient(t) + rr := tc.setupRR(cli, ctx) + runDependencyTest(t, ctx, tc, rr) + }) + } +} + +// runDependencyTest executes a single dependency test case. +func runDependencyTest(t *testing.T, ctx context.Context, tc struct { + name string + setupRR func(cli client.Client, ctx context.Context) *odhtypes.ReconciliationRequest + expectError bool + expectPanic bool + errorContains string + validateResult func(t *testing.T, rr *odhtypes.ReconciliationRequest) +}, rr *odhtypes.ReconciliationRequest) { + t.Helper() + g := NewWithT(t) + + if tc.expectPanic { + AssertPanics(t, func() { + _ = dashboard.ConfigureDependencies(ctx, rr) + }, "dashboard.ConfigureDependencies should panic") + return + } + + err := dashboard.ConfigureDependencies(ctx, rr) + + if tc.expectError { + g.Expect(err).Should(HaveOccurred()) + if tc.errorContains != "" { + g.Expect(err.Error()).Should(ContainSubstring(tc.errorContains)) + } + } else { + g.Expect(err).ShouldNot(HaveOccurred()) + } + + if tc.validateResult != nil { + tc.validateResult(t, rr) + } +} + +// validateSecretProperties validates secret properties for the specific test case. +func validateSecretProperties(t *testing.T, secret *unstructured.Unstructured, expectedName, expectedNamespace string) { + t.Helper() + g := NewWithT(t) + g.Expect(secret.GetAPIVersion()).Should(Equal("v1")) + g.Expect(secret.GetKind()).Should(Equal("Secret")) + g.Expect(secret.GetName()).Should(Equal(expectedName)) + g.Expect(secret.GetNamespace()).Should(Equal(expectedNamespace)) + + // Check the type field in the object + secretType, found, err := unstructured.NestedString(secret.Object, "type") + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(found).Should(BeTrue()) + g.Expect(secretType).Should(Equal("Opaque")) +} diff --git a/internal/controller/components/dashboard/dashboard_controller_hardware_profiles_test.go b/internal/controller/components/dashboard/dashboard_controller_hardware_profiles_test.go new file mode 100644 index 000000000000..5e0da8f658d4 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_hardware_profiles_test.go @@ -0,0 +1,818 @@ +package dashboard_test + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/log" + + infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/api/infrastructure/v1" + dashboardctrl "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" +) + +const ( + displayNameKey = "opendatahub.io/display-name" + descriptionKey = "opendatahub.io/description" + disabledKey = "opendatahub.io/disabled" + customKey = "custom-annotation" + customValue = "custom-value" + invalidSpec = "invalid-spec" +) + +// createDashboardHWP creates a dashboardctrl.DashboardHardwareProfile unstructured object with the specified parameters. +func createDashboardHWP(tb testing.TB, name string, enabled bool, nodeType string) *unstructured.Unstructured { + tb.Helper() + // Validate required parameters + if name == "" { + tb.Fatalf("name parameter cannot be empty") + } + if nodeType == "" { + tb.Fatalf("nodeType parameter cannot be empty") + } + + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName(name) + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = map[string]interface{}{ + "displayName": fmt.Sprintf("Display Name for %s", name), + "enabled": enabled, + "description": fmt.Sprintf("Description for %s", name), + "nodeSelector": map[string]interface{}{ + NodeTypeKey: nodeType, + }, + } + + return dashboardHWP +} + +func TestReconcileHardwareProfiles(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + setupRR func(t *testing.T) *odhtypes.ReconciliationRequest + expectError bool + validate func(t *testing.T, rr *odhtypes.ReconciliationRequest) + }{ + { + name: "CRDNotExists", + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + return rr + }, + expectError: false, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + // No specific validation needed for CRD not exists case + }, + }, + { + name: "WithValidProfiles", + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + // Create multiple mock dashboard hardware profiles + profile1 := createDashboardHWP(t, "profile1", true, "gpu") + profile2 := createDashboardHWP(t, "profile2", true, "cpu") + profile3 := createDashboardHWP(t, "profile3", false, "cpu") // Disabled profile + + cli, err := fakeclient.New(fakeclient.WithObjects(profile1, profile2, profile3)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + return rr + }, + expectError: false, + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + // Test ProcessHardwareProfile directly for each profile to bypass CRD check + profile1 := createDashboardHWP(t, "profile1", true, "gpu") + profile2 := createDashboardHWP(t, "profile2", true, "cpu") + profile3 := createDashboardHWP(t, "profile3", false, "cpu") + + ctx := t.Context() + logger := log.FromContext(ctx) + + g := gomega.NewWithT(t) + err := dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *profile1) + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *profile2) + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Disabled profile should not be processed + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *profile3) + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + }, + }, + { + name: "WithConversionError", + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + // This test verifies that ProcessHardwareProfile returns an error + // when the dashboard hardware profile has an invalid spec that cannot be converted. + // We test ProcessHardwareProfile directly to avoid CRD check issues. + + // Create a mock dashboard hardware profile with invalid spec + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName("invalid-profile") + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = invalidSpec // Invalid spec type + + cli, err := fakeclient.New(fakeclient.WithObjects(dashboardHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + return rr + }, + expectError: false, // ReconcileHardwareProfiles won't fail, but ProcessHardwareProfile will + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + // Test ProcessHardwareProfile directly to verify conversion error + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName("invalid-profile") + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = invalidSpec // Invalid spec type + + ctx := t.Context() + logger := log.FromContext(ctx) + + g := gomega.NewWithT(t) + err := dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + g.Expect(err).Should(gomega.HaveOccurred()) + g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to convert dashboard hardware profile")) + }, + }, + { + name: "WithCreateError", + setupRR: func(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + // This test verifies that the hardware profile processing returns an error + // when the Create operation fails for HardwareProfile objects. + // We test the ProcessHardwareProfile function directly to avoid CRD check issues. + + // Create a mock dashboard hardware profile + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName("test-profile") + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = map[string]interface{}{ + "displayName": TestDisplayName, + "enabled": true, + "description": TestDescription, + "nodeSelector": map[string]interface{}{ + NodeTypeKey: "gpu", + }, + } + + // Create a mock client that will fail on Create operations for HardwareProfile objects + cli, err := fakeclient.New( + fakeclient.WithObjects(dashboardHWP), + fakeclient.WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + // Fail on Create operations for HardwareProfile objects + t.Logf("Create interceptor called for object: %s, type: %T", obj.GetName(), obj) + + // Check if it's an infrastructure HardwareProfile by type + if _, ok := obj.(*infrav1.HardwareProfile); ok { + t.Logf("Triggering create error for HardwareProfile") + return errors.New("simulated create error") + } + return client.Create(ctx, obj, opts...) + }, + }), + ) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + return rr + }, + expectError: false, // ReconcileHardwareProfiles won't fail, but ProcessHardwareProfile will + validate: func(t *testing.T, rr *odhtypes.ReconciliationRequest) { + t.Helper() + // Test ProcessHardwareProfile directly to verify create error + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName("test-profile") + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = map[string]interface{}{ + "displayName": TestDisplayName, + "enabled": true, + "description": TestDescription, + "nodeSelector": map[string]interface{}{ + NodeTypeKey: "gpu", + }, + } + + ctx := t.Context() + logger := log.FromContext(ctx) + + g := gomega.NewWithT(t) + err := dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + t.Logf("ProcessHardwareProfile returned error: %v", err) + g.Expect(err).Should(gomega.HaveOccurred()) + g.Expect(err.Error()).Should(gomega.ContainSubstring("simulated create error")) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := gomega.NewWithT(t) + + rr := tc.setupRR(t) + ctx := t.Context() + + err := dashboardctrl.ReconcileHardwareProfiles(ctx, rr) + + if tc.expectError { + g.Expect(err).Should(gomega.HaveOccurred()) + } else { + g.Expect(err).ShouldNot(gomega.HaveOccurred()) + } + + tc.validate(t, rr) + }) + } +} + +// Test dashboardctrl.ProcessHardwareProfile function directly. +func TestProcessHardwareProfile(t *testing.T) { + t.Run("SuccessfulProcessing", testProcessHardwareProfileSuccessful) + t.Run("ConversionError", testProcessHardwareProfileConversionError) + t.Run("CreateNewProfile", testProcessHardwareProfileCreateNew) + t.Run("UpdateExistingProfile", testProcessHardwareProfileUpdateExisting) + t.Run("GetError", testProcessHardwareProfileGetError) +} + +func testProcessHardwareProfileSuccessful(t *testing.T) { + // Create a mock dashboard hardware profile + dashboardHWP := createDashboardHWP(t, TestProfile, true, "gpu") + + // Create an existing infrastructure hardware profile + existingInfraHWP := &infrav1.HardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: infrav1.HardwareProfileSpec{ + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: TestDisplayName, + Identifier: "gpu", + MinCount: intstr.FromInt32(1), + DefaultCount: intstr.FromInt32(1), + ResourceType: "Accelerator", + }, + }, + }, + } + + cli, err := fakeclient.New(fakeclient.WithObjects(existingInfraHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) +} + +func testProcessHardwareProfileConversionError(t *testing.T) { + // Create a mock dashboard hardware profile with invalid spec + dashboardHWP := &unstructured.Unstructured{} + dashboardHWP.SetGroupVersionKind(gvk.DashboardHardwareProfile) + dashboardHWP.SetName(TestProfile) + dashboardHWP.SetNamespace(TestNamespace) + dashboardHWP.Object["spec"] = "invalid-spec" // Invalid spec type + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + gomega.NewWithT(t).Expect(err).Should(gomega.HaveOccurred()) + gomega.NewWithT(t).Expect(err.Error()).Should(gomega.ContainSubstring("failed to convert dashboard hardware profile")) +} + +func testProcessHardwareProfileCreateNew(t *testing.T) { + // Create a mock dashboard hardware profile + dashboardHWP := createDashboardHWP(t, TestProfile, true, "gpu") + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) +} + +func testProcessHardwareProfileUpdateExisting(t *testing.T) { + // Create a mock dashboard hardware profile with different specs + dashboardHWP := createDashboardHWP(t, "updated-profile", true, "cpu") + + // Create an existing infrastructure hardware profile + existingInfraHWP := &infrav1.HardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "updated-profile", + Namespace: TestNamespace, + }, + Spec: infrav1.HardwareProfileSpec{ + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: "Updated Profile", + Identifier: "cpu", + MinCount: intstr.FromInt32(1), + DefaultCount: intstr.FromInt32(1), + ResourceType: "CPU", + }, + }, + }, + } + + cli, err := fakeclient.New(fakeclient.WithObjects(existingInfraHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + // This should update the existing profile + err = dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) +} + +func testProcessHardwareProfileGetError(t *testing.T) { + // Create a mock dashboard hardware profile + dashboardHWP := createDashboardHWP(t, TestProfile, true, "gpu") + + // Create a mock client that returns a controlled Get error + expectedError := errors.New("mock client Get error") + mockClient, err := fakeclient.New( + fakeclient.WithObjects(dashboardHWP), + fakeclient.WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return expectedError + }, + }), + ) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = mockClient + + ctx := t.Context() + logger := log.FromContext(ctx) + + // This should return the expected error from the mock client + processErr := dashboardctrl.ProcessHardwareProfile(ctx, rr, logger, *dashboardHWP) + gomega.NewWithT(t).Expect(processErr).Should(gomega.HaveOccurred()) + gomega.NewWithT(t).Expect(processErr.Error()).Should(gomega.ContainSubstring("failed to get infrastructure hardware profile")) + gomega.NewWithT(t).Expect(processErr.Error()).Should(gomega.ContainSubstring(expectedError.Error())) +} + +// Test dashboardctrl.CreateInfraHWP function directly. +func TestCreateInfraHWP(t *testing.T) { + t.Run("SuccessfulCreation", testCreateInfraHWPSuccessful) + t.Run("WithAnnotations", testCreateInfraHWPWithAnnotations) + t.Run("WithTolerations", testCreateInfraHWPWithTolerations) + t.Run("WithIdentifiers", testCreateInfraHWPWithIdentifiers) +} + +func testCreateInfraHWPSuccessful(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + }, + } + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.CreateInfraHWP(ctx, rr, logger, dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Verify the created InfrastructureHardwareProfile + var infraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert the created object's fields match expectations + gomega.NewWithT(t).Expect(infraHWP.Annotations[displayNameKey]).Should(gomega.Equal(TestDisplayName)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[descriptionKey]).Should(gomega.Equal(TestDescription)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[disabledKey]).Should(gomega.Equal("false")) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(map[string]string{NodeTypeKey: "gpu"})) +} + +func testCreateInfraHWPWithAnnotations(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + Annotations: map[string]string{ + customKey: customValue, + }, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + }, + } + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.CreateInfraHWP(ctx, rr, logger, dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Verify the created InfrastructureHardwareProfile + var infraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert the created object's fields match expectations + gomega.NewWithT(t).Expect(infraHWP.Annotations[displayNameKey]).Should(gomega.Equal(TestDisplayName)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[descriptionKey]).Should(gomega.Equal(TestDescription)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[disabledKey]).Should(gomega.Equal("false")) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(map[string]string{NodeTypeKey: "gpu"})) + + // Assert the custom annotation exists and equals customValue + gomega.NewWithT(t).Expect(infraHWP.Annotations[customKey]).Should(gomega.Equal(customValue)) +} + +func testCreateInfraHWPWithTolerations(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + Tolerations: []corev1.Toleration{ + { + Key: NvidiaGPUKey, + Value: "true", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + }, + } + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.CreateInfraHWP(ctx, rr, logger, dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Verify the created InfrastructureHardwareProfile + var infraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert the created object's fields match expectations + gomega.NewWithT(t).Expect(infraHWP.Annotations[displayNameKey]).Should(gomega.Equal(TestDisplayName)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[descriptionKey]).Should(gomega.Equal(TestDescription)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[disabledKey]).Should(gomega.Equal("false")) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(map[string]string{NodeTypeKey: "gpu"})) + + // Assert the toleration with key NvidiaGPUKey/value "true" and effect NoSchedule is present + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.Tolerations).Should(gomega.HaveLen(1)) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.Tolerations[0].Key).Should(gomega.Equal(NvidiaGPUKey)) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.Tolerations[0].Value).Should(gomega.Equal("true")) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.Tolerations[0].Effect).Should(gomega.Equal(corev1.TaintEffectNoSchedule)) +} + +func testCreateInfraHWPWithIdentifiers(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: "GPU", + Identifier: NvidiaGPUKey, + ResourceType: "Accelerator", + }, + }, + }, + } + + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.CreateInfraHWP(ctx, rr, logger, dashboardHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Verify the created InfrastructureHardwareProfile + var infraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert the created object's fields match expectations + gomega.NewWithT(t).Expect(infraHWP.Annotations[displayNameKey]).Should(gomega.Equal(TestDisplayName)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[descriptionKey]).Should(gomega.Equal(TestDescription)) + gomega.NewWithT(t).Expect(infraHWP.Annotations[disabledKey]).Should(gomega.Equal("false")) + gomega.NewWithT(t).Expect(infraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(map[string]string{NodeTypeKey: "gpu"})) + + // Assert the identifiers slice contains the expected HardwareIdentifier + gomega.NewWithT(t).Expect(infraHWP.Spec.Identifiers).Should(gomega.HaveLen(1)) + gomega.NewWithT(t).Expect(infraHWP.Spec.Identifiers[0].DisplayName).Should(gomega.Equal("GPU")) + gomega.NewWithT(t).Expect(infraHWP.Spec.Identifiers[0].Identifier).Should(gomega.Equal(NvidiaGPUKey)) + gomega.NewWithT(t).Expect(infraHWP.Spec.Identifiers[0].ResourceType).Should(gomega.Equal("Accelerator")) +} + +// Test dashboardctrl.UpdateInfraHWP function directly. +func TestUpdateInfraHWP(t *testing.T) { + t.Run("SuccessfulUpdate", testUpdateInfraHWPSuccessful) + t.Run("WithNilAnnotations", testUpdateInfraHWPWithNilAnnotations) + t.Run("WithExistingAnnotations", testUpdateInfraHWPWithExistingAnnotations) +} + +func testUpdateInfraHWPSuccessful(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + }, + } + + infraHWP := &infrav1.HardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: infrav1.HardwareProfileSpec{ + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: TestDisplayName, + Identifier: "gpu", + MinCount: intstr.FromInt32(1), + DefaultCount: intstr.FromInt32(1), + ResourceType: "Accelerator", + }, + }, + }, + } + + cli, err := fakeclient.New(fakeclient.WithObjects(infraHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.UpdateInfraHWP(ctx, rr, logger, dashboardHWP, infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Fetch the updated HardwareProfile from the fake client + var updatedInfraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &updatedInfraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert spec fields reflect the dashboardctrl.DashboardHardwareProfile changes + gomega.NewWithT(t).Expect(updatedInfraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(dashboardHWP.Spec.NodeSelector)) + + // Assert annotations were properly set + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(displayNameKey, TestDisplayName)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(descriptionKey, TestDescription)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(disabledKey, "false")) + + // Assert resource metadata remains correct + gomega.NewWithT(t).Expect(updatedInfraHWP.Name).Should(gomega.Equal(TestProfile)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Namespace).Should(gomega.Equal(TestNamespace)) +} + +func testUpdateInfraHWPWithNilAnnotations(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + }, + } + + infraHWP := &infrav1.HardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: infrav1.HardwareProfileSpec{ + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: TestDisplayName, + Identifier: "gpu", + MinCount: intstr.FromInt32(1), + DefaultCount: intstr.FromInt32(1), + ResourceType: "Accelerator", + }, + }, + }, + } + infraHWP.Annotations = nil // Test nil annotations case + + cli, err := fakeclient.New(fakeclient.WithObjects(infraHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.UpdateInfraHWP(ctx, rr, logger, dashboardHWP, infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Fetch the updated HardwareProfile from the fake client + var updatedInfraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &updatedInfraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert spec fields reflect the dashboardctrl.DashboardHardwareProfile changes + gomega.NewWithT(t).Expect(updatedInfraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(dashboardHWP.Spec.NodeSelector)) + + // Assert annotations were properly set (nil annotations become the dashboard annotations) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).ShouldNot(gomega.BeNil()) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(displayNameKey, TestDisplayName)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(descriptionKey, TestDescription)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(disabledKey, "false")) + + // Assert resource metadata remains correct + gomega.NewWithT(t).Expect(updatedInfraHWP.Name).Should(gomega.Equal(TestProfile)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Namespace).Should(gomega.Equal(TestNamespace)) +} + +func testUpdateInfraHWPWithExistingAnnotations(t *testing.T) { + dashboardHWP := &dashboardctrl.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + Annotations: map[string]string{ + customKey: customValue, + }, + }, + Spec: dashboardctrl.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + NodeSelector: map[string]string{ + NodeTypeKey: "gpu", + }, + }, + } + + infraHWP := &infrav1.HardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + Annotations: map[string]string{ + "existing-annotation": "existing-value", + }, + }, + Spec: infrav1.HardwareProfileSpec{ + Identifiers: []infrav1.HardwareIdentifier{ + { + DisplayName: TestDisplayName, + Identifier: "gpu", + MinCount: intstr.FromInt32(1), + DefaultCount: intstr.FromInt32(1), + ResourceType: "Accelerator", + }, + }, + }, + } + + cli, err := fakeclient.New(fakeclient.WithObjects(infraHWP)) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + rr := SetupTestReconciliationRequestSimple(t) + rr.Client = cli + + ctx := t.Context() + logger := log.FromContext(ctx) + + err = dashboardctrl.UpdateInfraHWP(ctx, rr, logger, dashboardHWP, infraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Fetch the updated HardwareProfile from the fake client + var updatedInfraHWP infrav1.HardwareProfile + err = cli.Get(ctx, client.ObjectKey{Name: TestProfile, Namespace: TestNamespace}, &updatedInfraHWP) + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + + // Assert spec fields reflect the dashboardctrl.DashboardHardwareProfile changes + gomega.NewWithT(t).Expect(updatedInfraHWP.Spec.SchedulingSpec.Node.NodeSelector).Should(gomega.Equal(dashboardHWP.Spec.NodeSelector)) + + // Assert annotations were properly merged (existing annotations preserved and merged with dashboard annotations) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(displayNameKey, TestDisplayName)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(descriptionKey, TestDescription)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(disabledKey, "false")) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue("existing-annotation", "existing-value")) + gomega.NewWithT(t).Expect(updatedInfraHWP.Annotations).Should(gomega.HaveKeyWithValue(customKey, customValue)) + + // Assert resource metadata remains correct + gomega.NewWithT(t).Expect(updatedInfraHWP.Name).Should(gomega.Equal(TestProfile)) + gomega.NewWithT(t).Expect(updatedInfraHWP.Namespace).Should(gomega.Equal(TestNamespace)) +} diff --git a/internal/controller/components/dashboard/dashboard_controller_init_test.go b/internal/controller/components/dashboard/dashboard_controller_init_test.go new file mode 100644 index 000000000000..7cc19f43a1e8 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_init_test.go @@ -0,0 +1,459 @@ +// This file contains tests for dashboard controller initialization functionality. +// These tests verify the dashboard.Initialize function and related initialization logic. +package dashboard_test + +import ( + "strings" + "testing" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/api/components/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/registry" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + + . "github.com/onsi/gomega" +) + +func TestInitialize(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Test success case + t.Run("success", func(t *testing.T) { + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: CreateTestDashboard(), + Release: common.Release{Name: cluster.OpenDataHub}, + } + + err = dashboard.Initialize(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(rr.Manifests).Should(HaveLen(1)) + g.Expect(rr.Manifests[0].ContextDir).Should(Equal(dashboard.ComponentName)) + }) + + // Test cases that should now succeed since we removed unnecessary validations + testCases := []struct { + name string + setupRR func() *odhtypes.ReconciliationRequest + expectError bool + errorMsg string + }{ + { + name: "nil client", + setupRR: func() *odhtypes.ReconciliationRequest { + return &odhtypes.ReconciliationRequest{ + Client: nil, + Instance: CreateTestDashboard(), + Release: common.Release{Name: cluster.OpenDataHub}, + } + }, + expectError: false, + errorMsg: "", + }, + { + name: "nil DSCI", + setupRR: func() *odhtypes.ReconciliationRequest { + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: nil, + Release: common.Release{Name: cluster.OpenDataHub}, + } + }, + expectError: false, + errorMsg: "", + }, + { + name: "invalid Instance type", + setupRR: func() *odhtypes.ReconciliationRequest { + // Use a different type that's not *componentApi.Dashboard + invalidInstance := &componentApi.TrustyAI{} + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: invalidInstance, + Release: common.Release{Name: cluster.OpenDataHub}, + } + }, + expectError: false, + errorMsg: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rr := tc.setupRR() + initialManifests := rr.Manifests // Capture initial state + + err := dashboard.Initialize(ctx, rr) + + if tc.expectError { + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring(tc.errorMsg)) + // Ensure Manifests remains unchanged when error occurs + g.Expect(rr.Manifests).Should(Equal(initialManifests)) + } else { + g.Expect(err).ShouldNot(HaveOccurred()) + // Assert that Manifests is populated with expected content + g.Expect(rr.Manifests).ShouldNot(Equal(initialManifests)) + g.Expect(rr.Manifests).Should(HaveLen(1)) + g.Expect(rr.Manifests[0].ContextDir).Should(Equal(dashboard.ComponentName)) + g.Expect(rr.Manifests[0].Path).Should(Equal(odhdeploy.DefaultManifestPath)) + } + }) + } +} + +func TestInitErrorPaths(t *testing.T) { + handler := getDashboardHandler() + + // Test with invalid platform that might cause errors + // This tests the error handling in the Init function + platforms := []common.Platform{ + "", // Empty platform + "invalid-platform", // Invalid platform name + } + + for _, platform := range platforms { + t.Run(string(platform), func(t *testing.T) { + // The Init function should handle invalid platforms gracefully + // It might fail due to missing manifest paths, but should not panic + result := runInitWithPanicRecovery(handler, platform) + + // Validate the result - should either succeed or fail with specific error + validateInitResult(t, struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + }{ + name: string(platform), + platform: platform, + expectErrorSubstring: ErrorUnsupportedPlatform, + expectPanic: false, + }, result) + }) + } +} + +// InitResult represents the result of running Init with panic recovery. +type InitResult struct { + PanicRecovered interface{} + Err error +} + +// runInitWithPanicRecovery runs Init with panic recovery and returns the result. +func runInitWithPanicRecovery(handler registry.ComponentHandler, platform common.Platform) InitResult { + var panicRecovered interface{} + defer func() { + if r := recover(); r != nil { + panicRecovered = r + } + }() + err := handler.Init(platform) + return InitResult{ + PanicRecovered: panicRecovered, + Err: err, + } +} + +// validateInitResult validates the result of Init call based on expectations. +func validateInitResult(t *testing.T, tc struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool +}, result InitResult) { + t.Helper() + g := NewWithT(t) + + if tc.expectPanic { + g.Expect(result.PanicRecovered).ShouldNot(BeNil()) + return + } + + if result.PanicRecovered != nil { + t.Errorf(ErrorInitPanicked, tc.platform, result.PanicRecovered) + return + } + + if tc.expectErrorSubstring != "" { + if result.Err != nil { + g.Expect(result.Err.Error()).Should(ContainSubstring(tc.expectErrorSubstring)) + } else { + t.Logf("Init handled platform %s gracefully without error", tc.platform) + } + } else { + g.Expect(result.Err).ShouldNot(HaveOccurred()) + } +} + +func TestInitErrorCases(t *testing.T) { + // Test cases for platforms that should fail during Init + // These are split into two categories: + // 1. Intentionally unsupported platforms (should return error) + // 2. Test platforms missing manifest fixtures (should return error due to missing manifests) + testCases := []struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + // category indicates the reason for the expected failure + category string // "unsupported" or "missing-manifests" + }{ + { + name: "unsupported-non-existent-platform", + platform: common.Platform(NonExistentPlatform), + expectErrorSubstring: ErrorUnsupportedPlatform, + expectPanic: false, + category: "unsupported", // This platform is intentionally not supported by the dashboard component + }, + { + name: "test-platform-missing-manifests", + platform: common.Platform(TestPlatform), + expectErrorSubstring: ErrorUnsupportedPlatform, + expectPanic: false, + category: "missing-manifests", // This is a test platform that should be supported but lacks manifest fixtures + }, + } + + // Group test cases by category for better organization + unsupportedPlatforms := []struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + }{} + missingManifestPlatforms := []struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + }{} + + // Categorize test cases + for _, tc := range testCases { + switch tc.category { + case "unsupported": + unsupportedPlatforms = append(unsupportedPlatforms, struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + }{tc.name, tc.platform, tc.expectErrorSubstring, tc.expectPanic}) + case "missing-manifests": + missingManifestPlatforms = append(missingManifestPlatforms, struct { + name string + platform common.Platform + expectErrorSubstring string + expectPanic bool + }{tc.name, tc.platform, tc.expectErrorSubstring, tc.expectPanic}) + } + } + + // Test intentionally unsupported platforms + // These platforms are not supported by the dashboard component and should fail + t.Run("UnsupportedPlatforms", func(t *testing.T) { + for _, tc := range unsupportedPlatforms { + t.Run(tc.name, func(t *testing.T) { + handler := getDashboardHandler() + result := runInitWithPanicRecovery(handler, tc.platform) + validateInitResult(t, tc, result) + }) + } + }) + + // Test platforms with missing manifest fixtures + // These platforms should be supported but are missing test manifest files + // Future maintainers should either: + // 1. Add manifest fixtures for these platforms, or + // 2. Mark these tests as skipped if the platforms are not intended for testing + t.Run("MissingManifestFixtures", func(t *testing.T) { + for _, tc := range missingManifestPlatforms { + t.Run(tc.name, func(t *testing.T) { + handler := getDashboardHandler() + result := runInitWithPanicRecovery(handler, tc.platform) + validateInitResult(t, tc, result) + }) + } + }) +} + +// TestInitPlatforms tests the Init function with various platform scenarios. +// This consolidated test replaces multiple redundant tests with a single table-driven test +// that covers all platform validation scenarios in a maintainable way. +func TestInitPlatforms(t *testing.T) { + testCases := []struct { + name string + platform common.Platform + category string // "supported", "unsupported", "edge-case" + expectError bool + errorSubstring string + }{ + // Supported platforms - should succeed + { + name: "supported-self-managed-rhoai", + platform: cluster.SelfManagedRhoai, + category: "supported", + expectError: false, + errorSubstring: "", + }, + { + name: "supported-managed-rhoai", + platform: cluster.ManagedRhoai, + category: "supported", + expectError: false, + errorSubstring: "", + }, + { + name: "supported-opendatahub", + platform: cluster.OpenDataHub, + category: "supported", + expectError: false, + errorSubstring: "", + }, + + // Unsupported platforms - should fail with clear error messages + { + name: "unsupported-openshift", + platform: common.Platform("OpenShift"), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-kubernetes", + platform: common.Platform("Kubernetes"), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-non-existent-platform", + platform: common.Platform(NonExistentPlatform), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-test-platform", + platform: common.Platform(TestPlatform), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-upstream", + platform: common.Platform("upstream"), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-downstream", + platform: common.Platform("downstream"), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-self-managed-test", + platform: common.Platform(TestSelfManagedPlatform), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "unsupported-managed", + platform: common.Platform("managed"), + category: "unsupported", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + + // Edge cases - should fail with clear error messages + { + name: "edge-case-empty-platform", + platform: common.Platform(""), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-nil-like-platform", + platform: common.Platform("nil-test"), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-special-chars", + platform: common.Platform("test-platform-with-special-chars!@#$%"), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-very-long-name", + platform: common.Platform(strings.Repeat("a", 1000)), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-platform-with-dashes", + platform: common.Platform("platform-with-dashes"), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-platform-with-underscores", + platform: common.Platform("platform_with_underscores"), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + { + name: "edge-case-platform-with-dots", + platform: common.Platform("platform.with.dots"), + category: "edge-case", + expectError: true, + errorSubstring: ErrorUnsupportedPlatform, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + handler := getDashboardHandler() + + // Test that Init handles platforms without panicking + defer func() { + if r := recover(); r != nil { + t.Errorf(ErrorInitPanicked, tc.platform, r) + } + }() + + // Call Init with the test platform + err := handler.Init(tc.platform) + + // Make deterministic assertions based on expected behavior + if tc.expectError { + g.Expect(err).Should(HaveOccurred(), "Expected error for %s platform: %s", tc.category, tc.platform) + if tc.errorSubstring != "" { + g.Expect(err.Error()).Should(ContainSubstring(tc.errorSubstring), + "Expected error to contain '%s' for %s platform: %s, got: %v", tc.errorSubstring, tc.category, tc.platform, err) + } + } else { + g.Expect(err).ShouldNot(HaveOccurred(), "Expected no error for %s platform: %s, got: %v", tc.category, tc.platform, err) + } + }) + } +} diff --git a/internal/controller/components/dashboard/dashboard_controller_reconciler_test.go b/internal/controller/components/dashboard/dashboard_controller_reconciler_test.go new file mode 100644 index 000000000000..7860242801d5 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_reconciler_test.go @@ -0,0 +1,111 @@ +package dashboard_test + +import ( + "strings" + "testing" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" +) + +func TestNewComponentReconcilerUnit(t *testing.T) { + t.Parallel() + + t.Run("WithNilManager", func(t *testing.T) { + t.Parallel() + testNewComponentReconcilerWithNilManager(t) + }) + t.Run("ComponentNameComputation", func(t *testing.T) { + t.Parallel() + testComponentNameComputation(t) + }) +} + +func testNewComponentReconcilerWithNilManager(t *testing.T) { + t.Helper() + handler := getDashboardHandler() + ctx := t.Context() + + t.Run("ReturnsErrorWithNilManager", func(t *testing.T) { + t.Helper() + err := handler.NewComponentReconciler(ctx, nil) + if err == nil { + t.Fatal("Expected function to return error with nil manager, but got nil error") + } + if !strings.Contains(err.Error(), "could not create the dashboard controller") { + t.Errorf("Expected error to contain 'could not create the dashboard controller', but got: %v", err) + } + }) +} + +// testComponentNameComputation tests that the component name computation logic works correctly. +func testComponentNameComputation(t *testing.T) { + t.Helper() + + // Test cases for different release types + testCases := []struct { + name string + release common.Release + expectedResult string + }{ + { + name: "OpenDataHub", + release: common.Release{Name: cluster.OpenDataHub}, + expectedResult: dashboard.LegacyComponentNameUpstream, + }, + { + name: "SelfManagedRhoai", + release: common.Release{Name: cluster.SelfManagedRhoai}, + expectedResult: dashboard.LegacyComponentNameDownstream, + }, + { + name: "ManagedRhoai", + release: common.Release{Name: cluster.ManagedRhoai}, + expectedResult: dashboard.LegacyComponentNameDownstream, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Test the new function with explicit release parameter + componentName := dashboard.ComputeComponentNameWithRelease(tc.release) + + // Verify the component name matches expected result + if componentName != tc.expectedResult { + t.Errorf("Expected %s for release %s, got %s", tc.expectedResult, tc.name, componentName) + } + + // Verify the component name is not empty + if componentName == "" { + t.Error("Expected dashboard.ComputeComponentNameWithRelease to return non-empty string") + } + }) + } + + // Test that the original function still works (backward compatibility) + componentName := dashboard.ComputeComponentName() + if componentName == "" { + t.Error("Expected dashboard.ComputeComponentName to return non-empty string") + } + + // Verify the component name is one of the expected values + validNames := []string{dashboard.LegacyComponentNameUpstream, dashboard.LegacyComponentNameDownstream} + valid := false + for _, validName := range validNames { + if componentName == validName { + valid = true + break + } + } + + if !valid { + t.Errorf("Expected dashboard.ComputeComponentName to return one of %v, but got: %s", validNames, componentName) + } + + // Test that multiple calls return the same result (deterministic) + componentName2 := dashboard.ComputeComponentName() + if componentName != componentName2 { + t.Error("Expected dashboard.ComputeComponentName to be deterministic, but got different results") + } +} diff --git a/internal/controller/components/dashboard/dashboard_controller_status_test.go b/internal/controller/components/dashboard/dashboard_controller_status_test.go new file mode 100644 index 000000000000..7d925a6a0944 --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_controller_status_test.go @@ -0,0 +1,211 @@ +// This file contains tests for dashboard controller status update functionality. +// These tests verify the dashboard.UpdateStatus function and related status logic. +package dashboard_test + +import ( + "context" + "errors" + "strings" + "testing" + + routev1 "github.com/openshift/api/route/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/api/components/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + + . "github.com/onsi/gomega" +) + +// mockClient implements client.Client interface for testing. +type mockClient struct { + client.Client + + listError error +} + +func (m *mockClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + // Check if this is a Route list call by examining the concrete type + if _, isRouteList := list.(*routev1.RouteList); isRouteList && m.listError != nil { + return m.listError + } + return m.Client.List(ctx, list, opts...) +} + +func TestUpdateStatusNoRoutes(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + err = cli.Create(ctx, dsci) + g.Expect(err).ShouldNot(HaveOccurred()) + + dashboardInstance := CreateTestDashboard() + + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + } + + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dashboardInstance.Status.URL).Should(BeEmpty()) +} + +func TestUpdateStatusWithRoute(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + err = cli.Create(ctx, dsci) + g.Expect(err).ShouldNot(HaveOccurred()) + + dashboardInstance := CreateTestDashboard() + + // Create a route with the expected label + route := createRoute("odh-dashboard", TestRouteHost, true) + + err = cli.Create(ctx, route) + g.Expect(err).ShouldNot(HaveOccurred()) + + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + } + + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dashboardInstance.Status.URL).Should(Equal("https://" + TestRouteHost)) +} + +func TestUpdateStatusInvalidInstance(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: &componentApi.Kserve{}, // Wrong type + } + + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("is not of type *componentApi.Dashboard")) +} + +func TestUpdateStatusWithMultipleRoutes(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + err = cli.Create(ctx, dsci) + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create multiple routes with the same label + route1 := createRouteWithLabels("odh-dashboard-1", "odh-dashboard-1-test-namespace.apps.example.com", true, map[string]string{ + labels.PlatformPartOf: strings.ToLower(componentApi.DashboardKind), + }) + + route2 := createRouteWithLabels("odh-dashboard-2", "odh-dashboard-2-test-namespace.apps.example.com", true, map[string]string{ + labels.PlatformPartOf: strings.ToLower(componentApi.DashboardKind), + }) + + err = cli.Create(ctx, route1) + g.Expect(err).ShouldNot(HaveOccurred()) + + err = cli.Create(ctx, route2) + g.Expect(err).ShouldNot(HaveOccurred()) + + dashboardInstance := CreateTestDashboard() + + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + } + + // When there are multiple routes, the URL should be empty + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dashboardInstance.Status.URL).Should(Equal("")) +} + +func TestUpdateStatusWithRouteNoIngress(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + err = cli.Create(ctx, dsci) + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create a route without ingress status + route := createRouteWithLabels("odh-dashboard", "odh-dashboard-test-namespace.apps.example.com", false, map[string]string{ + labels.PlatformPartOf: strings.ToLower(componentApi.DashboardKind), + }) + + err = cli.Create(ctx, route) + g.Expect(err).ShouldNot(HaveOccurred()) + + dashboardInstance := CreateTestDashboard() + + rr := &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboardInstance, + } + + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(dashboardInstance.Status.URL).Should(Equal("")) +} + +func TestUpdateStatusListError(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + // Create a fake client as the base + baseCli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + // Create DSCI resource + dsci := CreateTestDSCI(TestNamespace) + err = baseCli.Create(ctx, dsci) + g.Expect(err).ShouldNot(HaveOccurred()) + + // Inject a List error for Route objects to simulate a failing route list operation + mockCli := &mockClient{ + Client: baseCli, + listError: errors.New("failed to list routes"), + } + + dashboardInstance := CreateTestDashboard() + + rr := &odhtypes.ReconciliationRequest{ + Client: mockCli, + Instance: dashboardInstance, + } + + // Test the case where list fails + err = dashboard.UpdateStatus(ctx, rr) + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("failed to list routes")) +} diff --git a/internal/controller/components/dashboard/dashboard_support.go b/internal/controller/components/dashboard/dashboard_support.go index e7e731d66ff9..7ea82397d15a 100644 --- a/internal/controller/components/dashboard/dashboard_support.go +++ b/internal/controller/components/dashboard/dashboard_support.go @@ -29,43 +29,85 @@ const ( ) var ( + // Private maps to reduce API surface and prevent direct access. sectionTitle = map[common.Platform]string{ cluster.SelfManagedRhoai: "OpenShift Self Managed Services", cluster.ManagedRhoai: "OpenShift Managed Services", cluster.OpenDataHub: "OpenShift Open Data Hub", } + baseConsoleURL = map[common.Platform]string{ + cluster.SelfManagedRhoai: "https://rhods-dashboard-", + cluster.ManagedRhoai: "https://rhods-dashboard-", + cluster.OpenDataHub: "https://odh-dashboard-", + } + overlaysSourcePaths = map[common.Platform]string{ cluster.SelfManagedRhoai: "/rhoai/onprem", cluster.ManagedRhoai: "/rhoai/addon", cluster.OpenDataHub: "/odh", } - imagesMap = map[string]string{ + ImagesMap = map[string]string{ "odh-dashboard-image": "RELATED_IMAGE_ODH_DASHBOARD_IMAGE", "model-registry-ui-image": "RELATED_IMAGE_ODH_MOD_ARCH_MODEL_REGISTRY_IMAGE", "gen-ai-ui-image": "RELATED_IMAGE_ODH_MOD_ARCH_GEN_AI_IMAGE", "kube-rbac-proxy": "RELATED_IMAGE_OSE_KUBE_RBAC_PROXY_IMAGE", } - conditionTypes = []string{ + ConditionTypes = []string{ status.ConditionDeploymentsAvailable, } ) -func defaultManifestInfo(p common.Platform) odhtypes.ManifestInfo { +// GetSectionTitle returns the section title for the given platform. +// Returns an error if the platform is not supported. +func GetSectionTitle(platform common.Platform) (string, error) { + title, ok := sectionTitle[platform] + if !ok { + return "", fmt.Errorf(ErrUnsupportedPlatform, platform) + } + return title, nil +} + +// GetBaseConsoleURL returns the base console URL for the given platform. +// Returns an error if the platform is not supported. +func GetBaseConsoleURL(platform common.Platform) (string, error) { + url, ok := baseConsoleURL[platform] + if !ok { + return "", fmt.Errorf(ErrUnsupportedPlatform, platform) + } + return url, nil +} + +// GetOverlaysSourcePath returns the overlays source path for the given platform. +// Returns an error if the platform is not supported. +func GetOverlaysSourcePath(platform common.Platform) (string, error) { + path, ok := overlaysSourcePaths[platform] + if !ok { + return "", fmt.Errorf(ErrUnsupportedPlatform, platform) + } + return path, nil +} + +func DefaultManifestInfo(p common.Platform) (odhtypes.ManifestInfo, error) { + sourcePath, err := GetOverlaysSourcePath(p) + if err != nil { + return odhtypes.ManifestInfo{}, err + } + return odhtypes.ManifestInfo{ Path: odhdeploy.DefaultManifestPath, ContextDir: ComponentName, - SourcePath: overlaysSourcePaths[p], - } + SourcePath: sourcePath, + }, nil } -func bffManifestsPath() odhtypes.ManifestInfo { +func BffManifestsPath() odhtypes.ManifestInfo { return odhtypes.ManifestInfo{ Path: odhdeploy.DefaultManifestPath, ContextDir: ComponentName, - SourcePath: "modular-architecture", + SourcePath: ModularArchitectureSourcePath, } } @@ -76,15 +118,28 @@ func computeKustomizeVariable(ctx context.Context, cli client.Client, platform c return nil, fmt.Errorf("error getting gateway domain: %w", err) } + baseURL, err := GetBaseConsoleURL(platform) + if err != nil { + return nil, err + } + sectionTitle, err := GetSectionTitle(platform) + if err != nil { + return nil, err + } + return map[string]string{ "dashboard-url": fmt.Sprintf("https://%s/", consoleLinkDomain), "section-title": sectionTitle[platform], }, nil } -func computeComponentName() string { - release := cluster.GetRelease() - +// ComputeComponentNameWithRelease returns the appropriate legacy component name based on the provided release. +// Platforms whose release.Name equals cluster.SelfManagedRhoai or cluster.ManagedRhoai +// return LegacyComponentNameDownstream, while all others return LegacyComponentNameUpstream. +// This distinction exists because these specific platforms use legacy downstream vs upstream +// naming conventions. This is historical behavior that must be preserved - do not change +// return values as this maintains compatibility with existing deployments. +func ComputeComponentNameWithRelease(release common.Release) string { name := LegacyComponentNameUpstream if release.Name == cluster.SelfManagedRhoai || release.Name == cluster.ManagedRhoai { name = LegacyComponentNameDownstream @@ -92,3 +147,9 @@ func computeComponentName() string { return name } + +// ComputeComponentName returns the appropriate legacy component name based on the platform. +// This function maintains backward compatibility by using the global release state. +func ComputeComponentName() string { + return ComputeComponentNameWithRelease(cluster.GetRelease()) +} diff --git a/internal/controller/components/dashboard/dashboard_support_test.go b/internal/controller/components/dashboard/dashboard_support_test.go new file mode 100644 index 000000000000..75eaed32d83c --- /dev/null +++ b/internal/controller/components/dashboard/dashboard_support_test.go @@ -0,0 +1,266 @@ +package dashboard_test + +import ( + "testing" + + routev1 "github.com/openshift/api/route/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" + + . "github.com/onsi/gomega" +) + +const ( + dashboardURLKey = "dashboard-url" + sectionTitleKey = "section-title" + testNamespace = "test-namespace" + + // Test constants for platform names. + openDataHubPlatformName = "OpenDataHub platform" + selfManagedRhoaiPlatformName = "SelfManagedRhoai platform" + managedRhoaiPlatformName = "ManagedRhoai platform" + unsupportedPlatformName = "Unsupported platform" + unsupportedPlatformValue = "unsupported-platform" +) + +func TestDefaultManifestInfo(t *testing.T) { + tests := []struct { + name string + platform common.Platform + expected string + }{ + { + name: openDataHubPlatformName, + platform: cluster.OpenDataHub, + expected: "/odh", + }, + { + name: selfManagedRhoaiPlatformName, + platform: cluster.SelfManagedRhoai, + expected: "/rhoai/onprem", + }, + { + name: managedRhoaiPlatformName, + platform: cluster.ManagedRhoai, + expected: "/rhoai/addon", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + manifestInfo, err := dashboard.DefaultManifestInfo(tt.platform) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(manifestInfo.ContextDir).Should(Equal(dashboard.ComponentName)) + g.Expect(manifestInfo.SourcePath).Should(Equal(tt.expected)) + }) + } +} + +func TestBffManifestsPath(t *testing.T) { + g := NewWithT(t) + manifestInfo := dashboard.BffManifestsPath() + g.Expect(manifestInfo.ContextDir).Should(Equal(dashboard.ComponentName)) + g.Expect(manifestInfo.SourcePath).Should(Equal(dashboard.ModularArchitectureSourcePath)) +} + +func TestComputeKustomizeVariable(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + // Create a mock client with a console route + route := &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "console", + Namespace: "openshift-console", + }, + Spec: routev1.RouteSpec{ + Host: "console-openshift-console.apps.example.com", + }, + } + + // Create the OpenShift ingress resource that cluster.GetDomain() needs + ingress := &unstructured.Unstructured{} + ingress.SetGroupVersionKind(gvk.OpenshiftIngress) + ingress.SetName("cluster") + ingress.SetNamespace("") + + // Set the domain in the spec + err := unstructured.SetNestedField(ingress.Object, "apps.example.com", "spec", "domain") + g.Expect(err).ShouldNot(HaveOccurred()) + + cli, err := fakeclient.New(fakeclient.WithObjects(route, ingress)) + g.Expect(err).ShouldNot(HaveOccurred()) + + tests := []struct { + name string + platform common.Platform + expected map[string]string + }{ + { + name: "OpenDataHub platform", + platform: cluster.OpenDataHub, + expected: map[string]string{ + dashboardURLKey: "https://odh-dashboard-data-science-gateway.apps.example.com", + sectionTitleKey: "OpenShift Open Data Hub", + }, + }, + { + name: "SelfManagedRhoai platform", + platform: cluster.SelfManagedRhoai, + expected: map[string]string{ + dashboardURLKey: "https://rhods-dashboard-data-science-gateway.apps.example.com", + sectionTitleKey: "OpenShift Self Managed Services", + }, + }, + { + name: "ManagedRhoai platform", + platform: cluster.ManagedRhoai, + expected: map[string]string{ + dashboardURLKey: "https://rhods-dashboard-data-science-gateway.apps.example.com", + sectionTitleKey: "OpenShift Managed Services", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + variables, err := dashboard.ComputeKustomizeVariable(ctx, cli, tt.platform) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(variables).Should(Equal(tt.expected)) + }) + } +} + +func TestComputeKustomizeVariableNoConsoleRoute(t *testing.T) { + ctx := t.Context() + g := NewWithT(t) + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + + _, err = dashboard.ComputeKustomizeVariable(ctx, cli, cluster.OpenDataHub) + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("error getting gateway domain")) +} + +func TestInit(t *testing.T) { + handler := getDashboardHandler() + + // Test successful initialization for different platforms + platforms := []common.Platform{ + cluster.OpenDataHub, + cluster.SelfManagedRhoai, + cluster.ManagedRhoai, + } + + for _, platform := range platforms { + t.Run(string(platform), func(t *testing.T) { + g := NewWithT(t) + err := handler.Init(platform) + g.Expect(err).ShouldNot(HaveOccurred()) + }) + } +} + +func TestGetSectionTitle(t *testing.T) { + runPlatformTest(t, "GetSectionTitle", dashboard.GetSectionTitle) +} + +func TestGetBaseConsoleURL(t *testing.T) { + runPlatformTest(t, "GetBaseConsoleURL", dashboard.GetBaseConsoleURL) +} + +func TestGetOverlaysSourcePath(t *testing.T) { + runPlatformTest(t, "GetOverlaysSourcePath", dashboard.GetOverlaysSourcePath) +} + +// runPlatformTest is a helper function that reduces code duplication in platform testing. +func runPlatformTest(t *testing.T, testName string, testFunc func(platform common.Platform) (string, error)) { + t.Helper() + + tests := []struct { + name string + platform common.Platform + expected string + hasError bool + }{ + { + name: openDataHubPlatformName, + platform: cluster.OpenDataHub, + expected: getExpectedValue(testName, cluster.OpenDataHub), + hasError: false, + }, + { + name: selfManagedRhoaiPlatformName, + platform: cluster.SelfManagedRhoai, + expected: getExpectedValue(testName, cluster.SelfManagedRhoai), + hasError: false, + }, + { + name: managedRhoaiPlatformName, + platform: cluster.ManagedRhoai, + expected: getExpectedValue(testName, cluster.ManagedRhoai), + hasError: false, + }, + { + name: unsupportedPlatformName, + platform: unsupportedPlatformValue, + expected: "", + hasError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + result, err := testFunc(tt.platform) + if tt.hasError { + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring(ErrorUnsupportedPlatform)) + } else { + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(result).Should(Equal(tt.expected)) + } + }) + } +} + +// getExpectedValue returns the expected value for a given test name and platform. +func getExpectedValue(testName string, platform common.Platform) string { + switch testName { + case "GetSectionTitle": + switch platform { + case cluster.OpenDataHub: + return "OpenShift Open Data Hub" + case cluster.SelfManagedRhoai: + return "OpenShift Self Managed Services" + case cluster.ManagedRhoai: + return "OpenShift Managed Services" + } + case "GetBaseConsoleURL": + switch platform { + case cluster.OpenDataHub: + return "https://odh-dashboard-" + case cluster.SelfManagedRhoai, cluster.ManagedRhoai: + return "https://rhods-dashboard-" + } + case "GetOverlaysSourcePath": + switch platform { + case cluster.OpenDataHub: + return "/odh" + case cluster.SelfManagedRhoai: + return "/rhoai/onprem" + case cluster.ManagedRhoai: + return "/rhoai/addon" + } + } + return "" +} diff --git a/internal/controller/components/dashboard/dashboard_test.go b/internal/controller/components/dashboard/dashboard_test.go index 0d9b250462f9..d1097d60d494 100644 --- a/internal/controller/components/dashboard/dashboard_test.go +++ b/internal/controller/components/dashboard/dashboard_test.go @@ -1,5 +1,4 @@ -//nolint:testpackage,dupl -package dashboard +package dashboard_test import ( "encoding/json" @@ -8,13 +7,14 @@ import ( gt "github.com/onsi/gomega/types" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/runtime" "github.com/opendatahub-io/opendatahub-operator/v2/api/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/api/components/v1alpha1" + dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/api/datasciencecluster/v1" dscv2 "github.com/opendatahub-io/opendatahub-operator/v2/api/datasciencecluster/v2" - serviceApi "github.com/opendatahub-io/opendatahub-operator/v2/api/services/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/registry" "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/status" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" @@ -27,16 +27,35 @@ import ( . "github.com/onsi/gomega" ) +// assertDashboardManagedState verifies the expected relationship between ManagementState, +// presence of Dashboard CR, and InstalledComponents status for the Dashboard component. +// When ManagementState is Managed and no Dashboard CR exists, InstalledComponents should be false. +// When ManagementState is Unmanaged/Removed, the component should not be actively managed. +func assertDashboardManagedState(t *testing.T, dsc *dscv2.DataScienceCluster, state operatorv1.ManagementState) { + t.Helper() + g := NewWithT(t) + + if state == operatorv1.Managed { + // For Managed state, component should be enabled but not ready (no Dashboard CR) + // Note: InstalledComponents will be true when Dashboard CR exists regardless of Ready status + g.Expect(dsc.Status.Components.Dashboard.ManagementState).Should(Equal(operatorv1.Managed)) + } else { + // For Unmanaged and Removed states, component should not be actively managed + g.Expect(dsc.Status.Components.Dashboard.ManagementState).Should(Equal(state)) + g.Expect(dsc.Status.Components.Dashboard.DashboardCommonStatus).Should(BeNil()) + } +} + func TestGetName(t *testing.T) { g := NewWithT(t) - handler := &componentHandler{} + handler := getDashboardHandler() name := handler.GetName() g.Expect(name).Should(Equal(componentApi.DashboardComponentName)) } func TestNewCRObject(t *testing.T) { - handler := &componentHandler{} + handler := getDashboardHandler() g := NewWithT(t) dsc := createDSCWithDashboard(operatorv1.Managed) @@ -54,7 +73,7 @@ func TestNewCRObject(t *testing.T) { } func TestIsEnabled(t *testing.T) { - handler := &componentHandler{} + handler := getDashboardHandler() tests := []struct { name string @@ -93,114 +112,191 @@ func TestIsEnabled(t *testing.T) { } func TestUpdateDSCStatus(t *testing.T) { - handler := &componentHandler{} + handler := getDashboardHandler() - t.Run("should handle enabled component with ready Dashboard CR", func(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() + t.Run("enabled component with ready Dashboard CR", func(t *testing.T) { + testEnabledComponentWithReadyCR(t, handler) + }) + + t.Run("enabled component with not ready Dashboard CR", func(t *testing.T) { + testEnabledComponentWithNotReadyCR(t, handler) + }) - dsc := createDSCWithDashboard(operatorv1.Managed) - dashboard := createDashboardCR(true) + t.Run("disabled component", func(t *testing.T) { + testDisabledComponent(t, handler) + }) - cli, err := fakeclient.New(fakeclient.WithObjects(dsc, dashboard)) - g.Expect(err).ShouldNot(HaveOccurred()) + t.Run("empty management state as Removed", func(t *testing.T) { + testEmptyManagementState(t, handler) + }) - cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ - Client: cli, - Instance: dsc, - Conditions: conditions.NewManager(dsc, ReadyConditionType), - }) + t.Run("Dashboard CR not found", func(t *testing.T) { + testDashboardCRNotFound(t, handler) + }) + + t.Run("Dashboard CR without Ready condition", func(t *testing.T) { + testDashboardCRWithoutReadyCondition(t, handler) + }) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(cs).Should(Equal(metav1.ConditionTrue)) + t.Run("Dashboard CR with Ready Condition True", func(t *testing.T) { + testDashboardCRWithReadyConditionTrue(t, handler) + }) - g.Expect(dsc).Should(WithTransform(json.Marshal, And( - jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Managed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, ReadyConditionType, metav1.ConditionTrue), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, ReadyConditionType, status.ReadyReason), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "Component is ready"`, ReadyConditionType)), - )) + t.Run("different management states", func(t *testing.T) { + testDifferentManagementStates(t, handler) }) - t.Run("should handle enabled component with not ready Dashboard CR", func(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() + t.Run("nil Instance", func(t *testing.T) { + testNilInstance(t, handler) + }) +} - dsc := createDSCWithDashboard(operatorv1.Managed) - dashboard := createDashboardCR(false) +// testEnabledComponentWithReadyCR tests the enabled component with ready Dashboard CR scenario. +func testEnabledComponentWithReadyCR(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + testEnabledComponentWithCR(t, handler, true, metav1.ConditionTrue, status.ReadyReason, "Component is ready") +} - cli, err := fakeclient.New(fakeclient.WithObjects(dsc, dashboard)) - g.Expect(err).ShouldNot(HaveOccurred()) +// testEnabledComponentWithNotReadyCR tests the enabled component with not ready Dashboard CR scenario. +func testEnabledComponentWithNotReadyCR(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + testEnabledComponentWithCR(t, handler, false, metav1.ConditionFalse, status.NotReadyReason, "Component is not ready") +} - cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ - Client: cli, - Instance: dsc, - Conditions: conditions.NewManager(dsc, ReadyConditionType), - }) +// testEnabledComponentWithCR is a helper function that tests the enabled component with a Dashboard CR. +func testEnabledComponentWithCR(t *testing.T, handler registry.ComponentHandler, isReady bool, expectedStatus metav1.ConditionStatus, expectedReason, expectedMessage string) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(cs).Should(Equal(metav1.ConditionFalse)) + dsc := createDSCWithDashboard(operatorv1.Managed) + dashboardInstance := createDashboardCR(isReady) - g.Expect(dsc).Should(WithTransform(json.Marshal, And( - jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Managed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, ReadyConditionType, metav1.ConditionFalse), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, ReadyConditionType, status.NotReadyReason), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "Component is not ready"`, ReadyConditionType)), - )) + // Create a scheme that includes the Dashboard CR and DataScienceCluster + scheme := runtime.NewScheme() + err := componentApi.AddToScheme(scheme) + g.Expect(err).ShouldNot(HaveOccurred()) + err = dscv2.AddToScheme(scheme) + g.Expect(err).ShouldNot(HaveOccurred()) + + cli, err := fakeclient.New(fakeclient.WithObjects(dsc, dashboardInstance), fakeclient.WithScheme(scheme)) + g.Expect(err).ShouldNot(HaveOccurred()) + + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), }) - t.Run("should handle disabled component", func(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(expectedStatus)) + + g.Expect(dsc).Should(WithTransform(json.Marshal, And( + jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Managed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, dashboard.ReadyConditionType, expectedStatus), + jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, dashboard.ReadyConditionType, expectedReason), + jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "%s"`, dashboard.ReadyConditionType, expectedMessage)), + )) +} - dsc := createDSCWithDashboard(operatorv1.Removed) +// testDisabledComponent tests the disabled component scenario. +func testDisabledComponent(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() - cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) - g.Expect(err).ShouldNot(HaveOccurred()) + dsc := createDSCWithDashboard(operatorv1.Removed) - cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ - Client: cli, - Instance: dsc, - Conditions: conditions.NewManager(dsc, ReadyConditionType), - }) + cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) + g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), + }) - g.Expect(dsc).Should(WithTransform(json.Marshal, And( - jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Removed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, ReadyConditionType, metav1.ConditionFalse), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, ReadyConditionType, operatorv1.Removed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message | contains("Component ManagementState is set to Removed")`, ReadyConditionType)), - )) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) + + g.Expect(dsc).Should(WithTransform(json.Marshal, And( + jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Removed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, dashboard.ReadyConditionType, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, dashboard.ReadyConditionType, operatorv1.Removed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .message | contains("Component ManagementState is set to Removed")`, dashboard.ReadyConditionType)), + )) +} + +// testEmptyManagementState tests the empty management state scenario. +func testEmptyManagementState(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() + + dsc := createDSCWithDashboard("") + + cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) + g.Expect(err).ShouldNot(HaveOccurred()) + + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), }) - t.Run("should handle empty management state as Removed", func(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) + + g.Expect(dsc).Should(WithTransform(json.Marshal, And( + jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Removed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, dashboard.ReadyConditionType, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, dashboard.ReadyConditionType, operatorv1.Removed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .message | contains("Component ManagementState is set to Removed")`, dashboard.ReadyConditionType)), + )) +} + +// testDashboardCRNotFound tests the Dashboard CR not found scenario. +func testDashboardCRNotFound(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() + + dsc := createDSCWithDashboard(operatorv1.Managed) + cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) + g.Expect(err).ShouldNot(HaveOccurred()) - dsc := createDSCWithDashboard("") + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), + }) - cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) - g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(metav1.ConditionFalse)) +} - cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ - Client: cli, - Instance: dsc, - Conditions: conditions.NewManager(dsc, ReadyConditionType), - }) +// testDashboardCRWithoutReadyCondition tests the Dashboard CR without Ready condition scenario. +func testDashboardCRWithoutReadyCondition(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) + dsc := createDSCWithDashboard(operatorv1.Managed) + dashboardInstance := &componentApi.Dashboard{} + dashboardInstance.SetGroupVersionKind(gvk.Dashboard) + dashboardInstance.SetName(componentApi.DashboardInstanceName) + // Dashboard CR is cluster-scoped, so no namespace + cli, err := fakeclient.New(fakeclient.WithObjects(dsc, dashboardInstance)) + g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(dsc).Should(WithTransform(json.Marshal, And( - jq.Match(`.status.components.dashboard.managementState == "%s"`, operatorv1.Removed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, ReadyConditionType, metav1.ConditionFalse), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, ReadyConditionType, operatorv1.Removed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .severity == "%s"`, ReadyConditionType, common.ConditionSeverityInfo), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message | contains("Component ManagementState is set to Removed")`, ReadyConditionType)), - )) + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), }) + + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(metav1.ConditionFalse)) } func TestComputeKustomizeVariable(t *testing.T) { @@ -256,52 +352,100 @@ func TestComputeKustomizeVariable(t *testing.T) { clusterDomain: defaultDomain, // Should be ignored due to custom domain }, { - name: "Managed RHOAI platform with default domain", - platform: cluster.ManagedRhoai, - expectedURL: "https://data-science-gateway." + managedDomain + "/", - expectedTitle: "OpenShift Managed Services", - gatewayConfigFunc: defaultGatewayConfig, - clusterDomain: managedDomain, + Type: status.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: status.ReadyReason, }, } - for _, tt := range tests { - // Capture loop variable for parallel execution + cli, err := fakeclient.New(fakeclient.WithObjects(dsc, dashboardTrue)) + g.Expect(err).ShouldNot(HaveOccurred()) - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - ctx := t.Context() + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), + }) - // Pre-allocate slice with known capacity for better performance - objects := make([]client.Object, 0, 2) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(cs).Should(Equal(metav1.ConditionTrue)) +} - if gc := tt.gatewayConfigFunc(); gc != nil { - objects = append(objects, gc) - } +// testDifferentManagementStates tests different management states. +func testDifferentManagementStates(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() - // Mock cluster domain by creating a fake OpenShift Ingress object - if tt.clusterDomain != "" { - ingress := createMockOpenShiftIngress(tt.clusterDomain) - objects = append(objects, ingress) - } + managementStates := []operatorv1.ManagementState{ + operatorv1.Managed, + operatorv1.Removed, + operatorv1.Unmanaged, + } - cli, err := fakeclient.New(fakeclient.WithObjects(objects...)) + for _, state := range managementStates { + t.Run(string(state), func(t *testing.T) { + dsc := createDSCWithDashboard(state) + cli, err := fakeclient.New(fakeclient.WithObjects(dsc)) g.Expect(err).ShouldNot(HaveOccurred()) - result, err := computeKustomizeVariable(ctx, cli, tt.platform) + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: dsc, + Conditions: conditions.NewManager(dsc, dashboard.ReadyConditionType), + }) - if tt.expectError { - g.Expect(err).Should(HaveOccurred()) - return + g.Expect(err).ShouldNot(HaveOccurred()) + + if state == operatorv1.Managed { + g.Expect(cs).Should(Equal(metav1.ConditionFalse)) + } else { + g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) } - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(result).Should(HaveKeyWithValue("dashboard-url", tt.expectedURL)) - g.Expect(result).Should(HaveKeyWithValue("section-title", tt.expectedTitle)) + // Assert the expected relationship between ManagementState, Dashboard CR presence, and InstalledComponents + assertDashboardManagedState(t, dsc, state) + + // Assert specific status fields based on management state + switch state { + case operatorv1.Unmanaged: + // For Unmanaged: assert component status indicates not actively managed + g.Expect(dsc).Should(WithTransform(json.Marshal, And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, dashboard.ReadyConditionType, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, dashboard.ReadyConditionType, operatorv1.Unmanaged), + jq.Match(`.status.conditions[] | select(.type == "%s") | .severity == "%s"`, dashboard.ReadyConditionType, common.ConditionSeverityInfo), + ))) + case operatorv1.Removed: + // For Removed: assert cleanup-related status fields are set + g.Expect(dsc).Should(WithTransform(json.Marshal, And( + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, dashboard.ReadyConditionType, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, dashboard.ReadyConditionType, operatorv1.Removed), + jq.Match(`.status.conditions[] | select(.type == "%s") | .severity == "%s"`, dashboard.ReadyConditionType, common.ConditionSeverityInfo), + ))) + } }) } } +// testNilInstance tests the nil instance scenario. +func testNilInstance(t *testing.T, handler registry.ComponentHandler) { + t.Helper() + g := NewWithT(t) + ctx := t.Context() + + cli, err := fakeclient.New() + g.Expect(err).ShouldNot(HaveOccurred()) + cs, err := handler.UpdateDSCStatus(ctx, &types.ReconciliationRequest{ + Client: cli, + Instance: nil, + Conditions: conditions.NewManager(&dscv1.DataScienceCluster{}, dashboard.ReadyConditionType), + }) + + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("failed to convert to DataScienceCluster")) + g.Expect(cs).Should(Equal(metav1.ConditionUnknown)) +} + func TestComputeKustomizeVariableError(t *testing.T) { t.Parallel() // Enable parallel execution for better performance g := NewWithT(t) @@ -331,6 +475,7 @@ func createDashboardCR(ready bool) *componentApi.Dashboard { c := componentApi.Dashboard{} c.SetGroupVersionKind(gvk.Dashboard) c.SetName(componentApi.DashboardInstanceName) + // Dashboard CR is cluster-scoped, so no namespace if ready { c.Status.Conditions = []common.Condition{{ @@ -350,29 +495,3 @@ func createDashboardCR(ready bool) *componentApi.Dashboard { return &c } - -// createMockOpenShiftIngress creates an optimized mock OpenShift Ingress object -// for testing cluster domain resolution. -func createMockOpenShiftIngress(domain string) client.Object { - // Input validation for better error handling - if domain == "" { - domain = "default.example.com" // Fallback domain - } - - // Create OpenShift Ingress object (config.openshift.io/v1/Ingress) - // that cluster.GetDomain() looks for - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "config.openshift.io/v1", - "kind": "Ingress", - "metadata": map[string]interface{}{ - "name": "cluster", - }, - "spec": map[string]interface{}{ - "domain": domain, - }, - }, - } - - return obj -} diff --git a/internal/controller/components/dashboard/helpers_test.go b/internal/controller/components/dashboard/helpers_test.go new file mode 100644 index 000000000000..a972201393d5 --- /dev/null +++ b/internal/controller/components/dashboard/helpers_test.go @@ -0,0 +1,276 @@ +// This file contains shared helper functions for dashboard controller tests. +// These functions are used across multiple test files to reduce code duplication. +// Helper functions are intended for testing purposes only. +package dashboard_test + +import ( + "errors" + "testing" + + "github.com/onsi/gomega" + operatorv1 "github.com/openshift/api/operator/v1" + routev1 "github.com/openshift/api/route/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/opendatahub-io/opendatahub-operator/v2/api/common" + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/api/components/v1alpha1" + dscv2 "github.com/opendatahub-io/opendatahub-operator/v2/api/datasciencecluster/v2" + dsciv2 "github.com/opendatahub-io/opendatahub-operator/v2/api/dscinitialization/v2" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/dashboard" + "github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/registry" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/fakeclient" +) + +// getDashboardHandler returns the dashboard component handler from the registry. +// +//nolint:ireturn +func getDashboardHandler() registry.ComponentHandler { + var handler registry.ComponentHandler + _ = registry.ForEach(func(ch registry.ComponentHandler) error { + if ch.GetName() == componentApi.DashboardComponentName { + handler = ch + return errors.New("found") // Use error to break iteration + } + return nil + }) + return handler +} + +// TestData contains test fixtures and configuration values. +const ( + TestPath = "/test/path" + TestNamespace = "test-namespace" + TestPlatform = "test-platform" + TestSelfManagedPlatform = "self-managed" + TestProfile = "test-profile" + TestDisplayName = "Test Display Name" + TestDescription = "Test Description" + TestDomain = "apps.example.com" + TestRouteHost = "odh-dashboard-test-namespace.apps.example.com" + NodeTypeKey = "node-type" + NvidiaGPUKey = "nvidia.com/gpu" + DashboardHWPCRDName = "dashboardhardwareprofiles.dashboard.opendatahub.io" + NonExistentPlatform = "non-existent-platform" +) + +// ErrorMessages contains error message templates for test assertions. +const ( + ErrorFailedToUpdate = "failed to update" + ErrorFailedToUpdateParams = "failed to update params.env" + ErrorInitPanicked = "Init panicked with platform %s: %v" + ErrorFailedToSetVariable = "failed to set variable" + ErrorFailedToUpdateImages = "failed to update images on path" + ErrorFailedToUpdateModularImages = "failed to update " + dashboard.ModularArchitectureSourcePath + " images on path" + ErrorUnsupportedPlatform = "unsupported platform" +) + +// LogMessages contains log message templates for test assertions. +const ( + LogSetKustomizedParamsError = "setKustomizedParams returned error (expected): %v" +) + +// initialParamsEnvContent is the standard content for params.env files in tests. +const InitialParamsEnvContent = `# Initial params.env content +dashboard-url=https://odh-dashboard-test.example.com +section-title=Test Title +` + +// SetupTempManifestPath sets up a temporary directory for manifest downloads. +func SetupTempManifestPath(t *testing.T) { + t.Helper() + oldDeployPath := odhdeploy.DefaultManifestPath + t.Cleanup(func() { + odhdeploy.DefaultManifestPath = oldDeployPath + }) + odhdeploy.DefaultManifestPath = t.TempDir() +} + +// createTestClient creates a fake client for testing. +func CreateTestClient(t *testing.T) client.Client { + t.Helper() + cli, err := fakeclient.New() + gomega.NewWithT(t).Expect(err).ShouldNot(gomega.HaveOccurred()) + return cli +} + +// CreateTestDashboard creates a basic dashboard instance for testing. +func CreateTestDashboard() *componentApi.Dashboard { + return &componentApi.Dashboard{ + TypeMeta: metav1.TypeMeta{ + APIVersion: componentApi.GroupVersion.String(), + Kind: componentApi.DashboardKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: componentApi.DashboardInstanceName, + CreationTimestamp: metav1.Unix(1640995200, 0), // 2022-01-01T00:00:00Z + }, + Spec: componentApi.DashboardSpec{ + DashboardCommonSpec: componentApi.DashboardCommonSpec{}, + }, + Status: componentApi.DashboardStatus{ + Status: common.Status{ + Phase: "Ready", + ObservedGeneration: 1, + Conditions: []common.Condition{ + { + Type: componentApi.DashboardKind + "Ready", + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: metav1.Unix(1640995200, 0), // 2022-01-01T00:00:00Z + Reason: "ReconcileSucceeded", + Message: "Dashboard is ready", + }, + }, + }, + DashboardCommonStatus: componentApi.DashboardCommonStatus{ + URL: "https://odh-dashboard-" + TestNamespace + ".apps.example.com", + }, + }, + } +} + +// CreateDSCWithDashboard creates a DSC with dashboard component enabled. +func CreateDSCWithDashboard(managementState operatorv1.ManagementState) *dscv2.DataScienceCluster { + return &dscv2.DataScienceCluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "DataScienceCluster", + APIVersion: dscv2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dsc", + }, + Spec: dscv2.DataScienceClusterSpec{ + Components: dscv2.Components{ + Dashboard: componentApi.DSCDashboard{ + ManagementSpec: common.ManagementSpec{ + ManagementState: managementState, + }, + }, + }, + }, + } +} + +// createRoute creates a Route instance for testing. +func createRoute(name, host string, admitted bool) *routev1.Route { + labels := map[string]string{ + "platform.opendatahub.io/part-of": "dashboard", + } + return createRouteWithLabels(name, host, admitted, labels) +} + +// createRouteWithLabels creates a Route instance with custom labels for testing. +func createRouteWithLabels(name, host string, admitted bool, labels map[string]string) *routev1.Route { + route := &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: TestNamespace, + Labels: labels, + }, + Spec: routev1.RouteSpec{ + Host: host, + }, + } + + if admitted { + route.Status = routev1.RouteStatus{ + Ingress: []routev1.RouteIngress{ + { + Host: host, + Conditions: []routev1.RouteIngressCondition{ + { + Type: routev1.RouteAdmitted, + Status: corev1.ConditionTrue, + }, + }, + }, + }, + } + } + + return route +} + +// createTestReconciliationRequest creates a basic reconciliation request for testing. +func CreateTestReconciliationRequest(cli client.Client, dashboard *componentApi.Dashboard, release common.Release) *odhtypes.ReconciliationRequest { + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboard, + Release: release, + } +} + +// createTestReconciliationRequestWithManifests creates a reconciliation request with manifests for testing. +func CreateTestReconciliationRequestWithManifests( + cli client.Client, + dashboard *componentApi.Dashboard, + release common.Release, + manifests []odhtypes.ManifestInfo, +) *odhtypes.ReconciliationRequest { + return &odhtypes.ReconciliationRequest{ + Client: cli, + Instance: dashboard, + Release: release, + Manifests: manifests, + } +} + +// assertPanics is a helper function that verifies a function call panics. +func AssertPanics(t *testing.T, fn func(), message string) { + t.Helper() + + defer func() { + if r := recover(); r == nil { + t.Errorf("Expected function to panic, but it didn't. %s", message) + } else { + t.Logf("%s: %v", message, r) + } + }() + + fn() +} + +// CreateTestDashboardHardwareProfile creates a test dashboard hardware profile. +func CreateTestDashboardHardwareProfile() *dashboard.DashboardHardwareProfile { + return &dashboard.DashboardHardwareProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: TestProfile, + Namespace: TestNamespace, + }, + Spec: dashboard.DashboardHardwareProfileSpec{ + DisplayName: TestDisplayName, + Enabled: true, + Description: TestDescription, + }, + } +} + +// SetupTestReconciliationRequestSimple creates a simple test reconciliation request. +func SetupTestReconciliationRequestSimple(t *testing.T) *odhtypes.ReconciliationRequest { + t.Helper() + cli := CreateTestClient(t) + dashboard := CreateTestDashboard() + return CreateTestReconciliationRequest(cli, dashboard, common.Release{Name: cluster.OpenDataHub}) +} + +// CreateTestDSCI creates a test DSCI resource. +func CreateTestDSCI(namespace string) *dsciv2.DSCInitialization { + return &dsciv2.DSCInitialization{ + TypeMeta: metav1.TypeMeta{ + Kind: "DSCInitialization", + APIVersion: "dscinitialization.opendatahub.io/v2", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + Spec: dsciv2.DSCInitializationSpec{ + ApplicationsNamespace: namespace, + }, + } +} diff --git a/internal/controller/components/suite_test.go b/internal/controller/components/suite_test.go index 8668f47a482f..0571403671ef 100644 --- a/internal/controller/components/suite_test.go +++ b/internal/controller/components/suite_test.go @@ -62,14 +62,6 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) - DeferCleanup(func() { - By("DeferCleanup: tearing down the test environment") - if testEnv != nil { - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) - } - }) - err = componentApi.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/controller/datasciencecluster/suite_test.go b/internal/controller/datasciencecluster/suite_test.go index cfbf0c90d858..ad092547a296 100644 --- a/internal/controller/datasciencecluster/suite_test.go +++ b/internal/controller/datasciencecluster/suite_test.go @@ -80,9 +80,3 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) }) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}) diff --git a/internal/controller/dscinitialization/suite_test.go b/internal/controller/dscinitialization/suite_test.go index 0a0b7da0218c..a8cc338aa6f1 100644 --- a/internal/controller/dscinitialization/suite_test.go +++ b/internal/controller/dscinitialization/suite_test.go @@ -73,6 +73,26 @@ const ( interval = 250 * time.Millisecond ) +// cleanupTestEnv performs nil-safe cancellation and testEnv stop logic. +func cleanupTestEnv() { + By("cancelling context and tearing down test environment") + // Cancel context first to ensure proper shutdown order + if gCancel != nil { + gCancel() + gCancel = nil // Prevent double-cancellation + } + // Stop test environment with defensive error handling + if testEnv != nil { + By("tearing down the test environment") + err := testEnv.Stop() + if err != nil { + // Log but don't fail on cleanup errors (testEnv might already be stopped) + By("Warning: testEnv.Stop() returned error: " + err.Error()) + } + testEnv = nil // Prevent double-stop + } +} + func TestDataScienceClusterInitialization(t *testing.T) { RegisterFailHandler(Fail) @@ -112,13 +132,7 @@ var _ = BeforeSuite(func() { DeferCleanup(func() { By("DeferCleanup: cancelling context and tearing down test environment") - if gCancel != nil { - gCancel() - } - if testEnv != nil { - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) - } + cleanupTestEnv() }) utilruntime.Must(clientgoscheme.AddToScheme(testScheme)) @@ -173,10 +187,3 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred(), "Failed to run manager") }() }) - -var _ = AfterSuite(func() { - gCancel() - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}) diff --git a/internal/controller/services/gateway/gateway_support.go b/internal/controller/services/gateway/gateway_support.go index b75e877f2263..84ca55065052 100644 --- a/internal/controller/services/gateway/gateway_support.go +++ b/internal/controller/services/gateway/gateway_support.go @@ -92,7 +92,7 @@ var ( KubeAuthProxyLabels = map[string]string{"app": KubeAuthProxyName} ) -// make secrete data into sha256 as hash. +// calculateSecretHash computes a SHA256 hash of secret data to detect changes. func calculateSecretHash(secretData map[string][]byte) string { clientID := string(secretData[EnvClientID]) clientSecret := string(secretData[EnvClientSecret]) diff --git a/internal/controller/services/gateway/gateway_util_test.go b/internal/controller/services/gateway/gateway_util_test.go index eb483a9e6fcd..19c8b8973a37 100644 --- a/internal/controller/services/gateway/gateway_util_test.go +++ b/internal/controller/services/gateway/gateway_util_test.go @@ -80,7 +80,10 @@ func setupSupportTestClientWithClusterIngress(domain string) client.Client { clusterIngress.SetName("cluster") // Set the spec.domain field - _ = unstructured.SetNestedField(clusterIngress.Object, domain, "spec", "domain") + err := unstructured.SetNestedField(clusterIngress.Object, domain, "spec", "domain") + if err != nil { + panic(err) // In test helper, panic is acceptable for setup errors + } return fake.NewClientBuilder().WithScheme(createTestScheme()).WithObjects(clusterIngress).Build() } diff --git a/internal/controller/services/suite_test.go b/internal/controller/services/suite_test.go index 4d0962d046b1..28f46deb0bc0 100644 --- a/internal/controller/services/suite_test.go +++ b/internal/controller/services/suite_test.go @@ -80,9 +80,3 @@ var _ = BeforeSuite(func() { Expect(k8sClient).NotTo(BeNil()) }) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}) diff --git a/pkg/cluster/cluster_config.go b/pkg/cluster/cluster_config.go index 12411671bab4..f7112f550080 100644 --- a/pkg/cluster/cluster_config.go +++ b/pkg/cluster/cluster_config.go @@ -27,8 +27,13 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" ) +const ( + // DSCMonitoringNamespaceKey is the viper key for the DSC monitoring namespace. + DSCMonitoringNamespaceKey = "dsc-monitoring-namespace" +) + type ClusterInfo struct { - Type string `json:"type,omitempty"` // openshift , TODO: can be other value if we later support other type + Type string `json:"type,omitempty"` Version version.OperatorVersion `json:"version,omitempty"` FipsEnabled bool `json:"fips_enabled,omitempty"` } diff --git a/pkg/controller/reconciler/reconciler_finalizer_test.go b/pkg/controller/reconciler/reconciler_finalizer_test.go index 6ae690ed121e..0db509609247 100644 --- a/pkg/controller/reconciler/reconciler_finalizer_test.go +++ b/pkg/controller/reconciler/reconciler_finalizer_test.go @@ -56,7 +56,8 @@ func (f *MockManager) GetConfig() *rest.Config { return &rest.Config{} } func (f *MockManager) GetFieldIndexer() client.FieldIndexer { return nil } //nolint:ireturn -func (f *MockManager) GetEventRecorderFor(name string) record.EventRecorder { return nil } //nolint:ireturn +//nolint:ireturn +func (f *MockManager) GetEventRecorderFor(name string) record.EventRecorder { return nil } //nolint:ireturn func (f *MockManager) GetCache() cache.Cache { return nil } @@ -77,8 +78,7 @@ func (f *MockManager) GetControllerOptions() config.Controller { } func (f *MockManager) GetHTTPClient() *http.Client { return &http.Client{} } -//nolint:ireturn -func (f *MockManager) GetWebhookServer() webhook.Server { return nil } +func (f *MockManager) GetWebhookServer() webhook.Server { return nil } //nolint:ireturn //nolint:ireturn func setupTest(mockDashboard *componentApi.Dashboard) (context.Context, *MockManager, client.WithWatch) { @@ -116,7 +116,7 @@ func setupTest(mockDashboard *componentApi.Dashboard) (context.Context, *MockMan return ctx, mockMgr, mockClient } -func TestFinalizer_Add(t *testing.T) { +func TestFinalizerAdd(t *testing.T) { g := gomega.NewWithT(t) mockDashboard := &componentApi.Dashboard{ @@ -167,7 +167,7 @@ func TestFinalizer_Add(t *testing.T) { g.Expect(controllerutil.ContainsFinalizer(d, finalizerName)).To(gomega.BeTrue()) } -func TestFinalizer_NotPresent(t *testing.T) { +func TestFinalizerNotPresent(t *testing.T) { g := gomega.NewWithT(t) mockDashboard := &componentApi.Dashboard{ @@ -205,7 +205,7 @@ func TestFinalizer_NotPresent(t *testing.T) { g.Expect(controllerutil.ContainsFinalizer(d, finalizerName)).To(gomega.BeFalse()) } -func TestFinalizer_Remove(t *testing.T) { +func TestFinalizerRemove(t *testing.T) { g := gomega.NewWithT(t) mockDashboard := &componentApi.Dashboard{ diff --git a/pkg/utils/test/fakeclient/fakeclient.go b/pkg/utils/test/fakeclient/fakeclient.go index 1a68a870d3f6..81b96ed5b70b 100644 --- a/pkg/utils/test/fakeclient/fakeclient.go +++ b/pkg/utils/test/fakeclient/fakeclient.go @@ -80,6 +80,8 @@ func New(opts ...ClientOpts) (client.Client, error) { fakeMapper.Add(kt, meta.RESTScopeRoot) case gvk.Auth: fakeMapper.Add(kt, meta.RESTScopeRoot) + case gvk.DashboardHardwareProfile: + fakeMapper.Add(kt, meta.RESTScopeNamespace) default: fakeMapper.Add(kt, meta.RESTScopeNamespace) } diff --git a/tests/e2e/cleanup_test.go b/tests/e2e/cleanup_test.go index 54b6160c1b08..ba04e6032764 100644 --- a/tests/e2e/cleanup_test.go +++ b/tests/e2e/cleanup_test.go @@ -31,9 +31,36 @@ func CleanupPreviousTestResources(t *testing.T) { tc.DeleteResource( WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: tc.AppsNamespace}), WithIgnoreNotFound(true), - WithWaitForDeletion(false), + WithWaitForDeletion(true), // Wait for namespace deletion to complete before proceeding + ) + + // Also clean up the RHOAI applications namespace if it exists (for RHOAI mode) + rhoaiAppsNamespace := "redhat-ods-applications" + if tc.AppsNamespace != rhoaiAppsNamespace { + t.Logf("Cleaning up RHOAI applications namespace: %s", rhoaiAppsNamespace) + tc.DeleteResource( + WithMinimalObject(gvk.Namespace, types.NamespacedName{Name: rhoaiAppsNamespace}), + WithIgnoreNotFound(true), + WithWaitForDeletion(true), + ) + } + + // Recreate the applications namespace immediately after deletion to avoid cache issues + // The operator's cache expects this namespace to exist at startup + t.Logf("Recreating applications namespace: %s", tc.AppsNamespace) + tc.EventuallyResourceCreatedOrUpdated( + WithObjectToCreate(CreateNamespaceWithLabels(tc.AppsNamespace, nil)), ) + // Also recreate the RHOAI applications namespace to avoid operator cache errors + // The RHOAI-mode operator has this namespace hardcoded in its cache configuration + if tc.AppsNamespace != rhoaiAppsNamespace { + t.Logf("Recreating RHOAI applications namespace for cache: %s", rhoaiAppsNamespace) + tc.EventuallyResourceCreatedOrUpdated( + WithObjectToCreate(CreateNamespaceWithLabels(rhoaiAppsNamespace, nil)), + ) + } + // Cleanup Kueue cluster-scoped resources cleanupKueueOperatorAndResources(t, tc) diff --git a/tests/e2e/creation_test.go b/tests/e2e/creation_test.go index 86e5b14c571e..f9aa88599fff 100644 --- a/tests/e2e/creation_test.go +++ b/tests/e2e/creation_test.go @@ -24,9 +24,11 @@ import ( ) const ( - testNamespace = "test-model-registries" // Namespace used for model registry testing - dsciInstanceNameDuplicate = "e2e-test-dsci-duplicate" // Instance name for the duplicate DSCInitialization resource - dscInstanceNameDuplicate = "e2e-test-dsc-duplicate" // Instance name for the duplicate DataScienceCluster resource + testNamespace = "test-model-registries" // Namespace used for model registry testing + dsciInstanceNameDuplicate = "e2e-test-dsci-duplicate" // Instance name for the duplicate DSCInitialization resource + dscInstanceNameDuplicate = "e2e-test-dsc-duplicate" // Instance name for the duplicate DataScienceCluster resource + defaultHardwareProfileName = "default-profile" // Name of the default hardware profile used in tests + managedAnnotationKey = "opendatahub.io/managed" // Annotation key for managed resources ) // DSCTestCtx holds the context for the DSCInitialization and DataScienceCluster management tests. @@ -288,12 +290,12 @@ func (tc *DSCTestCtx) UpdateRegistriesNamespace(targetNamespace, expectedValue s func (tc *DSCTestCtx) ValidateHardwareProfileCR(t *testing.T) { t.Helper() - // verifed default hardwareprofile exists and api version is correct on v1. + // verified default hardwareprofile exists and api version is correct on v1. tc.EnsureResourceExists( - WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: "default-profile", Namespace: tc.AppsNamespace}), + WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: defaultHardwareProfileName, Namespace: tc.AppsNamespace}), WithCondition(And( jq.Match(`.spec.identifiers[0].defaultCount == 2`), - jq.Match(`.metadata.annotations["opendatahub.io/managed"] == "false"`), + jq.Match(`.metadata.annotations["`+managedAnnotationKey+`"] == "false"`), jq.Match(`.apiVersion == "infrastructure.opendatahub.io/v1"`), )), WithCustomErrorMsg("Default hardwareprofile should have defaultCount=2, managed=false, and use v1 API version"), @@ -301,34 +303,34 @@ func (tc *DSCTestCtx) ValidateHardwareProfileCR(t *testing.T) { // update default hardwareprofile to different value and check it is updated. tc.EventuallyResourceCreatedOrUpdated( - WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: "default-profile", Namespace: tc.AppsNamespace}), + WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: defaultHardwareProfileName, Namespace: tc.AppsNamespace}), WithMutateFunc(testf.Transform(` .spec.identifiers[0].defaultCount = 4 | - .metadata.annotations["opendatahub.io/managed"] = "false" + .metadata.annotations["`+managedAnnotationKey+`"] = "false" `)), WithCondition(And( Succeed(), jq.Match(`.spec.identifiers[0].defaultCount == 4`), - jq.Match(`.metadata.annotations["opendatahub.io/managed"] == "false"`), + jq.Match(`.metadata.annotations["`+managedAnnotationKey+`"] == "false"`), )), WithCustomErrorMsg("Failed to update defaultCount from 2 to 4"), ) tc.EnsureResourceExists( - WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: "default-profile", Namespace: tc.AppsNamespace}), + WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: defaultHardwareProfileName, Namespace: tc.AppsNamespace}), WithCondition(jq.Match(`.spec.identifiers[0].defaultCount == 4`)), WithCustomErrorMsg("Should have defaultCount to 4 but now got %s", jq.Match(`.spec.identifiers[0].defaultCount`)), ) // delete default hardwareprofile and check it is recreated with default values. tc.DeleteResource( - WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: "default-profile", Namespace: tc.AppsNamespace}), + WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: defaultHardwareProfileName, Namespace: tc.AppsNamespace}), ) tc.EventuallyResourceCreatedOrUpdated( - WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: "default-profile", Namespace: tc.AppsNamespace}), + WithMinimalObject(gvk.HardwareProfile, types.NamespacedName{Name: defaultHardwareProfileName, Namespace: tc.AppsNamespace}), WithCondition(And( jq.Match(`.spec.identifiers[0].defaultCount == 2`), - jq.Match(`.metadata.annotations["opendatahub.io/managed"] == "false"`), + jq.Match(`.metadata.annotations["`+managedAnnotationKey+`"] == "false"`), )), WithCustomErrorMsg("Hardware profile was not recreated with default values"), ) diff --git a/tests/e2e/monitoring_test.go b/tests/e2e/monitoring_test.go index 33af522f2ddc..66756d18ef66 100644 --- a/tests/e2e/monitoring_test.go +++ b/tests/e2e/monitoring_test.go @@ -49,13 +49,18 @@ const ( MetricsCPURequest = "100m" MetricsMemoryRequest = "256Mi" // TracesStorage backend types for testing. - TracesStorageBackendPV = "pv" - TracesStorageBackendS3 = "s3" - TracesStorageBackendGCS = "gcs" - TracesStorageSize1Gi = "1Gi" + TracesStorageBackendPV = "pv" + TracesStorageBackendS3 = "s3" + TracesStorageBackendGCS = "gcs" + TracesStorageBackendS3Secret = "s3-secret" + TracesStorageBackendGCSSecret = "gcs-secret" + TracesStorageRetention = "720h" + TracesStorageRetention24h = "24h" + TracesStorageSize10Gi = "10Gi" + TracesStorageSize1Gi = "1Gi" + monitoringTracesConfigMsg = "Monitoring resource should be updated with traces configuration by DSCInitialization controller" ) -// monitoringOwnerReferencesCondition is a reusable condition for validating owner references. var monitoringOwnerReferencesCondition = And( jq.Match(`.metadata.ownerReferences | length == 1`), jq.Match(`.metadata.ownerReferences[0].kind == "%s"`, gvk.Monitoring.Kind), @@ -485,7 +490,7 @@ func (tc *MonitoringTestCtx) ValidateTempoMonolithicCRCreation(t *testing.T) { WithMinimalObject(gvk.Monitoring, types.NamespacedName{Name: MonitoringCRName}), WithCondition(jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionTrue)), WithCondition(jq.Match(`.spec.traces != null`)), - WithCustomErrorMsg("Monitoring resource should be updated with traces configuration by DSCInitialization controller"), + WithCustomErrorMsg(monitoringTracesConfigMsg), ) // Ensure the TempoMonolithic CR is created by the controller (status conditions are set by external tempo operator). @@ -530,7 +535,7 @@ func (tc *MonitoringTestCtx) ValidateTempoStackCRCreationWithCloudStorage(t *tes name: "S3 backend", backend: TracesStorageBackendS3, monitoringCondition: jq.Match(`.spec.traces != null`), - monitoringErrorMsg: "Monitoring resource should be updated with traces configuration by DSCInitialization controller", + monitoringErrorMsg: monitoringTracesConfigMsg, }, { name: "GCS backend", @@ -573,7 +578,7 @@ func (tc *MonitoringTestCtx) ValidateInstrumentationCRTracesLifecycle(t *testing jq.Match(`.spec.traces.storage.retention == "%s"`, FormattedRetention), ), ), - WithCustomErrorMsg("Monitoring resource should be updated with traces configuration by DSCInitialization controller"), + WithCustomErrorMsg(monitoringTracesConfigMsg), ) // Step 3: Wait for Instrumentation CR to be created and fully configured