Check both name and ports for azure health probes

[feiskyer]

What this PR does / why we need it:

Check both name and ports for azure health probes, so that probe ports could follow nodePorts changes.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #56898

Special notes for your reviewer:

Should be cherry-picked in 1.7, 1.8, 1.9.

Release note:

BUG FIX: Check both name and ports for azure health probes

[feiskyer]

/sig azure

[feiskyer]

/area cloudprovider

[andyzhangx]

/lgtm

[feiskyer]

/assign @brendandburns

[andyzhangx]

/lgtm

[feiskyer]

/retest

[dixudx]

@feiskyer could you squash the commits? No need to have multiple commits for a single checking.

[feiskyer]

@dixudx Thanks for reviewing. Squashed commits. PTAL.

[dixudx]

/lgtm

[feiskyer]

@brendandburns Could you also help to approve milestone?

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

[colemickens]

I don't think this PR does anything in reality. The probe name already encodes the Port... so this check doesn't add anything.

See the way the load balancer probe names are determined:

kubernetes/pkg/cloudprovider/providers/azure/azure_util.go

Line 335 in 630dbed

func getLoadBalancerRuleName(service *v1.Service, port v1.ServicePort, subnetName *string) string {

(the lbRuleName is used as the expected Probe name as well, etc)

(Can you revert the impl and run the tests again and verify they fail? I think they're going to pass.)

[colemickens]

Also, your text in your PR mentions "nodePorts" but I don't see you checking that field.

[colemickens]

/lgtm cancel

Canceled because the tests pass without the implementation changes.

[k8s-github-robot]

[MILESTONENOTIFIER] Milestone Pull Request Current

@andyzhangx @brendandburns @colemickens @dixudx @feiskyer @rootfs

Note: This pull request is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required

Pull Request Labels

• sig/azure: Pull Request will be escalated to these SIGs if needed.
• priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
• kind/bug: Fixes a bug discovered during the current release.

Help

• Additional instructions
• Commands for setting labels

[colemickens]

Last comment, I'm not even sure it's worth checking the NodePort. The NodePort is assigned as Service creation time, as is the Service GUID. The service guid is also encoded in the probe name. I can't think of any time that a NodePort is updated for an existing Service.

I forget the NodePort is user edittable.

[enisoc]

I'm removing the milestone because it appears this isn't ready yet and the issue has existed since before 1.9, so I don't think it should block 1.9.0. Please let me know if you disagree.

[feiskyer]

@colemickens probe.Port is actually nodePort and it could be changed by user.

@enisoc This is a fix for #56898, and the PR itself has already ready. It should also be cherry-picked to old releases. Please help to add the milestone back.

[feiskyer]

I forget the NodePort is user edittable.

@colemickens add the magical lgtm lable back?

[colemickens]

Spoke with @feiskyer. I read this wrong, probe.Port is different than the servicePort.Port.
/lgtm
/status approved-for-milestone

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, brendandburns, colemickens, dixudx, feiskyer

Associated issue: #56898

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

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

• pkg/cloudprovider/providers/azure/OWNERS [brendandburns,colemickens]

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

[feiskyer]

@colemickens thanks. I also added the milestone back.

@enisoc @colemickens thanks for reviewing this.

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 56599, 56824, 56918, 56967, 56959). If you want to cherry-pick this change to another branch, please follow the instructions here.