🌱 Remove usage of FailureReason and FailureMessage (baremetal) #1716

guettli · 2025-11-17T11:26:54Z

Baremetal PR

Deprecated: This field is deprecated and is
going to be removed when support for v1beta1
will be dropped. Please see
https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md
for more details.

Related HCloud PR 1717

Here’s a shorter, clean PR description:

Summary

This PR removes all usage of the deprecated FailureReason and FailureMessage fields from the bare-metal (BM) provider. Status handling is now fully aligned with the newer CAPI conditions-based model.

Why

Upstream CAPI is deprecating these fields. Removing them keeps our provider consistent with the updated status API and prevents reliance on legacy status patterns.

Changes

BareMetalMachine.SetFailure() --> s.scope.SetErrorAndRemediate(). This sets the clusterv1.RemediateMachineAnnotation on the capiMachine.
During remediation the controller checks if the remediation was started by SetErrorAndRemediate(). If "yes", then no reboot gets done. The remediation stops, so that the machine gets deleted.
Removed all references to FailureReason and FailureMessage in BM controllers and API types.
Updated reconciliation logic to use standard Conditions exclusively.
Adjusted tests to match the new status behavior.
Test code was adapted to no longer use a fixed bmMachineName. Capi-machine and hbmm are exptected to have the same name. This was adapted, so test data structures match real structures.
The bm remediation does not take the Namespace from the InfrastructureRef. Cross namespace references are not allowed, because user1 of namespace1 must not reference namespace2.
Improved logging of unusual cases (conflictError, hbmm is deleting).
Duplicate code was removed and getAssociatedHost() was refactored to getAassociatedHostAndPatchHelper()
Remediation code expects that the capi-machine name and hbmm name are equal. Test code was adapted accordingly.

> Deprecated: This field is deprecated and is > going to be removed when support for v1beta1 > will be dropped. Please see > https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md > for more details.

…n controller would need to be changed.

Before the test did not realy test something because the state is none at the beginning (before provisioning), too.

…-baremetal

…e (baremetal)

…into tg/remove-failure-reason--baremetal

…e-reason--baremetal--on-top-main

…into tg/remove-failure-reason--baremetal

janiskemper · 2025-12-03T12:24:15Z

api/v1beta1/conditions_const.go

+const (
+	// RemediationSucceededCondition is:
+	// - False when the corresponding CAPI Machine has the "cluster.x-k8s.io/remediate-machine" annotation set and will be remediated by CAPI soon.
+	// - True otherwise.


it shouldn't be true otherwise. It should be only true IMO after it succeeded. Otherwise it should not be set at all

OK, what about deleting the condition 6 hours after it got set to True?

I think setting it to true is fine, once it actually got remediated. Then we can think about what you propose, but that would be an optimization that we could do later IMO. However, what I really would not like, and I think we currently do it, is to set the condition on true, even though we didn't even ever remediate the machine.

janiskemper · 2025-12-03T12:26:22Z

controllers/hetznerbaremetalhost_controller.go

 	ctx = ctrl.LoggerInto(ctx, log)

+	remediateConditionOfHbmm := conditions.Get(hetznerBareMetalMachine, infrav1.NoRemediateMachineAnnotationCondition)
+	if remediateConditionOfHbmm != nil && remediateConditionOfHbmm.Status == corev1.ConditionFalse {


One question: Does it work properly in case the hmbh is mirroring from hbmm, because in the other direction the hbmm is already mirroring from hbmh

janiskemper · 2025-12-03T12:30:23Z

controllers/hetznerbaremetalmachine_controller.go

+	remediateConditionOfHbmm := conditions.Get(hbmMachine, infrav1.RemediationSucceededCondition)
+	if remediateConditionOfHbmm != nil && remediateConditionOfHbmm.Status == corev1.ConditionFalse {
+		// The hbmm will be deleted. Do not reconcile it.
+		log.Info("hbmm has RemediationSucceededCondition=False. Waiting for deletion")


the condition is also set on false if it is still in progress, no? I don't understand this code snippet

@janiskemper maybe we should rename it to DeleteInfraMachineSucceeded. Because that is the intention. We do not want to reboot the machine.

This will be done via remediation, but this is just the path which need to be followed, because the controller can't delete the capi-machine. Remediation is not really the intention, the machine should get deleted.

Does this make more sense now?

I didn't understand it yet. We have the RemediationSucceeded condition, which is set on false with the reason "remediation ongoing". At least this is how we do it on hbmh. Do we have some different logic here on hbmm?

pkg/baremetalutils/baremetalutils.go

janiskemper · 2025-12-03T12:34:48Z

pkg/services/baremetal/remediation/remediation.go

 		return reconcile.Result{}, s.setOwnerRemediatedConditionToFailed(ctx,
 			"exit remediation because hbmm has no host annotation")
 	}
+	// if SetErrorAndRemediate() was used to stop provisioning, do not try to reboot server


I don't understand the name of the function. SetErrorAndRemediate sounds to me as if you explicitly want this remediation file to trigger. However, you instead do the opposite. Shouldn't we rename this function SetErrorAndRemediate so that readers are not let to the wrong direction?

If I understand correctly, you want to set an error so that remediation via reboot is skipped right?

Yes, SetErrorAndDeleteMachine() might be a better name. Do you agree?

yes. Just to confirm again: You wanna use this function in the cases when a remediation via reboot should not be done?

guettli · 2025-12-03T13:43:09Z

One question: Does it work properly in case the hmbh is mirroring from hbmm, because in the other direction the hbmm is already mirroring from hbmh

@janiskemper

This condition is only mirrored from hbmm to hbmh, so I think this is fine.

guettli added 3 commits November 17, 2025 12:25

make linter happy.

1fa1406

set condition.

8c7c7a9

guettli mentioned this pull request Nov 17, 2025

🌱 Remove usage of FailureReason and FailureMessage (hcloud) #1717

Open

feedback of AI.

8c81c6c

github-actions bot added size/M Denotes a PR that changes 50-200 lines, ignoring generated files. area/code Changes made in the code directory area/api Changes made in the api directory labels Nov 17, 2025

guettli added 15 commits November 17, 2025 14:38

set condition on host, too. Do not loose the message...

5b3a35d

cleanup, do not set condition from hbmm reconcile on hbmh.

98bcfff

copy condition from hbmm to host. No need to read capi machine.

fb67a97

delete via RemediateAnnotation does not work well. Code in Remediatio…

a5521cd

…n controller would need to be changed.

revert last commit. Delete via remediation is ok.

7d834ca

WIP: give msg to method, so that a more precise error gets created.

cda32ca

show exit message of remediation on capi machine condition.

700d1ec

no need to try a reboot.

f71d8a2

Merge branch 'main' into tg/remove-failure-reason--baremetal

7c57394

more logging, WIP

401a474

fixed test cases.

25890c4

🌱 Fix hbmm delete unit tests.

fc375ce

Before the test did not realy test something because the state is none at the beginning (before provisioning), too.

simplified code, so that AI understands it.

393ecd6

fix failing test.

8c1986a

deduplicated code, cleaned up PR.

7133120

github-actions bot added size/L Denotes a PR that changes 200-800 lines, ignoring generated files. and removed size/M Denotes a PR that changes 50-200 lines, ignoring generated files. labels Nov 20, 2025

guettli added 5 commits November 20, 2025 20:12

...

527de14

adapt comments to no longer use failure reason.

826c0d4

remove todo.

523dd7b

linting.

49a275c

make diff to main smaller.

0726e9a

guettli added 4 commits November 26, 2025 16:15

fix linting.

156f723

make generate.

5469266

Merge branch 'main' into tg/fix-hbmm-delete-test

8a5442e

Merge branch 'tg/fix-hbmm-delete-test' into tg/remove-failure-reason-…

c6d202f

…-baremetal

Base automatically changed from tg/fix-hbmm-delete-test to main November 27, 2025 06:28

guettli added 4 commits November 27, 2025 07:45

Merge branch 'main' into tg/remove-failure-reason--baremetal

6882d40

renamed to RemediationSucceededCondition.

566dd86

move check for rate-limit up.

2086a34

🌱 Refactoring PR for: Remove usage of FailureReason and FailureMessag…

8bcbe46

…e (baremetal)

guettli mentioned this pull request Nov 27, 2025

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

Merged

Merge branch 'tg/pre--remove-failure-reason--baremetal--on-top-main' …

3b45854

…into tg/remove-failure-reason--baremetal

guettli changed the base branch from main to tg/pre--remove-failure-reason--baremetal--on-top-main November 27, 2025 12:31

github-actions bot added size/L Denotes a PR that changes 200-800 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 800-2000 lines, ignoring generated files. labels Nov 27, 2025

guettli added 9 commits November 27, 2025 14:12

fix typo.

de7c875

fix linter warnings.

25696a2

Merge branch 'tg/pre--remove-failure-reason--baremetal--on-top-main' …

6eba6e0

…into tg/remove-failure-reason--baremetal

fix typo.

c4235a0

Merge remote-tracking branch 'origin/main' into tg/pre--remove-failur…

ffef8bb

…e-reason--baremetal--on-top-main

avoid to return a reconcile.Result which is always zero.

ae5a531

Merge remote-tracking branch 'origin/main' into tg/pre--remove-failur…

aa22b01

…e-reason--baremetal--on-top-main

handle not-found in GetAssociatedHost().

a4cd586

Merge branch 'tg/pre--remove-failure-reason--baremetal--on-top-main' …

8d17c62

…into tg/remove-failure-reason--baremetal

Base automatically changed from tg/pre--remove-failure-reason--baremetal--on-top-main to main December 3, 2025 12:04

Merge branch 'main' into tg/remove-failure-reason--baremetal

436d48c

janiskemper requested changes Dec 3, 2025

View reviewed changes

delete remediate condition on hbmh after remediation was done.

a2fb07c

guettli requested a review from janiskemper December 3, 2025 13:43

PR feedback.

b5eed48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🌱 Remove usage of FailureReason and FailureMessage (baremetal) #1716

🌱 Remove usage of FailureReason and FailureMessage (baremetal) #1716

guettli commented Nov 17, 2025 •

edited

Loading

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

guettli Dec 3, 2025

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

guettli Dec 3, 2025

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

guettli Dec 3, 2025

Uh oh!

janiskemper Dec 3, 2025

Uh oh!

guettli commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

🌱 Remove usage of FailureReason and FailureMessage (baremetal) #1716

Are you sure you want to change the base?

🌱 Remove usage of FailureReason and FailureMessage (baremetal) #1716

Conversation

guettli commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guettli commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guettli commented Nov 17, 2025 •

edited

Loading