bootstrap: Start hostNetwork pods even if network plugin not ready #33347

justinsb · 2016-09-23T03:06:08Z

bootstrap: Start hostNetwork pods even if network plugin not ready

This change is

k8s-github-robot · 2016-09-23T03:31:16Z

@k8s-ci-robot
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

justinsb · 2016-09-23T04:28:53Z

WIP so self-assigning for now

justinsb · 2016-09-23T06:15:56Z

This addresses (some of?) #32900 It is also fairly simple.

I realize this is a hard sell for 1.4.0. But if we want to put it into 1.4 at all, maybe we can incorporate it with a flag? Then installation tools could opt-in instead of setting pod-cidr. This is of course predicated on this not being obviously wrong!

cc @bprashanth @thockin

freehan · 2016-09-26T18:40:57Z

cluster/gce/trusty/configure-helper.sh

@@ -155,7 +155,7 @@ assemble_kubelet_flags() {
    if [ ! -z "${KUBELET_APISERVER:-}" ] && \
       [ ! -z "${KUBELET_CERT:-}" ] && \
       [ ! -z "${KUBELET_KEY:-}" ]; then
-      KUBELET_CMD_FLAGS="${KUBELET_CMD_FLAGS} --api-servers=https://${KUBELET_APISERVER} --register-schedulable=false --reconcile-cidr=false --pod-cidr=10.123.45.0/29"
+      KUBELET_CMD_FLAGS="${KUBELET_CMD_FLAGS} --api-servers=https://${KUBELET_APISERVER} --register-schedulable=false --reconcile-cidr=false"


If you drop pod-cidr, that means you have to set reconcile-cidr.

freehan · 2016-09-26T18:46:24Z

@roberthbailey Where does GKE take bootstrap parameter from?

Bascially, we want to maintain the current setup for GKE, where pod-cidr was set on master.
For other setups like AWS/GCE, master bootstraps without pod-cidr and uses node.PodCIDR once assigned.

justinsb · 2016-10-04T17:54:30Z

Can I get some feedback on this as fix? If so, I propose we're early enough in the cycle that we just merge it (once it is rebased and cleaned up and passes e2e), plenty of time to revert / fix any problems it uncovers.

freehan · 2016-10-04T18:44:28Z

What about the comment regarding reconcile-cidr?

This is a good one. Let us get it in.

justinsb · 2016-10-04T19:17:11Z

Thanks @freehan . Yes, I was probably over-eager when expunging pod-cidr - I will fix it up properly :-) Just wanted to check that we liked the general approach first!

caseydavenport · 2016-10-04T19:34:51Z

Haven't read the code yet, but @justinsb I thought this was already the case? It certainly is for CNI plugins. Is this just a kubenet fix?

justinsb · 2016-10-04T20:23:13Z

@caseydavenport are you sure? We hit this pretty hard in 1.4, with the /30 -> /29 issue... Maybe other plugins aren't correctly reporting status? Or maybe the plugin is just ready immediately, and doesn't need to wait for a podcidr? Check out the kubelet changes - they are pretty small...

caseydavenport · 2016-10-04T21:02:59Z

Yeah, my experience was with the Calico plugin, which is ready immediately and doesn't need to wait for a pod CIDR. I think I understand this PR now - it's essentially handling the case where --configure-cbr0 is set, which will fail until a pod CIDR is configured even for host networked pods. For cases like the Calico case where --configure-cbr0 isn't used, we weren't hitting this before.

Sorry for the confusion!

freehan · 2016-10-04T22:36:27Z

In the Calico case, it is using the cni network plugin right? I believe that one already separates hostnetwork and podnetwork.

dcbw · 2016-10-05T20:23:19Z

pkg/kubelet/kubelet.go

@@ -2197,6 +2197,15 @@ func (kl *Kubelet) rejectPod(pod *api.Pod, reason, message string) {
 // can be admitted, a brief single-word reason and a message explaining why
 // the pod cannot be admitted.
 func (kl *Kubelet) canAdmitPod(pods []*api.Pod, pod *api.Pod) (bool, string, string) {
+	if rs := kl.runtimeState.networkErrors(); len(rs) != 0 {
+		hostNetwork := false
+		if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.HostNetwork {


how about instead:

if (!podUsesHostNetwork(pod)) { return false, "NetworkNotReady", fmt.Sprintf(...) }

We added podUsesHostNetwork() specifically to cut down on this repeated pattern :)

Thanks - will do

dcbw · 2016-10-05T20:26:24Z

Yeah, my experience was with the Calico plugin, which is ready immediately and doesn't need to wait for a pod CIDR. I think I understand this PR now - it's essentially handling the case where --configure-cbr0 is set, which will fail until a pod CIDR is configured even for host networked pods. For cases like the Calico case where --configure-cbr0 isn't used, we weren't hitting this before.

@caseydavenport almost right. Currently, if the network plugin reports an error from Status(), kubelet will block running HostNetwork=true pods that shouldn't depend on the network plugin at all. This PR removes that erroneous inter-dependency and allows HostNetwork pods to run even when the network plugin isn't ready, since the two shouldn't have anything to do with each other.

So it's not really --configure-cbr0 specific, it's a problem for any network plugin that may report an error from Status(). But right now that's basically just kubenet, since CNI has no way to filter status back up to Kubernetes.

yujuhong · 2016-10-06T19:23:30Z

The downside is that there is no feedback loop to prevent schedulers from assigning non-host network pods to the node. There may be a period of time after kubelet startup that it reports ready, but rejects all the (non-host network) pods. /cc @kubernetes/sig-node

edevil · 2016-10-12T14:31:41Z

Is there any hope of seeing this in a near future in a point release?

mikedanese

This change looks ok to me.

mikedanese · 2016-10-12T14:58:09Z

pkg/kubelet/kubelet.go

@@ -2197,6 +2197,15 @@ func (kl *Kubelet) rejectPod(pod *api.Pod, reason, message string) {
 // can be admitted, a brief single-word reason and a message explaining why
 // the pod cannot be admitted.
 func (kl *Kubelet) canAdmitPod(pods []*api.Pod, pod *api.Pod) (bool, string, string) {
+	if rs := kl.runtimeState.networkErrors(); len(rs) != 0 {
+		hostNetwork := false
+		if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.HostNetwork {


dcbw · 2016-10-24T05:09:15Z

The pkg/kubelet bits LGTM

k8s-github-robot · 2016-10-24T07:35:15Z

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

k8s-github-robot · 2016-10-24T08:14:06Z

Automatic merge from submit-queue

edevil · 2016-10-24T09:12:34Z

Is this something that will end up on the next 1.4.x or will we have to wait for 1.5?

roberthbailey · 2016-10-24T16:51:25Z

@edevil - if you want it in the 1.4 branch feel free to send a cherry pick pull request.

k8s-cherrypick-bot · 2016-10-24T16:52:58Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

crassirostris · 2016-10-25T10:05:37Z

Guys, I'm sorry for not being vocal, but this PR broke suites gce-es-logging and gci-gce-es-logging and GCL-logging test

Logging doesn't work at all after this PR!

To give you more details:

Fluentd pod currently is in the node manifest. In kubelet logs you can see the following event

I1025 08:46:20.354155    1247 server.go:613] Event(api.ObjectReference{Kind:"Pod", Namespace:"kube-system", Name:"fluentd-elasticsearch-jenkins-e2e-minion-group-mndb", UID:"befba121e25c310938143db137604e03", APIVersion:"v1", ResourceVersion:"", FieldPath:""}): type: 'Warning' reason: 'NetworkNotReady' Network is not ready: [Kubenet does not have netConfig. This is most likely due to lack of PodCIDR]

And no mentions of fluentd after that.

justinsb · 2016-10-25T14:15:56Z

@crassirostris I'll take a look. Is there an open issue?

piosz · 2016-10-25T14:17:22Z

#35409

justinsb · 2016-10-25T14:20:02Z

Thanks - looking now.

yujuhong · 2016-10-25T15:01:36Z

FYI, this PR may have cause a lot of tests in serial suite to flake as well: #35519

foxish · 2016-10-25T15:28:16Z

@yujuhong @justinsb do we want to revert this PR?

yujuhong · 2016-10-25T15:34:06Z

I'd vote for reverting this first.

thockin · 2016-10-25T18:31:40Z

I would be open to a cherrypick, but it needs to be careful

On Mon, Oct 24, 2016 at 2:13 AM, André Cruz notifications@github.com
wrote:

Is this something that will end up on the next 1.4.x or will we have to
wait for 1.5?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33347 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVM8A235zZq-u6mNkr0iy29m2udhNks5q3HaigaJpZM4KEkYq
.

googlebot added the cla: yes label Sep 23, 2016

k8s-github-robot assigned yujuhong Sep 23, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Sep 23, 2016

justinsb assigned justinsb and unassigned yujuhong Sep 23, 2016

This was referenced Sep 23, 2016

Handle master node pod-cidr better #32900

Closed

[DO NOT MERGE] Kubenet allow podcidr change #33132

Closed

freehan reviewed Sep 26, 2016

View reviewed changes

justinsb changed the title ~~WIP: Start hostNetwork pods even if network plugin not ready~~ Start hostNetwork pods even if network plugin not ready Sep 27, 2016

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

justinsb removed their assignment Sep 29, 2016

k8s-github-robot assigned mikedanese Sep 29, 2016

dcbw reviewed Oct 5, 2016

View reviewed changes

mikedanese assigned freehan Oct 12, 2016

mikedanese reviewed Oct 12, 2016

View reviewed changes

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2016

k8s-github-robot merged commit 3c84164 into kubernetes:master Oct 24, 2016

crassirostris mentioned this pull request Oct 24, 2016

[k8s.io] Cluster level logging using Elasticsearch [Feature:Elasticsearch] should check that logs from containers are ingested into Elasticsearch {Kubernetes e2e suite} #35409

Closed

roberthbailey added the cherrypick-candidate label Oct 24, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Oct 24, 2016

roberthbailey added this to the v1.4 milestone Oct 24, 2016

roberthbailey added the cherrypick-candidate label Oct 24, 2016

edevil mentioned this pull request Oct 24, 2016

Automated cherry pick of #33347 #35451

Closed

jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 24, 2016

yujuhong mentioned this pull request Oct 25, 2016

(gci-)gce-serial suite flaking #35519

Closed

justinsb mentioned this pull request Oct 25, 2016

Kubelet may reject static pods when the node is out of disk space, and will not retry them #35521

Closed

yujuhong mentioned this pull request Oct 25, 2016

Revert "bootstrap: Start hostNetwork pods even if network plugin not ready" #35528

Merged

mikedanese removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 25, 2016

justinsb mentioned this pull request Oct 25, 2016

kubelet bootstrap: start hostNetwork pods before we have PodCIDR #35526

Merged

jessfraz removed the cherrypick-candidate label Oct 25, 2016

jessfraz removed this from the v1.4 milestone Oct 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bootstrap: Start hostNetwork pods even if network plugin not ready #33347

bootstrap: Start hostNetwork pods even if network plugin not ready #33347

justinsb commented Sep 23, 2016 •

edited by jessfraz

Loading

k8s-github-robot commented Sep 23, 2016

justinsb commented Sep 23, 2016

justinsb commented Sep 23, 2016

freehan Sep 26, 2016

freehan commented Sep 26, 2016 •

edited

Loading

justinsb commented Oct 4, 2016

freehan commented Oct 4, 2016

justinsb commented Oct 4, 2016

caseydavenport commented Oct 4, 2016

justinsb commented Oct 4, 2016

caseydavenport commented Oct 4, 2016

freehan commented Oct 4, 2016

dcbw Oct 5, 2016

mikedanese Oct 12, 2016

justinsb Oct 17, 2016

dcbw commented Oct 5, 2016 •

edited

Loading

yujuhong commented Oct 6, 2016

edevil commented Oct 12, 2016

mikedanese left a comment

mikedanese Oct 12, 2016

dcbw commented Oct 24, 2016

k8s-github-robot commented Oct 24, 2016

k8s-github-robot commented Oct 24, 2016

edevil commented Oct 24, 2016

roberthbailey commented Oct 24, 2016

k8s-cherrypick-bot commented Oct 24, 2016

crassirostris commented Oct 25, 2016 •

edited

Loading

justinsb commented Oct 25, 2016

piosz commented Oct 25, 2016

justinsb commented Oct 25, 2016

yujuhong commented Oct 25, 2016

foxish commented Oct 25, 2016

yujuhong commented Oct 25, 2016

thockin commented Oct 25, 2016

bootstrap: Start hostNetwork pods even if network plugin not ready #33347

bootstrap: Start hostNetwork pods even if network plugin not ready #33347

Conversation

justinsb commented Sep 23, 2016 • edited by jessfraz Loading

k8s-github-robot commented Sep 23, 2016

justinsb commented Sep 23, 2016

justinsb commented Sep 23, 2016

freehan Sep 26, 2016

Choose a reason for hiding this comment

freehan commented Sep 26, 2016 • edited Loading

justinsb commented Oct 4, 2016

freehan commented Oct 4, 2016

justinsb commented Oct 4, 2016

caseydavenport commented Oct 4, 2016

justinsb commented Oct 4, 2016

caseydavenport commented Oct 4, 2016

freehan commented Oct 4, 2016

dcbw Oct 5, 2016

Choose a reason for hiding this comment

mikedanese Oct 12, 2016

Choose a reason for hiding this comment

justinsb Oct 17, 2016

Choose a reason for hiding this comment

dcbw commented Oct 5, 2016 • edited Loading

yujuhong commented Oct 6, 2016

edevil commented Oct 12, 2016

mikedanese left a comment

Choose a reason for hiding this comment

mikedanese Oct 12, 2016

Choose a reason for hiding this comment

dcbw commented Oct 24, 2016

k8s-github-robot commented Oct 24, 2016

k8s-github-robot commented Oct 24, 2016

edevil commented Oct 24, 2016

roberthbailey commented Oct 24, 2016

k8s-cherrypick-bot commented Oct 24, 2016

crassirostris commented Oct 25, 2016 • edited Loading

justinsb commented Oct 25, 2016

piosz commented Oct 25, 2016

justinsb commented Oct 25, 2016

yujuhong commented Oct 25, 2016

foxish commented Oct 25, 2016

yujuhong commented Oct 25, 2016

thockin commented Oct 25, 2016

justinsb commented Sep 23, 2016 •

edited by jessfraz

Loading

freehan commented Sep 26, 2016 •

edited

Loading

dcbw commented Oct 5, 2016 •

edited

Loading

crassirostris commented Oct 25, 2016 •

edited

Loading