🐛 Requeue health check during the creation of a new cluster accessor

[aljoshare]

What this PR does / why we need it:
When a cluster accessor is created, the health check is started before the accessor is stored. If the health check hits the loadAccessor before the accessor is stored, it returns true, leaves the health check and never retries it. Together with expiring kubeconfigs (e.g. EKS), this will lead to a situation where a kubeconfig will never be refreshed and the cluster is basically dead until we restart the controller. This PR uses the TryLock to check if the cluster is currently locked which means that we're probably in the middle of the creation of a new cluster accessor and requeues the health check until the creation is done.

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 #7989

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: AljoschaP / name: Aljoscha Pörtner (42a36ad)

[k8s-ci-robot]

Welcome @AljoschaP!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. 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-sigs/cluster-api 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]

Hi @AljoschaP. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[killianmuldoon]

/ok-to-test

[codablock]

fyi, while analysing the code we figured out that the bug that is triggered by it (accessors not having a running health check) is something that is probably very common for all users, no matter which providers are used. It is however not seen/noticed by most users as the involved kubeconfig used for the accessor never gets invalid.

This can easily be verified by adding a log to the health check that prints out that a health check is performed. You'll see that it stops doing health checks after the first invocation.

[sbueringer]

Thx, very nice catch!!

Great work!

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: c133431490be719dac8dbbab1e99d3095e247111

[sbueringer]

/cherry-pick release-1.3

[k8s-infra-cherrypick-robot]

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.3 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.3

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.

[sbueringer]

/cherry-pick release-1.2

[k8s-infra-cherrypick-robot]

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.2

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.

[sbueringer]

/hold
just want to run the full tests

/test pull-cluster-api-e2e-full-main

[chrischdi]

Awesome finding 👍

/lgtm

[sbueringer]

/hold cancel
e2e green

[sbueringer]

/assign @fabriziopandini

[fabriziopandini]

@AljoschaP that's a great first PR!
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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:

• OWNERS [fabriziopandini]

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

[sbueringer]

independent flake

/retest

[sbueringer]

@AljoschaP if you have some time, can you please open a follow-up issue to add a unit test to cover this case?
(or a PR if you want to of course :))

Thank you!

[k8s-infra-cherrypick-robot]

@sbueringer: new pull request created: #8158

In response to this:

/cherry-pick release-1.3

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.

[k8s-infra-cherrypick-robot]

@sbueringer: new pull request created: #8159

In response to this:

/cherry-pick release-1.2

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.

[aljoshare]

@AljoschaP if you have some time, can you please open a follow-up issue to add a unit test to cover this case?

(or a PR if you want to of course :))

Thank you!

Of course 🙂 I will create an issue and start working on it beginning of next week 🙂 Thank you all!

[sbueringer]

Perfect, thank you very much!!