🐛 Remove the check for HCloudCredentialsInvalid to unblock reconcilement (#1930)

* remove the check for `HCloudCredentialsInvalid` to unblock reconcilement

Currently, we skip reconciling HCloudMachine completely if the condition
`HCloudTokenAvailable` is set to false with reason `HCloudCredentialsInvalid`.
This means that even when the CAPH controller restarts, the HCloudMachine is
not reconciled. We don't want this behaviour, as on CAPH restart we want to
reconcile all the related objects. Therefore, the early check that skips
reconciliation based on the condition is removed.

Note that this does not cause repeated reconciliation attempts with invalid
credentials as we still stop reconciling immediately after setting the
`HCloudCredentialsInvalid` condition during the normal reconcile loop.
The only difference is that we no longer prevent reconciliation from starting
based on a previously set condition.

Signed-off-by: Dhairya Arora <dhairya.arora@syself.com>

* if credentials are invalid on delete, set HCloudCredentialsInvalid condition

---------

Signed-off-by: Dhairya Arora <dhairya.arora@syself.com>

Author: Dhairya-Arora01

controllers/hcloudmachine_controller.go

@@ -100,15 +100,6 @@ func (r *HCloudMachineReconciler) Reconcile(ctx context.Context, req reconcile.R
 
 	log = log.WithValues("Machine", klog.KObj(machine))
 
-	// If the HCloudMachine has an unauthorized / HCloudCredentialsInvalid error, stop reconciling.
-	// There is no point in retrying API calls with an invalid token.
-	// The MachineHealthCheck will eventually remediate the machine by deleting and recreating it.
-	if conditions.IsFalse(hcloudMachine, infrav1.HCloudTokenAvailableCondition) &&
-		conditions.GetReason(hcloudMachine, infrav1.HCloudTokenAvailableCondition) == infrav1.HCloudCredentialsInvalidReason {
-		log.Info("HCloudMachine has HCloudCredentialsInvalid, stopping reconciliation")
-		return reconcile.Result{}, nil
-	}
-
 	// Fetch the Cluster.
 	cluster, err := util.GetClusterFromMetadata(ctx, r.Client, machine.ObjectMeta)
 	if err != nil {
@@ -167,6 +158,8 @@ func (r *HCloudMachineReconciler) Reconcile(ctx context.Context, req reconcile.R
 	defer func() {
 		if reterr != nil && errors.Is(reterr, hcloudclient.ErrUnauthorized) {
 			conditions.MarkFalse(hcloudMachine, infrav1.HCloudTokenAvailableCondition, infrav1.HCloudCredentialsInvalidReason, clusterv1.ConditionSeverityError, "wrong hcloud token")
+			// set reterr as nil, so that we don't get stuck in an infinite reconciliation loop if HCloud token is invalid.
+			reterr = nil
 		} else {
 			conditions.MarkTrue(hcloudMachine, infrav1.HCloudTokenAvailableCondition)
 		}

pkg/services/hcloud/server/server.go

@@ -277,6 +277,11 @@ func handleRateLimit(hm *infrav1.HCloudMachine, err error, functionName string,
 
 // Delete implements delete method of server.
 func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) {
+	// Nothing to do, if ProviderID is nil
+	if s.scope.HCloudMachine.Spec.ProviderID == nil {
+		return reconcile.Result{}, nil
+	}
+
 	server, err := s.findServer(ctx)
 	if err != nil {
 		return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "findServer", "failed to find server for deletion", s.scope.Logger)