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

Add a NodeCondition "NetworkUnavailable" to prevent scheduling onto a node until the routes have been created #26415

Merged
merged 2 commits into from
May 29, 2016
Merged

Add a NodeCondition "NetworkUnavailable" to prevent scheduling onto a node until the routes have been created #26415

merged 2 commits into from
May 29, 2016

Conversation

wojtek-t
Copy link
Member

@wojtek-t wojtek-t commented May 27, 2016 •
edited
Loading

This is new version of #26267 (based on top of that one).

The new workflow is:

  • we have an "NetworkNotReady" condition
  • Kubelet when it creates a node, it sets it to "true"
  • RouteController will set it to "false" when the route is created
  • Scheduler is scheduling only on nodes that doesn't have "NetworkNotReady ==true" condition

Release Note

In GCE/GKE route has to be created for a node, in order to make it schedulable.
As long as route is not created, a node will have "NetworkUnavailable" condition set, which will prevent scheduler from scheduling anything on that node.

@gmarek @bgrant0607 @zmerlynn @cjcullen @derekwaynecarr @danwinship @dcbw @lavalamp @vishh

@wojtek-t wojtek-t mentioned this pull request May 27, 2016
Closed
@k8s-github-robot 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. release-note-label-needed labels May 27, 2016
@wojtek-t wojtek-t force-pushed the network_not_ready branch from 2e49da0 to 18652e3 Compare May 27, 2016 11:38
@wojtek-t
Copy link
Member Author

There is currently a problem with this PR - kubelet is overwriting node status written by route controller.

@wojtek-t
Copy link
Member Author

Actually something different is happening here. Investigating.

@wojtek-t wojtek-t force-pushed the network_not_ready branch from 18652e3 to 8623c54 Compare May 27, 2016 13:24
@wojtek-t
Copy link
Member Author

OK - debugged and fixed. Hopefully tests will pass now.

@derekwaynecarr
Copy link
Member

This looks better to me.

@kubernetes/rh-networking - please take a pass.

@bprashanth bprashanth mentioned this pull request May 27, 2016
Closed
bgrant0607
bgrant0607 reviewed May 27, 2016
View reviewed changes
pkg/api/types.go
@@ -2009,6 +2009,8 @@ const (
NodeOutOfDisk NodeConditionType = "OutOfDisk"
// NodeMemoryPressure means the kubelet is under pressure due to insufficient available memory.
NodeMemoryPressure NodeConditionType = "MemoryPressure"
// NodeNetworkingNotReady means that network for the node is not correctly configured.
NodeNetworkingNotReady NodeConditionType = "NetworkingNotReady"
Copy link
Member

Choose a reason for hiding this comment

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

I prefer "NetworkUnavailable".

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

@wojtek-t wojtek-t force-pushed the network_not_ready branch from 8623c54 to be1b571 Compare May 27, 2016 17:33
@wojtek-t
Copy link
Member Author

@bgrant0607 - thanks a lot for quick review.

Comments applied. PTAL

BTW - do we need want a release note for this?

@bgrant0607 bgrant0607 added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note-label-needed labels May 27, 2016
@bgrant0607
Copy link
Member

LGTM

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 27, 2016
@bgrant0607 bgrant0607 added this to the v1.3 milestone May 27, 2016
@wojtek-t
Copy link
Member Author

Thanks @bgrant0607 !

@vishh
Copy link
Contributor

vishh commented May 27, 2016

LGTM

@davidopp davidopp changed the title When running on GCE prevent Nodes to become Ready before GCE routes are created. Add a NodeCondition "NetworkUnavaiable" to prevent scheduling onto a node until the routes have been created May 28, 2016
@k8s-bot
Copy link

k8s-bot commented May 29, 2016

GCE e2e build/test passed for commit be1b571.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 577cdf9 into kubernetes:master May 29, 2016
@gmarek gmarek mentioned this pull request May 30, 2016
Closed
This was referenced Jun 2, 2016
Closed
Closed
@wojtek-t wojtek-t deleted the network_not_ready branch June 15, 2016 14:33
@erictune erictune mentioned this pull request Jul 2, 2016
Closed
@david-mcmahon david-mcmahon changed the title Add a NodeCondition "NetworkUnavaiable" to prevent scheduling onto a node until the routes have been created Add a NodeCondition "NetworkUnavailable" to prevent scheduling onto a node until the routes have been created Jul 2, 2016
@erictune
Copy link
Member

erictune commented Jul 2, 2016

@wojtek-t Does this PR require action by the user when upgrading from 1.2.x to 1.3.0? (Think about non-developer users.) If so, please edit your first comment to have a release-note block, like in #28132. If it is just an optional feature, please change the label to just release-note. If it is not a complete feature by itself, then apply "release-note-none" label instead.

@wojtek-t wojtek-t added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Jul 4, 2016
@davidopp davidopp mentioned this pull request May 22, 2017
Closed
@marwanad marwanad mentioned this pull request Apr 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bgrant0607 bgrant0607

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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.3
Development

Successfully merging this pull request may close these issues.
8 participants
@wojtek-t @derekwaynecarr @bgrant0607 @vishh @k8s-bot @k8s-github-robot @erictune @googlebot