Repository: syself/cluster-api-provider-hetzner
Pull request #927 (2023-09-20 06:02:48)

🐛Don't reboot if HetzerBareMetalMachine not found
<!-- If this is your first PR, welcome! Please make sure you read the [contributing guidelines](../CONTRIBUTING.md#contributing-a-patch). -->
<!-- please add an icon to the title of this PR and delete this line and similar ones -->
<!-- the icon will be either ⚠️ (:warning:, major or breaking changes), ✨ (:sparkles:, feature additions), 🐛 (:bug:, patches and bugfixes), 📖 (:book:, documentation or proposals), or 🌱 (:seedling:, minor or other) -->

**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**TODOs**:

- [ ] squash commits
- [ ] include documentation
- [ ] add unit tests




Patch:
diff --git a/api/v1beta1/hetznerbaremetalhost_webhook.go b/api/v1beta1/hetznerbaremetalhost_webhook.go
index 8fdbf474..2c63d63c 100644
--- a/api/v1beta1/hetznerbaremetalhost_webhook.go
+++ b/api/v1beta1/hetznerbaremetalhost_webhook.go
@@ -17,10 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
-	"reflect"
-
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -51,17 +48,8 @@ func (host *HetznerBareMetalHost) ValidateCreate() (admission.Warnings, error) {
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (host *HetznerBareMetalHost) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	var allErrs field.ErrorList
-
-	oldHetznerBareMetalHost := old.(*HetznerBareMetalHost)
-	if !reflect.DeepEqual(host.Spec.ServerID, oldHetznerBareMetalHost.Spec.ServerID) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "serverID"), host.Spec.ServerID, "ServerID is immutable"),
-		)
-	}
-
-	return nil, aggregateObjErrors(host.GroupVersionKind().GroupKind(), host.Name, allErrs)
+func (host *HetznerBareMetalHost) ValidateUpdate(runtime.Object) (admission.Warnings, error) {
+	return nil, nil
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
diff --git a/controllers/hcloudmachine_controller.go b/controllers/hcloudmachine_controller.go
index c1be0ad6..c204f6eb 100644
--- a/controllers/hcloudmachine_controller.go
+++ b/controllers/hcloudmachine_controller.go
@@ -48,7 +48,6 @@ import (
 // HCloudMachineReconciler reconciles a HCloudMachine object.
 type HCloudMachineReconciler struct {
 	client.Client
-	RateLimitWaitTime   time.Duration
 	APIReader           client.Reader
 	HCloudClientFactory hcloudclient.Factory
 	WatchFilterValue    string
@@ -157,7 +156,7 @@ func (r *HCloudMachineReconciler) Reconcile(ctx context.Context, req reconcile.R
 	}()
 
 	// Check whether rate limit has been reached and if so, then wait.
-	if wait := reconcileRateLimit(hcloudMachine, r.RateLimitWaitTime); wait {
+	if wait := reconcileRateLimit(hcloudMachine); wait {
 		return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
diff --git a/controllers/hcloudmachinetemplate_controller.go b/controllers/hcloudmachinetemplate_controller.go
index 247f91ec..2bbbd620 100644
--- a/controllers/hcloudmachinetemplate_controller.go
+++ b/controllers/hcloudmachinetemplate_controller.go
@@ -43,7 +43,6 @@ import (
 // HCloudMachineTemplateReconciler reconciles a HCloudMachineTemplate object.
 type HCloudMachineTemplateReconciler struct {
 	client.Client
-	RateLimitWaitTime   time.Duration
 	APIReader           client.Reader
 	HCloudClientFactory hcloudclient.Factory
 	WatchFilterValue    string
@@ -126,7 +125,7 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec
 	}()
 
 	// check whether rate limit has been reached and if so, then wait.
-	if wait := reconcileRateLimit(machineTemplate, r.RateLimitWaitTime); wait {
+	if wait := reconcileRateLimit(machineTemplate); wait {
 		return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
diff --git a/controllers/hcloudremediation_controller.go b/controllers/hcloudremediation_controller.go
index b41a8163..347dbb31 100644
--- a/controllers/hcloudremediation_controller.go
+++ b/controllers/hcloudremediation_controller.go
@@ -45,7 +45,6 @@ import (
 // HCloudRemediationReconciler reconciles a HCloudRemediation object.
 type HCloudRemediationReconciler struct {
 	client.Client
-	RateLimitWaitTime   time.Duration
 	APIReader           client.Reader
 	HCloudClientFactory hcloudclient.Factory
 	WatchFilterValue    string
@@ -171,7 +170,7 @@ func (r *HCloudRemediationReconciler) Reconcile(ctx context.Context, req reconci
 	}()
 
 	// Check whether rate limit has been reached and if so, then wait.
-	if wait := reconcileRateLimit(hcloudRemediation, r.RateLimitWaitTime); wait {
+	if wait := reconcileRateLimit(hcloudRemediation); wait {
 		return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
diff --git a/controllers/hetznerbaremetalhost_controller.go b/controllers/hetznerbaremetalhost_controller.go
index d4ecee01..72a22514 100644
--- a/controllers/hetznerbaremetalhost_controller.go
+++ b/controllers/hetznerbaremetalhost_controller.go
@@ -51,7 +51,6 @@ import (
 // HetznerBareMetalHostReconciler reconciles a HetznerBareMetalHost object.
 type HetznerBareMetalHostReconciler struct {
 	client.Client
-	RateLimitWaitTime  time.Duration
 	APIReader          client.Reader
 	RobotClientFactory robotclient.Factory
 	SSHClientFactory   sshclient.Factory
@@ -139,7 +138,7 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl
 	}
 
 	// check whether rate limit has been reached and if so, then wait.
-	if wait := reconcileRateLimit(bmHost, r.RateLimitWaitTime); wait {
+	if wait := reconcileRateLimit(bmHost); wait {
 		log.Info("Rate limit exceeded - requeue in 30 seconds")
 		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go
index 9dea4187..94b11c65 100644
--- a/controllers/hetznercluster_controller.go
+++ b/controllers/hetznercluster_controller.go
@@ -62,12 +62,12 @@ import (
 
 const (
 	secretErrorRetryDelay = time.Second * 10
+	rateLimitWaitTime     = 5 * time.Minute
 )
 
 // HetznerClusterReconciler reconciles a HetznerCluster object.
 type HetznerClusterReconciler struct {
 	client.Client
-	RateLimitWaitTime              time.Duration
 	APIReader                      client.Reader
 	HCloudClientFactory            hcloudclient.Factory
 	targetClusterManagersStopCh    map[types.NamespacedName]chan struct{}
@@ -153,7 +153,7 @@ func (r *HetznerClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	}()
 
 	// check whether rate limit has been reached and if so, then wait.
-	if wait := reconcileRateLimit(hetznerCluster, r.RateLimitWaitTime); wait {
+	if wait := reconcileRateLimit(hetznerCluster); wait {
 		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
@@ -335,7 +335,7 @@ func (r *HetznerClusterReconciler) reconcileDelete(ctx context.Context, clusterS
 
 // reconcileRateLimit checks whether a rate limit has been reached and returns whether
 // the controller should wait a bit more.
-func reconcileRateLimit(setter conditions.Setter, rateLimitWaitTime time.Duration) bool {
+func reconcileRateLimit(setter conditions.Setter) bool {
 	condition := conditions.Get(setter, infrav1.HetznerAPIReachableCondition)
 	if condition != nil && condition.Status == corev1.ConditionFalse {
 		if time.Now().Before(condition.LastTransitionTime.Time.Add(rateLimitWaitTime)) {
diff --git a/main.go b/main.go
index efbe2284..8146dbdf 100644
--- a/main.go
+++ b/main.go
@@ -71,7 +71,6 @@ var (
 	hetznerBareMetalHostConcurrency    int
 	logLevel                           string
 	syncPeriod                         time.Duration
-	rateLimitWaitTime                  time.Duration
 )
 
 func main() {
@@ -86,8 +85,7 @@ func main() {
 	flag.IntVar(&hetznerBareMetalMachineConcurrency, "hetznerbaremetalmachine-concurrency", 1, "Number of HetznerBareMetalMachines to process simultaneously")
 	flag.IntVar(&hetznerBareMetalHostConcurrency, "hetznerbaremetalhost-concurrency", 1, "Number of HetznerBareMetalHosts to process simultaneously")
 	flag.StringVar(&logLevel, "log-level", "debug", "Specifies log level. Options are 'debug', 'info' and 'error'")
-	flag.DurationVar(&syncPeriod, "sync-period", 3*time.Minute, "The minimum interval at which watched resources are reconciled (e.g. 3m)")
-	flag.DurationVar(&rateLimitWaitTime, "rate-limit", 5*time.Minute, "The rate limiting for HCloud controller (e.g. 5m)")
+	flag.DurationVar(&syncPeriod, "sync-period", time.Duration(3*time.Minute), "The minimum interval at which watched resources are reconciled (e.g. 3m)")
 	flag.Parse()
 
 	ctrl.SetLogger(utils.GetDefaultLogger(logLevel))
@@ -127,7 +125,6 @@ func main() {
 	if err = (&controllers.HetznerClusterReconciler{
 		Client:                         mgr.GetClient(),
 		APIReader:                      mgr.GetAPIReader(),
-		RateLimitWaitTime:              rateLimitWaitTime,
 		HCloudClientFactory:            hcloudClientFactory,
 		WatchFilterValue:               watchFilterValue,
 		TargetClusterManagersWaitGroup: &wg,
diff --git a/pkg/services/baremetal/remediation/remediation.go b/pkg/services/baremetal/remediation/remediation.go
index 643c38eb..51223caa 100644
--- a/pkg/services/baremetal/remediation/remediation.go
+++ b/pkg/services/baremetal/remediation/remediation.go
@@ -24,6 +24,7 @@ import (
 	"strings"
 	"time"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	capi "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
@@ -53,6 +54,15 @@ func NewService(scope *scope.BareMetalRemediationScope) *Service {
 func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err error) {
 	host, err := s.getHost(ctx)
 	if err != nil {
+		if apierrors.IsNotFound(err) {
+			err = fmt.Errorf("HetzerBareMetalMachine not found")
+			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
+			}
+			return res, nil
+		}
 		err := fmt.Errorf("failed to find the unhealthy host: %w", err)
 		record.Warn(s.scope.BareMetalRemediation, "FailedToFindHost", err.Error())
 		return res, err
@@ -67,6 +77,7 @@ func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err erro
 			record.Warn(s.scope.BareMetalRemediation, "FailedSettingConditionOnMachine", err.Error())
 			return res, err
 		}
+		return res, nil
 	}
 
 	if s.scope.BareMetalRemediation.Spec.Strategy.Type != infrav1.RemediationTypeReboot {


Test patch:
diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go
index f7d84068..82f3f745 100644
--- a/controllers/controllers_suite_test.go
+++ b/controllers/controllers_suite_test.go
@@ -70,7 +70,6 @@ var _ = BeforeSuite(func() {
 	Expect((&HetznerClusterReconciler{
 		Client:                         testEnv.Manager.GetClient(),
 		APIReader:                      testEnv.Manager.GetAPIReader(),
-		RateLimitWaitTime:              5 * time.Minute,
 		HCloudClientFactory:            testEnv.HCloudClientFactory,
 		TargetClusterManagersWaitGroup: &wg,
 	}).SetupWithManager(ctx, testEnv.Manager, controller.Options{})).To(Succeed())
diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go
index 29d52d4c..eb20eeca 100644
--- a/controllers/hetznercluster_controller_test.go
+++ b/controllers/hetznercluster_controller_test.go
@@ -762,22 +762,22 @@ var _ = Describe("reconcileRateLimit", func() {
 
 	It("returns wait==true if rate limit exceeded is set and time is not over", func() {
 		conditions.MarkFalse(hetznerCluster, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason, clusterv1.ConditionSeverityWarning, "")
-		Expect(reconcileRateLimit(hetznerCluster, testEnv.RateLimitWaitTime)).To(BeTrue())
+		Expect(reconcileRateLimit(hetznerCluster)).To(BeTrue())
 	})
 
 	It("returns wait==false if rate limit exceeded is set and time is over", func() {
 		conditions.MarkFalse(hetznerCluster, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason, clusterv1.ConditionSeverityWarning, "")
 		conditionList := hetznerCluster.GetConditions()
 		conditionList[0].LastTransitionTime = metav1.NewTime(time.Now().Add(-time.Hour))
-		Expect(reconcileRateLimit(hetznerCluster, testEnv.RateLimitWaitTime)).To(BeFalse())
+		Expect(reconcileRateLimit(hetznerCluster)).To(BeFalse())
 	})
 
 	It("returns wait==false if rate limit condition is set to true", func() {
 		conditions.MarkTrue(hetznerCluster, infrav1.HetznerAPIReachableCondition)
-		Expect(reconcileRateLimit(hetznerCluster, testEnv.RateLimitWaitTime)).To(BeFalse())
+		Expect(reconcileRateLimit(hetznerCluster)).To(BeFalse())
 	})
 
 	It("returns wait==false if rate limit condition is not set", func() {
-		Expect(reconcileRateLimit(hetznerCluster, testEnv.RateLimitWaitTime)).To(BeFalse())
+		Expect(reconcileRateLimit(hetznerCluster)).To(BeFalse())
 	})
 })
diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go
index 274db0d0..a43db630 100644
--- a/test/helpers/envtest.go
+++ b/test/helpers/envtest.go
@@ -25,7 +25,6 @@ import (
 	"path"
 	"path/filepath"
 	goruntime "runtime"
-	"time"
 
 	g "github.com/onsi/ginkgo/v2"
 	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
@@ -115,7 +114,6 @@ type (
 		OSSSHClientAfterCloudInit    *sshmock.Client
 		RobotClient                  *robotmock.Client
 		cancel                       context.CancelFunc
-		RateLimitWaitTime            time.Duration
 	}
 )
 
@@ -196,7 +194,6 @@ func NewTestEnvironment() *TestEnvironment {
 		OSSSHClientAfterCloudInit:    osSSHClientAfterCloudInit,
 		RobotClientFactory:           mocks.NewRobotFactory(robotClient),
 		RobotClient:                  robotClient,
-		RateLimitWaitTime:            5 * time.Minute,
 	}
 }