Skip to content
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.

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

fix(kafka create): reduce number of calls made to AMS API #1596

Merged
merged 6 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions docs/commands/rhoas_kafka_create.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ require (
github.com/redhat-developer/app-services-sdk-go/accountmgmt v0.2.0
github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.5.0
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1
github.com/redhat-developer/app-services-sdk-go/registrymgmt v0.6.1
github.com/redhat-developer/service-binding-operator v0.9.0
Expand All @@ -39,7 +39,7 @@ require (
gitlab.com/c0b/go-ordered-json v0.0.0-20201030195603-febf46534d5a
golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29 // indirect
golang.org/x/net v0.0.0-20220403103023-749bd193bc2b // indirect
golang.org/x/oauth2 v0.0.0-20220524215830-622c5d57e401
golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb
golang.org/x/text v0.3.7
golang.org/x/tools v0.1.7 // indirect
google.golang.org/protobuf v1.28.0 // indirect
Expand Down
7 changes: 4 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,8 @@ github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.5.0 h1:cf+K96kW
github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.5.0/go.mod h1:JAedrXf/qLHd7lpOS+bOFh8nrOpp2j0sg4/VG/1um6c=
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0 h1:ExEHQaihnPNxN2nKXB0q5nrmSv4p8b3Idzt7TChxv+Q=
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0/go.mod h1:hMpejngP3BFnifCDH1gKRG9cU9Q4lr0WiQaW7A1LYo4=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0 h1:63UhOYB8TozKdnkkws2pXc0D1lEB+K3qX63/OxkjDas=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0/go.mod h1:m+m7d6xkC9WbSxemslyhjv0jVhquWLysRfdh+RQ5hH0=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1 h1:Gcyn2kLlslsVT6T8qoiCJpJFPrnD2i2KIFeKQJrXkTY=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1/go.mod h1:RoPo3tyHjv8apStFNVjChwWYdlWhg6hMzi1IrH3yQX8=
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1 h1:xRq5XJzRDs/Z7e/9SDt6zbNRIyesC4LTqN9ajHKwjHo=
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1/go.mod h1:Z/gr/snlpsqYg4vftmcx97vCR3qMQJhALGelDHx4pMA=
github.com/redhat-developer/app-services-sdk-go/registrymgmt v0.6.1 h1:3sUmQ3nAawsYWg7ZCO2Q8HF2J7MW6YA38h/YFL3ao6o=
Expand Down Expand Up @@ -932,8 +932,9 @@ golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602/go.mod h1:KelEdhl1UZF7XfJ
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220524215830-622c5d57e401 h1:zwrSfklXn0gxyLRX/aR+q6cgHbV/ItVyzbPlbA+dkAw=
golang.org/x/oauth2 v0.0.0-20220524215830-622c5d57e401/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb h1:8tDJ3aechhddbdPAxpycgXHJRMLpk/Ab+aa4OgdN5/g=
golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb/go.mod h1:jaDAt6Dkxork7LmZnYtzbRWj0W47D86a3TGe0YHBvmE=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down
44 changes: 39 additions & 5 deletions pkg/cmd/kafka/create/api_validators.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package create

import (
"errors"
"strings"

"github.com/redhat-developer/app-services-cli/pkg/core/cmdutil/flagutil"
"github.com/redhat-developer/app-services-cli/pkg/core/localize"
"github.com/redhat-developer/app-services-cli/pkg/shared/accountmgmtutil"
"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
Expand All @@ -13,17 +15,18 @@ import (
)

type ValidatorInput struct {
provider string
region string
size string

provider string
region string
size string
userAMSInstanceType *accountmgmtutil.QuotaSpec

f *factory.Factory
constants *remote.DynamicServiceConstants
conn connection.Connection
}

var validBillingModels []string = []string{accountmgmtutil.QuotaMarketplaceType, accountmgmtutil.QuotaStandardType}

func (input *ValidatorInput) ValidateProviderAndRegion() error {
f := input.f
f.Logger.Debug("Validating provider and region")
Expand Down Expand Up @@ -108,7 +111,7 @@ func (input *ValidatorInput) ValidateSize() error {
return nil
}

sizes, err := FetchValidKafkaSizesLabels(input.f, input.provider, input.region, *input.userAMSInstanceType)
sizes, err := FetchValidKafkaSizesLabels(input.f, input.provider, input.region, input.userAMSInstanceType)
if err != nil {
return err
}
Expand All @@ -119,3 +122,34 @@ func (input *ValidatorInput) ValidateSize() error {

return nil
}

// ValidateBillingModel validates if user provided a correct billing model
func ValidateBillingModel(billingModel string) error {
wtrocki marked this conversation as resolved.
Show resolved Hide resolved

if billingModel == "" {
return nil
}

isValid := flagutil.IsValidInput(billingModel, validBillingModels...)

if isValid {
return nil
}

return flagutil.InvalidValueError("billing-model", billingModel, validBillingModels...)
}

// ValidateMarketplaceFlags validates if flag combination is valid for chosing quota
func ValidateMarketplaceFlags(billingModel string, accountID string, marketplace string) error {

if billingModel == StandardType && (accountID != "" || marketplace != "") {
return errors.New("accountID cant be provided with standard billing model")
}

if (accountID != "") != (marketplace != "") {
return errors.New("accountID and marketplace should be provided together")
}

return nil

}
48 changes: 0 additions & 48 deletions pkg/cmd/kafka/create/completions.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package create

import (
"github.com/redhat-developer/app-services-cli/pkg/shared/accountmgmtutil"
"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
"github.com/redhat-developer/app-services-cli/pkg/shared/factory"
"github.com/redhat-developer/app-services-cli/pkg/shared/remote"
"github.com/spf13/cobra"
)

Expand All @@ -26,48 +23,3 @@ func GetCloudProviderRegionCompletionValues(f *factory.Factory, providerID strin

return validRegions, cobra.ShellCompDirectiveNoSpace
}

// GetKafkaSizeCompletionValues returns a list of valid kafka sizes for the specified region and ams instance types
func GetKafkaSizeCompletionValues(f *factory.Factory, providerID string, regionId string) (validRegions []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

// We need both values to provide a valid list of sizes
if providerID == "" || regionId == "" {
return nil, directive
}

err, constants := remote.GetRemoteServiceConstants(f.Context, f.Logger)
if err != nil {
return nil, directive
}

conn, err := f.Connection(connection.DefaultConfigSkipMasAuth)
if err != nil {
return nil, directive
}

userInstanceType, _ := accountmgmtutil.GetUserSupportedInstanceType(f.Context, &constants.Kafka.Ams, conn)

// Not including quota in this request as it takes very long time to list quota for all regions in suggestion mode
validRegions, _ = FetchValidKafkaSizesLabels(f, providerID, regionId, *userInstanceType)

return validRegions, cobra.ShellCompDirectiveNoSpace
}

// GetMarketplaceAcctIdCompletionValues returns a list of valid marketplace account IDs for the organization
func GetMarketplaceAcctIdCompletionValues(f *factory.Factory) (validMarketplaceAcctIDs []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

validMarketplaceAcctIDs, _ = accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, "")

return validMarketplaceAcctIDs, directive
}

// GetMarketplaceCompletionValues returns a list of valid marketplaces for the organization
func GetMarketplaceCompletionValues(f *factory.Factory) (validMarketplaces []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

validMarketplaces, _ = accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)

return validMarketplaces, directive
}
100 changes: 44 additions & 56 deletions pkg/cmd/kafka/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type options struct {

marketplaceAcctId string
marketplace string
billingModel string

outputFormat string
autoUse bool
Expand Down Expand Up @@ -93,7 +94,7 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
}
}

if opts.bypassChecks && (opts.marketplace != "" || opts.marketplaceAcctId != "") {
if opts.bypassChecks && (opts.marketplace != "" || opts.marketplaceAcctId != "" || opts.billingModel != "") {
return f.Localizer.MustLocalizeError("kafka.create.error.bypassChecks.marketplace")
}

Expand All @@ -111,26 +112,6 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
return flagutil.InvalidValueError("output", opts.outputFormat, validOutputFormats...)
}

if !opts.bypassChecks {
validMarketplaces, err := accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)
if err != nil {
return err
}

if opts.marketplace != "" && !flagutil.IsValidInput(opts.marketplace, validMarketplaces...) {
return flagutil.InvalidValueError(FlagMarketPlace, opts.marketplace, validMarketplaces...)
}

validMarketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, opts.marketplace)
if err != nil {
return err
}

if opts.marketplaceAcctId != "" && !flagutil.IsValidInput(opts.marketplaceAcctId, validMarketplaceAcctIDs...) {
return flagutil.InvalidValueError(FlagMarketPlaceAcctID, opts.marketplaceAcctId, validMarketplaceAcctIDs...)
}
}

return runCreate(opts)
},
}
Expand All @@ -147,6 +128,7 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
flags.BoolVar(&opts.autoUse, "use", true, f.Localizer.MustLocalize("kafka.create.flag.autoUse.description"))
flags.BoolVarP(&opts.wait, "wait", "w", false, f.Localizer.MustLocalize("kafka.create.flag.wait.description"))
flags.BoolVarP(&opts.dryRun, "dry-run", "", false, f.Localizer.MustLocalize("kafka.create.flag.dryrun.description"))
flags.StringVar(&opts.billingModel, "billing-model", "", f.Localizer.MustLocalize("kafka.create.flag.billingModel.description"))
flags.AddBypassTermsCheck(&opts.bypassChecks)

_ = cmd.RegisterFlagCompletionFunc(FlagProvider, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
Expand All @@ -157,18 +139,6 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
return GetCloudProviderRegionCompletionValues(f, opts.provider)
})

_ = cmd.RegisterFlagCompletionFunc(FlagSize, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetKafkaSizeCompletionValues(f, opts.provider, opts.region)
})

_ = cmd.RegisterFlagCompletionFunc(FlagMarketPlaceAcctID, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetMarketplaceAcctIdCompletionValues(f)
})

_ = cmd.RegisterFlagCompletionFunc(FlagMarketPlace, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetMarketplaceCompletionValues(f)
})

return cmd
}

Expand Down Expand Up @@ -212,10 +182,21 @@ func runCreate(opts *options) error {
return nil
}

userInstanceType, err = accountmgmtutil.GetUserSupportedInstanceType(f.Context, &constants.Kafka.Ams, conn)
if err != nil || userInstanceType == nil {
return f.Localizer.MustLocalizeError("kafka.create.error.userInstanceType.notFound")
err = ValidateBillingModel(opts.billingModel)
if err != nil {
return err
}

err = ValidateMarketplaceFlags(opts.billingModel, opts.marketplaceAcctId, opts.marketplace)
if err != nil {
return err
}

userInstanceType, err = accountmgmtutil.FetchQuotaCost(f, opts.billingModel, opts.marketplaceAcctId, opts.marketplace, &constants.Kafka.Ams)
if err != nil {
return err
}

}

var payload *kafkamgmtclient.KafkaRequestPayload
Expand All @@ -225,7 +206,7 @@ func runCreate(opts *options) error {
return f.Localizer.MustLocalizeError("kafka.create.error.noInteractiveMode")
}

payload, err = promptKafkaPayload(opts, *userInstanceType)
payload, err = promptKafkaPayload(opts, userInstanceType)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interactive mode needs more changes, currently the user will need to specify BillingModel as flag before starting the interactive mode.

if err != nil {
return err
}
Expand All @@ -249,6 +230,8 @@ func runCreate(opts *options) error {
payload.Marketplace.Set(&opts.marketplace)
payload.BillingCloudAccountId = kafkamgmtclient.NullableString{}
payload.BillingCloudAccountId.Set(&opts.marketplaceAcctId)
payload.BillingModel = kafkamgmtclient.NullableString{}
payload.BillingModel.Set(&opts.billingModel)
}

if !opts.bypassChecks {
Expand All @@ -271,7 +254,7 @@ func runCreate(opts *options) error {
return err1
}
if opts.size != "" {
sizes, err1 := FetchValidKafkaSizes(opts.f, opts.provider, opts.region, *userInstanceType)
sizes, err1 := FetchValidKafkaSizes(opts.f, opts.provider, opts.region, userInstanceType)
if err1 != nil {
return err1
}
Expand Down Expand Up @@ -400,7 +383,7 @@ type promptAnswers struct {

// Show a prompt to allow the user to interactively insert the data for their Kafka
// nolint:funlen
func promptKafkaPayload(opts *options, userQuotaType accountmgmtutil.QuotaSpec) (*kafkamgmtclient.KafkaRequestPayload, error) {
func promptKafkaPayload(opts *options, userQuotaType *accountmgmtutil.QuotaSpec) (*kafkamgmtclient.KafkaRequestPayload, error) {
f := opts.f

accountIDNullable := kafkamgmtclient.NullableString{}
Expand Down Expand Up @@ -438,7 +421,7 @@ func promptKafkaPayload(opts *options, userQuotaType accountmgmtutil.QuotaSpec)
return nil, err
}

regionIDs, err := GetEnabledCloudRegionIDs(opts.f, answers.CloudProvider, &userQuotaType)
regionIDs, err := GetEnabledCloudRegionIDs(opts.f, answers.CloudProvider, userQuotaType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -474,29 +457,34 @@ func promptKafkaPayload(opts *options, userQuotaType accountmgmtutil.QuotaSpec)
}
}

marketplaces, err := accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)
if err != nil {
return nil, err
}

if !opts.bypassChecks && len(marketplaces) > 0 {
if err = promptMarketplaceSelect(f.Localizer, marketplaces, answers); err != nil {
return nil, err
}
if !opts.bypassChecks && opts.billingModel == accountmgmtutil.QuotaMarketplaceType {

marketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, answers.Marketplace)
marketplaces, err := accountmgmtutil.GetValidMarketplaces(userQuotaType)
if err != nil {
return nil, err
}

if len(marketplaceAcctIDs) > 0 {
if err = promptMarketplaceAcctIDSelect(f.Localizer, marketplaceAcctIDs, answers); err != nil {
if len(marketplaces) > 0 {

if err = promptMarketplaceSelect(f.Localizer, marketplaces, answers); err != nil {
return nil, err
}

marketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(userQuotaType, answers.Marketplace)
if err != nil {
return nil, err
}
}

accountIDNullable.Set(&answers.MarketplaceAcctID)
marketplaceProviderNullable.Set(&answers.Marketplace)
if len(marketplaceAcctIDs) > 0 {
if err = promptMarketplaceAcctIDSelect(f.Localizer, marketplaceAcctIDs, answers); err != nil {
return nil, err
}
}

accountIDNullable.Set(&answers.MarketplaceAcctID)
marketplaceProviderNullable.Set(&answers.Marketplace)

}
}

payload := &kafkamgmtclient.KafkaRequestPayload{
Expand All @@ -507,7 +495,7 @@ func promptKafkaPayload(opts *options, userQuotaType accountmgmtutil.QuotaSpec)
Marketplace: marketplaceProviderNullable,
}
printSizeWarningIfNeeded(opts.f, answers.Size, sizes)
payload.SetPlan(mapAmsTypeToBackendType(&userQuotaType) + "." + answers.Size)
payload.SetPlan(mapAmsTypeToBackendType(userQuotaType) + "." + answers.Size)

return payload, nil
}
Expand Down
Loading