Final kubeadm-kubelet integration refactor PR by luxas · Pull Request #64624 · kubernetes/kubernetes

luxas · 2018-06-01T18:31:46Z

What this PR does / why we need it:
Note: Work in progress
This PR:

Updates the debs/rpms to do the "right thing" with the new integration flow
- Broken out into kubeadm: Update the dropin for the kubelet in v1.11 #64780
Uploads the CRISocket information to the Node object as an annotation
- Broken out into: kubeadm: Upload CRISocket information in kubeadm init/join #64792
Makes the kubeadm init / kubeadm join flow to be preflight, stop kubelet, write config/env files, daemon-reload, start kubelet
Renames .NodeRegistration.ExtraArgs to .NodeRegistration.KubeletExtraArgs as discussed in the SIG meeting
Adds a kubeadm upgrade node config command for fetching the latest configuration and writing it down to the node before upgrading the kubelet
Makes dynamic kubelet config actually get enabled when the feature gate in kubeadm is specifically opted into by the user
Fixes misc. minor bugs
Makes sure kubeadm init --dry-run works, so the dry-run functionality works for the kubelet integration as well

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes kubernetes/kubeadm#848

Special notes for your reviewer:

Release note:

kubeadm: Add a new `kubeadm upgrade node config` command

@kubernetes/sig-cluster-lifecycle-pr-reviews

chuckha

Looking forward to having this.

Please fix the glog issues then it's 👍from me.

In general, do not use glog.Infof/ln and favor fmt.Printf/ln. If you want it to not show up by default, use glog.V(x).Infof/ln.

The bare glog.Info calls are responsible for the gross default log output.

chuckha · 2018-06-01T18:58:43Z

cmd/kubeadm/app/preflight/checks.go

This applies to all uses of glog.Info{f,ln}
Either use glog as a glog.V(x).Infoln or fmt.Println

I'm gonna wait for your PR to merge, rebase and then fixup this (which then gets easier, given your PR merges tonight)

chuckha · 2018-06-01T19:00:11Z

cmd/kubeadm/app/preflight/checks.go

This seems like an error instead of a warning, unless there's a way we can recover automatically from this failure?

Additionally, instead of using glog.Warningf, please add a warning prefix and use fmt.Println. Or if you feel like warning isn't the right level, use fmt.Printf or glog.V(x).Infof

chuckha · 2018-06-01T19:14:03Z

cmd/kubeadm/app/cmd/upgrade/node.go

godoc this please

chuckha · 2018-06-01T19:14:31Z

cmd/kubeadm/app/cmd/upgrade/node.go

accidentally the rest of the sentence

timothysc

1st pass feedback

/cc @kad

b/c he had opinions earlier.

timothysc · 2018-06-01T19:12:28Z

cmd/kubeadm/app/cmd/init.go

? - this seems misplaced to me.

kubeletVersion is needed for the dynamic config func. This execs out to the kubelet to determine what version to target. This func call just moved to where it's actually used

timothysc · 2018-06-01T19:32:45Z

cmd/kubeadm/app/cmd/upgrade/node.go

I don't know how I feel about this vs. under the config subcommand.

UX is inconsistent for configs

This is gonna be pretty similar to kubeadm phase kubelet config download. WDYT?

timothysc · 2018-06-01T19:35:07Z

cmd/kubeadm/app/preflight/checks.go

Um then we should call this f(n) restart.

oops should be .ServiceStop here. fixed

liztio · 2018-06-04T17:33:05Z

build/rpms/10-kubeadm.conf

this needs to be =-, not -=

Thanks, fixed!

liztio · 2018-06-04T17:33:12Z

build/rpms/10-kubeadm.conf

liztio · 2018-06-04T17:33:24Z

build/debs/10-kubeadm.conf

liztio · 2018-06-04T17:33:30Z

build/debs/10-kubeadm.conf

fejta-bot · 2018-06-06T11:08:20Z

/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 comment for consistent failures.

fejta-bot · 2018-06-06T13:35:17Z

/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 comment for consistent failures.

timothysc · 2018-06-06T14:35:33Z

/test pull-kubernetes-e2e-gce

fejta-bot · 2018-06-06T17:05:18Z

/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 comment for consistent failures.

fejta-bot · 2018-06-06T19:11:20Z

/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 comment for consistent failures.

… integration work

luxas · 2018-06-06T20:28:50Z

(as part of the rebase, I added more unit test cases in cmd/kubeadm/app/phases/kubelet/flags_test.go, which made this go XXL)

neolit123 · 2018-06-06T21:26:17Z

/test pull-kubernetes-e2e-gce

neolit123 · 2018-06-06T22:32:13Z

re-adding
/lgtm

k8s-ci-robot · 2018-06-06T22:32:22Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: fabriziopandini, luxas, neolit123, timothysc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~cmd/kubeadm/OWNERS~~ [fabriziopandini,luxas,timothysc]
docs/OWNERS
pkg/OWNERS

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

k8s-github-robot · 2018-06-06T22:32:39Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@fabriziopandini @luxas @neolit123 @timothysc

Pull Request Labels

sig/cluster-lifecycle: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/feature: New functionality.

Help

neolit123 · 2018-06-07T00:08:43Z

/test pull-kubernetes-kubemark-e2e-gce

neolit123 · 2018-06-07T00:14:33Z

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

k8s-github-robot · 2018-06-07T02:37:35Z

/test all [submit-queue is verifying that this PR is safe to merge]

neolit123 · 2018-06-07T02:53:06Z

/test pull-kubernetes-node-e2e

k8s-github-robot · 2018-06-07T02:56:23Z

Automatic merge from submit-queue (batch tested with PRs 63386, 64624, 62297, 64847). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot · 2018-06-07T03:10:18Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-integration	`7a87cf1`	link	`/test pull-kubernetes-integration`
pull-kubernetes-e2e-gce	`7a87cf1`	link	`/test pull-kubernetes-e2e-gce`

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.

Details

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.

zparnold · 2018-06-13T15:00:00Z

Hello there! @luxas I'm Zach Arnold working on Docs for the 1.11 release. This PR was identified as one needing some documentation in the https://github.com/kubernetes/website repo around your contributions (thanks by the way!) When you have some time, could you please modify/add/remove the relevant content that needs changing in our documentation repo? Thanks! Please let me or my colleague Misty know (@zparnold/@misty on K8s Slack) if you need any assistance with the documentation.

k8s-ci-robot requested review from jbeda and lavalamp June 1, 2018 18:32

luxas assigned timothysc Jun 1, 2018

luxas requested review from fabriziopandini, kad, liztio and timothysc and removed request for jbeda and lavalamp June 1, 2018 18:32

luxas assigned fabriziopandini Jun 1, 2018

chuckha suggested changes Jun 1, 2018

View reviewed changes

chuckha reviewed Jun 1, 2018

View reviewed changes

timothysc suggested changes Jun 1, 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 Jun 2, 2018

timothysc added this to the v1.11 milestone Jun 4, 2018

timothysc added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. kind/feature Categorizes issue or PR as related to a new feature. status/approved-for-milestone labels Jun 4, 2018

liztio suggested changes Jun 4, 2018

View reviewed changes

liztio mentioned this pull request Jun 4, 2018

kubeadm: Let the --node-name flag flow down to --hostname-override for the kubelet #64706

Merged

luxas force-pushed the kubeadm_kubelet_final branch from cdc6e5b to 7964d7c Compare June 4, 2018 22:14

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 4, 2018

neolit123 mentioned this pull request Jun 6, 2018

1.11 command list in "Kubeadm overview" kubernetes/kubeadm#899

Closed

Add a 'kubeadm upgrade node config' command and finish up the kubelet…

1a0de94

… integration work

luxas force-pushed the kubeadm_kubelet_final branch from 7d3592b to d60aa40 Compare June 6, 2018 20:25

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 6, 2018

autogenerated

7a87cf1

luxas force-pushed the kubeadm_kubelet_final branch from d60aa40 to 7a87cf1 Compare June 6, 2018 20:41

k8s-ci-robot assigned neolit123 Jun 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2018

neolit123 mentioned this pull request Jun 6, 2018

kubeadm-init: update the example for the MasterConfiguration kubernetes/website#8958

Merged

k8s-github-robot merged commit 61a5809 into kubernetes:master Jun 7, 2018

AishSundar mentioned this pull request Jun 7, 2018

[Failing test] kubeadm-gce and kubeadm-gce-selfhosting are failing on release-master-blocking suite #64887

Closed

Conversation

luxas commented Jun 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chuckha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timothysc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fejta-bot commented Jun 6, 2018

Uh oh!

fejta-bot commented Jun 6, 2018

Uh oh!

timothysc commented Jun 6, 2018

Uh oh!

fejta-bot commented Jun 6, 2018

Uh oh!

fejta-bot commented Jun 6, 2018

Uh oh!

luxas commented Jun 6, 2018

Uh oh!

neolit123 commented Jun 6, 2018

Uh oh!

neolit123 commented Jun 6, 2018

Uh oh!

k8s-ci-robot commented Jun 6, 2018

Uh oh!

k8s-github-robot commented Jun 6, 2018

Uh oh!

neolit123 commented Jun 7, 2018

Uh oh!

neolit123 commented Jun 7, 2018

Uh oh!

k8s-github-robot commented Jun 7, 2018

Uh oh!

neolit123 commented Jun 7, 2018

Uh oh!

luxas commented Jun 1, 2018 •

edited

Loading

k8s-ci-robot commented Jun 7, 2018 •

edited

Loading