fix: Do not set RebootTriggeredAt to nil too early#2125
Conversation
… is nil When RebootTriggeredAt gets cleared (e.g. via ClearError path from connection error handling) but ErrorType is still SSHRebootTriggered, hasJustRebooted() incorrectly returns true forever because hasTimedOut(nil, ...) returns false. Fix: return false early when RebootTriggeredAt is nil. Also set RebootTriggeredAt when first detecting SSH timeout in emptyErrorType case so that escalation timeouts are computed correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com
Without the nil guard added in the fix, hasTimedOut(nil, ...) returns false, causing hasJustRebooted() to return true indefinitely when a reboot error type is set but RebootTriggeredAt was never populated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com
|
@guettli can you update the PR title and description again and also add a comment to the utility function you updated? Basically, I would say that this PR is about fixing a bug where we set RebootedAt too early to nil. Keeping the change in the utility function wouldn't be necessary but might be a safe guard for the future. |
PR title and description got updated. In ec53299 I added a comment and log, when the value is nil, which is not expected. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com
* fix: hasJustRebooted returns true indefinitely when RebootTriggeredAt is nil When RebootTriggeredAt gets cleared (e.g. via ClearError path from connection error handling) but ErrorType is still SSHRebootTriggered, hasJustRebooted() incorrectly returns true forever because hasTimedOut(nil, ...) returns false. Fix: return false early when RebootTriggeredAt is nil. Also set RebootTriggeredAt when first detecting SSH timeout in emptyErrorType case so that escalation timeouts are computed correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com * test: hasJustRebooted returns false when RebootTriggeredAt is nil Without the nil guard added in the fix, hasTimedOut(nil, ...) returns false, causing hasJustRebooted() to return true indefinitely when a reboot error type is set but RebootTriggeredAt was never populated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com * remove resetting RebootTriggeredAt. We need to wait until ssh works correctly. * add comment and info log. * fix lint: remove redundant embedded field name in Logger selector Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> # Committing as: thomas.guettler@syself.com --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com># Committing as: thomas.guettler@syself.com
|
Cherry-picked into release-1.1 via 240eaa5 |
Summary
RebootTriggeredAtwas set to nil to early.Then
hasJustRebooted()returnedtrueforever whenRebootTriggeredAtis nil butErrorTypeisSSHRebootTriggered.This was introduced in 4f9d7c6 in April this year:
if s.scope.HetznerBareMetalHost.Spec.Status.ErrorType == infrav1.ErrorTypeConnectionError { s.scope.HetznerBareMetalHost.ClearError() + s.scope.HetznerBareMetalHost.Spec.Status.RebootTriggeredAt = nil }How this state is reached
The exact path that produces
ErrorType=SSHRebootTriggeredwithRebootTriggeredAt=nil:handleIncompleteBootsetsErrorType=ConnectionError,RebootTriggeredAt=<time>.RebootTriggeredAt=nil.emptyErrorTypecase setsErrorType=SSHRebootTriggeredbut (before this fix) never re-setsRebootTriggeredAt.hasJustRebooted()seesSSHRebootTriggered+ nil timestamp →hasTimedOut(nil, …)returnsfalse→!false == true→ stuck forever.