From 67b79f66a9c73eb4299b264122c856c56031f63a Mon Sep 17 00:00:00 2001 From: Shubhadapaithankar Date: Thu, 11 Jun 2026 14:21:45 -0700 Subject: [PATCH] feat: add --install-scope flag to hypershift install Add a new --install-scope flag to the hypershift install command that controls which subset of manifests are applied: - "all" (default): installs CRDs and resources (existing behavior) - "crds": installs only CRDs - "resources": installs only resources (operator deployment and RBAC) Reuses the existing Outputs type from the render subcommand, adding IsValid(), IncludesCRDs(), and IncludesResources() helper methods. Both the install and render paths now use the shared methods, and the render path's switch statement is replaced with the same conditional pattern. The sets import is dropped. Includes unit tests for validation (valid/invalid scope values) and render output filtering (verifying CRDs vs resources in output). This enables consumers to split the hypershift install into phases, allowing CRD-dependent manifests to be applied between the CRD installation and operator startup. Ref: https://redhat.atlassian.net/browse/AROSLSRE-1158 Co-authored-by: celebdor Co-authored-by: Cursor --- cmd/install/install.go | 89 ++++++++++++++++++++--------------- cmd/install/install_render.go | 36 +++++++++----- cmd/install/install_test.go | 84 +++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 48 deletions(-) diff --git a/cmd/install/install.go b/cmd/install/install.go index c13fd743ec1d..0aa97c6f6885 100644 --- a/cmd/install/install.go +++ b/cmd/install/install.go @@ -156,6 +156,7 @@ type Options struct { ScaleFromZeroCredentialsSecret string ScaleFromZeroCredentialsSecretKey string RenderSensitive bool + InstallScope string } func (o *Options) Validate() error { @@ -166,6 +167,10 @@ func (o *Options) Validate() error { o.ScaleFromZeroProvider = strings.ToLower(o.ScaleFromZeroProvider) } + if o.InstallScope != "" && !Outputs(o.InstallScope).IsValid() { + errs = append(errs, fmt.Errorf("invalid --install-scope value %q: must be '%s', '%s', or '%s'", o.InstallScope, OutputAll, OutputCRDs, OutputResources)) + } + errs = append(errs, o.validatePlatformConfig()...) errs = append(errs, o.validateOIDCConfig()...) errs = append(errs, o.validateExternalDNSConfig()...) @@ -439,6 +444,7 @@ func NewCommand() *cobra.Command { cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCreds, "scale-from-zero-creds", opts.ScaleFromZeroCreds, "Path to credentials file for scale-from-zero instance type queries") cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCredentialsSecret, "scale-from-zero-secret", opts.ScaleFromZeroCredentialsSecret, "Name of existing secret containing scale-from-zero credentials (alternative to --scale-from-zero-creds)") cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCredentialsSecretKey, "scale-from-zero-secret-key", opts.ScaleFromZeroCredentialsSecretKey, "Key within the scale-from-zero credentials secret (default: credentials)") + cmd.PersistentFlags().StringVar(&opts.InstallScope, "install-scope", "all", "Scope of installation: 'all' installs CRDs and resources (default), 'crds' installs only CRDs, 'resources' installs only resources (operator deployment and RBAC)") cmd.RunE = func(cmd *cobra.Command, args []string) error { return InstallHyperShiftOperator(cmd.Context(), cmd.OutOrStdout(), opts) @@ -469,59 +475,68 @@ func InstallHyperShiftOperator(ctx context.Context, out io.Writer, opts Options) return err } - // Validate all CRDs via dry-run before applying - if err := dryRunValidateCRDs(ctx, out, crds); err != nil { - return err + scope := Outputs(opts.InstallScope) + if scope == "" { + scope = OutputAll } - // Coordinate with Cluster CAPI Operator if the ClusterAPI API is available. - // This is done after dry-run so the ClusterAPI config is not mutated if CRDs - // cannot be applied. - config, err := util.GetConfig() - if err != nil { - return fmt.Errorf("failed to get kubernetes config: %w", err) - } - discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) - if err != nil { - return fmt.Errorf("failed to create discovery client: %w", err) - } - registered, err := isClusterAPIRegistered(discoveryClient) - if err != nil { - return err - } - if registered { - fmt.Fprintf(out, "ClusterAPI API detected, coordinating with Cluster CAPI Operator\n") - generation, err := ensureUnmanagedCRDs(ctx, out, client, crds) + if scope.IncludesCRDs() { + // Validate all CRDs via dry-run before applying + if err := dryRunValidateCRDs(ctx, out, crds); err != nil { + return err + } + + // Coordinate with Cluster CAPI Operator if the ClusterAPI API is available. + // This is done after dry-run so the ClusterAPI config is not mutated if CRDs + // cannot be applied. + config, err := util.GetConfig() + if err != nil { + return fmt.Errorf("failed to get kubernetes config: %w", err) + } + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return fmt.Errorf("failed to create discovery client: %w", err) + } + registered, err := isClusterAPIRegistered(discoveryClient) if err != nil { return err } - if generation > 0 { - if err := waitForCAPIOperatorSync(ctx, out, client, generation); err != nil { + if registered { + fmt.Fprintf(out, "ClusterAPI API detected, coordinating with Cluster CAPI Operator\n") + generation, err := ensureUnmanagedCRDs(ctx, out, client, crds) + if err != nil { return err } + if generation > 0 { + if err := waitForCAPIOperatorSync(ctx, out, client, generation); err != nil { + return err + } + } } - } - - err = apply(ctx, out, crds) - if err != nil { - return err - } - if opts.WaitUntilAvailable || opts.WaitUntilEstablished { - if err := waitUntilEstablished(ctx, crds); err != nil { + err = apply(ctx, out, crds) + if err != nil { return err } - } - err = apply(ctx, out, objects) - if err != nil { - return err + if opts.WaitUntilAvailable || opts.WaitUntilEstablished { + if err := waitUntilEstablished(ctx, crds); err != nil { + return err + } + } } - if opts.WaitUntilAvailable { - if _, err := WaitUntilAvailable(ctx, opts); err != nil { + if scope.IncludesResources() { + err = apply(ctx, out, objects) + if err != nil { return err } + + if opts.WaitUntilAvailable { + if _, err := WaitUntilAvailable(ctx, opts); err != nil { + return err + } + } } return nil } diff --git a/cmd/install/install_render.go b/cmd/install/install_render.go index e420ecaed8d7..ee305114f960 100644 --- a/cmd/install/install_render.go +++ b/cmd/install/install_render.go @@ -12,7 +12,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/sets" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,6 +26,23 @@ const ( OutputResources Outputs = "resources" ) +func (o Outputs) IsValid() bool { + switch o { + case OutputAll, OutputCRDs, OutputResources: + return true + default: + return false + } +} + +func (o Outputs) IncludesCRDs() bool { + return o == OutputAll || o == OutputCRDs +} + +func (o Outputs) IncludesResources() bool { + return o == OutputAll || o == OutputResources +} + var ( RenderFormatYaml = "yaml" RenderFormatJson = "json" @@ -89,9 +105,8 @@ func (o *Options) ValidateRender() error { return fmt.Errorf("--format must be %s or %s", RenderFormatYaml, RenderFormatJson) } - outputs := sets.New(OutputAll, OutputCRDs, OutputResources) - if !outputs.Has(Outputs(o.OutputTypes)) { - return fmt.Errorf("--outputs must be one of %v", outputs.UnsortedList()) + if !Outputs(o.OutputTypes).IsValid() { + return fmt.Errorf("--outputs must be one of %s, %s, or %s", OutputAll, OutputCRDs, OutputResources) } return nil @@ -126,14 +141,13 @@ func RenderHyperShiftOperator(ctx context.Context, cmdOut io.Writer, opts *Optio } } + scope := Outputs(opts.OutputTypes) var objectsToRender []crclient.Object - switch Outputs(opts.OutputTypes) { - case OutputAll: - objectsToRender = append(crds, objects...) - case OutputCRDs: - objectsToRender = crds - case OutputResources: - objectsToRender = objects + if scope.IncludesCRDs() { + objectsToRender = append(objectsToRender, crds...) + } + if scope.IncludesResources() { + objectsToRender = append(objectsToRender, objects...) } if !opts.RenderSensitive { diff --git a/cmd/install/install_test.go b/cmd/install/install_test.go index 842f334651fe..957f7c010c6f 100644 --- a/cmd/install/install_test.go +++ b/cmd/install/install_test.go @@ -360,6 +360,34 @@ func TestOptions_Validate(t *testing.T) { }, expectError: false, }, + "when install-scope is all there is no error": { + inputOptions: Options{ + PrivatePlatform: string(hyperv1.NonePlatform), + InstallScope: string(OutputAll), + }, + expectError: false, + }, + "when install-scope is crds there is no error": { + inputOptions: Options{ + PrivatePlatform: string(hyperv1.NonePlatform), + InstallScope: string(OutputCRDs), + }, + expectError: false, + }, + "when install-scope is resources there is no error": { + inputOptions: Options{ + PrivatePlatform: string(hyperv1.NonePlatform), + InstallScope: string(OutputResources), + }, + expectError: false, + }, + "when install-scope is invalid it errors": { + inputOptions: Options{ + PrivatePlatform: string(hyperv1.NonePlatform), + InstallScope: "bogus", + }, + expectError: true, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { @@ -746,6 +774,62 @@ func TestRenderHyperShiftOperator_RenderSensitive(t *testing.T) { }) } +func TestRenderOutputsScope(t *testing.T) { + baseOpts := Options{ + PrivatePlatform: string(hyperv1.NonePlatform), + Format: RenderFormatYaml, + RenderSensitive: true, + } + + tests := []struct { + name string + outputs string + expectCRDs bool + expectResources bool + }{ + {"all includes CRDs and resources", string(OutputAll), true, true}, + {"crds includes only CRDs", string(OutputCRDs), true, false}, + {"resources includes only resources", string(OutputResources), false, true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + opts := baseOpts + opts.OutputTypes = tc.outputs + var buf bytes.Buffer + err := RenderHyperShiftOperator(t.Context(), &buf, &opts) + g.Expect(err).NotTo(HaveOccurred(), "RenderHyperShiftOperator failed for --outputs=%s", tc.outputs) + + var crdCount, resourceCount int + for doc := range strings.SplitSeq(buf.String(), "\n---\n") { + if strings.TrimSpace(doc) == "" { + continue + } + obj, _, err := hyperapi.YamlSerializer.Decode([]byte(doc), nil, nil) + g.Expect(err).NotTo(HaveOccurred(), "failed to decode rendered manifest") + if _, isCRD := obj.(*apiextensionsv1.CustomResourceDefinition); isCRD { + crdCount++ + } else { + resourceCount++ + } + } + + if tc.expectCRDs { + g.Expect(crdCount).To(BeNumerically(">", 0), "expected CRDs in output for --outputs=%s", tc.outputs) + } else { + g.Expect(crdCount).To(Equal(0), "expected no CRDs in output for --outputs=%s, got %d", tc.outputs, crdCount) + } + if tc.expectResources { + g.Expect(resourceCount).To(BeNumerically(">", 0), "expected resources in output for --outputs=%s", tc.outputs) + } else { + g.Expect(resourceCount).To(Equal(0), "expected no resources in output for --outputs=%s, got %d", tc.outputs, resourceCount) + } + }) + } +} + func TestHyperShiftOperatorManifests_SharedIngress(t *testing.T) { tests := []struct { name string