From 024537e2c894b9627629d5b7b4061966206814b5 Mon Sep 17 00:00:00 2001 From: Joseph Date: Tue, 21 Apr 2026 14:43:35 -0400 Subject: [PATCH 1/2] OADP-7868: Skip CLI/VMDP download setup when Console CRD is absent On OCP 4.22 clusters without Console capability (e.g. SNO), the ConsoleCLIDownload CRD does not exist. The CLIDownloadSetup and VMDPDownloadSetup runnables crash with a REST mapper error because the code only handles errors.IsNotFound, not CRD-level absence. Check CRD availability via RESTMapper before proceeding. When the CRD is missing, skip all resource creation gracefully. Signed-off-by: Joseph --- .../controller/cli_download_controller.go | 4 ++ .../cli_download_controller_test.go | 64 +++++++++++++++++++ internal/controller/console.go | 27 ++++++++ internal/controller/console_test.go | 50 +++++++++++++++ .../controller/vmdp_download_controller.go | 4 ++ .../vmdp_download_controller_test.go | 64 +++++++++++++++++++ 6 files changed, 213 insertions(+) create mode 100644 internal/controller/cli_download_controller_test.go create mode 100644 internal/controller/console.go create mode 100644 internal/controller/console_test.go create mode 100644 internal/controller/vmdp_download_controller_test.go diff --git a/internal/controller/cli_download_controller.go b/internal/controller/cli_download_controller.go index 0fe463812d..58c24a1676 100644 --- a/internal/controller/cli_download_controller.go +++ b/internal/controller/cli_download_controller.go @@ -44,6 +44,10 @@ func (c *CLIDownloadSetup) Start(ctx context.Context) error { c.Log = ctrl.Log.WithName("cli-download-setup") c.Log.Info("Starting CLI download setup") + if available, err := isConsoleCRDAvailable(c.Client.RESTMapper(), c.Log); !available { + return err + } + // Get the CLI server image from environment variable cliServerImage := os.Getenv("RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD") if cliServerImage == "" { diff --git a/internal/controller/cli_download_controller_test.go b/internal/controller/cli_download_controller_test.go new file mode 100644 index 0000000000..80fe58f520 --- /dev/null +++ b/internal/controller/cli_download_controller_test.go @@ -0,0 +1,64 @@ +package controller + +import ( + "context" + "testing" + + consolev1 "github.com/openshift/api/console/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestCLIDownloadSetup_StartSkipsWhenConsoleNotAvailable(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, appsv1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, routev1.Install(scheme)) + require.NoError(t, consolev1.Install(scheme)) + + mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{ + appsv1.SchemeGroupVersion, + corev1.SchemeGroupVersion, + routev1.GroupVersion, + }) + mapper.Add(appsv1.SchemeGroupVersion.WithKind("Deployment"), meta.RESTScopeNamespace) + mapper.Add(corev1.SchemeGroupVersion.WithKind("Service"), meta.RESTScopeNamespace) + mapper.Add(routev1.GroupVersion.WithKind("Route"), meta.RESTScopeNamespace) + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithRESTMapper(mapper). + Build() + + setup := &CLIDownloadSetup{ + Client: client, + Namespace: "openshift-adp", + OperatorName: "openshift-adp-controller-manager", + OperatorNamespace: "openshift-adp", + } + + err := setup.Start(context.Background()) + require.NoError(t, err) + + deploy := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: cliServerDeploymentName, + Namespace: "openshift-adp", + }, deploy) + assert.Error(t, err, "deployment should not exist") + + svc := &corev1.Service{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: cliServerServiceName, + Namespace: "openshift-adp", + }, svc) + assert.Error(t, err, "service should not exist") +} diff --git a/internal/controller/console.go b/internal/controller/console.go new file mode 100644 index 0000000000..edab4d8bbd --- /dev/null +++ b/internal/controller/console.go @@ -0,0 +1,27 @@ +package controller + +import ( + "fmt" + + "github.com/go-logr/logr" + consolev1 "github.com/openshift/api/console/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// isConsoleCRDAvailable checks whether the ConsoleCLIDownload CRD is registered +// with the API server. Returns false when the Console capability is not enabled +// (e.g. SNO clusters). +func isConsoleCRDAvailable(mapper meta.RESTMapper, log logr.Logger) (bool, error) { + _, err := mapper.RESTMapping( + schema.GroupKind{Group: consolev1.GroupVersion.Group, Kind: "ConsoleCLIDownload"}, + ) + if err != nil { + if meta.IsNoMatchError(err) { + log.Info("ConsoleCLIDownload CRD not available (Console capability not enabled), skipping setup") + return false, nil + } + return false, fmt.Errorf("failed to check ConsoleCLIDownload CRD availability: %w", err) + } + return true, nil +} diff --git a/internal/controller/console_test.go b/internal/controller/console_test.go new file mode 100644 index 0000000000..88da55ddf6 --- /dev/null +++ b/internal/controller/console_test.go @@ -0,0 +1,50 @@ +package controller + +import ( + "testing" + + consolev1 "github.com/openshift/api/console/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestIsConsoleCRDAvailable(t *testing.T) { + log := ctrl.Log.WithName("test") + + tests := []struct { + name string + includeConsoleCRD bool + wantAvailable bool + }{ + { + name: "returns false when ConsoleCLIDownload CRD is not registered", + includeConsoleCRD: false, + wantAvailable: false, + }, + { + name: "returns true when ConsoleCLIDownload CRD is registered", + includeConsoleCRD: true, + wantAvailable: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var groupVersions []schema.GroupVersion + if tt.includeConsoleCRD { + groupVersions = append(groupVersions, consolev1.GroupVersion) + } + mapper := meta.NewDefaultRESTMapper(groupVersions) + if tt.includeConsoleCRD { + mapper.Add(consolev1.GroupVersion.WithKind("ConsoleCLIDownload"), meta.RESTScopeRoot) + } + + available, err := isConsoleCRDAvailable(mapper, log) + require.NoError(t, err) + assert.Equal(t, tt.wantAvailable, available) + }) + } +} diff --git a/internal/controller/vmdp_download_controller.go b/internal/controller/vmdp_download_controller.go index 27c91a5b41..716bdfe3bb 100644 --- a/internal/controller/vmdp_download_controller.go +++ b/internal/controller/vmdp_download_controller.go @@ -42,6 +42,10 @@ func (v *VMDPDownloadSetup) Start(ctx context.Context) error { v.Log = ctrl.Log.WithName("vmdp-download-setup") v.Log.Info("Starting VMDP download setup") + if available, err := isConsoleCRDAvailable(v.Client.RESTMapper(), v.Log); !available { + return err + } + // Get the VMDP server image from environment variable vmdpServerImage := os.Getenv("RELATED_IMAGE_VMDP_CLI_DOWNLOAD") if vmdpServerImage == "" { diff --git a/internal/controller/vmdp_download_controller_test.go b/internal/controller/vmdp_download_controller_test.go new file mode 100644 index 0000000000..33a0e6aa56 --- /dev/null +++ b/internal/controller/vmdp_download_controller_test.go @@ -0,0 +1,64 @@ +package controller + +import ( + "context" + "testing" + + consolev1 "github.com/openshift/api/console/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestVMDPDownloadSetup_StartSkipsWhenConsoleNotAvailable(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, appsv1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, routev1.Install(scheme)) + require.NoError(t, consolev1.Install(scheme)) + + mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{ + appsv1.SchemeGroupVersion, + corev1.SchemeGroupVersion, + routev1.GroupVersion, + }) + mapper.Add(appsv1.SchemeGroupVersion.WithKind("Deployment"), meta.RESTScopeNamespace) + mapper.Add(corev1.SchemeGroupVersion.WithKind("Service"), meta.RESTScopeNamespace) + mapper.Add(routev1.GroupVersion.WithKind("Route"), meta.RESTScopeNamespace) + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithRESTMapper(mapper). + Build() + + setup := &VMDPDownloadSetup{ + Client: client, + Namespace: "openshift-adp", + OperatorName: "openshift-adp-controller-manager", + OperatorNamespace: "openshift-adp", + } + + err := setup.Start(context.Background()) + require.NoError(t, err) + + deploy := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: vmdpServerDeploymentName, + Namespace: "openshift-adp", + }, deploy) + assert.Error(t, err, "deployment should not exist") + + svc := &corev1.Service{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: vmdpServerServiceName, + Namespace: "openshift-adp", + }, svc) + assert.Error(t, err, "service should not exist") +} From 9aa2d02d5f0196c9f647e5dcf0be581861aceecc Mon Sep 17 00:00:00 2001 From: Joseph Date: Tue, 21 Apr 2026 16:27:20 -0400 Subject: [PATCH 2/2] Move check to main.go Signed-off-by: Joseph --- cmd/main.go | 15 +++-- .../controller/cli_download_controller.go | 4 -- .../cli_download_controller_test.go | 64 ------------------- internal/controller/console.go | 4 +- internal/controller/console_test.go | 2 +- .../controller/vmdp_download_controller.go | 4 -- .../vmdp_download_controller_test.go | 64 ------------------- 7 files changed, 13 insertions(+), 144 deletions(-) delete mode 100644 internal/controller/cli_download_controller_test.go delete mode 100644 internal/controller/vmdp_download_controller_test.go diff --git a/cmd/main.go b/cmd/main.go index 1dc0143a8e..6c721986ba 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -299,9 +299,16 @@ func main() { } //+kubebuilder:scaffold:builder - // Add CLI download setup runnable - // Only add if watchNamespace is set (skip for cluster-scoped mode) - if watchNamespace != "" { + // Add CLI/VMDP download setup runnables. + // Skip when namespace-scoped mode is off or the ConsoleCLIDownload CRD + // is absent (clusters without Console capability, e.g. SNO). + if watchNamespace == "" { + setupLog.Info("Skipping CLI and VMDP download setup - watchNamespace not set") + } else if available, err := controller.IsConsoleCRDAvailable(mgr.GetRESTMapper(), setupLog); !available { + if err != nil { + setupLog.Error(err, "unable to check ConsoleCLIDownload CRD availability, skipping CLI/VMDP download setup") + } + } else { if err := mgr.Add(&controller.CLIDownloadSetup{ Client: mgr.GetClient(), Namespace: watchNamespace, @@ -320,8 +327,6 @@ func main() { setupLog.Error(err, "unable to add VMDP download setup") os.Exit(1) } - } else { - setupLog.Info("Skipping CLI and VMDP download setup - watchNamespace not set") } if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/internal/controller/cli_download_controller.go b/internal/controller/cli_download_controller.go index 58c24a1676..0fe463812d 100644 --- a/internal/controller/cli_download_controller.go +++ b/internal/controller/cli_download_controller.go @@ -44,10 +44,6 @@ func (c *CLIDownloadSetup) Start(ctx context.Context) error { c.Log = ctrl.Log.WithName("cli-download-setup") c.Log.Info("Starting CLI download setup") - if available, err := isConsoleCRDAvailable(c.Client.RESTMapper(), c.Log); !available { - return err - } - // Get the CLI server image from environment variable cliServerImage := os.Getenv("RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD") if cliServerImage == "" { diff --git a/internal/controller/cli_download_controller_test.go b/internal/controller/cli_download_controller_test.go deleted file mode 100644 index 80fe58f520..0000000000 --- a/internal/controller/cli_download_controller_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package controller - -import ( - "context" - "testing" - - consolev1 "github.com/openshift/api/console/v1" - routev1 "github.com/openshift/api/route/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestCLIDownloadSetup_StartSkipsWhenConsoleNotAvailable(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, appsv1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) - require.NoError(t, routev1.Install(scheme)) - require.NoError(t, consolev1.Install(scheme)) - - mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{ - appsv1.SchemeGroupVersion, - corev1.SchemeGroupVersion, - routev1.GroupVersion, - }) - mapper.Add(appsv1.SchemeGroupVersion.WithKind("Deployment"), meta.RESTScopeNamespace) - mapper.Add(corev1.SchemeGroupVersion.WithKind("Service"), meta.RESTScopeNamespace) - mapper.Add(routev1.GroupVersion.WithKind("Route"), meta.RESTScopeNamespace) - - client := fake.NewClientBuilder(). - WithScheme(scheme). - WithRESTMapper(mapper). - Build() - - setup := &CLIDownloadSetup{ - Client: client, - Namespace: "openshift-adp", - OperatorName: "openshift-adp-controller-manager", - OperatorNamespace: "openshift-adp", - } - - err := setup.Start(context.Background()) - require.NoError(t, err) - - deploy := &appsv1.Deployment{} - err = client.Get(context.Background(), types.NamespacedName{ - Name: cliServerDeploymentName, - Namespace: "openshift-adp", - }, deploy) - assert.Error(t, err, "deployment should not exist") - - svc := &corev1.Service{} - err = client.Get(context.Background(), types.NamespacedName{ - Name: cliServerServiceName, - Namespace: "openshift-adp", - }, svc) - assert.Error(t, err, "service should not exist") -} diff --git a/internal/controller/console.go b/internal/controller/console.go index edab4d8bbd..0eb98625ef 100644 --- a/internal/controller/console.go +++ b/internal/controller/console.go @@ -9,10 +9,10 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -// isConsoleCRDAvailable checks whether the ConsoleCLIDownload CRD is registered +// IsConsoleCRDAvailable checks whether the ConsoleCLIDownload CRD is registered // with the API server. Returns false when the Console capability is not enabled // (e.g. SNO clusters). -func isConsoleCRDAvailable(mapper meta.RESTMapper, log logr.Logger) (bool, error) { +func IsConsoleCRDAvailable(mapper meta.RESTMapper, log logr.Logger) (bool, error) { _, err := mapper.RESTMapping( schema.GroupKind{Group: consolev1.GroupVersion.Group, Kind: "ConsoleCLIDownload"}, ) diff --git a/internal/controller/console_test.go b/internal/controller/console_test.go index 88da55ddf6..7f097cd281 100644 --- a/internal/controller/console_test.go +++ b/internal/controller/console_test.go @@ -42,7 +42,7 @@ func TestIsConsoleCRDAvailable(t *testing.T) { mapper.Add(consolev1.GroupVersion.WithKind("ConsoleCLIDownload"), meta.RESTScopeRoot) } - available, err := isConsoleCRDAvailable(mapper, log) + available, err := IsConsoleCRDAvailable(mapper, log) require.NoError(t, err) assert.Equal(t, tt.wantAvailable, available) }) diff --git a/internal/controller/vmdp_download_controller.go b/internal/controller/vmdp_download_controller.go index 716bdfe3bb..27c91a5b41 100644 --- a/internal/controller/vmdp_download_controller.go +++ b/internal/controller/vmdp_download_controller.go @@ -42,10 +42,6 @@ func (v *VMDPDownloadSetup) Start(ctx context.Context) error { v.Log = ctrl.Log.WithName("vmdp-download-setup") v.Log.Info("Starting VMDP download setup") - if available, err := isConsoleCRDAvailable(v.Client.RESTMapper(), v.Log); !available { - return err - } - // Get the VMDP server image from environment variable vmdpServerImage := os.Getenv("RELATED_IMAGE_VMDP_CLI_DOWNLOAD") if vmdpServerImage == "" { diff --git a/internal/controller/vmdp_download_controller_test.go b/internal/controller/vmdp_download_controller_test.go deleted file mode 100644 index 33a0e6aa56..0000000000 --- a/internal/controller/vmdp_download_controller_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package controller - -import ( - "context" - "testing" - - consolev1 "github.com/openshift/api/console/v1" - routev1 "github.com/openshift/api/route/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestVMDPDownloadSetup_StartSkipsWhenConsoleNotAvailable(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, appsv1.AddToScheme(scheme)) - require.NoError(t, corev1.AddToScheme(scheme)) - require.NoError(t, routev1.Install(scheme)) - require.NoError(t, consolev1.Install(scheme)) - - mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{ - appsv1.SchemeGroupVersion, - corev1.SchemeGroupVersion, - routev1.GroupVersion, - }) - mapper.Add(appsv1.SchemeGroupVersion.WithKind("Deployment"), meta.RESTScopeNamespace) - mapper.Add(corev1.SchemeGroupVersion.WithKind("Service"), meta.RESTScopeNamespace) - mapper.Add(routev1.GroupVersion.WithKind("Route"), meta.RESTScopeNamespace) - - client := fake.NewClientBuilder(). - WithScheme(scheme). - WithRESTMapper(mapper). - Build() - - setup := &VMDPDownloadSetup{ - Client: client, - Namespace: "openshift-adp", - OperatorName: "openshift-adp-controller-manager", - OperatorNamespace: "openshift-adp", - } - - err := setup.Start(context.Background()) - require.NoError(t, err) - - deploy := &appsv1.Deployment{} - err = client.Get(context.Background(), types.NamespacedName{ - Name: vmdpServerDeploymentName, - Namespace: "openshift-adp", - }, deploy) - assert.Error(t, err, "deployment should not exist") - - svc := &corev1.Service{} - err = client.Get(context.Background(), types.NamespacedName{ - Name: vmdpServerServiceName, - Namespace: "openshift-adp", - }, svc) - assert.Error(t, err, "service should not exist") -}