Repository: syself/cluster-api-provider-hetzner
Pull request #270 (2022-08-24 07:00:31)

:sparkles: Add condition for bootstrap ready
**What type of PR is this?**
/kind feature

**What this PR does / why we need it**:
It is not very clear, without looking at logs, that a machine is
currently waiting for its bootstrap data. In order to facilitate the
operator's job, we include this as a condition into the machine's
status.

**TODOs**:
- [x] squashed commits
- [ ] includes documentation
- [x] adds unit tests




Patch:
diff --git a/api/v1beta1/conditions_const.go b/api/v1beta1/conditions_const.go
index 500c762a..41fd8d3e 100644
--- a/api/v1beta1/conditions_const.go
+++ b/api/v1beta1/conditions_const.go
@@ -47,6 +47,13 @@ const (
 	InstanceAsControlPlaneUnreachableReason = "InstanceAsControlPlaneUnreachable"
 )
 
+const (
+	// InstanceBootstrapReadyCondition reports on current status of the instance. BootstrapReady indicates the bootstrap is ready.
+	InstanceBootstrapReadyCondition clusterv1.ConditionType = "InstanceBootstrapReady"
+	// InstanceBootstrapNotReadyReason bootstrap not ready yet.
+	InstanceBootstrapNotReadyReason = "InstanceBootstrapNotReady"
+)
+
 const (
 	// NetworkAttached reports on whether there is a network attached to the cluster.
 	NetworkAttached clusterv1.ConditionType = "NetworkAttached"
diff --git a/pkg/services/baremetal/baremetal/baremetal.go b/pkg/services/baremetal/baremetal/baremetal.go
index 2d9239e6..8ddc6191 100644
--- a/pkg/services/baremetal/baremetal/baremetal.go
+++ b/pkg/services/baremetal/baremetal/baremetal.go
@@ -93,9 +93,22 @@ func (s *Service) Reconcile(ctx context.Context) (_ *ctrl.Result, err error) {
 	// Make sure bootstrap data is available and populated. If not, return, we
 	// will get an event from the machine update when the flag is set to true.
 	if !s.scope.IsBootstrapReady(ctx) {
+		s.scope.V(1).Info("Bootstrap not ready - requeuing")
+		conditions.MarkFalse(
+			s.scope.BareMetalMachine,
+			infrav1.InstanceBootstrapReadyCondition,
+			infrav1.InstanceBootstrapNotReadyReason,
+			capi.ConditionSeverityInfo,
+			"bootstrap not ready yet",
+		)
 		return &ctrl.Result{}, nil
 	}
 
+	conditions.MarkTrue(
+		s.scope.BareMetalMachine,
+		infrav1.InstanceBootstrapReadyCondition,
+	)
+
 	errType := capierrors.CreateMachineError
 
 	// Check if the bareMetalmachine was associated with a baremetalhost
diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go
index b8fc0e54..c2d79dbb 100644
--- a/pkg/services/hcloud/server/server.go
+++ b/pkg/services/hcloud/server/server.go
@@ -69,10 +69,22 @@ func (s *Service) Reconcile(ctx context.Context) (_ *ctrl.Result, err error) {
 
 	// Waiting for bootstrap data to be ready
 	if !s.scope.IsBootstrapDataReady(ctx) {
-		s.scope.V(1).Info("Bootstrap not ready - requeuing")
+		s.scope.Info("Bootstrap not ready - requeuing")
+		conditions.MarkFalse(
+			s.scope.HCloudMachine,
+			infrav1.InstanceBootstrapReadyCondition,
+			infrav1.InstanceBootstrapNotReadyReason,
+			clusterv1.ConditionSeverityInfo,
+			"bootstrap not ready yet",
+		)
 		return &ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
+	conditions.MarkTrue(
+		s.scope.HCloudMachine,
+		infrav1.InstanceBootstrapReadyCondition,
+	)
+
 	// Try to find an existing server
 	server, err := s.findServer(ctx)
 	if err != nil {
@@ -92,7 +104,9 @@ func (s *Service) Reconcile(ctx context.Context) (_ *ctrl.Result, err error) {
 		)
 	}
 
+	c := s.scope.HCloudMachine.Status.Conditions.DeepCopy()
 	s.scope.HCloudMachine.Status = setStatusFromAPI(server)
+	s.scope.HCloudMachine.Status.Conditions = c
 
 	switch server.Status {
 	case hcloud.ServerStatusOff:


Test patch:
diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go
index 9e5c932b..411bda5b 100644
--- a/controllers/hcloudmachine_controller_test.go
+++ b/controllers/hcloudmachine_controller_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package controllers
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/hetznercloud/hcloud-go/hcloud"
@@ -48,6 +49,8 @@ var _ = Describe("HCloudMachineReconciler", func() {
 
 		hetznerSecret   *corev1.Secret
 		bootstrapSecret *corev1.Secret
+
+		key client.ObjectKey
 	)
 
 	BeforeEach(func() {
@@ -154,10 +157,11 @@ var _ = Describe("HCloudMachineReconciler", func() {
 
 			Expect(testEnv.Create(ctx, infraMachine)).To(Succeed())
 			Expect(testEnv.Create(ctx, infraCluster)).To(Succeed())
+
+			key = client.ObjectKey{Namespace: testNs.Name, Name: infraMachine.Name}
 		})
 
 		It("creates the infra machine", func() {
-			key := client.ObjectKey{Namespace: testNs.Name, Name: infraMachine.Name}
 			Eventually(func() bool {
 				if err := testEnv.Get(ctx, key, infraMachine); err != nil {
 					return false
@@ -179,6 +183,14 @@ var _ = Describe("HCloudMachineReconciler", func() {
 				return len(servers) == 0
 			}).Should(BeTrue())
 
+			// Check whether bootstrap condition is not ready
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, infraMachine); err != nil {
+					return false
+				}
+				return isPresentAndFalseWithReason(key, infraMachine, infrav1.InstanceBootstrapReadyCondition, infrav1.InstanceBootstrapNotReadyReason)
+			}, timeout, time.Second).Should(BeTrue())
+
 			By("setting the bootstrap data")
 			Eventually(func() error {
 				ph, err := patch.NewHelper(capiMachine, testEnv)
@@ -189,6 +201,16 @@ var _ = Describe("HCloudMachineReconciler", func() {
 				return ph.Patch(ctx, capiMachine, patch.WithStatusObservedGeneration{})
 			}, timeout, time.Second).Should(BeNil())
 
+			// Check whether bootstrap condition is ready
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, infraMachine); err != nil {
+					return false
+				}
+				objectCondition := conditions.Get(infraMachine, infrav1.InstanceBootstrapReadyCondition)
+				fmt.Println(objectCondition)
+				return isPresentAndTrue(key, infraMachine, infrav1.InstanceBootstrapReadyCondition)
+			}, timeout, time.Second).Should(BeTrue())
+
 			Eventually(func() int {
 				servers, err := hcloudClient.ListServers(ctx, hcloud.ServerListOpts{
 					ListOpts: hcloud.ListOpts{
diff --git a/controllers/hetznerbaremetalmachine_controller_test.go b/controllers/hetznerbaremetalmachine_controller_test.go
index 5abd4791..89c526ff 100644
--- a/controllers/hetznerbaremetalmachine_controller_test.go
+++ b/controllers/hetznerbaremetalmachine_controller_test.go
@@ -177,6 +177,100 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() {
 			hetznerSecret, rescueSSHSecret, osSSHSecret, bootstrapSecret)).To(Succeed())
 	})
 
+	Context("Basic test", func() {
+		BeforeEach(func() {
+			capiMachine = &clusterv1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "capi-machine-",
+					Namespace:    testNs.Name,
+					Finalizers:   []string{clusterv1.MachineFinalizer},
+					Labels: map[string]string{
+						clusterv1.ClusterLabelName: capiCluster.Name,
+					},
+				},
+				Spec: clusterv1.MachineSpec{
+					ClusterName: capiCluster.Name,
+					InfrastructureRef: corev1.ObjectReference{
+						APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
+						Kind:       "HetznerBareMetalMachine",
+						Name:       bmMachineName,
+					},
+					FailureDomain: &defaultFailureDomain,
+				},
+			}
+			Expect(testEnv.Create(ctx, capiMachine)).To(Succeed())
+
+			bmMachine = &infrav1.HetznerBareMetalMachine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      bmMachineName,
+					Namespace: testNs.Name,
+					Labels: map[string]string{
+						clusterv1.ClusterLabelName: capiCluster.Name,
+					},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "cluster.x-k8s.io/v1beta1",
+							Kind:       "Machine",
+							Name:       capiMachine.Name,
+							UID:        capiMachine.UID,
+						},
+					},
+				},
+				Spec: getDefaultHetznerBareMetalMachineSpec(),
+			}
+			Expect(testEnv.Create(ctx, bmMachine)).To(Succeed())
+
+			key = client.ObjectKey{Namespace: testNs.Name, Name: bmMachineName}
+		})
+
+		It("creates the bare metal machine and provisions host", func() {
+			// Check whether bootstrap condition is not ready
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, bmMachine); err != nil {
+					return false
+				}
+				return isPresentAndFalseWithReason(key, bmMachine, infrav1.InstanceBootstrapReadyCondition, infrav1.InstanceBootstrapNotReadyReason)
+			}, timeout, time.Second).Should(BeTrue())
+
+			Eventually(func() error {
+				ph, err := patch.NewHelper(capiMachine, testEnv)
+				Expect(err).ShouldNot(HaveOccurred())
+				capiMachine.Spec.Bootstrap = clusterv1.Bootstrap{
+					DataSecretName: pointer.String("bootstrap-secret"),
+				}
+				return ph.Patch(ctx, capiMachine, patch.WithStatusObservedGeneration{})
+			}, timeout, time.Second).Should(BeNil())
+
+			// Check whether bootstrap condition is ready
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, bmMachine); err != nil {
+					return false
+				}
+				return isPresentAndTrue(key, bmMachine, infrav1.InstanceBootstrapReadyCondition)
+			}, timeout, time.Second).Should(BeTrue())
+
+			defer func() {
+				Expect(testEnv.Delete(ctx, bmMachine)).To(Succeed())
+			}()
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, hostKey, host); err != nil {
+					return false
+				}
+				if host.Spec.Status.ProvisioningState == infrav1.StateProvisioned {
+					return true
+				}
+				return false
+			}, timeout).Should(BeTrue())
+
+			Eventually(func() bool {
+				if err := testEnv.Get(ctx, key, bmMachine); err != nil {
+					return false
+				}
+				return bmMachine.Status.Ready
+			}, timeout).Should(BeTrue())
+		})
+	})
+
 	Context("Basic test", func() {
 		BeforeEach(func() {
 			capiMachine = &clusterv1.Machine{