Add a label which prevents a node from being added to a cloud load balancer #53146

brendandburns · 2017-09-27T17:15:51Z

There are a variety of reasons that you may not want a node in a cluster to participate in a cloud load balancer. For example workload isolation for security, or managing network throughput, or because the node is not in the appropriate virtual network (cluster's that span environments)

This PR adds a label so that you can select which nodes you want to participate.

…lancer.

FengyunPan · 2017-09-28T02:49:36Z

/lgtm
The label can be used to node isolation.

brendandburns · 2017-09-28T03:34:47Z

/approve no-issue

k8s-github-robot · 2017-09-28T03:34:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, FengyunPan

Associated issue requirement bypassed by: brendandburns

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

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

~~cmd/kubeadm/OWNERS~~ [brendandburns]
~~pkg/controller/OWNERS~~ [brendandburns]

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

k8s-github-robot · 2017-09-28T03:35:49Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-09-28T04:28:52Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

After merging this PR(kubernetes#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic.

k8s-cherrypick-bot · 2017-09-29T18:21:08Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

rbitia · 2017-09-29T18:36:37Z

Solves this issue: https://github.com/Azure/aci-connector-k8s/issues/51

mikedanese · 2017-10-02T18:06:01Z

pkg/controller/service/service_controller.go

@@ -598,6 +598,10 @@ func getNodeConditionPredicate() corelisters.NodeConditionPredicate {
 			return false
 		}

+		if _, hasExcludeBalancerLabel := node.Labels[constants.LabelNodeRoleExcludeBalancer]; hasExcludeBalancerLabel {


I just opened #53341. We shouldn't depend on cmd/kubeadm from core controllers.

I can fix this if food have a better location for these annotations

Erm s/food/folks/

We can fix this later I think.

mikedanese · 2017-10-02T18:06:40Z

cmd/kubeadm/app/constants/constants.go

@@ -134,6 +134,10 @@ const (
 	// It's copied over to kubeadm until it's merged in core: https://github.com/kubernetes/kubernetes/pull/39112
 	LabelNodeRoleMaster = "node-role.kubernetes.io/master"

+	// LabelNodeRoleExcludeBalancer specifies that the node should be
+	// exclude from load balancers created by a cloud provider.
+	LabelNodeRoleExcludeBalancer = "node.role.kubernetes.io/exclude-balancer"


@kubernetes/sig-network-api-reviews

This doesn't follow the convetions of other labels in this file, nor does it have any versioning. Are we committing to this API?

Sorry I screwed up the cut/paste I guess. I'm happy to add the dash instead of the dot...

After merging this PR(kubernetes#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic.

Automatic merge from submit-queue (batch tested with PRs 53567, 53197, 52944, 49593). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. [OpenStack]Add codes to check the count of nodes(members) After merging this PR(#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic. **Release note**: ```release-note NONE ```

brendandburns · 2017-10-09T23:20:48Z

Fixed up @mikedanese 's concern in #53621

Apologies.

Mike, if you want me to extract from kubeadm let me know the right location and I'll happily do that.

@mikedanese

Automatic merge from submit-queue (batch tested with PRs 53621, 52320, 53625). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Fix a bad cut/paste of a node label. Fixes concerns [raised](#53146 (review)) in #53146 @mikedanese @jdumars

thockin · 2017-10-10T23:36:22Z

Hey, all.

This should not have been merged without an issue, and probably should not have merged without someone from sig-network OWNERS looking at it.

In truth, the whole notion of using nodes as LB targets should be folded down into per-cloud-provider logic. Service controller should not be doing this predicate check or passing nodes around.

Secondly, the node-role concept is highly contentious, so we should not overload it.

Thirdly, "don't do X" is not a role.

Given that the nodes ARE being passed by service controller, fine. I don't think it should be about "role" though. Probably it should be named "node" rather than "node-role", or even something new like "service-controller.kubernetes.io/exclude-lb-node" ?

@thockin

Automatic merge from submit-queue (batch tested with PRs 54644, 53072). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Flag gate node exclusion for service load balancers. @thockin @jdumars ```release-note Add a new feature gate for enabling an alpha annotation which, if present, excludes the annotated node from being added to a service load balancers. ``` Issue: #54743 Notes: The original PR for this feature was: #53146 Which didn't include a gate (or the alpha label). This was refined to add the `alpha` label in: #53678 Then in the cherry-pick review: #53656 (comment) @thockin requested a gate for an alpha feature, which is this PR.

jpbetz · 2017-11-02T04:41:17Z

Removing cherry-pick labels since this has been manually cherrypicked in #54955

…#53678-#54644-upstream-release-1.8 Automatic merge from submit-queue. Automated cherry pick of #53146 #53678 #54644 Cherry pick of #53146 #53678 #54644 on release-1.8. #53146: Add a label which prevents a node from being added to a cloud #53678: Append an alpha label to the exclude load balancer #54644: wqFlag gate node exclusion for service load balancers. ```release-note Add a label which prevents a node from being added to a cloud.Append an alpha label to the exclude load balancer. `wqFlag` gate node exclusion for service load balancers. ```

redbaron · 2017-11-09T13:33:38Z

Am I right that node using this label exclude itself from all LBs? So there is no way to do this per service?

After merging this PR(kubernetes#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic.

Automatic merge from submit-queue (batch tested with PRs 53567, 53197, 52944, 49593). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. [OpenStack]Add codes to check the count of nodes(members) After merging this PR(kubernetes#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic. **Release note**: ```release-note NONE ```

Add a label which prevents a node from being added to a cloud load ba…

422f5e3

…lancer.

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 27, 2017

brendandburns added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 27, 2017

k8s-github-robot assigned errordeveloper and mikedanese Sep 27, 2017

k8s-ci-robot assigned FengyunPan Sep 28, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 28, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 28, 2017

k8s-github-robot merged commit 6915fd5 into kubernetes:master Sep 28, 2017

FengyunPan mentioned this pull request Sep 28, 2017

ServiceController doesn't sync the status of LoadBalancer service into apiserver #53193

Closed

FengyunPan mentioned this pull request Sep 28, 2017

[OpenStack]Add codes to check the count of nodes(members) #53197

Merged

MrHohn mentioned this pull request Sep 28, 2017

[CLEANUP] Service controller should not import node label constants from kubeadm #53217

Closed

rbitia mentioned this pull request Sep 29, 2017

add connector support into the azure lb in kubernetes upstream rbitia/aci-demos#30

Closed

jdumars added the cherrypick-candidate label Sep 29, 2017

k8s-cherrypick-bot removed the cherrypick-candidate label Sep 29, 2017

jdumars added this to the v1.9 milestone Sep 29, 2017

jdumars added the cherrypick-candidate label Sep 29, 2017

brendandburns modified the milestones: v1.9, v1.8 Sep 29, 2017

rbitia mentioned this pull request Sep 29, 2017

Add a label which prevents a node from being added to a cloud load ba… #53277

Closed

mikedanese reviewed Oct 2, 2017

View reviewed changes

mikedanese assigned bowei Oct 2, 2017

brendandburns mentioned this pull request Oct 9, 2017

Fix a bad cut/paste of a node label. #53621

Merged

jdumars mentioned this pull request Oct 10, 2017

Automated cherry pick of #53146 #53621 #53656

Closed

thockin self-assigned this Oct 10, 2017

brendandburns mentioned this pull request Oct 28, 2017

Flag gate node exclusion for service load balancers. #54644

Merged

jdumars mentioned this pull request Nov 1, 2017

Automated cherry pick of #53146 #53678 #54644 #54955

Merged

jpbetz removed the cherrypick-candidate label Nov 2, 2017

jpbetz removed this from the v1.8 milestone Nov 2, 2017

dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018

[OpenStack]Add codes to check the count of nodes(members)

0c46bff

After merging this PR(kubernetes#53146), if there is no available nodes for the loadbalancer service, UpdateLoadBalancer() will run panic.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a label which prevents a node from being added to a cloud load balancer #53146

Add a label which prevents a node from being added to a cloud load balancer #53146

brendandburns commented Sep 27, 2017

FengyunPan commented Sep 28, 2017

brendandburns commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-cherrypick-bot commented Sep 29, 2017

rbitia commented Sep 29, 2017

mikedanese Oct 2, 2017

brendandburns Oct 3, 2017

brendandburns Oct 3, 2017

mikedanese Oct 3, 2017

mikedanese Oct 2, 2017

brendandburns Oct 3, 2017

brendandburns commented Oct 9, 2017

thockin commented Oct 10, 2017

jpbetz commented Nov 2, 2017

redbaron commented Nov 9, 2017

Add a label which prevents a node from being added to a cloud load balancer #53146

Add a label which prevents a node from being added to a cloud load balancer #53146

Conversation

brendandburns commented Sep 27, 2017

FengyunPan commented Sep 28, 2017

brendandburns commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-github-robot commented Sep 28, 2017

k8s-cherrypick-bot commented Sep 29, 2017

rbitia commented Sep 29, 2017

mikedanese Oct 2, 2017

Choose a reason for hiding this comment

brendandburns Oct 3, 2017

Choose a reason for hiding this comment

brendandburns Oct 3, 2017

Choose a reason for hiding this comment

mikedanese Oct 3, 2017

Choose a reason for hiding this comment

mikedanese Oct 2, 2017

Choose a reason for hiding this comment

brendandburns Oct 3, 2017

Choose a reason for hiding this comment

brendandburns commented Oct 9, 2017

thockin commented Oct 10, 2017

jpbetz commented Nov 2, 2017

redbaron commented Nov 9, 2017