From 73e976696761ef695c516aef4ff0c0d8d577f4ea Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 31 Jul 2023 09:45:54 -0700 Subject: [PATCH 01/11] Added subscription_type parameter to create cluster command --- cmd/ocm/create/cluster/cmd.go | 32 ++++++++++++++++++++++++++++++++ pkg/cluster/cluster.go | 2 ++ 2 files changed, 34 insertions(+) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 21faffe8..6e571b1a 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -39,11 +39,16 @@ import ( "github.com/spf13/pflag" ) +var ValidSubscriptionTypes = []string{standardSubscriptionType, marketplaceRhmSubscriptionType, marketplaceGcpSubscriptionType} + const ( defaultIngressRouteSelectorFlag = "default-ingress-route-selector" defaultIngressExcludedNamespacesFlag = "default-ingress-excluded-namespaces" defaultIngressWildcardPolicyFlag = "default-ingress-wildcard-policy" defaultIngressNamespaceOwnershipPolicyFlag = "default-ingress-namespace-ownership-policy" + standardSubscriptionType = "standard" + marketplaceRhmSubscriptionType = "marketplace-rhm" + marketplaceGcpSubscriptionType = "marketplace-gcp" ) var args struct { @@ -68,6 +73,7 @@ var args struct { clusterWideProxy c.ClusterWideProxy gcpServiceAccountFile arguments.FilePath etcdEncryption bool + subscriptionType string // Scaling options computeMachineType string @@ -301,6 +307,14 @@ func init() { fmt.Sprintf("Namespace Ownership Policy for ingress. Options are %s", strings.Join(ingress.ValidNamespaceOwnershipPolicies, ",")), ) + + fs.StringVar( + &args.subscriptionType, + "subscription-type", + standardSubscriptionType, + fmt.Sprintf("The subscription billing model for the cluster. Options are %s", + strings.Join(ValidSubscriptionTypes, ",")), + ) } func osdProviderOptions(_ *sdk.Connection) ([]arguments.Option, error) { @@ -377,6 +391,18 @@ func getVersionOptionsWithDefault(connection *sdk.Connection, channelGroup strin return } +func getSubscriptionTypeOptions() []arguments.Option { + options := []arguments.Option{} + for _, subscriptionType := range ValidSubscriptionTypes { + description := "" + options = append(options, arguments.Option{ + Value: subscriptionType, + Description: description, + }) + } + return options +} + func getMachineTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { return provider.GetMachineTypeOptions( connection.ClustersMgmt().V1(), @@ -530,6 +556,11 @@ func preRun(cmd *cobra.Command, argv []string) error { if err != nil { return err } + + if err = arguments.CheckOneOf(fs, "subscription-type", getSubscriptionTypeOptions()); err != nil { + return err + } + return nil } @@ -577,6 +608,7 @@ func run(cmd *cobra.Command, argv []string) error { Private: &args.private, EtcdEncryption: args.etcdEncryption, DefaultIngress: defaultIngress, + SubscriptionType: args.subscriptionType, } cluster, err := c.CreateCluster(connection.ClustersMgmt().V1(), clusterConfig, args.dryRun) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 67ad48d0..3395f7af 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -68,6 +68,7 @@ type Spec struct { ChannelGroup string Expiration time.Time EtcdEncryption bool + SubscriptionType string // Scaling config ComputeMachineType string @@ -312,6 +313,7 @@ func CreateCluster(cmv1Client *cmv1.Client, config Spec, dryRun bool) (*cmv1.Clu ID(config.Flavour), ). EtcdEncryption(config.EtcdEncryption). + BillingModel(cmv1.BillingModel(config.SubscriptionType)). Properties(clusterProperties) clusterBuilder = clusterBuilder.Version( From 206f64eaa79043df4e0431a1377968df855f9429 Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 6 Sep 2023 12:01:11 -0700 Subject: [PATCH 02/11] Fetched subscription type options from ocm-sdk and disabled ccs for interactive marketplace-gcp --- cmd/ocm/create/cluster/cmd.go | 96 +++++++++++++++++++++++++++++------ go.mod | 2 +- go.sum | 4 +- pkg/cluster/cluster.go | 4 +- pkg/cluster/describe.go | 2 + 5 files changed, 87 insertions(+), 21 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 6e571b1a..ba9f1969 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -33,20 +33,21 @@ import ( "github.com/openshift-online/ocm-cli/pkg/provider" "github.com/openshift-online/ocm-cli/pkg/utils" sdk "github.com/openshift-online/ocm-sdk-go" + amv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/rosa/pkg/interactive" "github.com/spf13/cobra" "github.com/spf13/pflag" ) -var ValidSubscriptionTypes = []string{standardSubscriptionType, marketplaceRhmSubscriptionType, marketplaceGcpSubscriptionType} +var ValidSubscriptionTypes = []string{StandardSubscriptionType, marketplaceRhmSubscriptionType, marketplaceGcpSubscriptionType} const ( defaultIngressRouteSelectorFlag = "default-ingress-route-selector" defaultIngressExcludedNamespacesFlag = "default-ingress-excluded-namespaces" defaultIngressWildcardPolicyFlag = "default-ingress-wildcard-policy" defaultIngressNamespaceOwnershipPolicyFlag = "default-ingress-namespace-ownership-policy" - standardSubscriptionType = "standard" + StandardSubscriptionType = "standard" marketplaceRhmSubscriptionType = "marketplace-rhm" marketplaceGcpSubscriptionType = "marketplace-gcp" ) @@ -311,10 +312,12 @@ func init() { fs.StringVar( &args.subscriptionType, "subscription-type", - standardSubscriptionType, + StandardSubscriptionType, fmt.Sprintf("The subscription billing model for the cluster. Options are %s", strings.Join(ValidSubscriptionTypes, ",")), ) + arguments.SetQuestion(fs, "subscription-type", "Subscription type:") + Cmd.RegisterFlagCompletionFunc("subscription-type", arguments.MakeCompleteFunc(getSubscriptionTypeOptions)) } func osdProviderOptions(_ *sdk.Connection) ([]arguments.Option, error) { @@ -391,16 +394,58 @@ func getVersionOptionsWithDefault(connection *sdk.Connection, channelGroup strin return } -func getSubscriptionTypeOptions() []arguments.Option { +func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { options := []arguments.Option{} - for _, subscriptionType := range ValidSubscriptionTypes { - description := "" + billingModels, err := getBillingModels(connection) + if err != nil { + return options, err + } + for _, billingModel := range billingModels { options = append(options, arguments.Option{ - Value: subscriptionType, - Description: description, + Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), + Description: billingModel.Description(), }) } - return options + return options, nil +} + +func getSubscriptionTypeIdFromDescription(connection *sdk.Connection, value string) string { + billingModels, _ := getBillingModels(connection) + for _, billingModel := range billingModels { + if value == fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()) { + return billingModel.Id() + } + } + return "" +} + +func getBillingModel(connection *sdk.Connection, billingModelID string) (*amv1.BillingModelItem, error) { + billingModels, _ := getBillingModels(connection) + var bm *amv1.BillingModelItem + for _, billingModel := range billingModels { + if billingModel.Id() == billingModelID { + bm = billingModel + break + } + } + return bm, nil +} + +func getBillingModels(connection *sdk.Connection) ([]*amv1.BillingModelItem, error) { + response, err := connection.AccountsMgmt().V1().BillingModels().List().Send() + if err != nil { + return nil, err + } + billingModels := response.Items().Slice() + var validBillingModel []*amv1.BillingModelItem + for _, billingModel := range billingModels { + for _, validSubscriptionTypeId := range ValidSubscriptionTypes { + if billingModel.Id() == validSubscriptionTypeId { + validBillingModel = append(validBillingModel, billingModel) + } + } + } + return validBillingModel, nil } func getMachineTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { @@ -447,9 +492,25 @@ func preRun(cmd *cobra.Command, argv []string) error { // Validate flags / ask for missing data. fs := cmd.Flags() + subscriptionTypeOptions, _ := getSubscriptionTypeOptions(connection) + err = arguments.PromptOneOf(fs, "subscription-type", subscriptionTypeOptions) + if err != nil { + return err + } + // Only offer the 2 providers known to support OSD now; // but don't validate if set, to not block `ocm` CLI from creating clusters on future providers. providers, _ := osdProviderOptions(connection) + + gcpBillingModel, _ := getBillingModel(connection, marketplaceGcpSubscriptionType) + isGcpSubscriptionType := args.subscriptionType == fmt.Sprintf("%s (%s)", gcpBillingModel.Id(), gcpBillingModel.Description()) + if isGcpSubscriptionType { + providers = []arguments.Option{ + {Value: c.ProviderGCP, Description: ""}, + } + args.ccs.Enabled = true + } + err = arguments.PromptOneOf(fs, "provider", providers) if err != nil { return err @@ -461,7 +522,7 @@ func preRun(cmd *cobra.Command, argv []string) error { args.clusterWideProxy.Enabled = true } - err = promptCCS(fs) + err = promptCCS(fs, isGcpSubscriptionType) if err != nil { return err } @@ -557,10 +618,6 @@ func preRun(cmd *cobra.Command, argv []string) error { return err } - if err = arguments.CheckOneOf(fs, "subscription-type", getSubscriptionTypeOptions()); err != nil { - return err - } - return nil } @@ -585,6 +642,10 @@ func run(cmd *cobra.Command, argv []string) error { return err } + if args.interactive { + args.subscriptionType = getSubscriptionTypeIdFromDescription(connection, args.subscriptionType) + } + clusterConfig := c.Spec{ Name: args.clusterName, Region: args.region, @@ -1068,8 +1129,11 @@ func promptExistingVPC(fs *pflag.FlagSet, connection *sdk.Connection) error { return err } -func promptCCS(fs *pflag.FlagSet) error { - err := arguments.PromptBool(fs, "ccs") +func promptCCS(fs *pflag.FlagSet, isGcpSubscriptionType bool) error { + var err error + if !isGcpSubscriptionType { + err = arguments.PromptBool(fs, "ccs") + } if err != nil { return err } diff --git a/go.mod b/go.mod index 91d732b9..651cd995 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/nwidger/jsoncolor v0.3.2 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.8 - github.com/openshift-online/ocm-sdk-go v0.1.356 + github.com/openshift-online/ocm-sdk-go v0.1.367 github.com/openshift/rosa v1.2.24 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 github.com/spf13/cobra v1.7.0 diff --git a/go.sum b/go.sum index 0f1ba27f..7516f87d 100644 --- a/go.sum +++ b/go.sum @@ -336,8 +336,8 @@ github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAl github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/onsi/gomega v1.27.8 h1:gegWiwZjBsf2DgiSbf5hpokZ98JVDMcWkUiigk6/KXc= github.com/onsi/gomega v1.27.8/go.mod h1:2J8vzI/s+2shY9XHRApDkdgPo1TKT7P2u6fXeJKFnNQ= -github.com/openshift-online/ocm-sdk-go v0.1.356 h1:FFUAi+mR5pdBsp3pisXp7M4SiFdxKQwNwKTQNY7QDY0= -github.com/openshift-online/ocm-sdk-go v0.1.356/go.mod h1:KYOw8kAKAHyPrJcQoVR82CneQ4ofC02Na4cXXaTq4Nw= +github.com/openshift-online/ocm-sdk-go v0.1.367 h1:17eJeoorPLhrHRte3vUGPxIKRDS8BEp1Uv+XhHPY87k= +github.com/openshift-online/ocm-sdk-go v0.1.367/go.mod h1:KYOw8kAKAHyPrJcQoVR82CneQ4ofC02Na4cXXaTq4Nw= github.com/openshift/rosa v1.2.24 h1:vv0yYnWHx6CCPEAau/0rS54P2ksaf+uWXb1TQPWxiYE= github.com/openshift/rosa v1.2.24/go.mod h1:MVXB27O3PF8WoOic23I03mmq6/9kVxpFx6FKyLMCyrQ= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 3395f7af..dbdb08a1 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -27,7 +27,7 @@ import ( "time" sdk "github.com/openshift-online/ocm-sdk-go" - amsv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" + amv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" ) @@ -705,7 +705,7 @@ func GetClusterAddOns(connection *sdk.Connection, clusterID string) ([]*AddOnIte } // Only display add-ons for which the org has quota - quotaCosts.Each(func(quotaCost *amsv1.QuotaCost) bool { + quotaCosts.Each(func(quotaCost *amv1.QuotaCost) bool { relatedResources := quotaCost.RelatedResources() for _, relatedResource := range relatedResources { if relatedResource.ResourceType() == "add-on" && diff --git a/pkg/cluster/describe.go b/pkg/cluster/describe.go index d0d26fd9..371cbaf7 100644 --- a/pkg/cluster/describe.go +++ b/pkg/cluster/describe.go @@ -196,6 +196,7 @@ func PrintClusterDescription(connection *sdk.Connection, cluster *cmv1.Cluster) "Infra: %d\n"+ "Computes: %s\n"+ "Product: %s\n"+ + "Subscription type: %s\n"+ "Provider: %s\n"+ "Version: %s\n"+ "Region: %s\n"+ @@ -221,6 +222,7 @@ func PrintClusterDescription(connection *sdk.Connection, cluster *cmv1.Cluster) cluster.Nodes().Infra(), computesStr, cluster.Product().ID(), + cluster.BillingModel(), cluster.CloudProvider().ID(), cluster.OpenshiftVersion(), cluster.Region().ID(), From 7d41647fcab8050ce49c152e28ab27ae4c328f97 Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 6 Sep 2023 12:19:22 -0700 Subject: [PATCH 03/11] Added comments and used specific billing model type function from sdk to get billing model --- cmd/ocm/create/cluster/cmd.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index ba9f1969..ee8f5697 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -313,8 +313,7 @@ func init() { &args.subscriptionType, "subscription-type", StandardSubscriptionType, - fmt.Sprintf("The subscription billing model for the cluster. Options are %s", - strings.Join(ValidSubscriptionTypes, ",")), + fmt.Sprintf("The subscription billing model for the cluster."), ) arguments.SetQuestion(fs, "subscription-type", "Subscription type:") Cmd.RegisterFlagCompletionFunc("subscription-type", arguments.MakeCompleteFunc(getSubscriptionTypeOptions)) @@ -420,15 +419,11 @@ func getSubscriptionTypeIdFromDescription(connection *sdk.Connection, value stri } func getBillingModel(connection *sdk.Connection, billingModelID string) (*amv1.BillingModelItem, error) { - billingModels, _ := getBillingModels(connection) - var bm *amv1.BillingModelItem - for _, billingModel := range billingModels { - if billingModel.Id() == billingModelID { - bm = billingModel - break - } + bilingModel, err := connection.AccountsMgmt().V1().BillingModels().BillingModel(billingModelID).Get().Send() + if err != nil { + return nil, err } - return bm, nil + return bilingModel.Body(), nil } func getBillingModels(connection *sdk.Connection) ([]*amv1.BillingModelItem, error) { @@ -492,6 +487,7 @@ func preRun(cmd *cobra.Command, argv []string) error { // Validate flags / ask for missing data. fs := cmd.Flags() + // Get options for subscription type subscriptionTypeOptions, _ := getSubscriptionTypeOptions(connection) err = arguments.PromptOneOf(fs, "subscription-type", subscriptionTypeOptions) if err != nil { @@ -502,6 +498,7 @@ func preRun(cmd *cobra.Command, argv []string) error { // but don't validate if set, to not block `ocm` CLI from creating clusters on future providers. providers, _ := osdProviderOptions(connection) + // If marketplace-gcp subscription type is used, provider can only be GCP gcpBillingModel, _ := getBillingModel(connection, marketplaceGcpSubscriptionType) isGcpSubscriptionType := args.subscriptionType == fmt.Sprintf("%s (%s)", gcpBillingModel.Id(), gcpBillingModel.Description()) if isGcpSubscriptionType { @@ -1131,6 +1128,7 @@ func promptExistingVPC(fs *pflag.FlagSet, connection *sdk.Connection) error { func promptCCS(fs *pflag.FlagSet, isGcpSubscriptionType bool) error { var err error + // If marketplace-gcp subscription type is used, ccs should by default be true if !isGcpSubscriptionType { err = arguments.PromptBool(fs, "ccs") } From 7881eb9c58aaf76b87ef24163b306d92c95f0951 Mon Sep 17 00:00:00 2001 From: tithakka Date: Thu, 7 Sep 2023 16:03:04 -0400 Subject: [PATCH 04/11] Added prompt messages for default selection, set standard as the first value for subscription type --- cmd/ocm/create/cluster/cmd.go | 44 +++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index ee8f5697..af87d38e 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -313,7 +313,8 @@ func init() { &args.subscriptionType, "subscription-type", StandardSubscriptionType, - fmt.Sprintf("The subscription billing model for the cluster."), + fmt.Sprintf("The subscription billing model for the cluster. Options are %s", + strings.Join(ValidSubscriptionTypes, ",")), ) arguments.SetQuestion(fs, "subscription-type", "Subscription type:") Cmd.RegisterFlagCompletionFunc("subscription-type", arguments.MakeCompleteFunc(getSubscriptionTypeOptions)) @@ -400,10 +401,22 @@ func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, return options, err } for _, billingModel := range billingModels { - options = append(options, arguments.Option{ - Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), - Description: billingModel.Description(), - }) + //Standard billing model should always be the first option + if billingModel.Id() == StandardSubscriptionType { + standardOption := arguments.Option{ + Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), + } + if len(options) == 0 { // nil or empty slice or after last element + options = append(options, standardOption) + } else { + options = append(options[:1], options[0:]...) + options[0] = standardOption + } + } else { + options = append(options, arguments.Option{ + Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), + }) + } } return options, nil } @@ -497,20 +510,18 @@ func preRun(cmd *cobra.Command, argv []string) error { // Only offer the 2 providers known to support OSD now; // but don't validate if set, to not block `ocm` CLI from creating clusters on future providers. providers, _ := osdProviderOptions(connection) - // If marketplace-gcp subscription type is used, provider can only be GCP gcpBillingModel, _ := getBillingModel(connection, marketplaceGcpSubscriptionType) isGcpSubscriptionType := args.subscriptionType == fmt.Sprintf("%s (%s)", gcpBillingModel.Id(), gcpBillingModel.Description()) + if isGcpSubscriptionType { - providers = []arguments.Option{ - {Value: c.ProviderGCP, Description: ""}, + fmt.Println("setting provider to", c.ProviderGCP) + args.provider = c.ProviderGCP + } else { + err = arguments.PromptOneOf(fs, "provider", providers) + if err != nil { + return err } - args.ccs.Enabled = true - } - - err = arguments.PromptOneOf(fs, "provider", providers) - if err != nil { - return err } if wasClusterWideProxyReceived() { @@ -1129,7 +1140,10 @@ func promptExistingVPC(fs *pflag.FlagSet, connection *sdk.Connection) error { func promptCCS(fs *pflag.FlagSet, isGcpSubscriptionType bool) error { var err error // If marketplace-gcp subscription type is used, ccs should by default be true - if !isGcpSubscriptionType { + if isGcpSubscriptionType { + fmt.Println("setting ccs to 'true'") + args.ccs.Enabled = true + } else { err = arguments.PromptBool(fs, "ccs") } if err != nil { From cbfc5f4afbba9352af093856fff372374c6956d2 Mon Sep 17 00:00:00 2001 From: tithakka Date: Sun, 10 Sep 2023 18:22:13 -0400 Subject: [PATCH 05/11] Added separate billing package and simplified function logic --- cmd/ocm/create/cluster/cmd.go | 83 ++++++++++++----------------------- pkg/billing/billing.go | 39 ++++++++++++++++ 2 files changed, 68 insertions(+), 54 deletions(-) create mode 100644 pkg/billing/billing.go diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index af87d38e..8e90b27e 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -19,6 +19,7 @@ package cluster import ( "encoding/json" "fmt" + "github.com/openshift-online/ocm-cli/pkg/billing" "io" "net" "os" @@ -33,23 +34,17 @@ import ( "github.com/openshift-online/ocm-cli/pkg/provider" "github.com/openshift-online/ocm-cli/pkg/utils" sdk "github.com/openshift-online/ocm-sdk-go" - amv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" "github.com/openshift/rosa/pkg/interactive" "github.com/spf13/cobra" "github.com/spf13/pflag" ) -var ValidSubscriptionTypes = []string{StandardSubscriptionType, marketplaceRhmSubscriptionType, marketplaceGcpSubscriptionType} - const ( defaultIngressRouteSelectorFlag = "default-ingress-route-selector" defaultIngressExcludedNamespacesFlag = "default-ingress-excluded-namespaces" defaultIngressWildcardPolicyFlag = "default-ingress-wildcard-policy" defaultIngressNamespaceOwnershipPolicyFlag = "default-ingress-namespace-ownership-policy" - StandardSubscriptionType = "standard" - marketplaceRhmSubscriptionType = "marketplace-rhm" - marketplaceGcpSubscriptionType = "marketplace-gcp" ) var args struct { @@ -312,9 +307,9 @@ func init() { fs.StringVar( &args.subscriptionType, "subscription-type", - StandardSubscriptionType, + billing.StandardSubscriptionType, fmt.Sprintf("The subscription billing model for the cluster. Options are %s", - strings.Join(ValidSubscriptionTypes, ",")), + strings.Join(billing.ValidSubscriptionTypes, ",")), ) arguments.SetQuestion(fs, "subscription-type", "Subscription type:") Cmd.RegisterFlagCompletionFunc("subscription-type", arguments.MakeCompleteFunc(getSubscriptionTypeOptions)) @@ -396,33 +391,36 @@ func getVersionOptionsWithDefault(connection *sdk.Connection, channelGroup strin func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { options := []arguments.Option{} - billingModels, err := getBillingModels(connection) + billingModels, err := billing.GetBillingModels(connection) if err != nil { return options, err } for _, billingModel := range billingModels { + option := subscriptionTypeTemplate(billingModel.Id(), billingModel.Description()) //Standard billing model should always be the first option - if billingModel.Id() == StandardSubscriptionType { - standardOption := arguments.Option{ - Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), - } + if billingModel.Id() == billing.StandardSubscriptionType { if len(options) == 0 { // nil or empty slice or after last element - options = append(options, standardOption) + options = append(options, option) } else { options = append(options[:1], options[0:]...) - options[0] = standardOption + options[0] = option } } else { - options = append(options, arguments.Option{ - Value: fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()), - }) + options = append(options, option) } } return options, nil } +func subscriptionTypeTemplate(id string, description string) arguments.Option { + option := arguments.Option{ + Value: fmt.Sprintf("%s (%s)", id, description), + } + return option +} + func getSubscriptionTypeIdFromDescription(connection *sdk.Connection, value string) string { - billingModels, _ := getBillingModels(connection) + billingModels, _ := billing.GetBillingModels(connection) for _, billingModel := range billingModels { if value == fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()) { return billingModel.Id() @@ -431,31 +429,6 @@ func getSubscriptionTypeIdFromDescription(connection *sdk.Connection, value stri return "" } -func getBillingModel(connection *sdk.Connection, billingModelID string) (*amv1.BillingModelItem, error) { - bilingModel, err := connection.AccountsMgmt().V1().BillingModels().BillingModel(billingModelID).Get().Send() - if err != nil { - return nil, err - } - return bilingModel.Body(), nil -} - -func getBillingModels(connection *sdk.Connection) ([]*amv1.BillingModelItem, error) { - response, err := connection.AccountsMgmt().V1().BillingModels().List().Send() - if err != nil { - return nil, err - } - billingModels := response.Items().Slice() - var validBillingModel []*amv1.BillingModelItem - for _, billingModel := range billingModels { - for _, validSubscriptionTypeId := range ValidSubscriptionTypes { - if billingModel.Id() == validSubscriptionTypeId { - validBillingModel = append(validBillingModel, billingModel) - } - } - } - return validBillingModel, nil -} - func getMachineTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { return provider.GetMachineTypeOptions( connection.ClustersMgmt().V1(), @@ -511,10 +484,11 @@ func preRun(cmd *cobra.Command, argv []string) error { // but don't validate if set, to not block `ocm` CLI from creating clusters on future providers. providers, _ := osdProviderOptions(connection) // If marketplace-gcp subscription type is used, provider can only be GCP - gcpBillingModel, _ := getBillingModel(connection, marketplaceGcpSubscriptionType) - isGcpSubscriptionType := args.subscriptionType == fmt.Sprintf("%s (%s)", gcpBillingModel.Id(), gcpBillingModel.Description()) + gcpBillingModel, _ := billing.GetBillingModel(connection, billing.MarketplaceGcpSubscriptionType) + gcpSubscriptionTypeTemplate := subscriptionTypeTemplate(gcpBillingModel.Id(), gcpBillingModel.Description()) + isGcpMarketplaceSubscriptionType := args.subscriptionType == gcpSubscriptionTypeTemplate.Value - if isGcpSubscriptionType { + if isGcpMarketplaceSubscriptionType { fmt.Println("setting provider to", c.ProviderGCP) args.provider = c.ProviderGCP } else { @@ -530,7 +504,12 @@ func preRun(cmd *cobra.Command, argv []string) error { args.clusterWideProxy.Enabled = true } - err = promptCCS(fs, isGcpSubscriptionType) + // If marketplace-gcp subscription type is used, ccs should by default be true + if isGcpMarketplaceSubscriptionType { + fmt.Println("setting ccs to 'true'") + args.ccs.Enabled = true + } + err = promptCCS(fs, args.ccs.Enabled == true) if err != nil { return err } @@ -1137,13 +1116,9 @@ func promptExistingVPC(fs *pflag.FlagSet, connection *sdk.Connection) error { return err } -func promptCCS(fs *pflag.FlagSet, isGcpSubscriptionType bool) error { +func promptCCS(fs *pflag.FlagSet, presetCCS bool) error { var err error - // If marketplace-gcp subscription type is used, ccs should by default be true - if isGcpSubscriptionType { - fmt.Println("setting ccs to 'true'") - args.ccs.Enabled = true - } else { + if !presetCCS { err = arguments.PromptBool(fs, "ccs") } if err != nil { diff --git a/pkg/billing/billing.go b/pkg/billing/billing.go new file mode 100644 index 00000000..176b8f73 --- /dev/null +++ b/pkg/billing/billing.go @@ -0,0 +1,39 @@ +package billing + +import ( + sdk "github.com/openshift-online/ocm-sdk-go" + amv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1" +) + +const ( + StandardSubscriptionType = "standard" + MarketplaceRhmSubscriptionType = "marketplace-rhm" + MarketplaceGcpSubscriptionType = "marketplace-gcp" +) + +var ValidSubscriptionTypes = []string{StandardSubscriptionType, MarketplaceRhmSubscriptionType, MarketplaceGcpSubscriptionType} + +func GetBillingModel(connection *sdk.Connection, billingModelID string) (*amv1.BillingModelItem, error) { + bilingModel, err := connection.AccountsMgmt().V1().BillingModels().BillingModel(billingModelID).Get().Send() + if err != nil { + return nil, err + } + return bilingModel.Body(), nil +} + +func GetBillingModels(connection *sdk.Connection) ([]*amv1.BillingModelItem, error) { + response, err := connection.AccountsMgmt().V1().BillingModels().List().Send() + if err != nil { + return nil, err + } + billingModels := response.Items().Slice() + var validBillingModel []*amv1.BillingModelItem + for _, billingModel := range billingModels { + for _, validSubscriptionTypeId := range ValidSubscriptionTypes { + if billingModel.Id() == validSubscriptionTypeId { + validBillingModel = append(validBillingModel, billingModel) + } + } + } + return validBillingModel, nil +} From c29ee4dfb5bf0e7982e03cc8199e953b16b5b10c Mon Sep 17 00:00:00 2001 From: tithakka Date: Sun, 10 Sep 2023 20:18:19 -0400 Subject: [PATCH 06/11] Modified subscription type logic to be consistent with subnet logic --- cmd/ocm/create/cluster/cmd.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 8e90b27e..3190f907 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -94,6 +94,8 @@ const clusterNameHelp = "will be used when generating a sub-domain for your clus const subnetTemplate = "%s (%s)" +const subscriptionTypeTemplate = "%s (%s)" + // Creates a subnet options using a predefined template. func setSubnetOption(subnet, zone string) string { return fmt.Sprintf(subnetTemplate, subnet, zone) @@ -104,6 +106,14 @@ func parseSubnet(subnetOption string) string { return strings.Split(subnetOption, " ")[0] } +func setSubscriptionTypeOption(id, description string) string { + return fmt.Sprintf(subscriptionTypeTemplate, id, description) +} + +func parseSubscriptionType(subscriptionTypeOption string) string { + return strings.Split(subscriptionTypeOption, " ")[0] +} + // Cmd Constant: var Cmd = &cobra.Command{ Use: "cluster [flags] NAME", @@ -396,7 +406,7 @@ func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, return options, err } for _, billingModel := range billingModels { - option := subscriptionTypeTemplate(billingModel.Id(), billingModel.Description()) + option := subscriptionTypeOption(billingModel.Id(), billingModel.Description()) //Standard billing model should always be the first option if billingModel.Id() == billing.StandardSubscriptionType { if len(options) == 0 { // nil or empty slice or after last element @@ -412,23 +422,13 @@ func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, return options, nil } -func subscriptionTypeTemplate(id string, description string) arguments.Option { +func subscriptionTypeOption(id string, description string) arguments.Option { option := arguments.Option{ - Value: fmt.Sprintf("%s (%s)", id, description), + Value: setSubscriptionTypeOption(id, description), } return option } -func getSubscriptionTypeIdFromDescription(connection *sdk.Connection, value string) string { - billingModels, _ := billing.GetBillingModels(connection) - for _, billingModel := range billingModels { - if value == fmt.Sprintf("%s (%s)", billingModel.Id(), billingModel.Description()) { - return billingModel.Id() - } - } - return "" -} - func getMachineTypeOptions(connection *sdk.Connection) ([]arguments.Option, error) { return provider.GetMachineTypeOptions( connection.ClustersMgmt().V1(), @@ -485,7 +485,7 @@ func preRun(cmd *cobra.Command, argv []string) error { providers, _ := osdProviderOptions(connection) // If marketplace-gcp subscription type is used, provider can only be GCP gcpBillingModel, _ := billing.GetBillingModel(connection, billing.MarketplaceGcpSubscriptionType) - gcpSubscriptionTypeTemplate := subscriptionTypeTemplate(gcpBillingModel.Id(), gcpBillingModel.Description()) + gcpSubscriptionTypeTemplate := subscriptionTypeOption(gcpBillingModel.Id(), gcpBillingModel.Description()) isGcpMarketplaceSubscriptionType := args.subscriptionType == gcpSubscriptionTypeTemplate.Value if isGcpMarketplaceSubscriptionType { @@ -630,7 +630,7 @@ func run(cmd *cobra.Command, argv []string) error { } if args.interactive { - args.subscriptionType = getSubscriptionTypeIdFromDescription(connection, args.subscriptionType) + args.subscriptionType = parseSubscriptionType(args.subscriptionType) } clusterConfig := c.Spec{ From dd79832be4903a9ec0f0a2678ec4b181316bdcc1 Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 11 Sep 2023 15:21:47 -0400 Subject: [PATCH 07/11] upgrade sdk version to 368 --- cmd/ocm/create/cluster/cmd.go | 6 +++--- go.mod | 2 +- go.sum | 4 ++-- pkg/billing/billing.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 3190f907..12fd8e0b 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -406,9 +406,9 @@ func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, return options, err } for _, billingModel := range billingModels { - option := subscriptionTypeOption(billingModel.Id(), billingModel.Description()) + option := subscriptionTypeOption(billingModel.ID(), billingModel.Description()) //Standard billing model should always be the first option - if billingModel.Id() == billing.StandardSubscriptionType { + if billingModel.ID() == billing.StandardSubscriptionType { if len(options) == 0 { // nil or empty slice or after last element options = append(options, option) } else { @@ -485,7 +485,7 @@ func preRun(cmd *cobra.Command, argv []string) error { providers, _ := osdProviderOptions(connection) // If marketplace-gcp subscription type is used, provider can only be GCP gcpBillingModel, _ := billing.GetBillingModel(connection, billing.MarketplaceGcpSubscriptionType) - gcpSubscriptionTypeTemplate := subscriptionTypeOption(gcpBillingModel.Id(), gcpBillingModel.Description()) + gcpSubscriptionTypeTemplate := subscriptionTypeOption(gcpBillingModel.ID(), gcpBillingModel.Description()) isGcpMarketplaceSubscriptionType := args.subscriptionType == gcpSubscriptionTypeTemplate.Value if isGcpMarketplaceSubscriptionType { diff --git a/go.mod b/go.mod index 651cd995..bd359d29 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/nwidger/jsoncolor v0.3.2 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.8 - github.com/openshift-online/ocm-sdk-go v0.1.367 + github.com/openshift-online/ocm-sdk-go v0.1.368 github.com/openshift/rosa v1.2.24 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 github.com/spf13/cobra v1.7.0 diff --git a/go.sum b/go.sum index 7516f87d..670df710 100644 --- a/go.sum +++ b/go.sum @@ -336,8 +336,8 @@ github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAl github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/onsi/gomega v1.27.8 h1:gegWiwZjBsf2DgiSbf5hpokZ98JVDMcWkUiigk6/KXc= github.com/onsi/gomega v1.27.8/go.mod h1:2J8vzI/s+2shY9XHRApDkdgPo1TKT7P2u6fXeJKFnNQ= -github.com/openshift-online/ocm-sdk-go v0.1.367 h1:17eJeoorPLhrHRte3vUGPxIKRDS8BEp1Uv+XhHPY87k= -github.com/openshift-online/ocm-sdk-go v0.1.367/go.mod h1:KYOw8kAKAHyPrJcQoVR82CneQ4ofC02Na4cXXaTq4Nw= +github.com/openshift-online/ocm-sdk-go v0.1.368 h1:qP+gkChV8WDwwpkUw1xUyjTXKdvrwyd70Gff2GMUSeU= +github.com/openshift-online/ocm-sdk-go v0.1.368/go.mod h1:KYOw8kAKAHyPrJcQoVR82CneQ4ofC02Na4cXXaTq4Nw= github.com/openshift/rosa v1.2.24 h1:vv0yYnWHx6CCPEAau/0rS54P2ksaf+uWXb1TQPWxiYE= github.com/openshift/rosa v1.2.24/go.mod h1:MVXB27O3PF8WoOic23I03mmq6/9kVxpFx6FKyLMCyrQ= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= diff --git a/pkg/billing/billing.go b/pkg/billing/billing.go index 176b8f73..7955abcd 100644 --- a/pkg/billing/billing.go +++ b/pkg/billing/billing.go @@ -30,7 +30,7 @@ func GetBillingModels(connection *sdk.Connection) ([]*amv1.BillingModelItem, err var validBillingModel []*amv1.BillingModelItem for _, billingModel := range billingModels { for _, validSubscriptionTypeId := range ValidSubscriptionTypes { - if billingModel.Id() == validSubscriptionTypeId { + if billingModel.ID() == validSubscriptionTypeId { validBillingModel = append(validBillingModel, billingModel) } } From 92422d8ae5b5c7ca4661f1e929f3a47c5471ec7d Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 11 Sep 2023 16:15:33 -0400 Subject: [PATCH 08/11] Fixed order of imports --- cmd/ocm/create/cluster/cmd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 12fd8e0b..76ee63eb 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -19,7 +19,6 @@ package cluster import ( "encoding/json" "fmt" - "github.com/openshift-online/ocm-cli/pkg/billing" "io" "net" "os" @@ -29,6 +28,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/openshift-online/ocm-cli/cmd/ocm/edit/ingress" "github.com/openshift-online/ocm-cli/pkg/arguments" + "github.com/openshift-online/ocm-cli/pkg/billing" c "github.com/openshift-online/ocm-cli/pkg/cluster" "github.com/openshift-online/ocm-cli/pkg/ocm" "github.com/openshift-online/ocm-cli/pkg/provider" From 1b0df071dda48c1c30f819942ed75d0bab628174 Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 11 Sep 2023 16:55:28 -0400 Subject: [PATCH 09/11] simplified the logic to add standard billing model as first option always --- cmd/ocm/create/cluster/cmd.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index 76ee63eb..e51e0d28 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -409,12 +409,7 @@ func getSubscriptionTypeOptions(connection *sdk.Connection) ([]arguments.Option, option := subscriptionTypeOption(billingModel.ID(), billingModel.Description()) //Standard billing model should always be the first option if billingModel.ID() == billing.StandardSubscriptionType { - if len(options) == 0 { // nil or empty slice or after last element - options = append(options, option) - } else { - options = append(options[:1], options[0:]...) - options[0] = option - } + options = append([]arguments.Option{option}, options...) } else { options = append(options, option) } From 1bfbd0f1da1bf6d2b44f2b191dc20b3338afa5b3 Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 11 Sep 2023 17:03:57 -0400 Subject: [PATCH 10/11] Simplified boolean condition --- cmd/ocm/create/cluster/cmd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ocm/create/cluster/cmd.go b/cmd/ocm/create/cluster/cmd.go index e51e0d28..c0c0ba3a 100644 --- a/cmd/ocm/create/cluster/cmd.go +++ b/cmd/ocm/create/cluster/cmd.go @@ -504,7 +504,7 @@ func preRun(cmd *cobra.Command, argv []string) error { fmt.Println("setting ccs to 'true'") args.ccs.Enabled = true } - err = promptCCS(fs, args.ccs.Enabled == true) + err = promptCCS(fs, args.ccs.Enabled) if err != nil { return err } From abe8d9cc56af1b2634f7083570387f0764392277 Mon Sep 17 00:00:00 2001 From: tithakka Date: Tue, 12 Sep 2023 09:52:27 -0400 Subject: [PATCH 11/11] Fixed linter error --- pkg/billing/billing.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/billing/billing.go b/pkg/billing/billing.go index 7955abcd..ef5371bf 100644 --- a/pkg/billing/billing.go +++ b/pkg/billing/billing.go @@ -11,7 +11,11 @@ const ( MarketplaceGcpSubscriptionType = "marketplace-gcp" ) -var ValidSubscriptionTypes = []string{StandardSubscriptionType, MarketplaceRhmSubscriptionType, MarketplaceGcpSubscriptionType} +var ValidSubscriptionTypes = []string{ + StandardSubscriptionType, + MarketplaceRhmSubscriptionType, + MarketplaceGcpSubscriptionType, +} func GetBillingModel(connection *sdk.Connection, billingModelID string) (*amv1.BillingModelItem, error) { bilingModel, err := connection.AccountsMgmt().V1().BillingModels().BillingModel(billingModelID).Get().Send()