🐛 Do not include BootstrapConfigReady condition for machine Ready c…

[apedriza]

…alculation if machine is already Available

What this PR does / why we need it:

Once a Machine becomes Available, the bootstrap config is no longer needed for its operation. This change excludes BootstrapConfigReady from the Ready aggregation once the machine is Available, and tolerates a missing bootstrap config when the node ref is already set.

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 #

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[k8s-ci-robot]

This PR is currently missing an area label, which is used to identify the modified component when generating release notes.

Area labels can be added by org members by writing /area ${COMPONENT} in a comment

Please see the labels list for possible areas.

Details

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-sigs/prow repository.

[k8s-ci-robot]

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

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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.

Details

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-sigs/prow repository.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[apedriza]

@fabriziopandini any chance to get this merged?

[fabriziopandini]

Top of mind a missing bootstrap config prevents CAPI to determine when to rollout a machine, it will have impact on the capability to perform in place updates (and probably there are more cases where CAPI expects a bootstrap config).

Considering this, we should keep BootstrapConfigReady as part of Machine's Ready condition
@sbueringer opinions?

[sbueringer]

Agree. I also think it's not a good idea to ignore BootstrapReady as long as Machine is Available. Once the Machine becomes unavailable for any reason we would suddenly include the condition again.

[apedriza]

Great, I understand that bootstrap config also plays a role after provisioning, during rollout decisions.

This was mainly about making the Ready status report for machines a bit smarter. From a machine lifecycle POV, config readiness only matters prior to boot, once the config has been consumed, it shouldn't affect whether a machine is determined to be not available, as technically it can't be a reason for a machine to not be available. It has already successfully done its duty on the machine so reporting it that way might be misleading.

That said, I don't think there's an issue with bringing that condition back into play if the machine becomes unavailable again for some other reason my concern is specifically about factoring it in while the machine is already available. Depending on where the machine state/phase is, some data should take relevance and others not, from my POV

But happy to go with whatever you all think makes more sense here

[sbueringer]

That said, I don't think there's an issue with bringing that condition back into play if the machine becomes unavailable again for some other reason my concern is specifically about factoring it in while the machine is already available.

Do you have a concrete issue with always using the BootstrapConfigReady condition for Available computation?

[apedriza]

Do you have a concrete issue with always using the BootstrapConfigReady condition for Available computation?

No, I was just confused for a moment about the machine status while I was deleting some config resources