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.

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

Auto-assign egress IPs from EgressCIDRs #20258

Merged

Conversation

danwinship
Copy link
Contributor

Building on #20155 (the first commit), which moves the EgressIP-tracking code from node/ to common/, and openshift/api#64 (the second commit), which adds HostSubnets.EgressCIDRs, this adds support to the common egressip code for tracking changes to EgressCIDRs and figuring out acceptable allocations of EgressIPs based on that; and adds support to master for actually updating HostSubnet objects to reflect these new figured-out allocations.

(This PR does not include HA.)

@danwinship danwinship added component/networking sig/networking do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/feature Categorizes issue or PR as related to a new feature. labels Jul 9, 2018
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 9, 2018
@openshift-bot
Copy link
Contributor

/test cross

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2018
@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 10, 2018
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2018
@danwinship danwinship removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2018
Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@knobunc
Copy link
Contributor

knobunc commented Jul 11, 2018

@deads2k we will need approval for the api glide change, once the api change goes in.

@danwinship danwinship force-pushed the auto-egress-ip-master branch 2 times, most recently from 131c316 to 76cc0aa Compare July 11, 2018 16:45
@danwinship
Copy link
Contributor Author

/test integration

@danwinship
Copy link
Contributor Author

/test extended_conformance_install

@danwinship
Copy link
Contributor Author

/test integration

@pravisankar
Copy link

/retest

@danwinship
Copy link
Contributor Author

/retest

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2018
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2018
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2018
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 17, 2018
@danwinship danwinship force-pushed the auto-egress-ip-master branch 2 times, most recently from 32fb13c to 6e2a7bc Compare July 17, 2018 18:08
@danwinship
Copy link
Contributor Author

/retest

@deads2k
Copy link
Contributor

deads2k commented Jul 19, 2018

bump looks clean

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, dcbw, deads2k, pravisankar, squeed

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 19, 2018
@danwinship
Copy link
Contributor Author

/retest

@liggitt
Copy link
Contributor

liggitt commented Jul 19, 2018

thanks for getting this updated. ready for tagging for merge?

@danwinship
Copy link
Contributor Author

@liggitt yes

@liggitt
Copy link
Contributor

liggitt commented Jul 19, 2018

readding lgtm from #20258 (comment)

@liggitt liggitt added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2018
@liggitt
Copy link
Contributor

liggitt commented Jul 19, 2018

/retest

1 similar comment
@danwinship
Copy link
Contributor Author

/retest

@danwinship
Copy link
Contributor Author

/test gcp

1 similar comment
@knobunc
Copy link
Contributor

knobunc commented Jul 19, 2018

/test gcp

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit dfe0224 into openshift:master Jul 20, 2018
@danwinship danwinship deleted the auto-egress-ip-master branch July 20, 2018 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/networking kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. sig/networking size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants