🌱 Refactoring PR for: Remove usage of FailureReason and FailureMessage (baremetal)

[guettli]

Some changes of this PR got extracted, so that the PR gets smaller and easier to review:

🌱 Remove usage of FailureReason and FailureMessage (baremetal) by guettli · Pull Request #1716 · syself/cluster-api-provider-hetzner

• Add/extended comments to some parts.
• move rate-limit handling code up, to return early if rate-limit is active.
• avoid usage of capierrors package.
• test code: Align capi-machine name and infra-machine name. Remediation code depends on that capi feature.
• tee tmp/test-unit-unfiltered.log in test-unis.sh: Write unmodfied output to tmp directory. This is handy if you want to see all logs, without running tests again.
• export func GetAssociatedHost(), so that it can be re-used.
• refactored getAssociatedHost() to getAssociatedHostAndPatchHelper(), so that the code can be re-used.
• add log when hbmm is deleting.
• avoid duplicate implemenation of splitHostKey()
• refactored setOwnerRemediatedConditionNew() to setOwnerRemediatedConditionToFailed(): Avoid duplicated code by adding functionality to setOwnerRemediatedConditionToFailed(): Use the same message for the created Event and the condition, and set Phase to PhaseDeleting.
• removed named return parameters: return res, nil --> return reconcile.Result{}, nil
• hbmm have an annotation pointing to the hbmh. The reference is a string in the format "namespace/hbmh-name". The namespace will be ignored from now on, because cross-namespace references are not allowed. This is not a breaking change, because we expect that both namespace strings are always equal.

[janiskemper]

I didn't fully understand the remediation part. Please add some explanation for the refactoring in the PR description

[guettli]

I didn't fully understand the remediation part. Please add some explanation for the refactoring in the PR description

can you please point me to a line in the code?

[janiskemper]

@guettli I didn't understand the reason for this part "refactored setOwnerRemediatedConditionNew() to setOwnerRemediatedConditionToFailed()"

[guettli]

@guettli I didn't understand the reason for this part "refactored setOwnerRemediatedConditionNew() to setOwnerRemediatedConditionToFailed()"

When you look at the places where setOwnerRemediatedConditionToFailed() gets used, then you might see that the same code was repeated. But it was not consistently. It makes sense to have the same message in the condition and in the event. Additionally, Phase = infrav1.PhaseDeleting should be set. So added this to the function and gave it a (from my perspective) better name.

do you want something to be changed here?

[janiskemper]

okay understood it now. Can you plz add this information to the PR description?

[guettli]

okay understood it now. Can you plz add this information to the PR description?

done. Anything else which is blocking? I would like to merge the PR.