[WIP] - Kubeadm phase ga #61631

fabriziopandini · 2018-03-24T12:17:57Z

What this PR does / why we need it:
This PR prototypes the approach described in KEP kubeadm phases to beta.

As a result you can execute kubeadm init [flags] as today - for executing the entire init workflow -, or use kubeadm init [phases] [flags] for executing only single steps of the workflow. e.g.

    # to execute only certs phases
    kubeadm init certs [flags]
    
    # to execute only certs/apiserver phases
    kubeadm init certs/apiserver [flags]

Which issue(s) this PR fixes:
Fixes #kubernetes/kubeadm#454

Special notes for your reviewer:
/CC @kubernetes/sig-cluster-lifecycle-pr-reviews

The following commits contains the relevant changes:

phases to beta
cmd factories
phased command

Other commits contains deleted files (many!) and autogenerate files.

The kubeadm init help shows all the available phases:

  preflight                  Run master pre-flight checks
  certs|certificates         Generates all PKI assets necessary to establish the control plane
    /ca                        Generates a self-signed kubernetes CA to provision identities for components of the cluster
    /apiserver                 Generates an API server serving certificate and key
    /apiserver-kubelet-client  Generates a client certificate for the API server to connect to the kubelets securely
    /etcd-ca                   Generates a self-signed CA to provision identities for etcd
    /etcd-server               Generates an etcd serving certificate and key
    /etcd-peer                 Generates an etcd peer certificate and key
    /etcd-healthcheck-client   Generates a client certificate for liveness probes to healthcheck etcd
    /apiserver-etcd-client     Generates a client certificate for the API server to connect to etcd securely
    /sa                        Generates a private key for signing service account tokens along with its public key
    /front-proxy-ca            Generates a front proxy CA certificate and key for a Kubernetes cluster
    /front-proxy-client        Generates a front proxy CA client certificate and key for a Kubernetes cluster
  kubeconfig                 Generates all kubeconfig files necessary to establish the control plane and the admin kubeconfig file
    /admin                     Generates a kubeconfig file for the admin to use and for kubeadm itself
    /kubelet                   Generates a kubeconfig file for the kubelet to use. Please note that this should be used *only* for bootstrapping purposes.
    /controller-manager        Generates a kubeconfig file for the controller manager to use
    /scheduler                 Generates a kubeconfig file for the scheduler to use
  audit-policy               Generates the audit-policy to use in the API server configuration (featureGated)
  controlplane               Generates all static Pod manifest files necessary to establish the control plane
    /apiserver                 Generates the API server static Pod manifest.
    /controller-manager        Generates the controller-manager static Pod manifest.
    /scheduler                 Generates the scheduler static Pod manifest.
  etcd                       Generates the static Pod manifest file for a local, single-node etcd instance
  init-kubeletconfig         (featureGated) Writes base configuration of kubelets to disk
  upload-kubeletconfig       (featureGated) Uploads kubelet config to a configMap and use it as a configSource for this node
  upload-config              Uploads the currently used configuration for kubeadm to a ConfigMap
  mark-master                Marks a node as master
  bootstrap-token            Makes all the bootstrap token configurations and creates an initial token
    /token                     Creates an initial bootstrap token to be used for node joining
    /cluster-info|clusterinfo  Uploads the cluster-info ConfigMap from the given kubeconfig file
    /allow-post-csr            Configures RBAC to allow node bootstrap tokens to post CSR in order for nodes to get long term certificate credentials
    /allow-auto-approve        Configures RBAC rules to allow the CSR approver controller automatically approve CSR from a node bootstrap token
  addons                     Installs required addons for passing Conformance tests
    /dns                       Installs the dns addon to a Kubernetes cluster
    /proxy                     Installs the proxy addon to a Kubernetes cluster
  selfhosting|self-hosting   Makes a kubeadm cluster self-hosted (featureGated)

Release note:

TBD

k8s-ci-robot · 2018-03-24T12:18:06Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fabriziopandini
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: brendandburns

Assign the PR to them by writing /assign @brendandburns in a comment when ready.

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:

cmd/kubeadm/OWNERS
docs/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jamiehannaford · 2018-03-26T13:46:20Z

@fabriziopandini Is there any way we can split up this PR into smaller chunks? 134 files is a lot to parse in one go! 😄 🤔

fabriziopandini · 2018-03-26T15:35:54Z

@jamiehannaford I'm going to remove temporarly auto-generated docs from the PR; this reduce the number of changes significantly, even if some check will complain.

The following commits will contains the relevant changes:

phases to beta
cmd factories
phased command

An this will sum up to about 2000 lines of code for the full kubeadm init & phases, plus some deleted files.

k8s-ci-robot · 2018-03-26T17:56:21Z

@fabriziopandini: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`e3e4d88`	link	`/test pull-kubernetes-e2e-kops-aws`
pull-kubernetes-verify	`e3e4d88`	link	`/test pull-kubernetes-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

liztio · 2018-03-26T21:11:24Z

cmd/kubeadm/app/cmd/util/phases/phases.go

+
+// HasByArgOrAlias returns true if the given phase (identified by phase.Arg)
+// is included in the workflow
+func (w PhaseWorkflow) HasByArgOrAlias(arg string) bool {


any reason this isn't HasOrgOrAlias like the individual phases?

liztio · 2018-03-27T20:39:19Z

cmd/kubeadm/app/cmd/util/factory/waiter.go

+		return dryrun.NewWaiter()
+	}
+
+	timeout := 30 * time.Minute


should these be constants maybe?

liztio · 2018-03-27T20:40:43Z

cmd/kubeadm/app/cmd/initworkflow.go

+	"upload-config", // because in this case it is not allowed to change the KubernetesVersion
+}
+
+// initWorkflow defines the main init workflow as a sequence of ordered phases


maybe call this kubeadm init workflow, to distinguish it from generic "initializer" functions?

liztio · 2018-03-27T20:48:01Z

cmd/kubeadm/app/cmd/util/phases/builder.go

+		Example:   pc.Example,
+		Args:      pc.validateArgs(pc.PhaseArgsValidator, pc.ArgsValidator), // triggers separated validation for for phaseArgs and other custom/positional args.
+		ValidArgs: pc.ValidArgs,
+		RunE: func(cmd *cobra.Command, args []string) error {


if we wanted to have seperate help functions, we could just programatically generate and nest a bunch of commands here using the same logic.

liztio · 2018-03-27T20:50:52Z

cmd/kubeadm/app/cmd/util/phases/builder.go

+
+// descriptionWithPhases returns the PhasedCommand long description with the addition of the
+// message for using phases
+func (pc *PhasedCommandBuilder) descriptionWithPhases() string {


be nice to have an example of what this looks like in comments, maybe?

nevermind, found it in doc.go

liztio · 2018-03-27T20:53:57Z

cmd/kubeadm/app/cmd/util/phases/builder.go

+
+		if customArgValidator != nil {
+			if err := customArgValidator(cmd, customArgs); err != nil {
+				return fmt.Errorf("invalid args: %v", err)


guessing this codebase doesn't use pkg/error?

xiangpengzhao · 2018-04-03T08:39:40Z

cmd/kubeadm/app/cmd/util/factory/masterconfiguration.go

+// MasterConfiguration returns the MasterConfiguration instance.
+func (f *MasterConfigurationFactory) MasterConfiguration() *kubeadmapi.MasterConfiguration {
+	if f.masterConfigurationInstance == nil {
+		panic("Invalid operation. InitMasterConfiguration must be executed before GetMasterConfiguration")


Maybe we want to make the panic info more clear? I think users may not be sure what happened or what to do when they see this info.

same as below

jamiehannaford

first round. I'll try to have a deeper look at this soon when I have time.

jamiehannaford · 2018-03-27T08:52:00Z

cmd/kubeadm/app/cmd/init.go

+// All the methods implementing the phases use this object as a receiver, so all the attributes
+// of this object will be passed through the init phases.
+type initContext struct {
+	// init input parameters


can we document each field?

jamiehannaford · 2018-03-27T08:52:24Z

cmd/kubeadm/app/cmd/init.go

+	factory.ClientFactory
+	factory.WaiterFactory
+
+	// worklow flags to be shared across init phases


jamiehannaford · 2018-03-27T08:53:20Z

cmd/kubeadm/app/cmd/init.go

-		Run: func(cmd *cobra.Command, args []string) {
+		// Phases provides the definition of the command workflow as a sequence of ordered phases
+		Phases: context.initWorkflow(),
+		// RunPhases provide the function the executes the entire command logic (all the phases) or


provides the function that

jamiehannaford · 2018-03-27T08:53:44Z

cmd/kubeadm/app/cmd/init.go

-			legacyscheme.Scheme.Default(cfg)
-			internalcfg := &kubeadmapi.MasterConfiguration{}
-			legacyscheme.Scheme.Convert(cfg, internalcfg, nil)
+			// If --config file is passed, errors if any overlapping flag is used too (mixedArguments)


an error is thrown

jamiehannaford · 2018-03-27T08:55:16Z

cmd/kubeadm/app/cmd/init.go

+
+			// If the kubernetes version is not used in the phasesToRun, inhibits kubernetes version lookup
+			// (an unnecessary access to internet in those cases) - by setting it to a default value -
+			context.inhibitVersionLookupWhenPossible(phasesToRun, cfg)


can we replace inhibit with prevent?

k8s-ci-robot · 2018-04-14T22:21:16Z

@fabriziopandini: PR needs rebase.

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.

fejta-bot · 2018-07-13T23:19:13Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2018-08-13T00:05:22Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

fabriziopandini · 2018-08-13T05:09:11Z

/remove-lifecycle rotten

timothysc · 2018-08-22T15:02:29Z

/hold

fejta-bot · 2018-09-21T15:46:09Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2018-09-21T15:46:16Z

@fejta-bot: Closing this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

k8s-ci-robot requested review from brendandburns and krousey March 24, 2018 12:18

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 24, 2018

k8s-github-robot added the kind/old-docs label Mar 24, 2018

fabriziopandini mentioned this pull request Mar 24, 2018

split phase selfhosting convert-from-staticpods kubernetes/kubeadm#735

Closed

fabriziopandini force-pushed the kubeadm-phase-GA branch from ad82c42 to 3622b9b Compare March 25, 2018 20:13

fabriziopandini added 5 commits March 26, 2018 19:19

generated files

1b74a3a

deleted files

2c9e02e

phased command

a8771f4

cmd factories

fefc0b5

phases to beta

e3e4d88

fabriziopandini force-pushed the kubeadm-phase-GA branch from 3622b9b to e3e4d88 Compare March 26, 2018 17:20

k8s-github-robot removed the kind/old-docs label Mar 26, 2018

liztio reviewed Mar 26, 2018

View reviewed changes

liztio reviewed Mar 27, 2018

View reviewed changes

xiangpengzhao reviewed Apr 3, 2018

View reviewed changes

timothysc self-requested a review April 7, 2018 00:50

jamiehannaford reviewed Apr 9, 2018

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 14, 2018

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2018

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 13, 2018

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 22, 2018

k8s-ci-robot closed this Sep 21, 2018

fabriziopandini deleted the kubeadm-phase-GA branch October 20, 2018 14:29

ghost mentioned this pull request Feb 12, 2021

kubeadm phases to beta kubernetes/enhancements#2501

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] - Kubeadm phase ga #61631

[WIP] - Kubeadm phase ga #61631

fabriziopandini commented Mar 24, 2018

k8s-ci-robot commented Mar 24, 2018

jamiehannaford commented Mar 26, 2018

fabriziopandini commented Mar 26, 2018 •

edited

Loading

k8s-ci-robot commented Mar 26, 2018 •

edited

Loading

liztio Mar 26, 2018

liztio Mar 27, 2018

liztio Mar 27, 2018

liztio Mar 27, 2018

liztio Mar 27, 2018

liztio Mar 27, 2018

liztio Mar 27, 2018

xiangpengzhao Apr 3, 2018

xiangpengzhao Apr 3, 2018

jamiehannaford left a comment

jamiehannaford Mar 27, 2018

jamiehannaford Mar 27, 2018

jamiehannaford Mar 27, 2018

jamiehannaford Mar 27, 2018

jamiehannaford Mar 27, 2018

k8s-ci-robot commented Apr 14, 2018

fejta-bot commented Jul 13, 2018

fejta-bot commented Aug 13, 2018

fabriziopandini commented Aug 13, 2018

timothysc commented Aug 22, 2018

fejta-bot commented Sep 21, 2018

k8s-ci-robot commented Sep 21, 2018

[WIP] - Kubeadm phase ga #61631

[WIP] - Kubeadm phase ga #61631

Conversation

fabriziopandini commented Mar 24, 2018

k8s-ci-robot commented Mar 24, 2018

jamiehannaford commented Mar 26, 2018

fabriziopandini commented Mar 26, 2018 • edited Loading

k8s-ci-robot commented Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamiehannaford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 14, 2018

fejta-bot commented Jul 13, 2018

fejta-bot commented Aug 13, 2018

fabriziopandini commented Aug 13, 2018

timothysc commented Aug 22, 2018

fejta-bot commented Sep 21, 2018

k8s-ci-robot commented Sep 21, 2018

fabriziopandini commented Mar 26, 2018 •

edited

Loading

k8s-ci-robot commented Mar 26, 2018 •

edited

Loading