-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat: allow select context for kubecm add and merge command #952
Conversation
The Label Bot has predicted the following:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #952 +/- ##
==========================================
- Coverage 19.54% 19.40% -0.15%
==========================================
Files 21 21
Lines 2062 2077 +15
==========================================
Hits 403 403
- Misses 1617 1630 +13
- Partials 42 44 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thank you for your valuable contribution to this project! Your work is greatly appreciated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to use String
for this flag.
@@ -46,6 +48,7 @@ func (ac *AddCommand) runAdd(cmd *cobra.Command, args []string) error { | |||
file, _ := ac.command.Flags().GetString("file") | |||
cover, _ := ac.command.Flags().GetBool("cover") | |||
contextName, _ := ac.command.Flags().GetString("context-name") | |||
selectContext, _ := ac.command.Flags().GetBool("select-context") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps GetString
is used here and followed by the relevant checks: exists, does not exist, is empty, etc.
This would allow for better automated testing in e2e test.
selectContext, _ := ac.command.Flags().GetString("select-context")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to choose contexts interactively, so I use bool type.
kubecm add -f kubeconfig1 --select-context
For my understanding, you want this:
kubecm add -f kubeconfig1 --select-context context1
It seems like we couldn't implement different types for the same flag (bool and string) in Cobra. So I think your idea should be implemented by a different flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, a new flag should be created here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cr7258 But this new flag is a new feature, which is not part of this PR, would you like to create a new PR to contribute this this feature? I will merge this PR first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunny0826 I will contribute this feature in a new PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extremely grateful !
LGTM |
@all-contributors please add @cr7258 for doc |
Description
Add the
--select-context
parameter allowing the user to choose which context should be added when running thekubecm add
andkubecm merge
commands.Test
Prepare two kubeconfig.
Run the
kubecm add
command with the--select-context
parameter. The user needs to confirm each context to be imported. If the--select-context
parameter is omitted, confirmation is not required.Run the
kubecm merge
command with the--select-context
parameter.If there is no context to add or merge, the program ends directly instead of asking
Does it overwrite File xxx ?
.Type of Change
Checklist
make build
andmake test
commands.make doc-gen
to generate new documentation.