WIP: Remove the legacy networking mode #34906

luxas · 2016-10-16T18:44:52Z

What this PR does / why we need it:
Removes the deprecated configure-cbr0 flag and networking mode to avoid having untested and maybe unstable code in kubelet, see: #33789

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

Special notes for your reviewer: There are a lot of deployments who rely on this networking mode. Not sure how we deal with that: force switch to kubenet or just delete the old deployment?

But please review the code changes first (the first commit)

Release note:

Removed the deprecated kubelet --configure-cbr0 flag, and with that the "classic" networking mode as well

PTAL @kubernetes/sig-network @kubernetes/sig-node @mikedanese

This change is

luxas · 2016-10-16T19:00:52Z

Will run hack/update-all.sh later...

k8s-ci-robot · 2016-10-16T19:13:08Z

Jenkins verification failed for commit e3207bf. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

thockin · 2016-10-17T05:30:22Z

I love it. This may be the first time we actually remove anything from componentconfig - @mikedanese can you think of any problems this will cause?

I am editing the subject for better relnote

@luxas a bunch of verify scripts failed - run update-all, please

k8s-ci-robot · 2016-10-17T13:31:40Z

Jenkins GCI GCE e2e failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-17T13:31:41Z

Jenkins GCE Node e2e failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-17T13:32:11Z

Jenkins GCE e2e failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-17T13:32:12Z

Jenkins GKE smoke e2e failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-17T13:32:41Z

Jenkins Kubemark GCE e2e failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-17T13:33:11Z

Jenkins unit/integration failed for commit 72c36c8. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

danwinship · 2016-10-17T14:53:30Z

cmd/kubelet/app/options/options.go

@@ -200,10 +198,10 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
 	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.Int64Var(&s.MaxOpenFiles, "max-open-files", s.MaxOpenFiles, "Number of files that can be opened by Kubelet process. [default=1000000]")
-	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]")
+	fs.BoolVar(&s.ReconcileCIDR, "reconcile-cidr", s.ReconcileCIDR, "Reconcile node CIDR with the CIDR specified by the API server. Won't have any effect if register-node is false. [default=true]")


Since this was previously a no-op if configure-cbr0 was false, and configure-cbr0 is now always false, this is now always a no-op. (Actually... it looks like it may have been a no-op already; reconcileCBR0 in kubelet_network.go always got called if configure-cbr0 was true, regardless of the value of reconcile-cidr.)

Actually my initial draft of this was removing reconcile-cidr as well, but I think we should take it separately.

If we agree to remove that as well, I'm fine, but please in another PR to make it easier for reviewing.

Another concern is that we didn't mark reconcile-cidr as deprecated...

well, if you don't remove it, the doc string that's there now is wrong; it implies that reconcile-cidr still does something if register-node is true, but it doesn't.

ping @luxas on this

luxas · 2016-10-17T16:49:24Z

Will run hack/update-all.sh later...

@thockin :)

Ok, so here's what we should sort out (in order):

Is the Go code change here?
- I'll prefer to deal with reconcile-cidr separately
There are a lot of deployments who rely on this networking mode.
- Not sure how we should deal with that: force switch to kubenet or just delete the old deployment?
Is the provisioning cluster/ changes ok?

If this is ok, I'm gonna rebase (will probably be needed at that time), fix up the commits and remove WIP.

However, I'm traveling to London the rest of this week and will be totally offline, so some time next week I can come back and update this. But please comment on it anyway :)

yujuhong · 2016-10-17T17:16:30Z

/cc @freehan

thockin · 2016-10-18T03:43:03Z

@freehan is out on leave, assign to me to @matchstick

On Mon, Oct 17, 2016 at 10:17 AM, Yu-Ju Hong notifications@github.com
wrote:

/cc @freehan https://github.com/freehan

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

thockin · 2016-10-19T07:02:15Z

I am LGTM on this. Consider also removing the second flag

k8s-github-robot · 2016-10-19T07:21:23Z

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

k8s-github-robot · 2016-10-19T08:03:39Z

Automatic merge from submit-queue

@thockin

Automatic merge from submit-queue Deprecate the --reconcile-cidr flag  **What this PR does / why we need it**: Follows up #34906 **Special notes for your reviewer**: I'm not sure why coreos had set `--reconcile-cidr` to `false` and what the implications are now. **Release note**:  ```release-note Deprecate the --reconcile-cidr kubelet flag because it has no function anymore ``` PTAL @thockin @freehan @justinsb @yujuhong @kubernetes/sig-node @kubernetes/sig-network **I will add `hack/update-all.sh` contents soon to fix builds**

The cbr0 configuration behavior this comment references was removed in kubernetes#34906

luxas added 2 commits October 16, 2016 21:26

Remove the legacy networking mode --configure-cbr0

0800df7

WIP: Remove --configure-cbr0 from kube-up

e3207bf

googlebot added the cla: yes label Oct 16, 2016

luxas added this to the v1.5 milestone Oct 16, 2016

luxas added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label Oct 16, 2016

luxas changed the title ~~Remove the legacy networking mode~~ WIP: Remove the legacy networking mode Oct 16, 2016

k8s-github-robot assigned vishh Oct 16, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2016

This was referenced Oct 16, 2016

Remove usage of brctl as a kubelet dependency #30056

Closed

Remove the ethtool kubelet dependency; use ip link instead of ethtool to determine pair interface. #30356

Closed

Run update-all

72c36c8

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 17, 2016

Remove unused net

565c5e6

danwinship reviewed Oct 17, 2016

View reviewed changes

luxas mentioned this pull request Oct 17, 2016

Deprecate the old cbr0 logic in kubelet? #31937

Closed

luxas unassigned vishh Oct 18, 2016

luxas assigned thockin and matchstick Oct 18, 2016

chrislovecnm mentioned this pull request Oct 19, 2016

Remove configure cbr0 options kubernetes/kops#700

Closed

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

k8s-github-robot merged commit 61e0113 into kubernetes:master Oct 19, 2016

luxas mentioned this pull request Oct 25, 2016

Deprecate the --reconcile-cidr flag #35523

Merged

ChenLingPeng mentioned this pull request Feb 15, 2017

kubelet pod bandwidth shaping not work any more? #41471

Closed

dcbw mentioned this pull request Apr 10, 2017

"classic" networking fails to start in k8s 1.4 #33789

Closed

islinwb mentioned this pull request Feb 3, 2018

--configure-cbr0=true not recognised on 1.9.2 #59257

Closed

islinwb mentioned this pull request Feb 26, 2018

Remove the deprecated '--configure-cbr0=' kubernetes/website#7518

Merged

mtaufen added a commit to mtaufen/kubernetes that referenced this pull request Jun 22, 2018

remove incorrect comment referencing removed functionality

6e9e306

The cbr0 configuration behavior this comment references was removed in kubernetes#34906

mtaufen added a commit to mtaufen/kubernetes that referenced this pull request Jul 16, 2018

remove incorrect comment referencing removed functionality

d245e72

The cbr0 configuration behavior this comment references was removed in kubernetes#34906

tanshanshan pushed a commit to tanshanshan/kubernetes that referenced this pull request Aug 10, 2018

remove incorrect comment referencing removed functionality

652c610

The cbr0 configuration behavior this comment references was removed in kubernetes#34906

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Remove the legacy networking mode #34906

WIP: Remove the legacy networking mode #34906

luxas commented Oct 16, 2016 •

edited by thockin

luxas commented Oct 16, 2016

k8s-ci-robot commented Oct 16, 2016

thockin commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

danwinship Oct 17, 2016

luxas Oct 17, 2016

danwinship Oct 17, 2016

thockin Oct 19, 2016

luxas commented Oct 17, 2016

yujuhong commented Oct 17, 2016

thockin commented Oct 18, 2016

thockin commented Oct 19, 2016

k8s-github-robot commented Oct 19, 2016

k8s-github-robot commented Oct 19, 2016

WIP: Remove the legacy networking mode #34906

WIP: Remove the legacy networking mode #34906

Conversation

luxas commented Oct 16, 2016 • edited by thockin

luxas commented Oct 16, 2016

k8s-ci-robot commented Oct 16, 2016

thockin commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

k8s-ci-robot commented Oct 17, 2016

danwinship Oct 17, 2016

Choose a reason for hiding this comment

luxas Oct 17, 2016

Choose a reason for hiding this comment

danwinship Oct 17, 2016

Choose a reason for hiding this comment

thockin Oct 19, 2016

Choose a reason for hiding this comment

luxas commented Oct 17, 2016

yujuhong commented Oct 17, 2016

thockin commented Oct 18, 2016

thockin commented Oct 19, 2016

k8s-github-robot commented Oct 19, 2016

k8s-github-robot commented Oct 19, 2016

luxas commented Oct 16, 2016 •

edited by thockin