Repository: syself/cluster-api-provider-hetzner
Pull request #884 (2023-09-01 09:06:06)

:seedling: Add suffix to Conditions and Reasons
**What this PR does / why we need it**:

Add suffix to Conditions and Reasons

Related:

https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20200506-conditions.md#data-model-changes



Patch:
diff --git a/api/v1beta1/conditions_const.go b/api/v1beta1/conditions_const.go
index 837a1fc5..079445b1 100644
--- a/api/v1beta1/conditions_const.go
+++ b/api/v1beta1/conditions_const.go
@@ -55,22 +55,22 @@ const (
 )
 
 const (
-	// NetworkAttached reports on whether there is a network attached to the cluster.
-	NetworkAttached clusterv1.ConditionType = "NetworkAttached"
+	// NetworkAttachedCondition reports on whether there is a network attached to the cluster.
+	NetworkAttachedCondition clusterv1.ConditionType = "NetworkAttached"
 	// NetworkUnreachableReason indicates that network is unreachable.
 	NetworkUnreachableReason = "NetworkUnreachable"
 )
 
 const (
-	// PlacementGroupsSynced reports on whether the placement groups are successfully synced.
-	PlacementGroupsSynced clusterv1.ConditionType = "PlacementGroupsSynced"
+	// PlacementGroupsSyncedCondition reports on whether the placement groups are successfully synced.
+	PlacementGroupsSyncedCondition clusterv1.ConditionType = "PlacementGroupsSynced"
 	// PlacementGroupsUnreachableReason indicates that network is disabled.
 	PlacementGroupsUnreachableReason = "PlacementGroupsUnreachable"
 )
 
 const (
-	// HetznerClusterReady reports on whether the Hetzner cluster is in ready state.
-	HetznerClusterReady clusterv1.ConditionType = "HetznerClusterReady"
+	// HetznerClusterReadyCondition reports on whether the Hetzner cluster is in ready state.
+	HetznerClusterReadyCondition clusterv1.ConditionType = "HetznerClusterReady"
 	// HetznerSecretUnreachableReason indicates that Hetzner secret is unreachable.
 	HetznerSecretUnreachableReason = "HetznerSecretUnreachable" // #nosec
 	// HCloudCredentialsInvalidReason indicates that credentials for HCloud are invalid.
@@ -78,12 +78,12 @@ const (
 )
 
 const (
-	// HetznerClusterTargetClusterReady reports on whether the kubeconfig in the target cluster is ready.
-	HetznerClusterTargetClusterReady clusterv1.ConditionType = "HetznerClusterTargetClusterReady"
-	// KubeConfigNotFound indicates that the Kubeconfig could not be found.
-	KubeConfigNotFound = "KubeConfigNotFound"
-	// KubeAPIServerNotResponding indicates that the api server cannot be reached.
-	KubeAPIServerNotResponding = "KubeAPIServerNotResponding"
+	// HetznerClusterTargetClusterReadyCondition reports on whether the kubeconfig in the target cluster is ready.
+	HetznerClusterTargetClusterReadyCondition clusterv1.ConditionType = "HetznerClusterTargetClusterReady"
+	// KubeConfigNotFoundReason indicates that the Kubeconfig could not be found.
+	KubeConfigNotFoundReason = "KubeConfigNotFound"
+	// KubeAPIServerNotRespondingReason indicates that the api server cannot be reached.
+	KubeAPIServerNotRespondingReason = "KubeAPIServerNotResponding"
 )
 
 const (
@@ -94,18 +94,18 @@ const (
 )
 
 const (
-	// HetznerBareMetalHostReady reports on whether the Hetzner cluster is in ready state.
-	HetznerBareMetalHostReady clusterv1.ConditionType = "HetznerBareMetalHostReady"
+	// HetznerBareMetalHostReadyCondition reports on whether the Hetzner cluster is in ready state.
+	HetznerBareMetalHostReadyCondition clusterv1.ConditionType = "HetznerBareMetalHostReady"
 	// RobotCredentialsInvalidReason indicates that credentials for Robot are invalid.
 	RobotCredentialsInvalidReason = "RobotCredentialsInvalid" // #nosec
-	// SSHCredentialsInSecretInvalid indicates that ssh credentials are invalid.
-	SSHCredentialsInSecretInvalid = "SSHCredentialsInSecretInvalid" // #nosec
-	// SSHKeyAlreadyExists indicates that the ssh key which is specified in the host spec exists already under a different name in Hetzner robot.
-	SSHKeyAlreadyExists = "SSHKeyAlreadyExists"
-	// OSSSHSecretMissing indicates that secret with the os ssh key is missing.
-	OSSSHSecretMissing = "OSSSHSecretMissing"
-	// RescueSSHSecretMissing indicates that secret with the rescue ssh key is missing.
-	RescueSSHSecretMissing = "RescueSSHSecretMissing"
+	// SSHCredentialsInSecretInvalidReason indicates that ssh credentials are invalid.
+	SSHCredentialsInSecretInvalidReason = "SSHCredentialsInSecretInvalid" // #nosec
+	// SSHKeyAlreadyExistsReason indicates that the ssh key which is specified in the host spec exists already under a different name in Hetzner robot.
+	SSHKeyAlreadyExistsReason = "SSHKeyAlreadyExists"
+	// OSSSHSecretMissingReason indicates that secret with the os ssh key is missing.
+	OSSSHSecretMissingReason = "OSSSHSecretMissing"
+	// RescueSSHSecretMissingReason indicates that secret with the rescue ssh key is missing.
+	RescueSSHSecretMissingReason = "RescueSSHSecretMissing"
 )
 
 const (
diff --git a/controllers/hetznerbaremetalhost_controller.go b/controllers/hetznerbaremetalhost_controller.go
index f865b154..f0d13bfe 100644
--- a/controllers/hetznerbaremetalhost_controller.go
+++ b/controllers/hetznerbaremetalhost_controller.go
@@ -214,13 +214,13 @@ func (r *HetznerBareMetalHostReconciler) getSecrets(
 			if apierrors.IsNotFound(err) {
 				conditions.MarkFalse(
 					bmHost,
-					infrav1.HetznerBareMetalHostReady,
-					infrav1.OSSSHSecretMissing,
+					infrav1.HetznerBareMetalHostReadyCondition,
+					infrav1.OSSSHSecretMissingReason,
 					clusterv1.ConditionSeverityError,
 					infrav1.ErrorMessageMissingOSSSHSecret,
 				)
 
-				record.Warnf(bmHost, infrav1.OSSSHSecretMissing, infrav1.ErrorMessageMissingOSSSHSecret)
+				record.Warnf(bmHost, infrav1.OSSSHSecretMissingReason, infrav1.ErrorMessageMissingOSSSHSecret)
 
 				result, err := host.SaveHostAndReturn(ctx, r.Client, bmHost)
 				if result != emptyResult || err != nil {
@@ -238,13 +238,13 @@ func (r *HetznerBareMetalHostReconciler) getSecrets(
 			if apierrors.IsNotFound(err) {
 				conditions.MarkFalse(
 					bmHost,
-					infrav1.HetznerBareMetalHostReady,
-					infrav1.RescueSSHSecretMissing,
+					infrav1.HetznerBareMetalHostReadyCondition,
+					infrav1.RescueSSHSecretMissingReason,
 					clusterv1.ConditionSeverityError,
 					infrav1.ErrorMessageMissingRescueSSHSecret,
 				)
 
-				record.Warnf(bmHost, infrav1.RescueSSHSecretMissing, infrav1.ErrorMessageMissingRescueSSHSecret)
+				record.Warnf(bmHost, infrav1.RescueSSHSecretMissingReason, infrav1.ErrorMessageMissingRescueSSHSecret)
 
 				result, err := host.SaveHostAndReturn(ctx, r.Client, bmHost)
 				if result != emptyResult || err != nil {
@@ -317,7 +317,7 @@ func hetznerSecretErrorResult(
 		// at some point in the future.
 		conditions.MarkFalse(
 			bmHost,
-			infrav1.HetznerBareMetalHostReady,
+			infrav1.HetznerBareMetalHostReadyCondition,
 			infrav1.HetznerSecretUnreachableReason,
 			clusterv1.ConditionSeverityError,
 			infrav1.ErrorMessageMissingHetznerSecret,
@@ -339,12 +339,12 @@ func hetznerSecretErrorResult(
 	if errors.As(err, &credValidationErr) {
 		conditions.MarkFalse(
 			bmHost,
-			infrav1.HetznerBareMetalHostReady,
+			infrav1.HetznerBareMetalHostReadyCondition,
 			infrav1.RobotCredentialsInvalidReason,
 			clusterv1.ConditionSeverityError,
 			infrav1.ErrorMessageMissingOrInvalidSecretData,
 		)
-		record.Warnf(bmHost, infrav1.SSHCredentialsInSecretInvalid, err.Error())
+		record.Warnf(bmHost, infrav1.SSHCredentialsInSecretInvalidReason, err.Error())
 
 		return host.SaveHostAndReturn(ctx, client, bmHost)
 	}
diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go
index e0329d81..21fc501c 100644
--- a/controllers/hetznercluster_controller.go
+++ b/controllers/hetznercluster_controller.go
@@ -125,7 +125,7 @@ func (r *HetznerClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	secretManager := secretutil.NewSecretManager(log, r.Client, r.APIReader)
 	hcloudToken, hetznerSecret, err := getAndValidateHCloudToken(ctx, req.Namespace, hetznerCluster, secretManager)
 	if err != nil {
-		return hcloudTokenErrorResult(ctx, err, hetznerCluster, infrav1.HetznerClusterReady, r.Client)
+		return hcloudTokenErrorResult(ctx, err, hetznerCluster, infrav1.HetznerClusterReadyCondition, r.Client)
 	}
 
 	hcloudClient := r.HCloudClientFactory.NewClient(hcloudToken)
@@ -178,7 +178,7 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 
 	// reconcile the network
 	if err := network.NewService(clusterScope).Reconcile(ctx); err != nil {
-		conditions.MarkFalse(hetznerCluster, infrav1.NetworkAttached, infrav1.NetworkUnreachableReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(hetznerCluster, infrav1.NetworkAttachedCondition, infrav1.NetworkUnreachableReason, clusterv1.ConditionSeverityError, err.Error())
 		return reconcile.Result{}, fmt.Errorf("failed to reconcile network for HetznerCluster %s/%s: %w", hetznerCluster.Namespace, hetznerCluster.Name, err)
 	}
 
@@ -191,10 +191,10 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 
 	// reconcile the placement groups
 	if err := placementgroup.NewService(clusterScope).Reconcile(ctx); err != nil {
-		conditions.MarkFalse(hetznerCluster, infrav1.PlacementGroupsSynced, infrav1.PlacementGroupsUnreachableReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(hetznerCluster, infrav1.PlacementGroupsSyncedCondition, infrav1.PlacementGroupsUnreachableReason, clusterv1.ConditionSeverityError, err.Error())
 		return reconcile.Result{}, fmt.Errorf("failed to reconcile placement groups for HetznerCluster %s/%s: %w", hetznerCluster.Namespace, hetznerCluster.Name, err)
 	}
-	conditions.MarkTrue(hetznerCluster, infrav1.PlacementGroupsSynced)
+	conditions.MarkTrue(hetznerCluster, infrav1.PlacementGroupsSyncedCondition)
 
 	if hetznerCluster.Spec.ControlPlaneLoadBalancer.Enabled {
 		if hetznerCluster.Status.ControlPlaneLoadBalancer.IPv4 != "<nil>" {
@@ -566,7 +566,7 @@ func (r *HetznerClusterReconciler) newTargetClusterManager(ctx context.Context,
 	clientConfig, err := clusterScope.ClientConfig(ctx)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			conditions.MarkFalse(hetznerCluster, infrav1.HetznerClusterTargetClusterReady, infrav1.KubeConfigNotFound, clusterv1.ConditionSeverityInfo, "kubeconfig not found (yet)")
+			conditions.MarkFalse(hetznerCluster, infrav1.HetznerClusterTargetClusterReadyCondition, infrav1.KubeConfigNotFoundReason, clusterv1.ConditionSeverityInfo, "kubeconfig not found (yet)")
 			return nil, nil
 		}
 		return nil, fmt.Errorf("failed to get a clientConfig for the API of HetznerCluster %s/%s: %w", hetznerCluster.Namespace, hetznerCluster.Name, err)
@@ -593,12 +593,12 @@ func (r *HetznerClusterReconciler) newTargetClusterManager(ctx context.Context,
 
 	// Check whether kubeapi server responds
 	if _, err := apiutil.NewDynamicRESTMapper(restConfig, httpClient); err != nil {
-		conditions.MarkFalse(hetznerCluster, infrav1.HetznerClusterTargetClusterReady, infrav1.KubeAPIServerNotResponding, clusterv1.ConditionSeverityInfo, "kubeapi server not responding (yet)")
+		conditions.MarkFalse(hetznerCluster, infrav1.HetznerClusterTargetClusterReadyCondition, infrav1.KubeAPIServerNotRespondingReason, clusterv1.ConditionSeverityInfo, "kubeapi server not responding (yet)")
 		return nil, nil //nolint:nilerr
 	}
 
 	// target cluster is ready
-	conditions.MarkTrue(hetznerCluster, infrav1.HetznerClusterTargetClusterReady)
+	conditions.MarkTrue(hetznerCluster, infrav1.HetznerClusterTargetClusterReadyCondition)
 
 	clusterMgr, err := ctrl.NewManager(
 		restConfig,
diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go
index 22da9be5..5ae5288c 100644
--- a/pkg/services/baremetal/host/host.go
+++ b/pkg/services/baremetal/host/host.go
@@ -251,12 +251,12 @@ func (s *Service) ensureSSHKey(sshSecretRef infrav1.SSHSecretRef, sshSecret *cor
 				msg := fmt.Sprintf("cannot upload ssh key %s - exists already under a different name", string(sshSecret.Data[sshSecretRef.Key.Name]))
 				conditions.MarkFalse(
 					s.scope.HetznerBareMetalHost,
-					infrav1.HetznerBareMetalHostReady,
-					infrav1.SSHKeyAlreadyExists,
+					infrav1.HetznerBareMetalHostReadyCondition,
+					infrav1.SSHKeyAlreadyExistsReason,
 					clusterv1.ConditionSeverityError,
 					msg,
 				)
-				record.Warnf(s.scope.HetznerBareMetalHost, infrav1.SSHKeyAlreadyExists, msg)
+				record.Warnf(s.scope.HetznerBareMetalHost, infrav1.SSHKeyAlreadyExistsReason, msg)
 				return infrav1.SSHKey{}, s.recordActionFailure(infrav1.FatalError, msg)
 			}
 			return infrav1.SSHKey{}, actionError{err: fmt.Errorf("failed to set ssh key: %w", err)}
diff --git a/pkg/services/baremetal/host/state_machine.go b/pkg/services/baremetal/host/state_machine.go
index 03c58d6f..735ff467 100644
--- a/pkg/services/baremetal/host/state_machine.go
+++ b/pkg/services/baremetal/host/state_machine.go
@@ -85,7 +85,7 @@ func (hsm *hostStateMachine) ReconcileState() (actionRes actionResult) {
 	}
 
 	// Set host ready now. This can be changed while the state is handled.
-	conditions.MarkTrue(hsm.host, infrav1.HetznerBareMetalHostReady)
+	conditions.MarkTrue(hsm.host, infrav1.HetznerBareMetalHostReadyCondition)
 
 	if stateHandler, found := hsm.handlers()[initialState]; found {
 		return stateHandler()
@@ -163,8 +163,8 @@ func (hsm *hostStateMachine) updateOSSSHStatusAndValidateKey(osSSHSecret *corev1
 	}
 	if err := validateSSHKey(osSSHSecret, hsm.host.Spec.Status.SSHSpec.SecretRef); err != nil {
 		msg := fmt.Sprintf("ssh credentials are invalid: %s", err.Error())
-		conditions.MarkFalse(hsm.host, infrav1.HetznerBareMetalHostReady, infrav1.SSHCredentialsInSecretInvalid, clusterv1.ConditionSeverityError, msg)
-		record.Warnf(hsm.host, infrav1.SSHKeyAlreadyExists, msg)
+		conditions.MarkFalse(hsm.host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.SSHCredentialsInSecretInvalidReason, clusterv1.ConditionSeverityError, msg)
+		record.Warnf(hsm.host, infrav1.SSHKeyAlreadyExistsReason, msg)
 		return hsm.reconciler.recordActionFailure(infrav1.PreparationError, infrav1.ErrorMessageMissingOrInvalidSecretData)
 	}
 	return nil
@@ -193,7 +193,7 @@ func (hsm *hostStateMachine) updateRescueSSHStatusAndValidateKey(rescueSSHSecret
 	}
 	if err := validateSSHKey(rescueSSHSecret, hsm.reconciler.scope.HetznerCluster.Spec.SSHKeys.RobotRescueSecretRef); err != nil {
 		msg := fmt.Sprintf("ssh credentials for rescue system are invalid: %s", err.Error())
-		conditions.MarkFalse(hsm.host, infrav1.HetznerBareMetalHostReady, infrav1.SSHCredentialsInSecretInvalid, clusterv1.ConditionSeverityError, msg)
+		conditions.MarkFalse(hsm.host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.SSHCredentialsInSecretInvalidReason, clusterv1.ConditionSeverityError, msg)
 		return hsm.reconciler.recordActionFailure(infrav1.PreparationError, infrav1.ErrorMessageMissingOrInvalidSecretData)
 	}
 	return nil
diff --git a/pkg/services/hcloud/network/network.go b/pkg/services/hcloud/network/network.go
index 52158945..61d0f4ba 100644
--- a/pkg/services/hcloud/network/network.go
+++ b/pkg/services/hcloud/network/network.go
@@ -62,7 +62,7 @@ func (s *Service) Reconcile(ctx context.Context) (err error) {
 		}
 	}
 
-	conditions.MarkTrue(s.scope.HetznerCluster, infrav1.NetworkAttached)
+	conditions.MarkTrue(s.scope.HetznerCluster, infrav1.NetworkAttachedCondition)
 	s.scope.HetznerCluster.Status.Network = statusFromHCloudNetwork(network)
 	return nil
 }


Test patch:
diff --git a/controllers/hetznerbaremetalhost_controller_test.go b/controllers/hetznerbaremetalhost_controller_test.go
index c368415e..686c21e6 100644
--- a/controllers/hetznerbaremetalhost_controller_test.go
+++ b/controllers/hetznerbaremetalhost_controller_test.go
@@ -724,7 +724,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 				if err := testEnv.Get(ctx, key, host); err != nil {
 					return false
 				}
-				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReady, infrav1.RescueSSHSecretMissing)
+				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.RescueSSHSecretMissingReason)
 			}, timeout).Should(BeTrue())
 		})
 
@@ -749,7 +749,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 				if err := testEnv.Get(ctx, key, host); err != nil {
 					return false
 				}
-				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReady, infrav1.SSHCredentialsInSecretInvalid)
+				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.SSHCredentialsInSecretInvalidReason)
 			}, timeout).Should(BeTrue())
 		})
 	})
@@ -782,7 +782,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 				if err := testEnv.Get(ctx, key, host); err != nil {
 					return false
 				}
-				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReady, infrav1.OSSSHSecretMissing)
+				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.OSSSHSecretMissingReason)
 			}, timeout).Should(BeTrue())
 		})
 
@@ -807,7 +807,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 				if err := testEnv.Get(ctx, key, host); err != nil {
 					return false
 				}
-				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReady, infrav1.SSHCredentialsInSecretInvalid)
+				return isPresentAndFalseWithReason(key, host, infrav1.HetznerBareMetalHostReadyCondition, infrav1.SSHCredentialsInSecretInvalidReason)
 			}, timeout).Should(BeTrue())
 		})
 	})
diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go
index 865ec0cf..41039e66 100644
--- a/controllers/hetznercluster_controller_test.go
+++ b/controllers/hetznercluster_controller_test.go
@@ -589,7 +589,7 @@ var _ = Describe("Hetzner ClusterReconciler", func() {
 					if err := testEnv.Get(ctx, key, instance); err != nil {
 						return false
 					}
-					return isPresentAndTrue(key, instance, infrav1.PlacementGroupsSynced)
+					return isPresentAndTrue(key, instance, infrav1.PlacementGroupsSyncedCondition)
 				}, timeout).Should(BeTrue())
 
 				By("checking for presence of HCloudPlacementGroup objects")
@@ -740,9 +740,9 @@ var _ = Describe("Hetzner ClusterReconciler", func() {
 						return false
 					}
 					if expectedConditionState {
-						return isPresentAndTrue(key, instance, infrav1.NetworkAttached)
+						return isPresentAndTrue(key, instance, infrav1.NetworkAttachedCondition)
 					}
-					return isPresentAndFalseWithReason(key, instance, infrav1.NetworkAttached, expectedReason)
+					return isPresentAndFalseWithReason(key, instance, infrav1.NetworkAttachedCondition, expectedReason)
 				}, timeout).Should(BeTrue())
 			},
 			Entry("with network", getDefaultHetznerClusterSpec(), true, ""),
@@ -883,7 +883,7 @@ var _ = Describe("Hetzner secret", func() {
 				if err := testEnv.Get(ctx, key, hetznerCluster); err != nil {
 					return false
 				}
-				return isPresentAndFalseWithReason(key, hetznerCluster, infrav1.HetznerClusterReady, expectedReason)
+				return isPresentAndFalseWithReason(key, hetznerCluster, infrav1.HetznerClusterReadyCondition, expectedReason)
 			}, timeout, time.Second).Should(BeTrue())
 		},
 		Entry("no Hetzner secret/wrong reference", corev1.Secret{