diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index 08b7d333b..a26c6235a 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -44,6 +44,8 @@ jobs: # if there's a diff then the workflow will exit here. - name: Run make verify run: make BUILD_IN_CONTAINER=false verify + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Lint Golang Code run: make BUILD_IN_CONTAINER=false lint-golang-ci diff --git a/Makefile b/Makefile index 72887dc58..0c3e3e303 100644 --- a/Makefile +++ b/Makefile @@ -762,6 +762,7 @@ ifeq ($(BUILD_IN_CONTAINER),true) $(BUILDER_IMAGE):$(BUILDER_IMAGE_VERSION) $@; else @lychee --version + @if [ -z "$${GITHUB_TOKEN}" ]; then echo "GITHUB_TOKEN is not set"; exit 1; fi lychee --verbose --config .lychee.toml ./*.md ./docs/**/*.md 2>&1 | grep -vP '\[(200|EXCLUDED)\]' endif diff --git a/api/v1beta1/conditions_const.go b/api/v1beta1/conditions_const.go index 9b5111199..8a1e8bbfb 100644 --- a/api/v1beta1/conditions_const.go +++ b/api/v1beta1/conditions_const.go @@ -238,3 +238,15 @@ const ( // RebootSucceededCondition indicates that the machine got rebooted successfully. RebootSucceededCondition clusterv1.ConditionType = "RebootSucceeded" ) + +const ( + // NoRemediateMachineAnnotationCondition is: + // - False when the corresponding CAPI Machine has the "cluster.x-k8s.io/remediate-machine" annotation set and will be remediated by CAPI soon. + // - True otherwise. + NoRemediateMachineAnnotationCondition clusterv1.ConditionType = "NoRemediateMachineAnnotation" + + // RemediateMachineAnnotationIsSetReason indicates that the CAPI machine has the + // "cluster.x-k8s.io/remediate-machine" annotation set. The CAPI machine and the corresponding + // infra-machine will be deleted by CAPI soon. + RemediateMachineAnnotationIsSetReason = "RemediateMachineAnnotationIsSet" +) diff --git a/api/v1beta1/hetznerbaremetalhost_types.go b/api/v1beta1/hetznerbaremetalhost_types.go index 95c40ab17..716df9435 100644 --- a/api/v1beta1/hetznerbaremetalhost_types.go +++ b/api/v1beta1/hetznerbaremetalhost_types.go @@ -120,8 +120,10 @@ const ( // RegistrationError is an error condition occurring when the // controller is unable to retrieve information on a specific server via robot. RegistrationError ErrorType = "registration error" + // PreparationError is an error condition occurring when something fails while preparing host reconciliation. PreparationError ErrorType = "preparation error" + // ProvisioningError is an error condition occurring when the controller // fails to provision or deprovision the Host. ProvisioningError ErrorType = "provisioning error" @@ -230,8 +232,9 @@ type HetznerBareMetalHostSpec struct { // +optional ConsumerRef *corev1.ObjectReference `json:"consumerRef,omitempty"` - // MaintenanceMode indicates that a machine is supposed to be deprovisioned - // and won't be selected by any Hetzner bare metal machine. + // MaintenanceMode indicates that a machine is supposed to be deprovisioned. The CAPI Machine + // will get the cluster.x-k8s.io/remediate-machine annotation, and CAPI will deprovision the + // machine. Accordingly, the host won't be selected by any Hetzner bare metal machine. MaintenanceMode *bool `json:"maintenanceMode,omitempty"` // Description is a human-entered text used to help identify the host. diff --git a/api/v1beta1/hetznerbaremetalmachine_types.go b/api/v1beta1/hetznerbaremetalmachine_types.go index a671c9806..ed46bc744 100644 --- a/api/v1beta1/hetznerbaremetalmachine_types.go +++ b/api/v1beta1/hetznerbaremetalmachine_types.go @@ -25,7 +25,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/selection" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck // we will handle that, when we update to capi v1.11 ) const ( @@ -298,8 +297,11 @@ type HetznerBareMetalMachineStatus struct { LastUpdated *metav1.Time `json:"lastUpdated,omitempty"` // FailureReason will be set in the event that there is a terminal problem. + // + // Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details. + // // +optional - FailureReason *capierrors.MachineStatusError `json:"failureReason,omitempty"` + FailureReason *string `json:"failureReason,omitempty"` // FailureMessage will be set in the event that there is a terminal problem. // +optional @@ -357,12 +359,6 @@ func (hbmm *HetznerBareMetalMachine) SetConditions(conditions clusterv1.Conditio hbmm.Status.Conditions = conditions } -// SetFailure sets a failure reason and message. -func (hbmm *HetznerBareMetalMachine) SetFailure(reason capierrors.MachineStatusError, message string) { - hbmm.Status.FailureReason = &reason - hbmm.Status.FailureMessage = &message -} - // GetImageSuffix tests whether the suffix is known and outputs it if yes. Otherwise it returns an error. func GetImageSuffix(url string) (string, error) { if strings.HasPrefix(url, "oci://") { diff --git a/api/v1beta1/hetznerbaremetalmachine_types_test.go b/api/v1beta1/hetznerbaremetalmachine_types_test.go index b00934ea2..b8b815686 100644 --- a/api/v1beta1/hetznerbaremetalmachine_types_test.go +++ b/api/v1beta1/hetznerbaremetalmachine_types_test.go @@ -23,7 +23,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/require" - capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck // we will handle that, when we update to capi v1.11 ) var _ = Describe("Test Image.GetDetails", func() { @@ -177,35 +176,6 @@ var _ = Describe("Test GetImageSuffix", func() { ) }) -var _ = Describe("Test SetFailure", func() { - bmMachine := HetznerBareMetalMachine{} - newFailureMessage := "bad error" - newFailureReason := capierrors.CreateMachineError - - It("sets new failure on the machine with existing failure", func() { - failureMessage := "first message" - failureReason := capierrors.MachineStatusError("first error") - bmMachine.Status.FailureMessage = &failureMessage - bmMachine.Status.FailureReason = &failureReason - - bmMachine.SetFailure(newFailureReason, newFailureMessage) - - Expect(bmMachine.Status.FailureMessage).ToNot(BeNil()) - Expect(bmMachine.Status.FailureReason).ToNot(BeNil()) - Expect(*bmMachine.Status.FailureMessage).To(Equal(newFailureMessage)) - Expect(*bmMachine.Status.FailureReason).To(Equal(newFailureReason)) - }) - - It("sets new failure on the machine without existing failure", func() { - bmMachine.SetFailure(newFailureReason, newFailureMessage) - - Expect(bmMachine.Status.FailureMessage).ToNot(BeNil()) - Expect(bmMachine.Status.FailureReason).ToNot(BeNil()) - Expect(*bmMachine.Status.FailureMessage).To(Equal(newFailureMessage)) - Expect(*bmMachine.Status.FailureReason).To(Equal(newFailureReason)) - }) -}) - var _ = Describe("Test HasHostAnnotation", func() { type testCaseHasHostAnnotation struct { annotations map[string]string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index ab12dee41..7d1fa5b2d 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -896,7 +896,7 @@ func (in *HetznerBareMetalMachineStatus) DeepCopyInto(out *HetznerBareMetalMachi } if in.FailureReason != nil { in, out := &in.FailureReason, &out.FailureReason - *out = new(errors.MachineStatusError) + *out = new(string) **out = **in } if in.FailureMessage != nil { diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalhosts.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalhosts.yaml index e347061c3..8ad6d2bfa 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalhosts.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalhosts.yaml @@ -133,8 +133,9 @@ spec: type: string maintenanceMode: description: |- - MaintenanceMode indicates that a machine is supposed to be deprovisioned - and won't be selected by any Hetzner bare metal machine. + MaintenanceMode indicates that a machine is supposed to be deprovisioned. The CAPI Machine + will get the cluster.x-k8s.io/remediate-machine annotation, and CAPI will deprovision the + machine. Accordingly, the host won't be selected by any Hetzner bare metal machine. type: boolean rootDeviceHints: description: |- diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml index 9d7cfe261..9ecd6cfe5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml @@ -399,8 +399,10 @@ spec: a terminal problem. type: string failureReason: - description: FailureReason will be set in the event that there is - a terminal problem. + description: |- + FailureReason will be set in the event that there is a terminal problem. + + Deprecated: This field is deprecated and is going to be removed when support for v1beta1 will be dropped. Please see https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20240916-improve-status-in-CAPI-resources.md for more details. type: string lastUpdated: description: LastUpdated identifies when this status was last observed. diff --git a/controllers/hetznerbaremetalhost_controller.go b/controllers/hetznerbaremetalhost_controller.go index 74d2ca0d3..61a7bf1e1 100644 --- a/controllers/hetznerbaremetalhost_controller.go +++ b/controllers/hetznerbaremetalhost_controller.go @@ -185,6 +185,9 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl return res, nil } + // Case "Delete" was handled in reconcileSelectedStates. From now we know that the host has not + // DeletionTimestamp set. + hetznerCluster := &infrav1.HetznerCluster{} hetznerClusterName := client.ObjectKey{ @@ -226,9 +229,21 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl } log = log.WithValues("HetznerBareMetalMachine", klog.KObj(hetznerBareMetalMachine)) - ctx = ctrl.LoggerInto(ctx, log) + remediateConditionOfHbmm := conditions.Get(hetznerBareMetalMachine, infrav1.NoRemediateMachineAnnotationCondition) + if remediateConditionOfHbmm != nil && remediateConditionOfHbmm.Status == corev1.ConditionFalse { + // The hbmm of this host is in remediation. Do not reconcile it. + // Take the Condition of the hbmm and make it available on the hbmh. + msg := "hbmm has NoRemediateMachineAnnotationCondition=False. Not reconciling this host." + log.Info(msg) + conditions.MarkFalse(bmHost, infrav1.NoRemediateMachineAnnotationCondition, + remediateConditionOfHbmm.Reason, remediateConditionOfHbmm.Severity, + "%s", remediateConditionOfHbmm.Message) + return reconcile.Result{}, nil + } + conditions.MarkTrue(bmHost, infrav1.NoRemediateMachineAnnotationCondition) + // Get Hetzner robot api credentials secretManager := secretutil.NewSecretManager(log, r.Client, r.APIReader) robotCreds, err := getAndValidateRobotCredentials(ctx, req.Namespace, hetznerCluster, secretManager) @@ -307,7 +322,7 @@ func (r *HetznerBareMetalHostReconciler) reconcileSelectedStates(ctx context.Con return ctrl.Result{RequeueAfter: 10 * time.Second}, nil - // Handle StateDeleting + // Handle StateDeleting case infrav1.StateDeleting: if controllerutil.RemoveFinalizer(bmHost, infrav1.HetznerBareMetalHostFinalizer) || controllerutil.RemoveFinalizer(bmHost, infrav1.DeprecatedBareMetalHostFinalizer) { diff --git a/controllers/hetznerbaremetalmachine_controller.go b/controllers/hetznerbaremetalmachine_controller.go index 084ad06fa..8c02f6741 100644 --- a/controllers/hetznerbaremetalmachine_controller.go +++ b/controllers/hetznerbaremetalmachine_controller.go @@ -73,20 +73,20 @@ func (r *HetznerBareMetalMachineReconciler) Reconcile(ctx context.Context, req r log = log.WithValues("HetznerBareMetalMachine", klog.KObj(hbmMachine)) // Fetch the Machine. - machine, err := util.GetOwnerMachine(ctx, r.Client, hbmMachine.ObjectMeta) + capiMachine, err := util.GetOwnerMachine(ctx, r.Client, hbmMachine.ObjectMeta) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to get owner machine. BareMetalMachine.ObjectMeta.OwnerReferences %v: %w", hbmMachine.ObjectMeta.OwnerReferences, err) } - if machine == nil { + if capiMachine == nil { log.Info("Machine Controller has not yet set OwnerRef") return reconcile.Result{}, nil } - log = log.WithValues("Machine", klog.KObj(machine)) + log = log.WithValues("Machine", klog.KObj(capiMachine)) // Fetch the Cluster. - cluster, err := util.GetClusterFromMetadata(ctx, r.Client, machine.ObjectMeta) + cluster, err := util.GetClusterFromMetadata(ctx, r.Client, capiMachine.ObjectMeta) if err != nil { log.Info("Machine is missing cluster label or cluster does not exist") return reconcile.Result{}, nil @@ -125,7 +125,7 @@ func (r *HetznerBareMetalMachineReconciler) Reconcile(ctx context.Context, req r machineScope, err := scope.NewBareMetalMachineScope(scope.BareMetalMachineScopeParams{ Client: r.Client, Logger: log, - Machine: machine, + Machine: capiMachine, BareMetalMachine: hbmMachine, HetznerCluster: hetznerCluster, HCloudClient: hcc, diff --git a/controllers/hetznerbaremetalmachine_controller_test.go b/controllers/hetznerbaremetalmachine_controller_test.go index 8dfc1afa9..3c5b7edfc 100644 --- a/controllers/hetznerbaremetalmachine_controller_test.go +++ b/controllers/hetznerbaremetalmachine_controller_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "fmt" "testing" "time" @@ -36,13 +37,14 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" - "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/baremetal" robotmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/robot" sshmock "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/mocks/ssh" sshclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/ssh" @@ -411,7 +413,7 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { }, timeout, time.Second).Should(BeTrue()) }) - It("sets a failure reason when maintenance mode is set on the host", func() { + It("sets RemediateMachineAnnotation when maintenance mode is set on the host", func() { By("making sure that machine is ready") Eventually(func() bool { @@ -431,14 +433,34 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() { Expect(ph.Patch(ctx, host, patch.WithStatusObservedGeneration{})).To(Succeed()) - By("checking that failure message is set on machine") + By("checking that RemediateMachineAnnotation is set on machine") - Eventually(func() bool { + Eventually(func() error { if err := testEnv.Get(ctx, key, bmMachine); err != nil { - return false + return err } - return bmMachine.Status.FailureMessage != nil && *bmMachine.Status.FailureMessage == baremetal.FailureMessageMaintenanceMode - }, timeout).Should(BeTrue()) + + capiMachine, err := util.GetOwnerMachine(ctx, testEnv, bmMachine.ObjectMeta) + if err != nil { + return err + } + + _, exists := capiMachine.Annotations[clusterv1.RemediateMachineAnnotation] + if !exists { + return fmt.Errorf("RemediateMachineAnnotation not set on capi machine") + } + + c := conditions.Get(bmMachine, infrav1.NoRemediateMachineAnnotationCondition) + if c == nil { + return fmt.Errorf("condition NoRemediateMachineAnnotationCondition does not exist") + } + + if c.Status != corev1.ConditionFalse { + return fmt.Errorf("condition NoRemediateMachineAnnotationCondition should be False") + } + + return nil + }, timeout).Should(Succeed()) }) It("checks the hetznerBareMetalMachine status running phase", func() { diff --git a/pkg/baremetalutils/baremetalutils.go b/pkg/baremetalutils/baremetalutils.go new file mode 100644 index 000000000..3d6e20b6e --- /dev/null +++ b/pkg/baremetalutils/baremetalutils.go @@ -0,0 +1,67 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package baremetalutils implements helper functions for working with baremetal. +package baremetalutils + +import ( + "context" + "fmt" + "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" + + infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" +) + +func splitHostKey(key string) (namespace, name string) { + parts := strings.Split(key, "/") + if len(parts) != 2 { + panic("unexpected host key") + } + return parts[0], parts[1] +} + +// GetAssociatedHost gets the associated host by looking for an annotation on the +// machine that contains a reference to the host. Returns nil if not found. Assumes the host is in +// the same namespace as the machine. +func GetAssociatedHost(ctx context.Context, crClient client.Client, hbmm *infrav1.HetznerBareMetalMachine) (*infrav1.HetznerBareMetalHost, error) { + annotations := hbmm.GetAnnotations() + // if no annotations exist on machine, no host can be associated + if annotations == nil { + return nil, nil + } + + // check if host annotation is set and return if not + hostKey, ok := annotations[infrav1.HostAnnotation] + if !ok { + return nil, nil + } + + // find associated host object and return it + hostNamespace, hostName := splitHostKey(hostKey) + + host := &infrav1.HetznerBareMetalHost{} + key := client.ObjectKey{ + Name: hostName, + Namespace: hostNamespace, + } + + if err := crClient.Get(ctx, key, host); err != nil { + return nil, fmt.Errorf("failed to get host object: %w", err) + } + return host, nil +} diff --git a/pkg/baremetalutils/baremetalutils_test.go b/pkg/baremetalutils/baremetalutils_test.go new file mode 100644 index 000000000..ed81f537e --- /dev/null +++ b/pkg/baremetalutils/baremetalutils_test.go @@ -0,0 +1,28 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package baremetalutils + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Test splitHostKey", func() { + namespace, name := splitHostKey("namespace/name") + Expect(namespace).To(Equal("namespace")) + Expect(name).To(Equal("name")) +}) diff --git a/pkg/scope/baremetalmachine.go b/pkg/scope/baremetalmachine.go index 8b8ebc354..fa65b225f 100644 --- a/pkg/scope/baremetalmachine.go +++ b/pkg/scope/baremetalmachine.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/record" "sigs.k8s.io/controller-runtime/pkg/client" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" @@ -45,7 +46,7 @@ type BareMetalMachineScopeParams struct { // This is meant to be called for each reconcile iteration. func NewBareMetalMachineScope(params BareMetalMachineScopeParams) (*BareMetalMachineScope, error) { if params.Client == nil { - return nil, fmt.Errorf("cannot create baremetal host scope without client") + return nil, fmt.Errorf("cannot create baremetal machine scope without client") } if params.Machine == nil { return nil, fmt.Errorf("failed to generate new scope from nil Machine") @@ -123,3 +124,33 @@ func (m *BareMetalMachineScope) IsControlPlane() bool { func (m *BareMetalMachineScope) IsBootstrapReady() bool { return m.Machine.Spec.Bootstrap.DataSecretName != nil } + +// SetErrorAndRemediate sets "cluster.x-k8s.io/remediate-machine" annotation on the corresponding +// CAPI machine. CAPI will remediate that machine. Additionally, an event of type Warning will be +// created, and the condition will be set on both the BareMetalMachine and the corresponding +// HetznerBareMetalHost (if found). The Condition NoRemediateMachineAnnotationCondition will be set +// on the hbmm. +func (m *BareMetalMachineScope) SetErrorAndRemediate(ctx context.Context, message string) error { + obj := m.Machine + + // Create a patch base + patch := client.MergeFrom(obj.DeepCopy()) + + // Modify only annotations on the in-memory copy + if obj.Annotations == nil { + obj.Annotations = map[string]string{} + } + obj.Annotations[clusterv1.RemediateMachineAnnotation] = "" + + // Apply patch – only the diff (annotations) is sent to the API server + if err := m.Client.Patch(ctx, obj, patch); err != nil { + return err + } + + record.Warnf(m.BareMetalMachine, "HetznerBareMetalMachineWillBeRemediated", + "HetznerBareMetalMachine will be remediated: %s", message) + + conditions.MarkFalse(m.BareMetalMachine, infrav1.NoRemediateMachineAnnotationCondition, + infrav1.RemediateMachineAnnotationIsSetReason, clusterv1.ConditionSeverityInfo, "%s", message) + return nil +} diff --git a/pkg/services/baremetal/baremetal/baremetal.go b/pkg/services/baremetal/baremetal/baremetal.go index 50a8f9275..5a9f8e638 100644 --- a/pkg/services/baremetal/baremetal/baremetal.go +++ b/pkg/services/baremetal/baremetal/baremetal.go @@ -39,7 +39,6 @@ import ( "k8s.io/apimachinery/pkg/selection" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - capierrors "sigs.k8s.io/cluster-api/errors" //nolint:staticcheck // we will handle that, when we update to capi v1.11 "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/record" @@ -47,6 +46,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" + "github.com/syself/cluster-api-provider-hetzner/pkg/baremetalutils" "github.com/syself/cluster-api-provider-hetzner/pkg/scope" hcloudutil "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/util" "github.com/syself/cluster-api-provider-hetzner/pkg/utils" @@ -129,9 +129,9 @@ func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err erro } // Delete implements delete method of bare metal machine. -func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) { +func (s *Service) Delete(ctx context.Context) (reconcile.Result, error) { // get host - ignore if not found - host, helper, err := s.getAssociatedHost(ctx) + host, helper, err := s.getAssociatedHostAndPatchHelper(ctx) if err != nil && !apierrors.IsNotFound(err) { return reconcile.Result{}, fmt.Errorf("failed to get associated host: %w", err) } @@ -151,7 +151,7 @@ func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) // remove the ownerRef to this host, even if the consumerRef references another machine host.OwnerReferences = s.removeOwnerRef(host.OwnerReferences) - return res, nil + return reconcile.Result{}, nil } if removeMachineSpecsFromHost(host) { @@ -165,6 +165,8 @@ func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) // check if deprovisioning is done if host.Spec.Status.ProvisioningState != infrav1.StateNone { + s.scope.Logger.Info("hbmm is deleting, but host is not deprovisioned yet. Requeueing", + "ProvisioningState", host.Spec.Status.ProvisioningState) return reconcile.Result{RequeueAfter: requeueAfter}, nil } @@ -198,25 +200,26 @@ func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) ) s.scope.BareMetalMachine.Status.Phase = clusterv1.MachinePhaseDeleted - return res, nil + return reconcile.Result{}, nil } // update updates a machine and is invoked by the Machine Controller. func (s *Service) update(ctx context.Context) error { - host, helper, err := s.getAssociatedHost(ctx) + host, helper, err := s.getAssociatedHostAndPatchHelper(ctx) if err != nil { return fmt.Errorf("failed to get host: %w", err) } if host == nil { - s.scope.BareMetalMachine.SetFailure(capierrors.UpdateMachineError, "host not found") + err = errors.Join(s.scope.SetErrorAndRemediate(ctx, "Reconcile of hbmm: host not found")) return fmt.Errorf("host not found for machine %s: %w", s.scope.Machine.Name, err) } readyCondition := conditions.Get(host, clusterv1.ReadyCondition) if readyCondition != nil { - if readyCondition.Status == corev1.ConditionTrue { + switch readyCondition.Status { + case corev1.ConditionTrue: conditions.MarkTrue(s.scope.BareMetalMachine, infrav1.HostReadyCondition) - } else if readyCondition.Status == corev1.ConditionFalse { + case corev1.ConditionFalse: conditions.MarkFalse( s.scope.BareMetalMachine, infrav1.HostReadyCondition, @@ -229,30 +232,23 @@ func (s *Service) update(ctx context.Context) error { } // maintenance mode on the host is a fatal error for the machine object - if host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode && s.scope.BareMetalMachine.Status.FailureReason == nil { - s.scope.BareMetalMachine.SetFailure(capierrors.UpdateMachineError, FailureMessageMaintenanceMode) - record.Eventf( - s.scope.BareMetalMachine, - "BareMetalMachineSetFailure", - "set failure reason due to maintenance mode of underlying host", - ) + if host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode { + err := s.scope.SetErrorAndRemediate(ctx, FailureMessageMaintenanceMode) + if err != nil { + return err + } return nil } - // if host has a fatal error, then it should be set on the machine object as well - if (host.Spec.Status.ErrorType == infrav1.FatalError || host.Spec.Status.ErrorType == infrav1.PermanentError) && - s.scope.BareMetalMachine.Status.FailureReason == nil { - s.scope.BareMetalMachine.SetFailure(capierrors.UpdateMachineError, host.Spec.Status.ErrorMessage) - record.Eventf(s.scope.BareMetalMachine, "BareMetalMachineSetFailure", host.Spec.Status.ErrorMessage) + // if host has a fatal error, then it should be set on the hbmm object as well + if host.Spec.Status.ErrorType == infrav1.FatalError || host.Spec.Status.ErrorType == infrav1.PermanentError { + err := s.scope.SetErrorAndRemediate(ctx, host.Spec.Status.ErrorMessage) + if err != nil { + return err + } return nil } - // if host is healthy, the machine is healthy as well - if host.Spec.Status.ErrorType == infrav1.ErrorType("") { - s.scope.BareMetalMachine.Status.FailureMessage = nil - s.scope.BareMetalMachine.Status.FailureReason = nil - } - // ensure that the references are correctly set on host s.setReferencesOnHost(host) @@ -283,7 +279,7 @@ func (s *Service) update(ctx context.Context) error { func (s *Service) associate(ctx context.Context) error { // look for associated host - associatedHost, _, err := s.getAssociatedHost(ctx) + associatedHost, _, err := s.getAssociatedHostAndPatchHelper(ctx) if err != nil { return fmt.Errorf("failed to get associated host: %w", err) } @@ -352,41 +348,25 @@ func (s *Service) associate(ctx context.Context) error { return nil } -// getAssociatedHost gets the associated host by looking for an annotation on the machine -// that contains a reference to the host. Returns nil if not found. Assumes the -// host is in the same namespace as the machine. -func (s *Service) getAssociatedHost(ctx context.Context) (*infrav1.HetznerBareMetalHost, *patch.Helper, error) { - annotations := s.scope.BareMetalMachine.ObjectMeta.GetAnnotations() - // if no annotations exist on machine, no host can be associated - if annotations == nil { - return nil, nil, nil +// getAssociatedHostAndPatchHelper gets the associated host by looking for an annotation on the +// machine that contains a reference to the host. Returns nil if not found. Assumes the host is in +// the same namespace as the machine. Additionally, a PatchHelper gets returned. +func (s *Service) getAssociatedHostAndPatchHelper(ctx context.Context) (*infrav1.HetznerBareMetalHost, *patch.Helper, error) { + host, err := baremetalutils.GetAssociatedHost(ctx, s.scope.Client, s.scope.BareMetalMachine) + if err != nil { + return nil, nil, err } - // check if host annotation is set and return if not - hostKey, ok := annotations[infrav1.HostAnnotation] - if !ok { + if host == nil { return nil, nil, nil } - // find associated host object and return it - hostNamespace, hostName := splitHostKey(hostKey) - - host := infrav1.HetznerBareMetalHost{} - key := client.ObjectKey{ - Name: hostName, - Namespace: hostNamespace, - } - - if err := s.scope.Client.Get(ctx, key, &host); err != nil { - return nil, nil, fmt.Errorf("failed to get host object: %w", err) - } - - helper, err := patch.NewHelper(&host, s.scope.Client) + helper, err := patch.NewHelper(host, s.scope.Client) if err != nil { return nil, nil, fmt.Errorf("failed to create patch helper: %w", err) } - return &host, helper, nil + return host, helper, nil } // ChooseHost tries to find a free hbmh. @@ -668,14 +648,17 @@ func (s *Service) setProviderID(ctx context.Context) error { } // providerID is based on the ID of the host machine - host, _, err := s.getAssociatedHost(ctx) + host, _, err := s.getAssociatedHostAndPatchHelper(ctx) if err != nil { return fmt.Errorf("failed to get host: %w", err) } if host == nil { - s.scope.BareMetalMachine.SetFailure(capierrors.UpdateMachineError, "host not found") - return fmt.Errorf("host not found for machine %s: %w", s.scope.Machine.Name, err) + err := s.scope.SetErrorAndRemediate(ctx, "setProviderID failed: host not found") + if err != nil { + return err + } + return fmt.Errorf("host not found for machine %q", s.scope.Machine.Name) } if host.Spec.Status.ProvisioningState != infrav1.StateProvisioned { @@ -880,14 +863,6 @@ func hostKey(host *infrav1.HetznerBareMetalHost) string { return host.GetNamespace() + "/" + host.GetName() } -func splitHostKey(key string) (namespace, name string) { - parts := strings.Split(key, "/") - if len(parts) != 2 { - panic("unexpected host key") - } - return parts[0], parts[1] -} - func checkForRequeueError(err error, errMessage string) (res reconcile.Result, reterr error) { if err == nil { return res, nil diff --git a/pkg/services/baremetal/baremetal/baremetal_test.go b/pkg/services/baremetal/baremetal/baremetal_test.go index 6bbb0e20c..cea63ffdd 100644 --- a/pkg/services/baremetal/baremetal/baremetal_test.go +++ b/pkg/services/baremetal/baremetal/baremetal_test.go @@ -714,12 +714,6 @@ var _ = Describe("Test hostKey", func() { Expect(hostKey(host)).To(Equal("namespace/name")) }) -var _ = Describe("Test splitHostKey", func() { - namespace, name := splitHostKey("namespace/name") - Expect(namespace).To(Equal("namespace")) - Expect(name).To(Equal("name")) -}) - var _ = Describe("Test checkForRequeueError", func() { type testCaseCheckForRequeueError struct { err error @@ -759,12 +753,6 @@ var _ = Describe("Test checkForRequeueError", func() { ) }) -var _ = Describe("Test splitHostKey", func() { - namespace, name := splitHostKey("namespace/name") - Expect(namespace).To(Equal("namespace")) - Expect(name).To(Equal("name")) -}) - var _ = Describe("Test analyzePatchError", func() { type testCaseAnalyzePatchError struct { ignoreNotFound bool diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go index 33710fbab..2afede1ed 100644 --- a/pkg/services/baremetal/host/host.go +++ b/pkg/services/baremetal/host/host.go @@ -160,8 +160,8 @@ func SaveHostAndReturn(ctx context.Context, cl client.Client, host *infrav1.Hetz if err := cl.Update(ctx, host); err != nil { if apierrors.IsConflict(err) { - log := ctrl.LoggerFrom(ctx) - log.Info("conflict error. Retrying", "err", err) + logger := ctrl.LoggerFrom(ctx) + logger.Info("conflict error. Retrying", "err", err) return reconcile.Result{Requeue: true}, nil } return reconcile.Result{}, fmt.Errorf("failed to update host object: %w", err) diff --git a/pkg/services/baremetal/remediation/remediation.go b/pkg/services/baremetal/remediation/remediation.go index 371dc9dec..63d05fa61 100644 --- a/pkg/services/baremetal/remediation/remediation.go +++ b/pkg/services/baremetal/remediation/remediation.go @@ -21,9 +21,9 @@ import ( "context" "encoding/json" "fmt" - "strings" "time" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -31,10 +31,10 @@ import ( "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/record" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" + "github.com/syself/cluster-api-provider-hetzner/pkg/baremetalutils" "github.com/syself/cluster-api-provider-hetzner/pkg/scope" ) @@ -51,56 +51,49 @@ func NewService(scope *scope.BareMetalRemediationScope) *Service { } // Reconcile implements reconcilement of HetznerBareMetalRemediations. -func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err error) { - // try to get information about host from bare metal machine annotations - key, err := objectKeyFromAnnotations(s.scope.BareMetalMachine.ObjectMeta.GetAnnotations()) +func (s *Service) Reconcile(ctx context.Context) (reconcile.Result, error) { + host, err := baremetalutils.GetAssociatedHost(ctx, s.scope.Client, s.scope.BareMetalMachine) if err != nil { - if err := s.setOwnerRemediatedConditionNew(ctx); err != nil { - err = fmt.Errorf("failed to set remediated condition on capi machine: %w", err) - record.Warn(s.scope.BareMetalRemediation, "FailedSettingConditionOnMachine", err.Error()) - return res, err - } - record.Event(s.scope.BareMetalRemediation, "ExitRemediation", "exit remediation because bare metal machine has no host annotation") - return res, nil - } - - // get host - var host infrav1.HetznerBareMetalHost - if err := s.scope.Client.Get(ctx, key, &host); err != nil { if apierrors.IsNotFound(err) { - if err := s.setOwnerRemediatedConditionNew(ctx); err != nil { - err = fmt.Errorf("failed to set remediated condition on capi machine: %w", err) - record.Warn(s.scope.BareMetalRemediation, "FailedSettingConditionOnMachine", err.Error()) - return res, err - } - record.Event(s.scope.BareMetalRemediation, "ExitRemediation", "host not found - exit remediation") - return res, nil + return s.setOwnerRemediatedConditionToFailed(ctx, + "exit remediation because host not found") } - err := fmt.Errorf("failed to find the unhealthy host: %w", err) + + // retry + err := fmt.Errorf("failed to find the unhealthy host (will retry): %w", err) record.Warn(s.scope.BareMetalRemediation, "FailedToFindHost", err.Error()) - return res, err + return reconcile.Result{}, err } - // if host is not provisioned or in maintenance mode, then we do not try to reboot server - if host.Spec.Status.ProvisioningState != infrav1.StateProvisioned || - host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode { - if err := s.setOwnerRemediatedConditionNew(ctx); err != nil { - err := fmt.Errorf("failed to set remediated condition on capi machine: %w", err) - record.Warn(s.scope.BareMetalRemediation, "FailedSettingConditionOnMachine", err.Error()) - return res, err - } - record.Eventf( - s.scope.BareMetalRemediation, - "ExitRemediation", - "exit remediation because host is not provisioned or in maintenance mode. Provisioning state: %s. Maintenance mode: %v", - host.Spec.Status.ProvisioningState, host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode, - ) - return res, nil + if host == nil { + return s.setOwnerRemediatedConditionToFailed(ctx, + "exit remediation because hbmm has no host annotation") + } + // if SetErrorAndRemediate() was used to stop provisioning, do not try to reboot server + infraMachineCondition := conditions.Get(s.scope.BareMetalMachine, infrav1.NoRemediateMachineAnnotationCondition) + if infraMachineCondition != nil && infraMachineCondition.Status == corev1.ConditionFalse { + return s.setOwnerRemediatedConditionToFailed(ctx, + fmt.Sprintf("exit remediation because infra machine has condition set: %s: %s", + infraMachineCondition.Reason, + infraMachineCondition.Message)) + } + + // if host is not provisioned, do not try to reboot server + if host.Spec.Status.ProvisioningState != infrav1.StateProvisioned { + return s.setOwnerRemediatedConditionToFailed(ctx, + fmt.Sprintf("exit remediation because host is not provisioned. Provisioning state: %s.", + host.Spec.Status.ProvisioningState)) + } + + // host is in maintenance mode, do not try to reboot server + if host.Spec.MaintenanceMode != nil && *host.Spec.MaintenanceMode { + return s.setOwnerRemediatedConditionToFailed(ctx, + "exit remediation because host is in maintenance mode") } if s.scope.BareMetalRemediation.Spec.Strategy.Type != infrav1.RemediationTypeReboot { record.Warn(s.scope.BareMetalRemediation, "UnsupportedRemediationStrategy", "unsupported remediation strategy") - return res, nil + return reconcile.Result{}, nil } // If no phase set, default to running @@ -113,12 +106,14 @@ func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err erro return s.handlePhaseRunning(ctx, host) case infrav1.PhaseWaiting: return s.handlePhaseWaiting(ctx) + case infrav1.PhaseDeleting: + return reconcile.Result{}, nil + default: + return reconcile.Result{}, fmt.Errorf("internal error, unhandled BareMetalRemediation.Status.Phase: %v", s.scope.BareMetalRemediation.Status.Phase) } - - return res, nil } -func (s *Service) handlePhaseRunning(ctx context.Context, host infrav1.HetznerBareMetalHost) (res reconcile.Result, err error) { +func (s *Service) handlePhaseRunning(ctx context.Context, host *infrav1.HetznerBareMetalHost) (res reconcile.Result, err error) { // if host has not been remediated yet, do that now if s.scope.BareMetalRemediation.Status.LastRemediated == nil { if err := s.remediate(ctx, host); err != nil { @@ -147,10 +142,10 @@ func (s *Service) handlePhaseRunning(ctx context.Context, host infrav1.HetznerBa return res, nil } -func (s *Service) remediate(ctx context.Context, host infrav1.HetznerBareMetalHost) error { +func (s *Service) remediate(ctx context.Context, host *infrav1.HetznerBareMetalHost) error { var err error - patchHelper, err := patch.NewHelper(&host, s.scope.Client) + patchHelper, err := patch.NewHelper(host, s.scope.Client) if err != nil { return fmt.Errorf("failed to init patch helper: %s %s/%s %w", host.Kind, host.Namespace, host.Name, err) } @@ -162,7 +157,7 @@ func (s *Service) remediate(ctx context.Context, host infrav1.HetznerBareMetalHo return fmt.Errorf("failed to add reboot annotation: %w", err) } - if err := patchHelper.Patch(ctx, &host); err != nil { + if err := patchHelper.Patch(ctx, host); err != nil { return fmt.Errorf("failed to patch: %s %s/%s %w", host.Kind, host.Namespace, host.Name, err) } @@ -184,19 +179,7 @@ func (s *Service) handlePhaseWaiting(ctx context.Context) (res reconcile.Result, return reconcile.Result{RequeueAfter: nextCheck}, nil } - // When machine is still unhealthy after remediation, setting of OwnerRemediatedCondition - // moves control to CAPI machine controller. The owning controller will do - // preflight checks and handles the Machine deletion - s.scope.BareMetalRemediation.Status.Phase = infrav1.PhaseDeleting - - if err := s.setOwnerRemediatedConditionNew(ctx); err != nil { - err := fmt.Errorf("failed to set remediated condition on capi machine: %w", err) - record.Warn(s.scope.BareMetalRemediation, "FailedSettingConditionOnMachine", err.Error()) - return res, err - } - record.Event(s.scope.BareMetalRemediation, "SetOwnerRemediatedCondition", "because retryLimit is reached and reboot timed out") - - return res, nil + return s.setOwnerRemediatedConditionToFailed(ctx, "because retryLimit is reached and reboot timed out") } // timeUntilNextRemediation checks if it is time to execute a next remediation step @@ -217,48 +200,50 @@ func (s *Service) timeUntilNextRemediation(now time.Time) time.Duration { return nextRemediation } -// setOwnerRemediatedConditionNew sets MachineOwnerRemediatedCondition on CAPI machine object +// setOwnerRemediatedConditionToFailed sets MachineOwnerRemediatedCondition on CAPI machine object // that have failed a healthcheck. -func (s *Service) setOwnerRemediatedConditionNew(ctx context.Context) error { +func (s *Service) setOwnerRemediatedConditionToFailed(ctx context.Context, msg string) (reconcile.Result, error) { capiMachine, err := util.GetOwnerMachine(ctx, s.scope.Client, s.scope.BareMetalRemediation.ObjectMeta) if err != nil { - return fmt.Errorf("failed to get capi machine: %w", err) + if !apierrors.IsNotFound(err) { + // Maybe a network error. Retry + return reconcile.Result{}, fmt.Errorf("failed to get capi machine: %w", err) + } + record.Event(s.scope.BareMetalRemediation, "CapiMachineGone", "CAPI machine does not exist. Remediation will be stopped. Infra Machine will be deleted soon by GC.") + + // do not retry + s.scope.BareMetalRemediation.Status.Phase = infrav1.PhaseDeleting + return reconcile.Result{}, nil } + // There is a capi-machine. Set condition. patchHelper, err := patch.NewHelper(capiMachine, s.scope.Client) if err != nil { - return fmt.Errorf("failed to init patch helper: %s %s/%s %w", capiMachine.Kind, capiMachine.Namespace, capiMachine.Name, err) + // retry + return reconcile.Result{}, fmt.Errorf("failed to init patch helper: %s %s/%s %w", capiMachine.Kind, capiMachine.Namespace, capiMachine.Name, err) } + // When machine is still unhealthy after remediation, setting of OwnerRemediatedCondition + // moves control to CAPI machine controller. The owning controller will do + // preflight checks and handles the Machine deletion conditions.MarkFalse( capiMachine, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, - "remediation through reboot failed", + "Remediation finished: %s", msg, ) if err := patchHelper.Patch(ctx, capiMachine); err != nil { - return fmt.Errorf("failed to patch: %s %s/%s %w", capiMachine.Kind, capiMachine.Namespace, capiMachine.Name, err) - } - return nil -} - -func objectKeyFromAnnotations(annotations map[string]string) (client.ObjectKey, error) { - if annotations == nil { - return client.ObjectKey{}, fmt.Errorf("unable to get annotations") - } - hostKey, ok := annotations[infrav1.HostAnnotation] - if !ok { - return client.ObjectKey{}, fmt.Errorf("unable to get HostAnnotation") + // retry + return reconcile.Result{}, fmt.Errorf("failed to patch: %s %s/%s %w", capiMachine.Kind, capiMachine.Namespace, capiMachine.Name, err) } - hostNamespace, hostName, err := splitHostKey(hostKey) - if err != nil { - return client.ObjectKey{}, fmt.Errorf("failed to parse host key %q: %w", hostKey, err) - } + record.Event(s.scope.BareMetalRemediation, "ExitRemediation", msg) - return client.ObjectKey{Name: hostName, Namespace: hostNamespace}, nil + // do not retry + s.scope.BareMetalRemediation.Status.Phase = infrav1.PhaseDeleting + return reconcile.Result{}, nil } // addRebootAnnotation sets reboot annotation on unhealthy host. @@ -277,11 +262,3 @@ func addRebootAnnotation(annotations map[string]string) (map[string]string, erro annotations[infrav1.RebootAnnotation] = string(b) return annotations, nil } - -func splitHostKey(key string) (namespace, name string, err error) { - parts := strings.Split(key, "/") - if len(parts) != 2 { - return "", "", fmt.Errorf("unexpected host key") - } - return parts[0], parts[1], nil -} diff --git a/pkg/services/baremetal/remediation/remediation_test.go b/pkg/services/baremetal/remediation/remediation_test.go index 3fbf68de0..38b5800a4 100644 --- a/pkg/services/baremetal/remediation/remediation_test.go +++ b/pkg/services/baremetal/remediation/remediation_test.go @@ -24,7 +24,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" "github.com/syself/cluster-api-provider-hetzner/pkg/scope" @@ -77,96 +76,6 @@ var _ = Describe("Test TimeUntilNextRemediation", func() { ) }) -var _ = Describe("Test ObjectKeyFromAnnotations", func() { - type testCaseObjectKeyFromAnnotations struct { - annotations map[string]string - expectError bool - expectKey client.ObjectKey - } - - DescribeTable("Test ObjectKeyFromAnnotations", - func(tc testCaseObjectKeyFromAnnotations) { - key, err := objectKeyFromAnnotations(tc.annotations) - - if tc.expectError { - Expect(err).ToNot(BeNil()) - } else { - Expect(err).To(BeNil()) - } - - Expect(key).To(Equal(tc.expectKey)) - }, - Entry("correct host key", testCaseObjectKeyFromAnnotations{ - annotations: map[string]string{ - infrav1.HostAnnotation: "mynamespace/myname", - }, - expectError: false, - expectKey: client.ObjectKey{Namespace: "mynamespace", Name: "myname"}, - }), - Entry("nil annotations", testCaseObjectKeyFromAnnotations{ - annotations: nil, - expectError: true, - expectKey: client.ObjectKey{}, - }), - Entry("no host annotation", testCaseObjectKeyFromAnnotations{ - annotations: map[string]string{ - "other-key": "mynamespace/myname", - }, - expectError: true, - expectKey: client.ObjectKey{}, - }), - Entry("incorrect host key", testCaseObjectKeyFromAnnotations{ - annotations: map[string]string{ - infrav1.HostAnnotation: "mynamespace-myname", - }, - expectError: true, - expectKey: client.ObjectKey{}, - }), - ) -}) - -var _ = Describe("Test SplitHostKey", func() { - type testCaseSplitHostKey struct { - hostKey string - expectNamespace string - expectName string - expectError bool - } - - DescribeTable("Test SplitHostKey", - func(tc testCaseSplitHostKey) { - ns, name, err := splitHostKey(tc.hostKey) - - Expect(ns).To(Equal(tc.expectNamespace)) - Expect(name).To(Equal(tc.expectName)) - - if tc.expectError { - Expect(err).ToNot(BeNil()) - } else { - Expect(err).To(BeNil()) - } - }, - Entry("correct host key", testCaseSplitHostKey{ - hostKey: "ns/name", - expectNamespace: "ns", - expectName: "name", - expectError: false, - }), - Entry("incorrect host key 1", testCaseSplitHostKey{ - hostKey: "ns-name", - expectNamespace: "", - expectName: "", - expectError: true, - }), - Entry("incorrect host key 2", testCaseSplitHostKey{ - hostKey: "ns/name/other", - expectNamespace: "", - expectName: "", - expectError: true, - }), - ) -}) - var _ = Describe("Test AddRebootAnnotation", func() { type testCaseAddRebootAnnotation struct { annotations map[string]string