register master kubelet with the apiserver #14054

mikedanese · 2015-09-16T17:44:48Z

dchen1107 · 2015-09-16T17:48:32Z

cc/ @roberthbailey to make sure this doesn't break GKE's configurations.

dchen1107 · 2015-09-16T18:13:08Z

shippable failed due to:

godep go install ./...

k8s.io/kubernetes/contrib/mesos/pkg/executor/service

contrib/mesos/pkg/executor/service/service.go:341: not enough arguments in call to kubelet.NewMainKubelet
godep: go exit status 2

mikedanese · 2015-09-16T18:23:25Z

@dchen1107 fixed mesos

k8s-bot · 2015-09-16T19:44:39Z

GCE e2e build/test failed for commit 1a3e0b8cadbc97a3d48606404bc6690dbba90ddb.

roberthbailey · 2015-09-16T19:52:09Z

cmd/kubelet/app/server.go

@@ -268,6 +272,8 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&s.ReallyCrashForTesting, "really-crash-for-testing", s.ReallyCrashForTesting, "If true, when panics occur crash. Intended for testing.")
 	fs.Float64Var(&s.ChaosChance, "chaos-chance", s.ChaosChance, "If > 0.0, introduce random client errors and latency. Intended for testing. [default=0.0]")
 	fs.BoolVar(&s.Containerized, "containerized", s.Containerized, "Experimental support for running kubelet in a container.  Intended for testing. [default=false]")
+	fs.BoolVar(&s.ReconcileCIDR, "reconcile-cidr", s.ReconcileCIDR, "Reconcile node CIDR with the CIDR specified by the API server. No-op if register-node or configure-cbr0 is false. [default=true]")


Can you briefly explain why we need this in addition to the configureCBR0 flag? Why would we want to configure cbr0 but not reconcile the CIDR from the master?

/cc @cjcullen

@roberthbailey To clarify, do you want me to explain it here or in a comment? (or both?)

If it's non-trivial, a comment would be best.

Here is the reason: We removed salt-based of concileCIDR completely on all nodes, no matter master node or worker nodes, then master node's cidr bridge won't be configured by Kubelet at all. Introducing this one is to reconfige cidr for minion per apiserver.

I think we can eventually consolidate some of those flags, but ok as it is today. :-)

I guess I'm confused as to why passing --configure-cbr0=false doesn't have the desired effect of preventing the kubelet running on the master from messing with the bridge.

The bridge cbr0 doesn't exist and docker is configured to use cbr0. We used to have container_bridge.py to create the bridge on the master so we could either readd container_bridge.py it our use kubelet to configure the bridge.

So configure-cbr0 tells the kubelet whether or not to create / configure cbr0 and reconcile-cidr tells the kubelet whether or not to reconfigure the bridge based on the cidr allocation from the apiserver. Is the reason that we don't want to reconfigure the bridge because the interaction of restarting docker + supervisord causes the cluster to be unhealthy? If we want to make the master node schedulable at some point then it will need to be able to reconfigure cbr0 when it gets a cidr assigned.

roberthbailey · 2015-09-16T19:54:19Z

@dchen1107 thanks for the cc. This should be fine for GKE.

roberthbailey · 2015-09-16T20:04:38Z

pkg/kubelet/kubelet.go

@@ -795,6 +802,9 @@ func (kl *Kubelet) initialNodeStatus() (*api.Node, error) {
 			Name:   kl.nodeName,
 			Labels: map[string]string{"kubernetes.io/hostname": kl.hostname},
 		},
+		Spec: api.NodeSpec{


If a node is unschedulable, should the kubelet not watch for pods to run? That would be much better security than setting a flag that could be overridden by anyone with api access.

@mikedanese had a brief chat over a similar topic earlier this morning. At the end, we still want master node(s) be schedulable, not ready today. This is not recommended by us, we are going to document it, but it is up to the user / cluster admin to decide if making such node schedulable in a OSS environment.

The behavior would still be controlled by this flag though.

Right now, you could post a node status manually to change the node to scehduleable, but that will get flipped back every N seconds as the kubelet keeps posting status. In the intervening window, you could pin a pod to the master node to run arbitrary code on it.

If the kubelet (when told to be unschedulable) both told the master it's intent and explicitly didn't run any pods assigned to it, that would close the hole. A cluster admin could still create a master as a schedulable node by changing this flag.

Right now, you could post a node status manually to change the node to scehduleable, but that will get flipped back every N seconds as the kubelet keeps posting status.

Unschedulable is part of Spec not Status, and is posted only once during initial registration.

The --register-schedulable=false flag is meant to tell a kubelet to register as unscheduled initially but makes no guarantees that the node will remain unschedulable. It may even have a broader use case than just the master's kubelet.

I can make this explicit in the flag documentation.

If the kubelet (when told to be unschedulable) both told the master it's intent and explicitly didn't run any pods assigned to it, that would close the hole. A cluster admin could still create a master as a schedulable node by changing this flag.

IMHO that doesn't provide any better security or user experince than just locking down the node resource.

I don't think this introduces any extra security hole. Honestly we don't have a real security solution for OSS kubernetes yet, and the feature is completely disabled for GKE environment.

One of major benefits introduced by Kubernetes is decoupling cluster operations from application operations. If a cluster admin decide to allow master node schedulable, that is his decision. It shouldn't allow arbitrary application users to change node configurations / flags. If they do, that is bad operations!

@mikedanese: What is the use case for making --register-schedulable flag an initialization only option instead of being valid for the entire lifetime?
In general, I find multiple ways to configure the same behavior confusing. As a user, I'd prefer to either flip a flag or post an update to an API server object and not deal with how these two options interact.

@mikedanese: Ping.

The intent is only to control initial registration configuration, not to control it continuously. We don't want to take away power from the API. That's the domain of auth policy, as discussed above.

If were to control behavior continuously, that would require a different means of communicating Kubelet's configuration back up to the scheduler. More details are discussed in #3885. We're not ready to do that, though.

If we wanted to materially impact the security profile of Kubelet, we need a more comprehensive design. For instance, if exec weren't blocked as well as pod creation, there's no point. OTOH, blocking exec would impact debuggability.

I am glad to see that we are converging on this. To me that flag --register-schedulable is a temporary hack because we don't have config object defined to generate NodeSpec yet. But to Kubelet, I want NodeSpec is the only source of truth here.

mikedanese · 2015-09-16T21:22:07Z

This is not intended to be enabled in GKE, correct? My understanding from a conversation with @dchen1107 is that this feature is for kubernetes OSS for now.

k8s-github-robot · 2015-09-16T21:30:23Z

Labelling this PR as size/M

dchen1107 · 2015-09-16T22:49:26Z

Yes, this is only for OSS Kubernetes. For GKE, this should be disabled completely for now. But we can extend this one after 1.1 for GKE.

dchen1107 · 2015-09-17T20:58:58Z

@mikedanese Please rebase your PR.

Had offline discussion with @roberthbailey, and we can change Kubelet with unschedulable mode to post status, start static pods, and refuse other allocated pods from kube-apiserver, but that shouldn't block this PR. I will file a separate issue to address that.

mikedanese · 2015-09-18T17:14:27Z

@k8s-bot ok to test

k8s-bot · 2015-09-18T17:38:22Z

GCE e2e build/test failed for commit 990982a2d96f3771b6b6acb8364d8df42bd2fdb4.

k8s-bot · 2015-09-18T21:35:22Z

GCE e2e build/test failed for commit ff2a00acc26eb141ed4b5a30b968f6497b3f6517.

roberthbailey · 2015-09-18T22:41:55Z

pkg/kubelet/node_manager.go

+	if nm.reconcileCIDR {
+		nm.lock.Lock()
+		nm.podCIDR = node.Spec.PodCIDR
+		nm.lock.Unlock()


This lock was previously held during the setNodeStatus & UpdateStatus calls (but I'm not sure whether that was necessary or just a side effect using defer). @cjcullen?

k8s-bot · 2015-10-07T21:06:12Z

GCE e2e test build/test passed for commit 15798cd94bbef37f7566f1a290cb741ed4e92b98.

k8s-github-robot · 2015-10-09T04:17:26Z

PR changed after LGTM, removing LGTM.

…n the master

k8s-bot · 2015-10-09T04:52:22Z

GCE e2e test build/test passed for commit fa60bbe.

k8s-github-robot · 2015-10-09T05:34:59Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2015-10-09T06:10:56Z

GCE e2e build/test failed for commit fa60bbe.

mikedanese · 2015-10-09T14:14:28Z

@k8s-bot ok to test. Flake.

23:04:29 Summarizing 3 Failures:
23:04:29 
23:04:29 [Fail] Deployment [It] deployment should scale up and down in the right order 
23:04:29 /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/deployment.go:264
23:04:29 
23:04:29 [Fail] Kubectl client Kubectl expose [It] should create services for rc 
23:04:29 /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/examples.go:602
23:04:29 
23:04:29 [Fail] Job [It] should run a job to completion when tasks sometimes fail and are locally restarted 
23:04:29 /go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/job.go:74
23:04:29 
23:04:29 Ran 96 of 186 Specs in 1183.504 seconds
23:04:57 FAIL! -- 93 Passed | 3 Failed | 2 Pending | 88 Skipped

k8s-bot · 2015-10-09T14:15:31Z

GCE e2e build/test failed for commit fa60bbe.

mikedanese · 2015-10-09T14:16:18Z

Git pull flake. @k8s-bot ok to test.

k8s-bot · 2015-10-09T14:17:26Z

GCE e2e build/test failed for commit fa60bbe.

mikedanese · 2015-10-09T17:52:37Z

@k8s-bot test

mikedanese · 2015-10-09T20:12:04Z

@k8s-bot OK to test

k8s-bot · 2015-10-09T20:40:40Z

GCE e2e build/test failed for commit fa60bbe.

mikedanese · 2015-10-09T23:35:12Z

@k8s-bot ok to test

k8s-bot · 2015-10-09T23:59:01Z

GCE e2e test build/test passed for commit fa60bbe.

register master kubelet with the apiserver

I have 140 commits in this directory and I get a lot of cleanup reviews and want to be able to approve changes to hack/.golint_failures. 0e69316 delete unused cache b9c7007 enable token review when openapi is generated d5bbc35 make deps-approvers the approvers of sample-cli-plugin/Godeps 4186abf bzl: fix update-bazel.sh 7b47229 remove deprecated /proxy paths b973840 gke-certificates-controller: rm -rf 4961065 cluster: remove unused functions 1e2b644 cluster: move logging library to hack/ bef68f7 cluster: build gci mounter like other go binaries fe7ba9e kubeadm: use kubelet bootstrap instead of reimplementing 3c39173 fixit: break sig-cluster-lifecycle tests into subpackage 64f77eb enable race detection on integration tests cdcfa35 promote tls-bootstrap to beta ff4a814 migrate set generation to go genrule 3600d49 delete benchmark integration tests that don't work at all 21617a6 don't use build tags to mark integration tests 59fc948 bump rules_go and go version for bazel builds ba5c285 bazel: implement git build stamping ad42b42 move kubeadm api group testing to kubeadm package c8ce55f Revert "Merge pull request kubernetes#41132 from kubernetes/revert-40893-kubelet-auth" cbe5bd9 bump gazel to v14 86d9493 remove second CA used for kubelet auth in favor of webhook auth 04a7880 update repo local config to allow redirects from gopkg.in 44b7246 autogenerated 96c146c promote certificates.k8s.io to beta 087016d update gazel to v8 837eee4 pin gazel to v3 e225625 add a configuration for kubelet to register as a node with taints 584689f implement kubectl procelain csr commands 93f737e fix verify-bazel.sh on mac and windows 5dc7554 bazel: implement set-gen as a bazel genrule 61bd6aa remove docs/user-guide from bindata search path 224e32b make godep licenses/copyright check case insensitive 1cd2968 godep: vendor go-bindata d380cb1 fix realpath issue on mac ea632fa Revert "disable bazel build" 27116c6 rename build/ to build-tools/ ee15c80 disable bazel build 999c967 ignore BUILD in the flags-underscore.py validation b250a88 don't check BUILD file when verifying godeps a2eec91 add bazel presubmits to verify BUILD files are up to date c17a8a7 kubectl: apply prune should fallback to basic delete when a resource has no reaper 25e4dcc kubeadm: fix conversion macros and add kubeadm to round trip testing 6d17a87 kubectl: add two more test of kubectl apply --prune 62960aa add a test for kubectl apply --prune 6339d91 add a test to test-cmd.sh for apply -f with label selector b421bf4 build kube-discovery and kubeadm with release 0c76cf5 fix hack/verify-codegen.sh 9f379df add an option to controller-manager to auto approve all CSRs 95e2e29 move kube-dns to the cluster/addons/ directory f3de21b move integration tests into individual pacakges af0177e cleanup hack/verify-govet.sh to throttle process creation 2c93ea5 Merge pull request kubernetes#27289 from mikedanese/split-verify ee34c76 split verify out of unit/integration suite d046275 now that go test runs iteration loops, use that instead of custom executor 1ef1906 Merge pull request kubernetes#26197 from wonderfly/update_default_master_image fbf6bbc Merge pull request kubernetes#25596 from derekparker/inotify 3e1c0b5 run kube-addon-manager in a pod c5cc0c3 Merge pull request kubernetes#24277 from ihmccreery/upgrade-timeout 132c427 add linux fastbuild option to ./build/release.sh 2857baa use defaults in test-dockerized for etcd prefix and api versions 695211e Merge pull request kubernetes#21105 from caesarxuchao/watchCacheForIntegration 2172e0d Merge pull request kubernetes#21108 from mml/slow-flake 1478cf3 Merge pull request kubernetes#21090 from ihmccreery/feature-reboot b3172a4 kubelet: add a pidfile b1743a6 this is a manual reversion of kubernetes#20702 5b27055 Merge pull request kubernetes#19378 from ihmccreery/remove-update-jobs b743827 Merge pull request kubernetes#19659 from ihmccreery/timeout-reboot a6589f7 hack: ignore cluster/env.sh in boilerplate check f71657d retrofit the scheduler with the leader election client. bf763bb Merge pull request kubernetes#19498 from pwittrock/nodelabels 22cfa5e build: move some of hack/lib/ into a new cluster/lib/ b174fc9 Merge pull request kubernetes#18994 from bprashanth/flannel_suite a09d85b expose master count configuration in a cli option on apiserver c2753d7 bump ci go version to 1.5.2 0655e65 fall back to old behavior when deciding mem availablity during build 1d9d11c run kube-proxy in a static pod 91de3a1 cleanup some nits in hack/get-build.sh cd79c6c fix unbound variable error in hace/get-build.sh 5e64590 renable enable var to correct name and only use it when needed 9bdb860 add apigroup installer and tests e6d3b47 add componentconfig api group to autogen stuff 88008de Merge pull request kubernetes#16459 from mikedanese/enable-exp d28d134 Merge pull request kubernetes#16533 from ihmccreery/upgrade-test-fixes 3343522 enable deployment and daemonset in gce upgrade tests 7cbf249 Merge pull request kubernetes#15836 from wojtek-t/codecgen_from_godeps 92404e7 add upgrade test between 1.0 and 1.1 for gce 95b8394 Merge pull request kubernetes#15861 from mikedanese/upgrade-num-minion ece5779 increase NUM_MINIONS for jenkins gce upgrade test b8b35af actually promote daemonset simple test out of flaky and skip all daemonset tests in gke d379a36 copy directory not contents of directory 402e68e add slow test for terminated pod garbage collection c0943f1 add intermediate e2e runs to gce upgrade 10d56ff promote simple daemonset test out of flaky b635fc5 Merge pull request kubernetes#15228 from mesosphere/sttts-conformance-tags 392f33e Merge pull request kubernetes#14054 from mikedanese/register-master fa60bbe add flag to kubelet to ignore the cidr passed down by the apiserver on the master 53e14c7 diff all of pkg/ when verifying swagerspec instead of just pkg/api/ 05ef8ed Merge pull request kubernetes#15104 from mikedanese/ds-e2e fe820fc break up daemonset test into two tests 833be48 enable all experimental flags with one controller 905e971 be explicit about minion group size in upgrade test ae7d3d5 add gce-upgrade to jenkins/e2e.sh 376faea add pod garbage collection b0457be Merge pull request kubernetes#13058 from mvdan/go1.5 a48f218 Merge pull request kubernetes#13754 from tummychow/labels-deps 1fec199 Merge pull request kubernetes#13824 from kubernetes/revert-13547-hpa-kubeup fa40ced move contrib/for-tests to test/images f061875 updating all references in .sh scripts 8326697 rewrite all links to prs to k8s links fb02b33 fix build 8e48431 Revert "demote to flaky tests from parallel e2e" b56edd1 Merge pull request kubernetes#11727 from ZJU-SEL/build-nonstatic-hyperkube cf4cb1a Merge pull request kubernetes#10474 from kargakis/scale-multiple-controllers e376a09 demote to flaky service tests from parallel e2e 7c47d6b Merge pull request kubernetes#12009 from smarterclayton/fix_cmd_config 0269e2b Merge pull request kubernetes#11941 from GoogleCloudPlatform/enact_version_md 94a387d Revert "Improve conversion to support multiple packages" 1a613c4 Merge pull request kubernetes#9971 from smarterclayton/make_conversion_more_flexible 0ae48c4 Merge pull request kubernetes#11927 from wojtek-t/remove_shell_services 59a1dd4 Merge pull request kubernetes#11789 from mbforbes/nodesNetwork 6294070 Merge pull request kubernetes#11803 from wojtek-t/move_back_from_flaky daa6d4d Merge pull request kubernetes#11285 from liggitt/ca 9f16fd9 Merge pull request kubernetes#11860 from ingvagabund/delimiter-for-X-option-eparis c0acfbc Merge pull request kubernetes#11421 from nikhiljindal/exposeServcPort ae1c8e5 Merge pull request kubernetes#11737 from thockin/cleanup-remove-v1beta3 01ee1b8 Merge pull request kubernetes#10840 from jbeda/master d4d99de make mungedoc exit 1 if manual changes are needed and wire up erro message. 337772a fix all tests 055115a fake realpath, and standardize treatment of trailing / of dirs in gendoc b4514ee fix run-gendocs to point to new repo location c053b9a add documentation and script on how to get recent and "nightly" builds 719870f add publishing of latest-green.txt to jenkins e2e tests on success 1e130e0 remove --machines from code and docs dbb47fe remove e2e run before cluster upgrade de55e17 e2e test cluster stability during upgrade c9fcf45 fix bad cmd-test for patch. 9f91532 fix error where we can't use patch and add cmd-test for patch and file update

googlebot added the cla: yes label Sep 16, 2015

dchen1107 self-assigned this Sep 16, 2015

mikedanese force-pushed the register-master branch 2 times, most recently from 7ab4260 to 2280a00 Compare September 16, 2015 17:49

mikedanese force-pushed the register-master branch 2 times, most recently from 1a3e0b8 to 427863f Compare September 16, 2015 18:22

mikedanese force-pushed the register-master branch from 427863f to 0c09323 Compare September 16, 2015 18:27

roberthbailey reviewed Sep 16, 2015
View reviewed changes

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 16, 2015

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 17, 2015

dchen1107 mentioned this pull request Sep 17, 2015

Prodswitch-off Kubelet should reject allocated pods from kube-apiserver (was: Unschedulable Kubelet should reject allocated pods from kube-apiserver) #14140

Closed

mikedanese force-pushed the register-master branch from 0c09323 to 990982a Compare September 18, 2015 16:48

mikedanese removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 18, 2015

dchen1107 mentioned this pull request Sep 18, 2015

kubelet rejects pods from apiserver if node is unschedulable #14143

Closed

mikedanese force-pushed the register-master branch from 990982a to ff2a00a Compare September 18, 2015 21:10

roberthbailey reviewed Sep 18, 2015
View reviewed changes

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2015

add flag to kubelet to ignore the cidr passed down by the apiserver o…

fa60bbe

…n the master

mikedanese force-pushed the register-master branch from 15798cd to fa60bbe Compare October 9, 2015 04:21

mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2015

dchen1107 mentioned this pull request Oct 9, 2015

e2e flaky: Job should run a job to completion when tasks sometimes fail and are locally restarted #15389

Closed

mikedanese added a commit that referenced this pull request Oct 10, 2015

Merge pull request #14054 from mikedanese/register-master

392f33e

register master kubelet with the apiserver

mikedanese merged commit 392f33e into kubernetes:master Oct 10, 2015

mikedanese deleted the register-master branch October 10, 2015 00:12

yujuhong mentioned this pull request Oct 21, 2015

kubelet with "--register-node=false" still tries to find that node #15108

Closed

mikedanese mentioned this pull request Nov 15, 2015

DaemonSet should not/currently cannot schedule pods on SchedulingDisabled nodes #17297

Closed

mikedanese mentioned this pull request Jan 27, 2016

daemonset: apply predicate when placing daemon pods #20061

Merged

mikedanese mentioned this pull request Feb 29, 2016

DeamonSet test expect daemons on unschedulable Nodes #22172

Merged

eparis removed the cherrypick-candidate label Mar 8, 2016

mikedanese mentioned this pull request Feb 4, 2020

add myself to hack/OWNERS #87826

Merged

register master kubelet with the apiserver #14054

register master kubelet with the apiserver #14054

Conversation

mikedanese commented Sep 16, 2015

dchen1107 commented Sep 16, 2015

dchen1107 commented Sep 16, 2015

k8s.io/kubernetes/contrib/mesos/pkg/executor/service

mikedanese commented Sep 16, 2015

k8s-bot commented Sep 16, 2015

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

roberthbailey commented Sep 16, 2015

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

Choose a reason for hiding this comment

mikedanese commented Sep 16, 2015

k8s-github-robot commented Sep 16, 2015

dchen1107 commented Sep 16, 2015

dchen1107 commented Sep 17, 2015

mikedanese commented Sep 18, 2015

k8s-bot commented Sep 18, 2015

k8s-bot commented Sep 18, 2015

Choose a reason for hiding this comment

k8s-bot commented Oct 7, 2015

k8s-github-robot commented Oct 9, 2015

k8s-bot commented Oct 9, 2015

k8s-github-robot commented Oct 9, 2015

k8s-bot commented Oct 9, 2015

mikedanese commented Oct 9, 2015

k8s-bot commented Oct 9, 2015

mikedanese commented Oct 9, 2015

k8s-bot commented Oct 9, 2015

mikedanese commented Oct 9, 2015

mikedanese commented Oct 9, 2015

k8s-bot commented Oct 9, 2015

mikedanese commented Oct 9, 2015

k8s-bot commented Oct 9, 2015