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

Add kubeadm alpha certs certificate-key command #77848

Merged
merged 1 commit into from May 29, 2019

Conversation

@yagonobre
Copy link
Member

commented May 14, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Add kubeadm certificate-key command to generate secure random key to use on kubeadm init --experimental-upload-certs

Which issue(s) this PR fixes:
Does this PR introduce a user-facing change?:

Add `kubeadm alpha certs certificate-key` command to generate secure random key to use on `kubeadm init --experimental-upload-certs`

/priority important-longterm

@yagonobre

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

// NewCmdCertificateKey returns cobra.Command for certificate key generate
func NewCmdCertificateKey() *cobra.Command {
return &cobra.Command{
Use: "certificate-key",

This comment has been minimized.

Copy link
@neolit123

neolit123 May 14, 2019

Member

hm, i wonder if we should just use the init phase for this.
adding a top level command would be fine if has more than one sub commands and multiple flags, but right now it would only serve this single purpose.

on the other hand we already have kubeadm token which covers existing functionality in init.

we also have the option here to have kubeadm certificate-key do multiple things:

  • only generate key
  • upload the certs & generate key (duplicates what the init phase does)

This comment has been minimized.

Copy link
@neolit123

neolit123 May 14, 2019

Member

also, a problem related to UX that i've been seeing is that users want the new full join command when generating a new cert-key.

so they use token ... --print-join-command and combine it with the new cert key manually.

there are no good solutions for this unless we have a command that generates both a new bootstrap token + hash + cert key.

This comment has been minimized.

Copy link
@yagonobre

yagonobre May 14, 2019

Author Member

this command just generate a certificate-key, so user can only use it on init and init print the join command, we can print the init command but token generate just print the token.

This comment has been minimized.

Copy link
@yagonobre

yagonobre May 14, 2019

Author Member

due that the certificate-key can't be manually generate on a easy way, it would be nice to have a command for it, but I agree that a top level command only for this seems weird.

This comment has been minimized.

Copy link
@rosti

rosti May 22, 2019

Member

I agree with @neolit123 . My idea is that this is an utility for certificates and should go with the rest of the certs commands. Probably adding it under kubeadm alpha certs is a good idea.
@fabriziopandini @yagonobre @neolit123 WDYT?

This comment has been minimized.

Copy link
@yagonobre

yagonobre May 22, 2019

Author Member

+1 to move to kubeadm alpha certs

This comment has been minimized.

Copy link
@neolit123

@yagonobre yagonobre force-pushed the yagonobre:certificate-key-command branch from d2864b6 to 81c4b37 May 27, 2019

@yagonobre yagonobre force-pushed the yagonobre:certificate-key-command branch 2 times, most recently from f746458 to b6e6b46 May 27, 2019

@yagonobre

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

@neolit123 @rosti updated!

@neolit123
Copy link
Member

left a comment

@yagonobre one minor comment about date but LGTM.
/approve

@@ -1,5 +1,5 @@
/*
Copyright 2018 The Kubernetes Authors.
Copyright 2019 The Kubernetes Authors.

This comment has been minimized.

Copy link
@neolit123

neolit123 May 27, 2019

Member

apparently, the project does not change dates of old source files.
2018 should be kept here.

This comment has been minimized.

Copy link
@yagonobre

yagonobre May 27, 2019

Author Member

til

This comment has been minimized.

Copy link
@yagonobre

yagonobre May 27, 2019

Author Member

updated!

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 27, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123, yagonobre

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yagonobre yagonobre force-pushed the yagonobre:certificate-key-command branch from b6e6b46 to 0a005e3 May 27, 2019

@yagonobre yagonobre changed the title Add kubeadm certificate-key command Add kubeadm alpha phase certs certificate-key command May 27, 2019

@rosti

rosti approved these changes May 27, 2019

Copy link
Member

left a comment

Thanks @yagonobre !
Given that this is an alpha command ATM, it's late in the cycle and it's simple enough, I think that it's OK to merge it as is (although it needs a simple test case and some UX polishing upon graduation).
/lgtm
/hold
for the release note to be reflecting kubeadm alpha certs certificate-key instead of kubeadm certificate-key.

@yagonobre yagonobre changed the title Add kubeadm alpha phase certs certificate-key command Add kubeadm alpha certs certificate-key command May 27, 2019

@rosti

This comment has been minimized.

Copy link
Member

commented May 27, 2019

Thanks @yagonobre !
/hold cancel

@rosti

This comment has been minimized.

Copy link
Member

commented May 27, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@neolit123

This comment has been minimized.

Copy link
Member

commented May 27, 2019

/retest

1 similar comment
@neolit123

This comment has been minimized.

Copy link
Member

commented May 27, 2019

/retest

@yagonobre

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@neolit123

This comment has been minimized.

Copy link
Member

commented May 27, 2019

/retest

1 similar comment
@neolit123

This comment has been minimized.

Copy link
Member

commented May 27, 2019

/retest

@fejta-bot

This comment has been minimized.

Copy link

commented May 28, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

2 similar comments
@fejta-bot

This comment has been minimized.

Copy link

commented May 28, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot

This comment has been minimized.

Copy link

commented May 28, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 6118b8a into kubernetes:master May 29, 2019

20 checks passed

cla/linuxfoundation yagonobre authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.