-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
kubeadm: print the join command #70795
kubeadm: print the join command #70795
Conversation
Hi @yuexiao-wang. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
thanks for this WIP @yuexiao-wang
added one initial comment.
cmd/kubeadm/app/cmd/init.go
Outdated
@@ -154,6 +154,32 @@ func NewCmdInit(out io.Writer) *cobra.Command { | |||
// TODO: the code in runInit should be progressively converted in phases; each phase will be exposed | |||
// via the subcommands automatically created by initRunner.BindToCommand | |||
err = runInit(&data, out) | |||
|
|||
// printing the join command should happen after all the phases in init have finished | |||
showJoinCommand := func(i *initData, out io.Writer) error { |
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.
let's not have it as a anonymous function but rather define it in the file level:
func showJoinCommand(....) {
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.
Thanks, I will fix it
/kind cleanup |
cmd/kubeadm/app/cmd/init.go
Outdated
} | ||
|
||
// Prints the join command, multiple times in case the user has multiple tokens | ||
for _, token := range tokens { |
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.
You can use initData.Tokens
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.
Yes, agree with it.
Thanks.
89a608a
to
231e3be
Compare
231e3be
to
75e7153
Compare
PR is updated |
|
||
// Prints the join command, multiple times in case the user has multiple tokens | ||
for _, token := range tokens { | ||
if err := printJoinCommand(out, adminKubeConfigPath, token, i.skipTokenPrint); err != nil { |
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.
initData already holds a method to get the kubeconfig path:
adminKubeConfigPath
-> i.KubeConfigPath()
it can replace the code above :)
_, kubeConfigDir, _, _, err := getDirectoriesToUse(i.dryRun, i.dryRunDir, i.cfg.CertificatesDir)
if err != nil {
return errors.Wrap(err, "failed to get directories to use")
}
adminKubeConfigPath := filepath.Join(kubeConfigDir, kubeadmconstants.AdminKubeConfigFileName)
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.
Updated
fb0bf65
to
7ad7cd6
Compare
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.
@yuexiao-wang thanks!
Looks ok, but we still need some PR to merge
@@ -458,19 +460,16 @@ func (d initData) Tokens() []string { | |||
|
|||
// runInit executes master node provisioning | |||
func runInit(i *initData, out io.Writer) error { |
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'm expecting this PR to remove this func after rebasing when all the phases merge
/test pull-kubernetes-verify |
7ad7cd6
to
389f8b6
Compare
/retest |
1 similar comment
/retest |
/milestone v1.13 |
389f8b6
to
5851fd5
Compare
… init have finished Signed-off-by: yuexiao-wang <wang.yuexiao@zte.com.cn>
@@ -397,6 +400,9 @@ func (d initData) KubeConfigDir() string { | |||
|
|||
// KubeConfigPath returns the path to the kubeconfig file to use for connecting to Kubernetes | |||
func (d initData) KubeConfigPath() string { | |||
if d.dryRun { | |||
d.kubeconfigPath = filepath.Join(d.dryRunDir, kubeadmconstants.AdminKubeConfigFileName) |
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.
that was a bug indeed, thanks for fixing. @yuexiao-wang
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.
thanks for your help
/lgtm |
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.
@yuexiao-wang many thanks!
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini, yuexiao-wang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Signed-off-by: yuexiao-wang wang.yuexiao@zte.com.cn
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
move
printJoinCommand
out ofrunInit
and create a new function to print the join command.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Ref # kubernetes/kubeadm#1163
Fixes # kubernetes/kubeadm#1233
Special notes for your reviewer:
Does this PR introduce a user-facing change?: