:seedling: Refactor action EnsureProvisioned by Dhairya-Arora01 · Pull Request #2091 · syself/cluster-api-provider-hetzner

Dhairya-Arora01 · 2026-06-09T11:25:07Z

What this PR does / why we need it:
Refactor actionEnsureProvisioned to reliably detect failed rescue mode reboot.

actionEnsureProvisioned is refactored into 2 steps:
Step-1: Check if the host is still in rescue mode SSH on port 22 and check hostname.

If hostname is rescue:

Check if the host was just rebooted (within 15s period)
- If yes, reque with delay.
Ensure rescue is not set for the next reboot.
- If rescue is set for next boot, unset it via robot API.
Based on the current ErrorType, decide what to do:
- Empty: This means no prior reboot triggered. Use trigger software reboot (if the host supports it) otherwise hardware reboot.
- ErrorTypeSoftwareRebootTriggered and not timed out yet (within softwareResetTimeout of 10 min): The software reboot is still in progress, so reque with delay.
- ErrorTypeSoftwareRebootTriggered and timed out: Software reboot failed to bring the host out of rescue. Escalate to hardware reboot. Trigger hardware reboot via Robot API, set ErrorTypeHardwareRebootTriggered and move to Step-2.

If SSH error or hostname is not rescue, then also move to step-2

Step-2:
If SSHAfterInstallImage is disabled, mark the action as completed, clear the ErrorType and move to the next phase Provisioned. Otherwise, SSH via configured OS port and check hostname.

If hostname matches the desired hostname: mark the action as completed, clear the errorType and move to the next phase.
If the hostname doesn't match:
- If ErrorTypeHardwareRebootTriggered and not yet timed out (within hardwareResetTimeout of 10 min): requeue with delay as the hardware reboot is still in progress.
- Otherwise set a permanent error.

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

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

TODO: Instead of setting permanent error in step-2 when hardware reboot timed out set a fatal error so that it can retry.

squash commits
include documentation
add unit tests

Refactor `actionEnsureProvisioned` to reliably detect failed rescue mode reboot. actionEnsureProvisioned is refactored into 2 steps: **Step-1**: Check if the host is still in rescue mode SSH on port 22 and check `hostname`. If hostname is `rescue`: - Check if the host was just rebooted (within 15s period) - If yes, reque with delay. - Ensure rescue is not set for the next reboot. - If rescue is set for next boot, unset it via robot API. - Based on the current `ErrorType`, decide what to do: - *Empty*: This means no prior reboot triggered. Use trigger software reboot (if the host supports it) otherwise hardware reboot. - *`ErrorTypeSoftwareRebootTriggered` and not timed out yet (within `softwareResetTimeout` of 10 min)*: The software reboot is still in progress, so reque with delay. - *`ErrorTypeSoftwareRebootTriggered` and timed out*: Software reboot failed to bring the host out of rescue. Escalate to hardware reboot. Trigger hardware reboot via Robot API, set `ErrorTypeHardwareRebootTriggered` and move to Step-2. If SSH error or hostname is not rescue, then also move to step-2 **Step-2**: If `SSHAfterInstallImage` is disabled, mark the action as completed, clear the `ErrorType` and move to the next phase `Provisioned`. Otherwise, SSH via configured OS port and check hostname. - If SSH error (connection reset or timed out): reque with delay. - If hostname matches the desired hostname: mark the action as completed, clear the errorType and move to the next phase. - If the hostname doesn't match: - If `ErrorTypeHardwareRebootTriggered` and not yet timed out (within `hardwareResetTimeout` of 10 min): requeue with delay as the hardware reboot is still in progress. - Otherwise set a permanent error. Signed-off-by: Dhairya Arora <dhairya.arora@syself.com>

janiskemper · 2026-06-10T10:20:36Z

 func (s *Service) actionEnsureProvisioned(ctx context.Context) (ar actionResult) {
 	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateEnsureProvisioned)

+	// If hardware reboot was triggered, then move to Step-2 i.e. ensuring host is booted into the desired OS.


explain why you do this check...

IMO the explanation is: in case the hardware reboot is triggered already, we have reached the last phase of escalation already.

janiskemper · 2026-06-10T10:21:24Z

 func (s *Service) actionEnsureProvisioned(ctx context.Context) (ar actionResult) {
 	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateEnsureProvisioned)

+	// If hardware reboot was triggered, then move to Step-2 i.e. ensuring host is booted into the desired OS.


can you explain the two steps a bit better here in this function? It's not really undrestandable

janiskemper · 2026-06-10T10:25:27Z

+	// Check hostname with sshClient.
+	out := sshClient.GetHostName(ctx)
+	if out.Err != nil || out.StdErr != "" {
+		// Rescue system is not reachable, we can move to step-2 i.e. verifying if host is booted into provisioned OS.


I think we can write this a bit differently: "We failed to reach the server via ssh and assume that it rebooting, or has rebooted already. Therefore, we can go to step 2 to verify that it is provisions successfully.

janiskemper · 2026-06-10T10:26:40Z

+	if hostname != rescue {
+		// Host is not in rescue mode, we can move to step-2 i.e. verifying if host is booted into provisioned OS.
+		//
+		// NOTE: We are not analyzing the SSH errors because in this step we only need to detect if the host is in rescue mode.


didn't we analyze SSH errors before? I don't understand this.. Do you mean that we don't check for specific errors and differentiate them? In this case, you should probably add this to the if statement above

janiskemper · 2026-06-10T10:29:49Z

+		return actionContinue{delay: 5 * time.Second}
+
+	case infrav1.ErrorTypeSoftwareRebootTriggered:
+		// If the software reboot timed out, then trigger a hardware reboot, otherwise reque.


janiskemper · 2026-06-10T10:30:33Z

+		// If the software reboot timed out, then trigger a hardware reboot, otherwise reque.
+		if !hasTimedOut(s.scope.HetznerBareMetalHost.Spec.Status.RebootTriggeredAt, softwareResetTimeout) {
+			markProvisionPendingWithInfo(s.scope.HetznerBareMetalHost, infrav1.StateEnsureProvisioned, "waiting for software reboot to complete")
+			return actionContinue{delay: 5 * time.Second}


should we requeue every 5 seconds? Seems very quickly. How do we do it in other places? We should keep that aligned probably.

janiskemper · 2026-06-10T10:33:50Z

+
+	if hostname != desiredHostName {
+		// if the host was just rebooted, give it some time.
 		if s.hasJustRebooted() {


this function is something you don't re-use above - but you have the same case, don't you?

janiskemper · 2026-06-10T10:37:02Z

-			msg := "reboot handling failed"
-			if err != nil {
-				msg = err.Error()
+			if sshclient.IsConnectionRefusedError(out.Err) || os.IsTimeout(out.Err) || sshclient.IsTimeoutError(out.Err) {


I don't think that this makes a lot of sense. If it is another error why we cannot reach the server, then we would say "host name doesn't match" even though we didn't even manage to ssh onto the server.

Dhairya-Arora01 requested a review from janiskemper June 9, 2026 11:25

Dhairya-Arora01 force-pushed the 2073 branch from 9b5f84f to 439cb1b Compare June 9, 2026 11:27

github-actions Bot added area/code Changes made in the code directory size/L Denotes a PR that changes 200-800 lines, ignoring generated files. labels Jun 9, 2026

fix lint

6f29b9e

janiskemper requested changes Jun 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🌱 Refactor action EnsureProvisioned#2091

🌱 Refactor action EnsureProvisioned#2091
Dhairya-Arora01 wants to merge 2 commits into
mainfrom
2073

Dhairya-Arora01 commented Jun 9, 2026 •

edited

Loading

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

janiskemper Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Dhairya-Arora01 commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dhairya-Arora01 commented Jun 9, 2026 •

edited

Loading