Repository: syself/cluster-api-provider-hetzner
Pull request #673 (2023-03-17 08:44:46)

:seedling: Make host.MaintenanceMode a pointer
**What this PR does / why we need it**:
Currently, we cannot distinguish cases where maintenanceMode is set and false from those once where it is not set. This creates some issues with e.g. ArgoCD. Therefore, we use a pointer to clearly distinguish the cases. If not set, the maintenanceMode is omitted in the Kubernetes object.

**TODOs**:
- [x] squashed commits
- [ ] includes documentation
- [ ] adds unit tests




Patch:
diff --git a/Makefile b/Makefile
index 2a81fa6f..e32a615e 100644
--- a/Makefile
+++ b/Makefile
@@ -258,7 +258,7 @@ create-workload-cluster-hcloud-network-packer: $(KUSTOMIZE) $(ENVSUBST) ## Creat
 create-workload-cluster-hetzner-hcloud-control-plane: $(KUSTOMIZE) $(ENVSUBST) ## Creates a workload-cluster. ENV Variables need to be exported or defined in the tilt-settings.json
 	# Create workload Cluster.
 	kubectl create secret generic hetzner --from-literal=hcloud=$(HCLOUD_TOKEN) --from-literal=robot-user=$(HETZNER_ROBOT_USER) --from-literal=robot-password=$(HETZNER_ROBOT_PASSWORD) --save-config --dry-run=client -o yaml | kubectl apply -f -
-	kubectl create secret generic robot-ssh --from-literal=sshkey-name=cluster --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
+	kubectl create secret generic robot-ssh --from-literal=sshkey-name=test --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
 	$(KUSTOMIZE) build templates/cluster-templates/hetzner-hcloud-control-planes --load-restrictor LoadRestrictionsNone  > templates/cluster-templates/cluster-template-hetzner-hcloud-control-planes.yaml
 	cat templates/cluster-templates/cluster-template-hetzner-hcloud-control-planes.yaml | $(ENVSUBST) - | kubectl apply -f -
 	$(MAKE) wait-and-get-secret
@@ -268,7 +268,7 @@ create-workload-cluster-hetzner-hcloud-control-plane: $(KUSTOMIZE) $(ENVSUBST) #
 create-workload-cluster-hetzner-baremetal-control-plane: $(KUSTOMIZE) $(ENVSUBST) ## Creates a workload-cluster. ENV Variables need to be exported or defined in the tilt-settings.json
 	# Create workload Cluster.
 	kubectl create secret generic hetzner --from-literal=hcloud=$(HCLOUD_TOKEN) --from-literal=robot-user=$(HETZNER_ROBOT_USER) --from-literal=robot-password=$(HETZNER_ROBOT_PASSWORD) --save-config --dry-run=client -o yaml | kubectl apply -f -
-	kubectl create secret generic robot-ssh --from-literal=sshkey-name=cluster --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
+	kubectl create secret generic robot-ssh --from-literal=sshkey-name=test --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
 	$(KUSTOMIZE) build templates/cluster-templates/hetzner-baremetal-control-planes --load-restrictor LoadRestrictionsNone  > templates/cluster-templates/cluster-template-hetzner-baremetal-control-planes.yaml
 	cat templates/cluster-templates/cluster-template-hetzner-baremetal-control-planes.yaml | $(ENVSUBST) - | kubectl apply -f -
 	$(MAKE) wait-and-get-secret
@@ -278,7 +278,7 @@ create-workload-cluster-hetzner-baremetal-control-plane: $(KUSTOMIZE) $(ENVSUBST
 create-workload-cluster-hetzner-baremetal-control-plane-remediation: $(KUSTOMIZE) $(ENVSUBST) ## Creates a workload-cluster. ENV Variables need to be exported or defined in the tilt-settings.json
 	# Create workload Cluster.
 	kubectl create secret generic hetzner --from-literal=hcloud=$(HCLOUD_TOKEN) --from-literal=robot-user=$(HETZNER_ROBOT_USER) --from-literal=robot-password=$(HETZNER_ROBOT_PASSWORD) --save-config --dry-run=client -o yaml | kubectl apply -f -
-	kubectl create secret generic robot-ssh --from-literal=sshkey-name=cluster --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
+	kubectl create secret generic robot-ssh --from-literal=sshkey-name=test --from-file=ssh-privatekey=${HETZNER_SSH_PRIV_PATH} --from-file=ssh-publickey=${HETZNER_SSH_PUB_PATH} --save-config --dry-run=client -o yaml | kubectl apply -f -
 	$(KUSTOMIZE) build templates/cluster-templates/hetzner-baremetal-control-planes-remediation --load-restrictor LoadRestrictionsNone  > templates/cluster-templates/cluster-template-hetzner-baremetal-control-planes-remediation.yaml
 	cat templates/cluster-templates/cluster-template-hetzner-baremetal-control-planes-remediation.yaml | $(ENVSUBST) - | kubectl apply -f -
 	$(MAKE) wait-and-get-secret
diff --git a/api/v1beta1/hetznerbaremetalhost_types.go b/api/v1beta1/hetznerbaremetalhost_types.go
index 1b61d9e5..b9df10c6 100644
--- a/api/v1beta1/hetznerbaremetalhost_types.go
+++ b/api/v1beta1/hetznerbaremetalhost_types.go
@@ -192,7 +192,7 @@ type HetznerBareMetalHostSpec struct {
 
 	// MaintenanceMode indicates that a machine is supposed to be deprovisioned
 	// and won't be selected by any Hetzner bare metal machine.
-	MaintenanceMode bool `json:"maintenanceMode,omitempty"`
+	MaintenanceMode *bool `json:"maintenanceMode,omitempty"`
 
 	// Description is a human-entered text used to help identify the host
 	// +optional
@@ -413,8 +413,7 @@ type HardwareDetails struct {
 }
 
 // HetznerBareMetalHostStatus defines the observed state of HetznerBareMetalHost.
-type HetznerBareMetalHostStatus struct {
-}
+type HetznerBareMetalHostStatus struct{}
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
index 4f961591..c9bd370f 100644
--- a/api/v1beta1/zz_generated.deepcopy.go
+++ b/api/v1beta1/zz_generated.deepcopy.go
@@ -522,6 +522,11 @@ func (in *HetznerBareMetalHostSpec) DeepCopyInto(out *HetznerBareMetalHostSpec)
 		*out = new(v1.ObjectReference)
 		**out = **in
 	}
+	if in.MaintenanceMode != nil {
+		in, out := &in.MaintenanceMode, &out.MaintenanceMode
+		*out = new(bool)
+		**out = **in
+	}
 	in.Status.DeepCopyInto(&out.Status)
 }
 
diff --git a/pkg/services/baremetal/baremetal/baremetal.go b/pkg/services/baremetal/baremetal/baremetal.go
index 0a701660..5b31e4b3 100644
--- a/pkg/services/baremetal/baremetal/baremetal.go
+++ b/pkg/services/baremetal/baremetal/baremetal.go
@@ -253,7 +253,7 @@ func (s *Service) update(ctx context.Context, log logr.Logger) error {
 		return fmt.Errorf("host not found for machine %s: %w", s.scope.Machine.Name, err)
 	}
 
-	if host.Spec.MaintenanceMode && s.scope.BareMetalMachine.Status.FailureReason == nil {
+	if host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode && s.scope.BareMetalMachine.Status.FailureReason == nil {
 		s.scope.BareMetalMachine.SetFailure(capierrors.UpdateMachineError, "host machine in maintenance mode")
 		record.Eventf(
 			s.scope.BareMetalMachine,
@@ -564,7 +564,7 @@ func (s *Service) chooseHost(ctx context.Context) (*infrav1.HetznerBareMetalHost
 		if host.Spec.ConsumerRef != nil {
 			continue
 		}
-		if host.Spec.MaintenanceMode {
+		if host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode {
 			continue
 		}
 		if host.GetDeletionTimestamp() != nil {


Test patch:
diff --git a/controllers/hetznerbaremetalmachine_controller_test.go b/controllers/hetznerbaremetalmachine_controller_test.go
index 721b9f06..6359c334 100644
--- a/controllers/hetznerbaremetalmachine_controller_test.go
+++ b/controllers/hetznerbaremetalmachine_controller_test.go
@@ -411,7 +411,8 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() {
 
 			ph, err := patch.NewHelper(host, testEnv)
 			Expect(err).ShouldNot(HaveOccurred())
-			host.Spec.MaintenanceMode = true
+			maintenanceMode := true
+			host.Spec.MaintenanceMode = &maintenanceMode
 			Expect(ph.Patch(ctx, host, patch.WithStatusObservedGeneration{})).To(Succeed())
 
 			// It sets a failure reason
diff --git a/pkg/services/baremetal/baremetal/baremetal_test.go b/pkg/services/baremetal/baremetal/baremetal_test.go
index 88febaaa..346662bc 100644
--- a/pkg/services/baremetal/baremetal/baremetal_test.go
+++ b/pkg/services/baremetal/baremetal/baremetal_test.go
@@ -31,7 +31,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
-
 )
 
 var _ = Describe("chooseHost", func() {
@@ -78,13 +77,14 @@ var _ = Describe("chooseHost", func() {
 		},
 	}
 
+	maintenanceMode := true
 	hostInMaintenanceMode := infrav1.HetznerBareMetalHost{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "hostInMaintenanceMode",
 			Namespace: defaultNamespace,
 		},
 		Spec: infrav1.HetznerBareMetalHostSpec{
-			MaintenanceMode: true,
+			MaintenanceMode: &maintenanceMode,
 			Status: infrav1.ControllerGeneratedStatus{
 				ProvisioningState: infrav1.StateNone,
 			},
@@ -188,7 +188,7 @@ var _ = Describe("chooseHost", func() {
 			Labels:    map[string]string{"key": "value"},
 		},
 		Spec: infrav1.HetznerBareMetalHostSpec{
-			MaintenanceMode: true,
+			MaintenanceMode: &maintenanceMode,
 			Status: infrav1.ControllerGeneratedStatus{
 				ProvisioningState: infrav1.StateNone,
 			},
@@ -512,7 +512,6 @@ var _ = Describe("Test deleteOwnerRefFromList", func() {
 
 	expectedRefList3 := make([]metav1.OwnerReference, 0, 2)
 	expectedRefList3 = append(expectedRefList3, metav1.OwnerReference{
-
 		Name:       "bm-machine2",
 		Kind:       "HetznerBareMetalMachine",
 		APIVersion: "v1beta1",
@@ -523,7 +522,6 @@ var _ = Describe("Test deleteOwnerRefFromList", func() {
 			refList, err := deleteOwnerRefFromList(tc.RefList, objectType, objectMeta)
 			Expect(err).To(Succeed())
 			Expect(refList).To(Equal(tc.ExpectedRefList))
-
 		},
 		Entry("List of one matching entry", testCaseFindOwnerRefFromList{
 			RefList: []metav1.OwnerReference{
@@ -590,7 +588,6 @@ var _ = Describe("Test setOwnerRefInList", func() {
 			refList, err := setOwnerRefInList(tc.RefList, tc.Controller, objectType, objectMeta)
 			Expect(err).To(Succeed())
 			Expect(refList).To(Equal(tc.ExpectedRefList))
-
 		},
 		Entry("List of one non-matching entry", testCaseSetOwnerRefInList{
 			RefList: []metav1.OwnerReference{