kubeadm: Make kubeadm use the right CSR approver for the right version #46864

luxas · 2017-06-02T15:20:19Z

What this PR does / why we need it:

fixes regression caused in: #45619

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Fixes: kubernetes/kubeadm#289

Special notes for your reviewer:

cc @pipejakob our e2e CI should probably go green after this change

Release note:

NONE

@mikedanese @pipejakob @timothysc @liggitt

k8s-ci-robot · 2017-06-02T15:20:22Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

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.

k8s-ci-robot · 2017-06-02T15:20:22Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

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.

k8s-ci-robot · 2017-06-02T15:20:22Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

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.

pipejakob · 2017-06-02T16:51:00Z

@luxas Cool, but as a heads-up, our CI job was failing because of quota issues (fixed now), but hasn't rerun in a while because the submit queue is a little broken now (it seems that someone manually merged some commits that broke verification). No new commits have been merged into kubernetes for 16 hours or so, so it hasn't run in that long. That's not to say there weren't also other regressions caused by #45619 that might appear now, though.

I believe @spxtr is the test-infra on-call and is looking into unblocking the submit queue.

pipejakob · 2017-06-02T16:54:44Z

Actually, looks like #46863 should take care of it.

k8s-ci-robot · 2017-06-02T16:57:41Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-federation-e2e-gce	ead8e401a1f10a4e0b30b69018a67f4c67754f51	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

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.

liggitt · 2017-06-02T17:24:09Z

it wasn't a merge that broke master, it was a new git tag :-/

liggitt · 2017-06-02T17:26:30Z

cmd/kubeadm/app/master/manifests.go

+	if k8sVersion.LessThan(kubeadmconstants.MinimumCSRSARApproverVersion) {
+		// enable the former CSR group approver for v1.6 clusters.
+		// TODO(luxas): Remove this once we're targeting v1.8 at HEAD
+		defaultArguments["insecure-experimental-approve-all-kubelet-csrs-for-group"] = bootstrapapi.BootstrapGroup


how was this being set before?

@liggitt It was being set unconditionally in the defaultArguments slice initializer above. Now only for v1.6

ah, and got removed by the SAR PR. ok.

pipejakob · 2017-06-02T17:29:36Z

@liggitt Yeah, Joe pointed out #46863 to me. Thanks for the fix!

liggitt · 2017-06-02T18:37:33Z

this LGTM, but bazel is very unhappy

luxas · 2017-06-02T20:24:54Z

I'll fixup the tests tomorrow or so, late here now.
@mikedanese it would be awesome if I could get a soft approval on this and be able to self-apply once I have generated code and tests uploaded.

luxas · 2017-06-02T20:25:37Z

@pipejakob Is there a way to invoke the kubeadm CI with a (@)k8s-bot command btw?

pipejakob · 2017-06-02T21:33:45Z

@k8s-bot pull-kubernetes-e2e-kubeadm-gce test this

pipejakob · 2017-06-02T21:34:49Z

Oh, the kubeadm pull job actually depends on a successful completion of the bazel pull job (that's the step that builds/uploads the .debs we use), so I can't successfully trigger it until you have pull-kubernetes-bazel passing.

timothysc · 2017-06-04T21:39:35Z

cmd/kubeadm/app/master/manifests.go

@@ -54,7 +55,7 @@ const (
 )

 var (
-	v170 = version.MustParseSemantic("v1.7.0-alpha.0")
+	v170alpha0 = version.MustParseSemantic("v1.7.0-alpha.0")


I actually prefer the v170 b/c we should only support released versions, correct?

liggitt · 2017-06-06T00:48:30Z

@luxas are you able to update this to fix bazel and tests?

luxas · 2017-06-06T02:24:04Z

@liggitt Will do in a couple of hours. Sorry for the slight delay, am partially/have been OOO.

luxas · 2017-06-06T04:49:10Z

@k8s-bot pull-kubernetes-e2e-kubeadm-gce test this

liggitt · 2017-06-06T05:39:31Z

/lgtm

liggitt · 2017-06-06T05:42:46Z

would like to see a green CI run for kubeadm - https://k8s-gubernator.appspot.com/pr/46864

luxas · 2017-06-06T06:05:29Z

/approve no-issue

I must do this due to kubernetes/test-infra#2975

k8s-github-robot · 2017-06-06T06:05:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liggitt, luxas

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

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

Needs approval from an approver in each of these OWNERS Files:

~~cmd/kubeadm/OWNERS~~ [luxas]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-06-06T12:22:43Z

Automatic merge from submit-queue (batch tested with PRs 46897, 46899, 46864, 46854, 46875)

@mikedanese

Automatic merge from submit-queue kubeadm: Enable the Node Authorizer/Admission plugin in v1.7 **What this PR does / why we need it**: This is similar to #46796, but for kubeadm. Basically it was a part of #46796, but there were some other upgradability and compability concerns for kubeadm I took care of while working today. Example: ```console $ kubeadm init --kubernetes-version v1.7.0-beta.0 [kubeadm] WARNING: kubeadm is in beta, please do not use it for production clusters. [init] Using Kubernetes version: v1.7.0-beta.0 [init] Using Authorization mode: [RBAC Node] ... $ sudo kubectl --kubeconfig=/etc/kubernetes/kubelet.conf get secret foo Error from server (Forbidden): User "system:node:thegopher" cannot get secrets in the namespace "default".: "no path found to object" (get secrets foo) $ echo '{"apiVersion":"v1","kind":"Node","metadata":{"name":"foo"}}' | sudo kubectl create -f - --kubeconfig=/etc/kubernetes/kubelet.conf Error from server (Forbidden): error when creating "STDIN": nodes "foo" is forbidden: node thegopher cannot modify node foo ``` **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: Depends on #46864 (uses that PR as a base, will rebase once it's merged) Please only review the second commit. Will also fix tests in a minute. **Release note**: ```release-note kubeadm: Enable the Node Authorizer/Admission plugin in v1.7 ``` @mikedanese @liggitt @pipejakob @roberthbailey @jbeda @timothysc

luxas added the kind/bug Categorizes issue or PR as related to a bug. label Jun 2, 2017

luxas added this to the v1.7 milestone Jun 2, 2017

luxas assigned mikedanese Jun 2, 2017

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jun 2, 2017

luxas force-pushed the kubeadm_fix_v16_csrs branch from 0bcdcb8 to 3da0f28 Compare June 2, 2017 15:21

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 2, 2017

k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Jun 2, 2017

luxas force-pushed the kubeadm_fix_v16_csrs branch from 3da0f28 to ead8e40 Compare June 2, 2017 16:05

liggitt reviewed Jun 2, 2017

View reviewed changes

luxas mentioned this pull request Jun 2, 2017

kubeadm: Enable the Node Authorizer/Admission plugin in v1.7 #46879

Merged

luxas assigned pipejakob Jun 2, 2017

timothysc reviewed Jun 4, 2017

View reviewed changes

k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2017

liggitt added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Jun 6, 2017

kubeadm: Make kubeadm use the right CSR approver for the right version

de2ef8f

luxas force-pushed the kubeadm_fix_v16_csrs branch from ead8e40 to de2ef8f Compare June 6, 2017 04:47

k8s-ci-robot assigned liggitt Jun 6, 2017

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

luxas mentioned this pull request Jun 6, 2017

Deploying v1.6 clusters with kubeadm at HEAD is broken kubernetes/kubeadm#289

Closed

luxas added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 6, 2017

pipejakob mentioned this pull request Jun 6, 2017

Reenable kubeadm presubmit test. kubernetes/test-infra#2976

Closed

k8s-github-robot merged commit f04a774 into kubernetes:master Jun 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: Make kubeadm use the right CSR approver for the right version #46864

kubeadm: Make kubeadm use the right CSR approver for the right version #46864

luxas commented Jun 2, 2017 •

edited

Loading

k8s-ci-robot commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017

pipejakob commented Jun 2, 2017

pipejakob commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017 •

edited

Loading

liggitt commented Jun 2, 2017

liggitt Jun 2, 2017

luxas Jun 2, 2017

liggitt Jun 2, 2017

pipejakob commented Jun 2, 2017

liggitt commented Jun 2, 2017

luxas commented Jun 2, 2017

luxas commented Jun 2, 2017

pipejakob commented Jun 2, 2017

pipejakob commented Jun 2, 2017

timothysc Jun 4, 2017

liggitt commented Jun 6, 2017

luxas commented Jun 6, 2017

luxas commented Jun 6, 2017

liggitt commented Jun 6, 2017

liggitt commented Jun 6, 2017 •

edited

Loading

luxas commented Jun 6, 2017 •

edited

Loading

k8s-github-robot commented Jun 6, 2017

k8s-github-robot commented Jun 6, 2017

kubeadm: Make kubeadm use the right CSR approver for the right version #46864

kubeadm: Make kubeadm use the right CSR approver for the right version #46864

Conversation

luxas commented Jun 2, 2017 • edited Loading

k8s-ci-robot commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017

pipejakob commented Jun 2, 2017

pipejakob commented Jun 2, 2017

k8s-ci-robot commented Jun 2, 2017 • edited Loading

liggitt commented Jun 2, 2017

liggitt Jun 2, 2017

Choose a reason for hiding this comment

luxas Jun 2, 2017

Choose a reason for hiding this comment

liggitt Jun 2, 2017

Choose a reason for hiding this comment

pipejakob commented Jun 2, 2017

liggitt commented Jun 2, 2017

luxas commented Jun 2, 2017

luxas commented Jun 2, 2017

pipejakob commented Jun 2, 2017

pipejakob commented Jun 2, 2017

timothysc Jun 4, 2017

Choose a reason for hiding this comment

liggitt commented Jun 6, 2017

luxas commented Jun 6, 2017

luxas commented Jun 6, 2017

liggitt commented Jun 6, 2017

liggitt commented Jun 6, 2017 • edited Loading

luxas commented Jun 6, 2017 • edited Loading

k8s-github-robot commented Jun 6, 2017

k8s-github-robot commented Jun 6, 2017

luxas commented Jun 2, 2017 •

edited

Loading

k8s-ci-robot commented Jun 2, 2017 •

edited

Loading

liggitt commented Jun 6, 2017 •

edited

Loading

luxas commented Jun 6, 2017 •

edited

Loading