feat(kafka): add promote command #1805

rkpattnaik780 · 2023-02-09T08:02:15Z

rhoas kafka promote command to promote eval instances to use paid subscriptions.

Verification Steps

Create an eval instance:

rhoas kafka create --name test-instance --billing-model eval

Run promote command to change billing model to standard.

rhoas kafka promote --billing-model standard

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation change
Other (please specify)

ziccardi · 2023-02-09T11:36:19Z

I gave a run to the code and it works very well.

I have only a few comments:

If you run the promote command and the kafka is not in a valid state for promotion (ready/suspended/resuming), the cli gives a misleading error : Only Kafka instances with billing model "eval" can be promoted
The promote is valid only for eval instances, but when I list the kafkas, the billing model is not visualised. How can I know what are the kafkas I can promote?
When the promote is called and it is successful, it returns an 'instance has been successfully promoted' message. That is misleading. When you invoke the promotion endpoint, the promotion request gets accepted. A reconciler then manages the promotion. At this point, the promotion can still fail. The cli should take into consideration the promotion_status and the promotion_details fields.
The billing_model is not shown in the kafka list: I have no way to know the current billing model and I don't have any way to check that the promotion has been really done.

rkpattnaik780 · 2023-02-10T09:48:23Z

Hello @ziccardi
Thanks for the review. While going through the wrong error message issue, it turns out that the custom error code KAFKAS-MGMT-9 is associated with two failed reasons - kafka request \"cfivi8ubtv3kjdprf0v0\" has a kafka billing model \"standard\". Only kafka requests with a kafka billing model 'eval' can be promoted and kafka request \"cfivi8ubtv3kjdprf0v0\" with status \"accepted\" cannot be promoted: promotable status are: [ready suspended resuming]". Is there any reason we have two errors mapped to single error code? Shouldn't they be unique?

ziccardi · 2023-02-10T09:59:06Z

Is there any reason we have two errors mapped to single error code? Shouldn't they be unique?

Currently we return those errors as general errors, so they share the same code. If you need those to use different codes, please fire a jira and we will look into it

rkpattnaik780 · 2023-02-10T10:05:46Z

Is there any reason we have two errors mapped to single error code? Shouldn't they be unique?

Currently we return those errors as general errors, so they share the same code. If you need those to use different codes, please fire a jira and we will look into it

Thank you for explaining. Will be creating a JIRA.

dimakis

LGTM, much better success message 👍

* feat(kafka create): enable creating long-lived trial instances (#1800) * feat(kafka): add promote command (#1805)

jbyrne-redhat · 2023-02-14T10:32:37Z

pkg/core/localize/locales/en/cmd/kafka.en.toml

@@ -54,6 +54,15 @@ one = 'Kafka instance ID. Uses the current instance if not set'
 [kafkas.common.flag.output.description]
 one = 'Format in which to display the Kafka instances (choose from: "json", "yml", "yaml")'

+[kafka.common.flag.marketplaceId.description]
+one = 'Cloud Account ID for the marketplace'


Suggest: "Account ID for cloud marketplace"

jbyrne-redhat · 2023-02-14T10:33:46Z

pkg/core/localize/locales/en/cmd/kafka.en.toml

+one = 'Billing model to be used'
+
+[kafka.common.flag.marketplaceType.description]
+one = 'Name of the marketplace where the instance is purchased on'


Suggest: 'Name of cloud marketplace where Kafka instance was purchased'

jbyrne-redhat · 2023-02-14T10:42:58Z

pkg/core/localize/locales/en/cmd/kafka.en.toml

@@ -396,6 +396,9 @@ one = 'unable to create new Kafka instance at this time in specified cloud provi
 [kafka.create.error.instance.limit]
 one = 'maximum number of allowed kafka instances has been reached. Please review all instances that your user has access to and delete one or more instances before creating a new one'


Suggest: 'the maximum number of Kafka instances for your organization has been reached. Before you can create a new instance, you must delete one or more of the existing instances'

jbyrne-redhat

Descriptions look good overall, @rkpattnaik780 . A couple of minor suggestions.

feat(kafka): add promote command

02446d7

rkpattnaik780 requested review from dimakis and jackdelahunt February 9, 2023 08:02

docs: improve promote success message

ae8b23d

dimakis approved these changes Feb 13, 2023

View reviewed changes

rkpattnaik780 merged commit b7f8c4b into feat-long-lived Feb 13, 2023

rkpattnaik780 deleted the promote-eval branch February 13, 2023 12:54

rkpattnaik780 added a commit that referenced this pull request Feb 13, 2023

feat(kafka create): enable creating long-lived trial instances (#1809)

a2e1b6a

* feat(kafka create): enable creating long-lived trial instances (#1800) * feat(kafka): add promote command (#1805)

rkpattnaik780 requested a review from jbyrne-redhat February 13, 2023 14:47

jbyrne-redhat reviewed Feb 14, 2023

View reviewed changes

rkpattnaik780 mentioned this pull request Feb 15, 2023

docs(kafka): improve flag descriptions #1811

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(kafka): add promote command #1805

feat(kafka): add promote command #1805

rkpattnaik780 commented Feb 9, 2023 •

edited

Loading

ziccardi commented Feb 9, 2023 •

edited

Loading

rkpattnaik780 commented Feb 10, 2023

ziccardi commented Feb 10, 2023 •

edited

Loading

rkpattnaik780 commented Feb 10, 2023

dimakis left a comment

jbyrne-redhat Feb 14, 2023

jbyrne-redhat Feb 14, 2023 •

edited

Loading

jbyrne-redhat Feb 14, 2023

jbyrne-redhat left a comment

		@@ -396,6 +396,9 @@ one = 'unable to create new Kafka instance at this time in specified cloud provi
		[kafka.create.error.instance.limit]
		one = 'maximum number of allowed kafka instances has been reached. Please review all instances that your user has access to and delete one or more instances before creating a new one'

feat(kafka): add promote command #1805

feat(kafka): add promote command #1805

Conversation

rkpattnaik780 commented Feb 9, 2023 • edited Loading

Verification Steps

Type of change

ziccardi commented Feb 9, 2023 • edited Loading

rkpattnaik780 commented Feb 10, 2023

ziccardi commented Feb 10, 2023 • edited Loading

rkpattnaik780 commented Feb 10, 2023

dimakis left a comment

Choose a reason for hiding this comment

jbyrne-redhat Feb 14, 2023

Choose a reason for hiding this comment

jbyrne-redhat Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

jbyrne-redhat Feb 14, 2023

Choose a reason for hiding this comment

jbyrne-redhat left a comment

Choose a reason for hiding this comment

rkpattnaik780 commented Feb 9, 2023 •

edited

Loading

ziccardi commented Feb 9, 2023 •

edited

Loading

ziccardi commented Feb 10, 2023 •

edited

Loading

jbyrne-redhat Feb 14, 2023 •

edited

Loading