Merge pull request #3091 from hunterkepley/ocm-20194

OCM-20194 | feat: Win-Li support (machinepool type flag)

Author: openshift-merge-bot[bot]

cmd/create/machinepool/cmd.go

@@ -77,6 +77,10 @@ func CreateMachinepoolRunner(userOptions *mpOpts.CreateMachinepoolUserOptions) r
 			return err
 		}
 
+		if err := machinepool.ValidateImageType(cmd, options.args, cluster); err != nil {
+			return err
+		}
+
 		r.AWSClient, err = aws.NewClient().
 			Region(cluster.Region().ID()).
 			Logger(r.Logger).

cmd/rosa/structure_test/command_args/rosa/create/machinepool/command_args.yml

@@ -26,3 +26,4 @@
 - name: use-spot-instances
 - name: version
 - name: capacity-reservation-id
+- name: type

go.mod

@@ -29,6 +29,7 @@ require (
 	github.com/nathan-fiscaletti/consolesize-go v0.0.0-20210105204122-a87d9f614b9d
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.30.0
+	github.com/openshift-online/ocm-api-model/clientapi v0.0.437
 	github.com/openshift-online/ocm-common v0.0.31
 	github.com/openshift-online/ocm-sdk-go v0.1.482
 	github.com/pkg/errors v0.9.1
@@ -50,7 +51,6 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/ram v1.26.1 // indirect
 	github.com/go-jose/go-jose/v4 v4.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
-	github.com/openshift-online/ocm-api-model/clientapi v0.0.437 // indirect
 	github.com/openshift-online/ocm-api-model/model v0.0.437 // indirect
 	go.yaml.in/yaml/v2 v2.4.3 // indirect
 )

pkg/helper/machinepools/helpers.go

@@ -10,6 +10,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation"
 
+	v1 "github.com/openshift-online/ocm-api-model/clientapi/clustersmgmt/v1"
 	commonUtils "github.com/openshift-online/ocm-common/pkg/utils"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/spf13/cobra"
@@ -32,6 +33,10 @@ const (
 	MaxNodeDrainTimeInHours   = 168
 )
 
+var (
+	ImageTypes = []string{string(v1.ImageTypeDefault), string(v1.ImageTypeWindows)}
+)
+
 var allowedTaintEffects = []string{
 	"NoSchedule",
 	"NoExecute",
@@ -363,3 +368,12 @@ func ValidateUpgradeMaxSurgeUnavailable(val interface{}) error {
 
 	return nil
 }
+
+func IsValidImageType(imageType string) bool {
+	for t := range ImageTypes {
+		if ImageTypes[t] == imageType {
+			return true
+		}
+	}
+	return false
+}

pkg/machinepool/helper.go

@@ -59,6 +59,22 @@ func ValidateLabels(cmd *cobra.Command, args *mpOpts.CreateMachinepoolUserOption
 	return nil
 }
 
+// Validate that the image type is a real one
+func ValidateImageType(cmd *cobra.Command, args *mpOpts.CreateMachinepoolUserOptions, cluster *cmv1.Cluster) error {
+	if cmd.Flags().Changed("type") {
+		if !cluster.Hypershift().Enabled() {
+			return fmt.Errorf("the '--type' flag can only be used with Hosted Control Plane clusters")
+		}
+		imageType := args.Type
+
+		if mpHelpers.IsValidImageType(imageType) {
+			return nil
+		}
+		return fmt.Errorf("invalid image type: '%s' - please use one of: %v", imageType, prettyPrintImageTypes())
+	}
+	return nil
+}
+
 // Validate the cluster's state is ready
 func ValidateClusterState(cluster *cmv1.Cluster, clusterKey string) error {
 	if cluster.State() != cmv1.ClusterStateReady {
@@ -462,3 +478,15 @@ func isDedicatedHost(machinePool *cmv1.MachinePool, runtime *rosa.Runtime) strin
 	}
 	return displayValueNo
 }
+
+func prettyPrintImageTypes() string {
+	s := "'"
+	for i, t := range mpHelpers.ImageTypes {
+		if i == 0 {
+			s += t
+		} else {
+			s += "', '" + t
+		}
+	}
+	return s + "'"
+}

pkg/machinepool/helper_test.go

@@ -9,11 +9,13 @@ import (
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	v1 "github.com/openshift-online/ocm-api-model/clientapi/clustersmgmt/v1"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/spf13/cobra"
 
 	mock "github.com/openshift/rosa/pkg/aws"
 	"github.com/openshift/rosa/pkg/helper/features"
+	mpHelpers "github.com/openshift/rosa/pkg/helper/machinepools"
 	mpOpts "github.com/openshift/rosa/pkg/options/machinepool"
 	"github.com/openshift/rosa/pkg/reporter"
 	"github.com/openshift/rosa/pkg/rosa"
@@ -909,3 +911,71 @@ var _ = Describe("isDedicatedHost Function", func() {
 		Expect(result).To(Equal(displayValueUnknown))
 	})
 })
+
+var _ = Describe("ValidateImageType", func() {
+	It("OK: Pass Windows + Default string as image type", func() {
+		Expect(mpHelpers.IsValidImageType(string(v1.ImageTypeWindows))).To(BeTrue())
+		Expect(mpHelpers.IsValidImageType(string(v1.ImageTypeDefault))).To(BeTrue())
+	})
+	It("KO: Do not pass anything else as image type", func() {
+		Expect(mpHelpers.IsValidImageType("")).To(BeFalse())
+		Expect(mpHelpers.IsValidImageType("win-li")).To(BeFalse())
+		Expect(mpHelpers.IsValidImageType("windows")).To(BeFalse())
+		Expect(mpHelpers.IsValidImageType("unix")).To(BeFalse())
+		Expect(mpHelpers.IsValidImageType("123123123")).To(BeFalse())
+	})
+	It("OK: Validate full flow works", func() {
+		cmd := &cobra.Command{}
+		args := &mpOpts.CreateMachinepoolUserOptions{}
+		cmd.Flags().StringVar(&args.Type, "type", "", "")
+		cluster, err := cmv1.NewCluster().Hypershift(cmv1.NewHypershift().Enabled(true)).Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		args.Type = string(v1.ImageTypeWindows)
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, cluster)
+		Expect(err).ToNot(HaveOccurred())
+
+		args.Type = string(v1.ImageTypeDefault)
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, cluster)
+		Expect(err).ToNot(HaveOccurred())
+	})
+	It("KO: Validate full flow fails", func() {
+		cmd := &cobra.Command{}
+		args := &mpOpts.CreateMachinepoolUserOptions{}
+		cluster, err := cmv1.NewCluster().Hypershift(cmv1.NewHypershift().Enabled(true)).Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		cmd.Flags().StringVar(&args.Type, "type", "", "")
+
+		args.Type = ""
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, cluster)
+		Expect(err).To(HaveOccurred())
+
+		args.Type = "windows"
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, cluster)
+		Expect(err).To(HaveOccurred())
+
+		args.Type = "w123123123"
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, cluster)
+		Expect(err).To(HaveOccurred())
+
+		clusterClassic, err := cmv1.NewCluster().Hypershift(cmv1.NewHypershift().Enabled(false)).Build()
+		Expect(err).ToNot(HaveOccurred())
+
+		args.Type = "w123123123"
+		cmd.SetArgs([]string{fmt.Sprintf("--type=%s", args.Type)})
+		Expect(cmd.Execute()).ToNot(HaveOccurred())
+		err = ValidateImageType(cmd, args, clusterClassic)
+		Expect(err).To(HaveOccurred())
+	})
+})

pkg/machinepool/machinepool.go

@@ -540,6 +540,28 @@ func (m *machinePool) CreateNodePools(r *rosa.Runtime, cmd *cobra.Command, clust
 		return fmt.Errorf("expected a valid name for the machine pool")
 	}
 
+	imageType := ""
+	if args.Type != "" && cluster.Hypershift().Enabled() {
+		imageType = args.Type
+	}
+
+	// Image type (supports things such as WinLI // Windows VMs)
+	if interactive.Enabled() && cluster.Hypershift().Enabled() {
+		imageType, err = interactive.GetOption(interactive.Input{
+			Question: "Image Type",
+			Default:  cmv1.ImageTypeDefault,
+			Required: false,
+			Options:  mpHelpers.ImageTypes,
+		})
+		if err != nil {
+			return fmt.Errorf("expected a valid image type: '%s'", err)
+		}
+	}
+
+	if imageType != "" && !mpHelpers.IsValidImageType(imageType) {
+		return fmt.Errorf("expected a valid image type for the machine pool: '%s'", imageType)
+	}
+
 	// OpenShift version:
 	isVersionSet := cmd.Flags().Changed("version")
 	version := args.Version
@@ -1025,6 +1047,10 @@ func (m *machinePool) CreateNodePools(r *rosa.Runtime, cmd *cobra.Command, clust
 		}
 	}
 
+	if imageType != "" {
+		npBuilder.ImageType(cmv1.ImageType(imageType))
+	}
+
 	if version != "" {
 		npBuilder.Version(cmv1.NewVersion().ID(version))
 	}

pkg/options/machinepool/create.go

@@ -35,6 +35,7 @@ type CreateMachinepoolUserOptions struct {
 	MaxUnavailable        string
 	EC2MetadataHttpTokens string
 	CapacityReservationId string
+	Type                  string
 }
 
 const (
@@ -65,12 +66,6 @@ func NewCreateMachinepoolUserOptions() *CreateMachinepoolUserOptions {
 	return &CreateMachinepoolUserOptions{}
 }
 
-func NewCreateMachinepoolOptions() *CreateMachinepoolOptions {
-	return &CreateMachinepoolOptions{
-		args: &CreateMachinepoolUserOptions{},
-	}
-}
-
 func (m *CreateMachinepoolOptions) Machinepool() *CreateMachinepoolUserOptions {
 	return m.args
 }
@@ -279,6 +274,13 @@ func BuildMachinePoolCreateCommandWithOptions() (*cobra.Command, *CreateMachinep
 		"",
 		"The ID of an AWS On-Demand Capacity Reservation. The 'capacity-reservation-id' must be pre-created "+
 			"in advance, before creating a NodePool.")
+
+	flags.StringVar(&options.Type,
+		"type",
+		"",
+		"Specifies the type of AMI this machinepool uses. You may supply '--type Windows' for example if you want "+
+			"support for Windows VMs.")
+
 	output.AddFlag(cmd)
 	interactive.AddFlag(flags)
 	return cmd, options