Skip to content
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.

Sign up for GitHub

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

Jump to bottom

AWS: Remove blackhole routes in our managed range #47572

Merged
merged 1 commit into from
Jun 21, 2017
Merged

AWS: Remove blackhole routes in our managed range #47572

merged 1 commit into from
Jun 21, 2017

Conversation

justinsb
Copy link
Member

Blackhole routes otherwise acccumulate unboundedly. We also are careful
to ensure that we do so only within the managed range, which requires
enlisting the help of the routecontroller.

Fix #47524

AWS: clean up blackhole routes when using kubenet

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 15, 2017
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 15, 2017
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 15, 2017
@justinsb
AWS: Remove blackhole routes in our managed range
11f8886 
Blackhole routes otherwise acccumulate unboundedly.  We also are careful
to ensure that we do so only within the managed range, which requires
enlisting the help of the routecontroller.

Fix kubernetes#47524
@justinsb
Copy link
Member Author

So I think this is relatively low risk (because it is gated behind the blackhole boolean so it should be aws only). The issue is problematic for bigger users - multiple users have independently come up with their own workarounds.

Assuming this merges, I will then immediately propose it for backport to the 1.6 branch, and the 1.7 branch if needed. Given that, @dchen1107 do you think we should put this into 1.7?

@justinsb justinsb mentioned this pull request Jun 15, 2017
Closed
@justinsb justinsb added this to the v1.7 milestone Jun 16, 2017
@justinsb
Copy link
Member Author

ping @stts / @vishh for review

For approve:
/assign @wojtek-t

@dchen1107
Copy link
Member

/approve

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2017
@dchen1107
Copy link
Member

@justinsb I approved this pr for 1.7, and left @wojtek-t for review. If @wojtek-t cannot review this, who should be the reviewer for this?

@chrislovecnm
Copy link
Contributor

@dchen1107 looking at it now

/assign chrislovecnm

chrislovecnm
chrislovecnm reviewed Jun 20, 2017
View reviewed changes
Copy link
Contributor

@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@chrislovecnm
Copy link
Contributor

/assign @chrislovecnm

@chrislovecnm
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrislovecnm, dchen1107, justinsb

Associated issue: 47524

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

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

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

@chrislovecnm
Copy link
Contributor

/test pull-kubernetes-kubemark-e2e-gce

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 5780cd0 into kubernetes:master Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrislovecnm chrislovecnm chrislovecnm left review comments

Assignees

@chrislovecnm chrislovecnm

@wojtek-t wojtek-t

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
8 participants
@justinsb @dchen1107 @chrislovecnm @k8s-github-robot @sttts @vishh @wojtek-t @k8s-ci-robot