From 70748765a33c5cf5ab86637041e39689df9116b2 Mon Sep 17 00:00:00 2001
From: janiskemper <janis.kemper@syself.com>
Date: Thu, 12 Oct 2023 08:54:38 +0200
Subject: [PATCH] :bug: Fix provision condition time lag

There has been a time lag in the condition that shows the provisioning
status of the HetznerBareMetalHost. It was only updated after a new
reconcilement has started, not during the reconcilement where the state
actually changed.

This commit places the update of the condition to the start of the
respective functions to ensure it is up to date.
---
 pkg/services/baremetal/host/host.go          | 23 ++++++++++++++++++--
 pkg/services/baremetal/host/state_machine.go |  9 --------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go
index f33f0a5b2..fb55c19ec 100644
--- a/pkg/services/baremetal/host/host.go
+++ b/pkg/services/baremetal/host/host.go
@@ -115,6 +115,7 @@ func (s *Service) Reconcile(ctx context.Context) (result reconcile.Result, err e
 
 	// reconcile state
 	actResult := hostStateMachine.ReconcileState()
+
 	result, err = actResult.Result()
 	if err != nil {
 		return reconcile.Result{Requeue: true}, fmt.Errorf("action %q failed: %w", initialState, err)
@@ -144,6 +145,8 @@ func SaveHostAndReturn(ctx context.Context, cl client.Client, host *infrav1.Hetz
 }
 
 func (s *Service) actionPreparing() actionResult {
+	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StatePreparing)
+
 	server, err := s.scope.RobotClient.GetBMServer(s.scope.HetznerBareMetalHost.Spec.ServerID)
 	if err != nil {
 		s.handleRobotRateLimitExceeded(err, "GetBMServer")
@@ -498,6 +501,8 @@ func (s *Service) ensureRescueMode() error {
 }
 
 func (s *Service) actionRegistering() actionResult {
+	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateRegistering)
+
 	creds := sshclient.CredentialsFromSecret(s.scope.RescueSSHSecret, s.scope.HetznerCluster.Spec.SSHKeys.RobotRescueSecretRef)
 	in := sshclient.Input{
 		PrivateKey: creds.PrivateKey,
@@ -907,6 +912,8 @@ func validateStdOut(stdOut string) (string, error) {
 }
 
 func (s *Service) actionImageInstalling() actionResult {
+	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateImageInstalling)
+
 	creds := sshclient.CredentialsFromSecret(s.scope.RescueSSHSecret, s.scope.HetznerCluster.Spec.SSHKeys.RobotRescueSecretRef)
 	in := sshclient.Input{
 		PrivateKey: creds.PrivateKey,
@@ -1028,6 +1035,8 @@ func getDeviceNames(wwn []string, storageDevices []infrav1.Storage) []string {
 }
 
 func (s *Service) actionProvisioning() actionResult {
+	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateProvisioning)
+
 	host := s.scope.HetznerBareMetalHost
 
 	portAfterInstallImage := host.Spec.Status.SSHSpec.PortAfterInstallImage
@@ -1079,8 +1088,6 @@ func (s *Service) actionProvisioning() actionResult {
 	}
 
 	host.ClearError()
-	conditions.MarkTrue(host, infrav1.ProvisionSucceededCondition)
-
 	return actionComplete{}
 }
 
@@ -1190,6 +1197,7 @@ func verifyConnectionRefused(sshClient sshclient.Client, port int) bool {
 }
 
 func (s *Service) actionEnsureProvisioned() (ar actionResult) {
+	markProvisionPending(s.scope.HetznerBareMetalHost, infrav1.StateEnsureProvisioned)
 	sshClient := s.scope.SSHClientFactory.NewClient(sshclient.Input{
 		PrivateKey: sshclient.CredentialsFromSecret(s.scope.OSSSHSecret, s.scope.HetznerBareMetalHost.Spec.Status.SSHSpec.SecretRef).PrivateKey,
 		Port:       s.scope.HetznerBareMetalHost.Spec.Status.SSHSpec.PortAfterCloudInit,
@@ -1276,6 +1284,7 @@ func (s *Service) actionEnsureProvisioned() (ar actionResult) {
 		}
 	}
 
+	conditions.MarkTrue(s.scope.HetznerBareMetalHost, infrav1.ProvisionSucceededCondition)
 	s.scope.HetznerBareMetalHost.ClearError()
 	return actionComplete{}
 }
@@ -1537,3 +1546,13 @@ func (s *Service) hasJustRebooted() bool {
 		s.scope.HetznerBareMetalHost.Spec.Status.ErrorType == infrav1.ErrorTypeHardwareRebootTriggered) &&
 		!hasTimedOut(s.scope.HetznerBareMetalHost.Spec.Status.LastUpdated, rebootWaitTime)
 }
+
+func markProvisionPending(host *infrav1.HetznerBareMetalHost, state infrav1.ProvisioningState) {
+	conditions.MarkFalse(
+		host,
+		infrav1.ProvisionSucceededCondition,
+		infrav1.StillProvisioningReason,
+		clusterv1.ConditionSeverityInfo,
+		"host is still provisioning - state %q", state,
+	)
+}
diff --git a/pkg/services/baremetal/host/state_machine.go b/pkg/services/baremetal/host/state_machine.go
index 2e295a30d..b83540ebb 100644
--- a/pkg/services/baremetal/host/state_machine.go
+++ b/pkg/services/baremetal/host/state_machine.go
@@ -87,15 +87,6 @@ func (hsm *hostStateMachine) ReconcileState() (actionRes actionResult) {
 	// Assume credentials are ready for now. This can be changed while the state is handled.
 	conditions.MarkTrue(hsm.host, infrav1.CredentialsAvailableCondition)
 
-	// Set condition on false. It will be set on true if host is provisioned during reconcilement
-	conditions.MarkFalse(
-		hsm.host,
-		infrav1.ProvisionSucceededCondition,
-		infrav1.StillProvisioningReason,
-		clusterv1.ConditionSeverityInfo,
-		"host is provisioning - current state: %q", hsm.host.Spec.Status.ProvisioningState,
-	)
-
 	if stateHandler, found := hsm.handlers()[initialState]; found {
 		return stateHandler()
 	}