Add LoadBalancerClass field in service #98277

XudongLiuHarold · 2021-01-21T21:37:17Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR implements the KEP-1959 Add LoadBalancerClassName field in service.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Setting loadBalancerClass in load balancer type of service is available with this PR. 
Users who want to use a custom load balancer can specify loadBalancerClass to achieve it.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP-1959]:<https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/1959-service-lb-class-field>

k8s-ci-robot · 2021-01-21T21:37:20Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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. I understand the commands that are listed here.

k8s-ci-robot · 2021-01-21T21:37:26Z

Welcome @XudongLiuHarold!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-01-21T21:37:26Z

Hi @XudongLiuHarold. 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.

XudongLiuHarold · 2021-01-21T21:46:01Z

I signed it

andrewsykim · 2021-01-21T21:58:03Z

/assign

fejta-bot · 2021-01-21T22:14:29Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

andrewsykim

During alpha we need to update Service REST code to drop the field when the feature gate is not enabled. See dropServiceDisabledFields():

a0e6133

staging/src/k8s.io/api/core/v1/types.go

pkg/apis/core/types.go

staging/src/k8s.io/cloud-provider/controllers/service/controller.go

thockin

Code freeze is coming - ping me when this is ready

staging/src/k8s.io/api/core/v1/types.go

thockin · 2021-03-04T18:43:06Z

pkg/apis/core/validation/validation.go

@@ -122,6 +122,15 @@ func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList {
 	return allErrs
 }

+// ValidateQualifiedName validates if name is what Kubernetes calls a "qualified name".
+func ValidateQualifiedName(value string, fldPath *field.Path) field.ErrorList {


Thanks for this! We should file a "Good first Issue" issue to change most of the users of IsQualifiedName to this simpler form

Sure, will submit the issue once get this PR merged, thanks @thockin.

Submit issue here: #99828

pkg/apis/core/validation/validation.go

pkg/apis/core/validation/validation_test.go

pkg/registry/core/service/strategy_test.go

staging/src/k8s.io/cloud-provider/controllers/service/controller.go

desponda · 2021-03-04T20:22:19Z

👋 Hello! I am from the Bug Triage team! As @thockin mentioned, friendly reminder that code freeze is starting March 9th, 2021 (about 1 week from now). We want to ensure that each PR has a chance to be merged before code freeze!

thockin

1 testcase request., Approving and if you get it in good, else followup.

Thanks!

/lgtm
/approve

pkg/apis/core/validation/validation_test.go

thockin · 2021-03-05T00:38:01Z

/lgtm

KEP-1959: https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/1959-service-lb-class-field

k8s-ci-robot · 2021-03-05T01:13:11Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~api/OWNERS~~ [thockin]
~~pkg/apis/core/OWNERS~~ [thockin]
~~pkg/features/OWNERS~~ [thockin]
~~pkg/registry/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/OWNERS~~ [thockin]
~~staging/src/k8s.io/cloud-provider/controllers/service/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

XudongLiuHarold · 2021-03-05T02:10:26Z

/retest

XudongLiuHarold · 2021-03-05T02:50:53Z

Sorry, didn’t realize that need to re-gen code after this one #99012 got merged. Test case failures resolved.
Could I get /lgtm label again? @thockin Thanks! ❤️

thockin · 2021-03-05T05:35:01Z

/lgtm

XudongLiuHarold · 2021-03-05T17:20:53Z

Should that have been +featureGate=LoadBalancerClass?

Hi @pohly , according to @nikhita's suggestion here: #98277 (comment), dropped + prefix.

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 21, 2021

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 21, 2021

k8s-ci-robot requested review from andrewsykim and brendandburns January 21, 2021 21:38

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 21, 2021

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch from 60184da to f052dde Compare January 21, 2021 21:52

k8s-ci-robot added area/cloudprovider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/network Categorizes an issue or PR as relevant to SIG Network. labels Jan 21, 2021

k8s-ci-robot assigned andrewsykim Jan 21, 2021

andrewsykim reviewed Jan 22, 2021

View reviewed changes

staging/src/k8s.io/api/core/v1/types.go Outdated Show resolved Hide resolved

pkg/apis/core/types.go Outdated Show resolved Hide resolved

staging/src/k8s.io/cloud-provider/controllers/service/controller.go Outdated Show resolved Hide resolved

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch 2 times, most recently from d4eb02e to 96f6cea Compare January 22, 2021 03:38

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 22, 2021

thockin reviewed Mar 4, 2021

View reviewed changes

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch from 7733bb4 to 1a8d693 Compare March 4, 2021 22:43

thockin mentioned this pull request Mar 4, 2021

Service Type=LoadBalancer Class kubernetes/enhancements#1959

Closed

XudongLiuHarold requested a review from thockin March 4, 2021 22:48

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch from 1a8d693 to 3569cc0 Compare March 4, 2021 23:54

thockin reviewed Mar 4, 2021

View reviewed changes

pkg/apis/core/validation/validation_test.go Show resolved Hide resolved

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 4, 2021

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch from 3569cc0 to f181ad2 Compare March 5, 2021 00:09

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2021

XudongLiuHarold requested a review from thockin March 5, 2021 00:12

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2021

Add LoadBalancerClass field in service

72da0b1

KEP-1959: https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/1959-service-lb-class-field

XudongLiuHarold force-pushed the add-loadbalancerclass-field branch from f181ad2 to 72da0b1 Compare March 5, 2021 01:12

k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 5, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 5, 2021

k8s-ci-robot merged commit 66cbf01 into kubernetes:master Mar 5, 2021

XudongLiuHarold mentioned this pull request Mar 5, 2021

Change the users of IsQualifiedName to ValidateQualifiedName #99828

Closed

pohly mentioned this pull request Mar 5, 2021

CSIStorageCapacity beta API #99641

Merged

XudongLiuHarold mentioned this pull request Jun 23, 2021

Promote loadbalancerclass feature to beta #103129

Merged

XudongLiuHarold mentioned this pull request Jan 24, 2022

REQUEST: New membership for XudongLiuHarold kubernetes/org#3207

Closed

7 tasks

XudongLiuHarold mentioned this pull request Feb 7, 2022

Promote load balancer class to GA #107979

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LoadBalancerClass field in service #98277

Add LoadBalancerClass field in service #98277

XudongLiuHarold commented Jan 21, 2021 •

edited

k8s-ci-robot commented Jan 21, 2021

k8s-ci-robot commented Jan 21, 2021

k8s-ci-robot commented Jan 21, 2021

XudongLiuHarold commented Jan 21, 2021

andrewsykim commented Jan 21, 2021

fejta-bot commented Jan 21, 2021

andrewsykim left a comment

thockin left a comment

thockin Mar 4, 2021

XudongLiuHarold Mar 4, 2021

XudongLiuHarold Mar 5, 2021

desponda commented Mar 4, 2021

thockin left a comment

thockin commented Mar 5, 2021

k8s-ci-robot commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021 •

edited

thockin commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021 •

edited

Add LoadBalancerClass field in service #98277

Add LoadBalancerClass field in service #98277

Conversation

XudongLiuHarold commented Jan 21, 2021 • edited

k8s-ci-robot commented Jan 21, 2021

k8s-ci-robot commented Jan 21, 2021

k8s-ci-robot commented Jan 21, 2021

XudongLiuHarold commented Jan 21, 2021

andrewsykim commented Jan 21, 2021

fejta-bot commented Jan 21, 2021

andrewsykim left a comment

Choose a reason for hiding this comment

thockin left a comment

Choose a reason for hiding this comment

thockin Mar 4, 2021

Choose a reason for hiding this comment

XudongLiuHarold Mar 4, 2021

Choose a reason for hiding this comment

XudongLiuHarold Mar 5, 2021

Choose a reason for hiding this comment

desponda commented Mar 4, 2021

thockin left a comment

Choose a reason for hiding this comment

thockin commented Mar 5, 2021

k8s-ci-robot commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021 • edited

thockin commented Mar 5, 2021

XudongLiuHarold commented Mar 5, 2021 • edited

XudongLiuHarold commented Jan 21, 2021 •

edited

XudongLiuHarold commented Mar 5, 2021 •

edited

XudongLiuHarold commented Mar 5, 2021 •

edited