Hi @zhouhaibing089. 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 @k8s-bot 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.

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://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

Once you've signed, please reply here (e.g. "I signed it!") 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.

This change is 

I signed it!

ping @vishh please take a look.

I'm torn with this PR. Although it is technically correct, from a user standpoint, when they create a external LB they do not know that a Node Port is also being consumed which might confuse users.

@derekwaynecarr WDYT?

well, I have to admit that your concern is totally reasonable. anyway, I would like to clarify the background a little bit more: in some environments(openstack LBaaS as an example), a creation of vip with multiple ports will create multiple vips actually under the hood(the same ip though), so if we really want to limit the ports number, we can either:

1. count the nodeport usage, as it actually limits how many ports can be created.(which is proposed in this pr)
2. add another resource name like services.loadbalancer.ports, and add this into resource quota.

@vishh @derekwaynecarr what do you think?

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

• pkg/quota/evaluator/OWNERS

We suggest the following people:
cc @vishh
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

/lgtm

LGTM from the bulk LGTM tool

@thockin thinks this is fine. So LGTM

/lgtm

Automatic merge from submit-queue

@zhouhaibing089 sorry, this fell of my radar.

this is a behavior change that warrants a release-note.

i am not sure i am fully groking the rationale. is this behavior Iaas specific?

i would have liked a unit test can you do that as well?

@thockin -- am i mistaken that this pr would have made the value be 2 and not 1 node port per load balancer service? see: #29409

I think there needs to be a check that a NodePort is actually present in a given port spec rather than blindly counting the number of ports with the assumption that services of type LoadBalancer will always allocate a NodePort.

NodePort allocation is a function of the services controller when a cloud provider is configured, but that is deployment-specific. OpenShift can be configured to run a controller that allocates externalIPs to LoadBalancer services when running without a cloud provider, and no NodePort would be allocated:

https://docs.openshift.com/container-platform/latest/admin_guide/tcp_ingress_external_ports.html

Disregard, nodeports are allocated on creation so services of type LoadBalancer will always consume nodeports whether they will be used or not.

cc: @smarterclayton @knobunc

added unit testing #42998