Skip to content

/kubernetes

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate cleanup-ipvs flag #83832

Merged
merged 1 commit into from Nov 2, 2019
+1 −0

Conversation

@gongguan
Copy link
Contributor

gongguan commented Oct 12, 2019 •
edited

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
deprecate cleanup-ipvs flag, and always flushing IPVS on --cleanup
see more at issue#78797
Which issue(s) this PR fixes:

Fixes #78797

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

deprecate cleanup-ipvs flag

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Oct 12, 2019

Hi @gongguan. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.
@k8s-ci-robot k8s-ci-robot requested review from caseydavenport and freehan Oct 12, 2019
@k8s-ci-robot k8s-ci-robot requested a review from andrewsykim Oct 12, 2019
@gongguan

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

gongguan commented Oct 12, 2019

/assign @thockin
/cc @andrewsykim @vllry
@k8s-ci-robot k8s-ci-robot requested review from vllry and removed request for freehan and caseydavenport Oct 12, 2019
@gongguan

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

gongguan commented Oct 12, 2019

/assign @bowei
@gongguan gongguan force-pushed the gongguan:deprecate_cleanup-ipvs_flag branch from e824bd4 to 23da7a0 Oct 12, 2019
@gongguan

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

gongguan commented Oct 12, 2019

/kind cleanup
@neolit123
neolit123 reviewed Oct 12, 2019
View changes
cmd/kube-proxy/app/server.go Show resolved Hide resolved
@gongguan gongguan closed this Oct 13, 2019
@gongguan gongguan force-pushed the gongguan:deprecate_cleanup-ipvs_flag branch from 23da7a0 to 8577711 Oct 13, 2019
@k8s-ci-robot k8s-ci-robot added size/XS approved and removed size/M labels Oct 13, 2019
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Oct 13, 2019

@gongguan: Failed to re-open PR: state cannot be changed. There are no new commits on the gongguan:deprecate_cleanup-ipvs_flag branch.

In response to this:

/reopen

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.
@k8s-ci-robot k8s-ci-robot reopened this Oct 13, 2019
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Oct 13, 2019

@gongguan: Reopened this PR.

In response to this:

/reopen

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.
@k8s-ci-robot k8s-ci-robot removed the approved label Oct 13, 2019
@vllry
vllry reviewed Oct 13, 2019
View changes
cmd/kube-proxy/app/server.go Outdated Show resolved Hide resolved
@gongguan gongguan force-pushed the gongguan:deprecate_cleanup-ipvs_flag branch from e53cca6 to ebe4496 Oct 14, 2019
@thockin
thockin reviewed Nov 1, 2019
View changes
Copy link
Member

thockin left a comment

Thanks!

/lgtm
/approve
@k8s-ci-robot k8s-ci-robot added the lgtm label Nov 1, 2019
@k8s-ci-robot

This comment has been minimized.

Sign in to view
Copy link
Contributor

k8s-ci-robot commented Nov 1, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gongguan, thockin

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@andrewsykim
andrewsykim reviewed Nov 1, 2019
View changes
cmd/kube-proxy/app/server.go
@@ -153,6 +153,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) {

fs.BoolVar(&o.CleanupAndExit, "cleanup", o.CleanupAndExit, "If true cleanup iptables and ipvs rules and exit.")
fs.BoolVar(&o.CleanupIPVS, "cleanup-ipvs", o.CleanupIPVS, "If true and --cleanup is specified, kube-proxy will also flush IPVS rules, in addition to normal cleanup.")
fs.MarkDeprecated("cleanup-ipvs", "In a future release, running --cleanup will always flush IPVS rules")

This comment has been minimized.

Sign in to view
Copy link
@andrewsykim

andrewsykim Nov 1, 2019

Member

@gongguan can you open an issue to track this work please?

This comment has been minimized.

Sign in to view
Copy link
@gongguan

gongguan Nov 2, 2019

Author Contributor

Can #78797 be used as the tracking issue or a new one raised by me?

This comment has been minimized.

Sign in to view
Copy link
@gongguan

gongguan Nov 2, 2019

Author Contributor

Should I add a comment with the tracking issue above the sentence?

This comment has been minimized.

Sign in to view
Copy link
@andrewsykim

andrewsykim Nov 2, 2019

Member

Ah, yes it can, just make sure the issue name is updated and it's not closed when this PR is merged

This comment has been minimized.

Sign in to view
Copy link
@gongguan

gongguan Nov 2, 2019

Author Contributor

rebased current code to pass the failed check. Could you please label an ok-to-test?
@gongguan
deprecate cleanup-ipvs flag
Loading status checks…
f147b6e
@gongguan gongguan force-pushed the gongguan:deprecate_cleanup-ipvs_flag branch from ebe4496 to f147b6e Nov 2, 2019
@k8s-ci-robot k8s-ci-robot removed the lgtm label Nov 2, 2019
@andrewsykim

This comment has been minimized.

Sign in to view
Copy link
Member

andrewsykim commented Nov 2, 2019

/lgtm
/ok-to-test
@k8s-ci-robot k8s-ci-robot merged commit dc88809 into kubernetes:master Nov 2, 2019
15 checks passed
15 checks passed
cla/linuxfoundation gongguan authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Nov 2, 2019
@gongguan gongguan mentioned this pull request Nov 2, 2019
Closed
@gongguan gongguan deleted the gongguan:deprecate_cleanup-ipvs_flag branch Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123

@thockin thockin

@vllry vllry

@andrewsykim andrewsykim

Assignees

@bowei bowei

@thockin thockin

@andrewsykim andrewsykim

Labels
approved area/ipvs cncf-cla: yes kind/cleanup lgtm needs-priority ok-to-test release-note sig/network size/XS
Projects
None yet
Milestone
  v1.17
7 participants
@gongguan @k8s-ci-robot @andrewsykim @neolit123 @thockin @vllry @bowei
You can’t perform that action at this time.