Skip to content

AROSLSRE-1158: add --install-scope flag to hypershift install #8725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shubhadapaithankar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

AROSLSRE-1158: add --install-scope flag to hypershift install #8725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 52 additions & 37 deletions cmd/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ type Options struct {
ScaleFromZeroCredentialsSecret string
ScaleFromZeroCredentialsSecretKey string
RenderSensitive bool
InstallScope string
}

func (o *Options) Validate() error {
Expand All @@ -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()...)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
36 changes: 25 additions & 11 deletions cmd/install/install_render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
84 changes: 84 additions & 0 deletions cmd/install/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down