From 46dc5d8adb99e7de9befdc15a7545b2732cd3572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Mon, 13 Apr 2026 16:40:47 +0200 Subject: [PATCH 01/12] :seedling: Fix unit-tests on v1.1.x. --- pkg/services/hcloud/server/server.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index bac8c4805..b23056da6 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -253,9 +253,11 @@ func (s *Service) handleBootStateUnset(ctx context.Context) (reconcile.Result, e if errors.Is(err, errServerCreateNotPossible) { err = fmt.Errorf("createServerFromImageNameOrURL failed: %w", err) s.scope.Error(err, "") - conditions.MarkFalse(hm, infrav1.ServerCreateSucceededCondition, - "ServerCreateNotPossible", clusterv1.ConditionSeverityWarning, - "%s", err.Error()) + if !conditions.IsFalse(hm, infrav1.ServerCreateSucceededCondition) { + conditions.MarkFalse(hm, infrav1.ServerCreateSucceededCondition, + "ServerCreateNotPossible", clusterv1.ConditionSeverityWarning, + "%s", err.Error()) + } return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil } return reconcile.Result{}, fmt.Errorf("failed to create server: %w", err) From cfdf5388fc8294844b79cb4cd22e5e839989e721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 10:00:48 +0200 Subject: [PATCH 02/12] provide hint, when env test is missing binaries. --- test/helpers/envtest.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go index 048f79830..9f863b095 100644 --- a/test/helpers/envtest.go +++ b/test/helpers/envtest.go @@ -25,6 +25,7 @@ import ( "path" "path/filepath" goruntime "runtime" + "strings" "time" g "github.com/onsi/ginkgo/v2" @@ -146,7 +147,7 @@ func NewTestEnvironment() *TestEnvironment { initializeWebhookInEnvironment() if _, err := env.Start(); err != nil { - panic(err) + panic(wrapEnvtestStartError(err)) } logLevel := "info" @@ -221,6 +222,21 @@ func NewTestEnvironment() *TestEnvironment { } } +func wrapEnvtestStartError(err error) error { + if err == nil { + return err + } + + errText := err.Error() + if !strings.Contains(errText, "$PATH") { + // This is not: unable to start control plane itself: failed to start the controlplane. + // retried 5 times: exec: "etcd": executable file not found in $PATH + return err + } + + return fmt.Errorf("%w\nHint: set KUBEBUILDER_ASSETS=$PWD/hack/tools/bin/k8s/1.??.0-linux-amd64", err) +} + // StartManager starts the manager and sets a cancel function into the testEnv object. func (t *TestEnvironment) StartManager(ctx context.Context) error { ctx, cancel := context.WithCancel(ctx) From de68006621e0926cc3cd28d9814d77d9440959ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 10:20:23 +0200 Subject: [PATCH 03/12] better fix. --- controllers/hcloudmachine_controller_test.go | 26 +++++++- pkg/services/hcloud/server/server.go | 65 +++++++++++++------- 2 files changed, 65 insertions(+), 26 deletions(-) diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index 751da28a7..38542e8c1 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -513,9 +513,29 @@ var _ = Describe("HCloudMachineReconciler", func() { }) It("checks that ImageNotFound is visible in conditions if image does not exist", func() { - Eventually(func() bool { - return isPresentAndFalseWithReason(key, hcloudMachine, infrav1.ServerCreateSucceededCondition, infrav1.ImageNotFoundReason) - }, timeout, interval).Should(BeTrue()) + Eventually(func() error { + if err := testEnv.Get(ctx, key, hcloudMachine); err != nil { + return err + } + + c := conditions.Get(hcloudMachine, infrav1.ServerCreateSucceededCondition) + if c == nil { + return fmt.Errorf("%s not set", infrav1.ServerCreateSucceededCondition) + } + + if c.Status != corev1.ConditionFalse || c.Reason != infrav1.ImageNotFoundReason { + return fmt.Errorf( + "expected %s to be False with reason %q, got status=%q reason=%q message=%q", + infrav1.ServerCreateSucceededCondition, + infrav1.ImageNotFoundReason, + c.Status, + c.Reason, + c.Message, + ) + } + + return nil + }, timeout, interval).Should(Succeed()) }) }) }) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index bccbc8a77..37e03026e 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -59,7 +59,21 @@ const ( var hcloudImageURLCommandDir = "/shared" -var errServerCreateNotPossible = fmt.Errorf("server create not possible - need action") +var errServerCreateNotPossible = errors.New("server create not possible - need action") + +type serverCreateConditionError struct { + reason string + severity clusterv1.ConditionSeverity + message string +} + +func (e *serverCreateConditionError) Error() string { + return e.message +} + +func (e *serverCreateConditionError) Unwrap() error { + return errServerCreateNotPossible +} // Service defines struct with machine scope to reconcile HCloudMachines. type Service struct { @@ -1065,6 +1079,18 @@ func (s *Service) createServerFromImageName(ctx context.Context) (*hcloud.Server image, err := s.getServerImage(ctx, hm.Spec.ImageName) if err != nil { + var conditionErr *serverCreateConditionError + if errors.As(err, &conditionErr) { + conditions.MarkFalse( + hm, + infrav1.ServerCreateSucceededCondition, + conditionErr.reason, + conditionErr.severity, + "%s", + conditionErr.message, + ) + } + err = fmt.Errorf("create server from imageName (%q): %w", hm.Spec.ImageName, err) msg := err.Error() record.Warn(hm, "FailedGetServerImage", msg) @@ -1273,14 +1299,11 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud if serverType == nil { msg := fmt.Sprintf("failed to get server type %q", string(s.scope.HCloudMachine.Spec.Type)) - conditions.MarkFalse( - s.scope.HCloudMachine, - infrav1.ServerCreateSucceededCondition, - infrav1.ServerTypeNotFoundReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) + return nil, &serverCreateConditionError{ + reason: infrav1.ServerTypeNotFoundReason, + severity: clusterv1.ConditionSeverityError, + message: msg, + } } // query for an existing image by label @@ -1313,24 +1336,20 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud msg := fmt.Sprintf("image is ambiguous - %d images have name %s", len(images), imageName) record.Warn(s.scope.HCloudMachine, "ImageNameAmbiguous", msg) - conditions.MarkFalse(s.scope.HCloudMachine, - infrav1.ServerCreateSucceededCondition, - infrav1.ImageAmbiguousReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) + return nil, &serverCreateConditionError{ + reason: infrav1.ImageAmbiguousReason, + severity: clusterv1.ConditionSeverityError, + message: msg, + } } if len(images) == 0 { msg := fmt.Sprintf("no image found with name %s", s.scope.HCloudMachine.Spec.ImageName) record.Warn(s.scope.HCloudMachine, "ImageNotFound", msg) - conditions.MarkFalse(s.scope.HCloudMachine, - infrav1.ServerCreateSucceededCondition, - infrav1.ImageNotFoundReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) + return nil, &serverCreateConditionError{ + reason: infrav1.ImageNotFoundReason, + severity: clusterv1.ConditionSeverityError, + message: msg, + } } return images[0], nil From 4dea9074fab87c2535443f85434343f76dcdff41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 10:25:14 +0200 Subject: [PATCH 04/12] Preserve specific server create error reasons --- pkg/services/hcloud/server/server.go | 104 +++++++++++++++------------ 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index 37e03026e..5e01f1059 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -60,19 +60,45 @@ const ( var hcloudImageURLCommandDir = "/shared" var errServerCreateNotPossible = errors.New("server create not possible - need action") - -type serverCreateConditionError struct { - reason string - severity clusterv1.ConditionSeverity - message string +var errServerTypeNotFound = errors.New("server type not found") +var errImageAmbiguous = errors.New("image ambiguous") +var errImageNotFound = errors.New("image not found") + +func isServerCreateNotPossibleError(err error) bool { + return errors.Is(err, errServerCreateNotPossible) || + errors.Is(err, errServerTypeNotFound) || + errors.Is(err, errImageAmbiguous) || + errors.Is(err, errImageNotFound) } -func (e *serverCreateConditionError) Error() string { - return e.message -} +func markServerCreateConditionFromError(hm *infrav1.HCloudMachine, err error, msg string) bool { + switch { + case errors.Is(err, errServerTypeNotFound): + conditions.MarkFalse(hm, + infrav1.ServerCreateSucceededCondition, + infrav1.ServerTypeNotFoundReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + case errors.Is(err, errImageAmbiguous): + conditions.MarkFalse(hm, + infrav1.ServerCreateSucceededCondition, + infrav1.ImageAmbiguousReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + case errors.Is(err, errImageNotFound): + conditions.MarkFalse(hm, + infrav1.ServerCreateSucceededCondition, + infrav1.ImageNotFoundReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + default: + return false + } -func (e *serverCreateConditionError) Unwrap() error { - return errServerCreateNotPossible + return true } // Service defines struct with machine scope to reconcile HCloudMachines. @@ -266,7 +292,7 @@ func (s *Service) handleBootStateUnset(ctx context.Context) (reconcile.Result, e return reconcile.Result{}, nil } - if errors.Is(err, errServerCreateNotPossible) { + if isServerCreateNotPossibleError(err) { err = fmt.Errorf("createServerFromImageNameOrURL failed: %w", err) s.scope.Error(err, "") if !conditions.IsFalse(hm, infrav1.ServerCreateSucceededCondition) { @@ -1042,9 +1068,14 @@ func (s *Service) createServerFromImageURL(ctx context.Context) (*hcloud.Server, // Validate that ImageURLCommand is given hm := s.scope.HCloudMachine - image, err := s.getServerImage(ctx, preRescueOSImage) + image, imageErrMsg, err := s.getServerImage(ctx, preRescueOSImage) if err != nil { - err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %w", preRescueOSImage, err) + markServerCreateConditionFromError(hm, err, imageErrMsg) + if imageErrMsg != "" { + err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %s: %w", preRescueOSImage, imageErrMsg, err) + } else { + err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %w", preRescueOSImage, err) + } msg := err.Error() record.Warn(hm, "FailedGetServerImage", msg) s.scope.Error(nil, msg) @@ -1077,21 +1108,14 @@ func (s *Service) createServerFromImageName(ctx context.Context) (*hcloud.Server return nil, nil, err } - image, err := s.getServerImage(ctx, hm.Spec.ImageName) + image, imageErrMsg, err := s.getServerImage(ctx, hm.Spec.ImageName) if err != nil { - var conditionErr *serverCreateConditionError - if errors.As(err, &conditionErr) { - conditions.MarkFalse( - hm, - infrav1.ServerCreateSucceededCondition, - conditionErr.reason, - conditionErr.severity, - "%s", - conditionErr.message, - ) + markServerCreateConditionFromError(hm, err, imageErrMsg) + if imageErrMsg != "" { + err = fmt.Errorf("create server from imageName (%q): %s: %w", hm.Spec.ImageName, imageErrMsg, err) + } else { + err = fmt.Errorf("create server from imageName (%q): %w", hm.Spec.ImageName, err) } - - err = fmt.Errorf("create server from imageName (%q): %w", hm.Spec.ImageName, err) msg := err.Error() record.Warn(hm, "FailedGetServerImage", msg) s.scope.Error(nil, msg) @@ -1288,22 +1312,18 @@ func (s *Service) getSSHKeys(ctx context.Context) ( return caphSSHKeys, hcloudSSHKeys, nil } -func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud.Image, error) { +func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud.Image, string, error) { key := fmt.Sprintf("%s%s", infrav1.NameHetznerProviderPrefix, "image-name") // Get server type so we can filter for images with correct architecture serverType, err := s.scope.HCloudClient.GetServerType(ctx, string(s.scope.HCloudMachine.Spec.Type)) if err != nil { - return nil, handleRateLimit(s.scope.HCloudMachine, err, "GetServerType", "failed to get server type in HCloud") + return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "GetServerType", "failed to get server type in HCloud") } if serverType == nil { msg := fmt.Sprintf("failed to get server type %q", string(s.scope.HCloudMachine.Spec.Type)) - return nil, &serverCreateConditionError{ - reason: infrav1.ServerTypeNotFoundReason, - severity: clusterv1.ConditionSeverityError, - message: msg, - } + return nil, msg, errServerTypeNotFound } // query for an existing image by label @@ -1317,7 +1337,7 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud images, err := s.scope.HCloudClient.ListImages(ctx, listOpts) if err != nil { - return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by label in HCloud") + return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by label in HCloud") } // query for an existing image by name. @@ -1327,7 +1347,7 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud } imagesByName, err := s.scope.HCloudClient.ListImages(ctx, listOpts) if err != nil { - return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by name in HCloud") + return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by name in HCloud") } images = append(images, imagesByName...) @@ -1336,23 +1356,15 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud msg := fmt.Sprintf("image is ambiguous - %d images have name %s", len(images), imageName) record.Warn(s.scope.HCloudMachine, "ImageNameAmbiguous", msg) - return nil, &serverCreateConditionError{ - reason: infrav1.ImageAmbiguousReason, - severity: clusterv1.ConditionSeverityError, - message: msg, - } + return nil, msg, errImageAmbiguous } if len(images) == 0 { msg := fmt.Sprintf("no image found with name %s", s.scope.HCloudMachine.Spec.ImageName) record.Warn(s.scope.HCloudMachine, "ImageNotFound", msg) - return nil, &serverCreateConditionError{ - reason: infrav1.ImageNotFoundReason, - severity: clusterv1.ConditionSeverityError, - message: msg, - } + return nil, msg, errImageNotFound } - return images[0], nil + return images[0], "", nil } func (s *Service) handleServerStatusOff(ctx context.Context, server *hcloud.Server) (res reconcile.Result, err error) { From 7e69a750cb2a8dcb8ae78c70f6db347e01ce10a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 10:29:55 +0200 Subject: [PATCH 05/12] Simplify server create condition handling --- pkg/services/hcloud/server/server.go | 96 ++++++++++------------------ 1 file changed, 33 insertions(+), 63 deletions(-) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index 5e01f1059..41ee559cc 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -60,46 +60,6 @@ const ( var hcloudImageURLCommandDir = "/shared" var errServerCreateNotPossible = errors.New("server create not possible - need action") -var errServerTypeNotFound = errors.New("server type not found") -var errImageAmbiguous = errors.New("image ambiguous") -var errImageNotFound = errors.New("image not found") - -func isServerCreateNotPossibleError(err error) bool { - return errors.Is(err, errServerCreateNotPossible) || - errors.Is(err, errServerTypeNotFound) || - errors.Is(err, errImageAmbiguous) || - errors.Is(err, errImageNotFound) -} - -func markServerCreateConditionFromError(hm *infrav1.HCloudMachine, err error, msg string) bool { - switch { - case errors.Is(err, errServerTypeNotFound): - conditions.MarkFalse(hm, - infrav1.ServerCreateSucceededCondition, - infrav1.ServerTypeNotFoundReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - case errors.Is(err, errImageAmbiguous): - conditions.MarkFalse(hm, - infrav1.ServerCreateSucceededCondition, - infrav1.ImageAmbiguousReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - case errors.Is(err, errImageNotFound): - conditions.MarkFalse(hm, - infrav1.ServerCreateSucceededCondition, - infrav1.ImageNotFoundReason, - clusterv1.ConditionSeverityError, - "%s", msg, - ) - default: - return false - } - - return true -} // Service defines struct with machine scope to reconcile HCloudMachines. type Service struct { @@ -292,10 +252,11 @@ func (s *Service) handleBootStateUnset(ctx context.Context) (reconcile.Result, e return reconcile.Result{}, nil } - if isServerCreateNotPossibleError(err) { + if errors.Is(err, errServerCreateNotPossible) { err = fmt.Errorf("createServerFromImageNameOrURL failed: %w", err) s.scope.Error(err, "") if !conditions.IsFalse(hm, infrav1.ServerCreateSucceededCondition) { + // Preserve a more specific condition reason already set by lower-level helpers. conditions.MarkFalse(hm, infrav1.ServerCreateSucceededCondition, "ServerCreateNotPossible", clusterv1.ConditionSeverityWarning, "%s", err.Error()) @@ -1068,14 +1029,9 @@ func (s *Service) createServerFromImageURL(ctx context.Context) (*hcloud.Server, // Validate that ImageURLCommand is given hm := s.scope.HCloudMachine - image, imageErrMsg, err := s.getServerImage(ctx, preRescueOSImage) + image, err := s.getServerImage(ctx, preRescueOSImage) if err != nil { - markServerCreateConditionFromError(hm, err, imageErrMsg) - if imageErrMsg != "" { - err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %s: %w", preRescueOSImage, imageErrMsg, err) - } else { - err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %w", preRescueOSImage, err) - } + err = fmt.Errorf("failed to get pre-rescue-OS server image %q: %w", preRescueOSImage, err) msg := err.Error() record.Warn(hm, "FailedGetServerImage", msg) s.scope.Error(nil, msg) @@ -1108,14 +1064,9 @@ func (s *Service) createServerFromImageName(ctx context.Context) (*hcloud.Server return nil, nil, err } - image, imageErrMsg, err := s.getServerImage(ctx, hm.Spec.ImageName) + image, err := s.getServerImage(ctx, hm.Spec.ImageName) if err != nil { - markServerCreateConditionFromError(hm, err, imageErrMsg) - if imageErrMsg != "" { - err = fmt.Errorf("create server from imageName (%q): %s: %w", hm.Spec.ImageName, imageErrMsg, err) - } else { - err = fmt.Errorf("create server from imageName (%q): %w", hm.Spec.ImageName, err) - } + err = fmt.Errorf("create server from imageName (%q): %w", hm.Spec.ImageName, err) msg := err.Error() record.Warn(hm, "FailedGetServerImage", msg) s.scope.Error(nil, msg) @@ -1312,18 +1263,25 @@ func (s *Service) getSSHKeys(ctx context.Context) ( return caphSSHKeys, hcloudSSHKeys, nil } -func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud.Image, string, error) { +func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud.Image, error) { key := fmt.Sprintf("%s%s", infrav1.NameHetznerProviderPrefix, "image-name") // Get server type so we can filter for images with correct architecture serverType, err := s.scope.HCloudClient.GetServerType(ctx, string(s.scope.HCloudMachine.Spec.Type)) if err != nil { - return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "GetServerType", "failed to get server type in HCloud") + return nil, handleRateLimit(s.scope.HCloudMachine, err, "GetServerType", "failed to get server type in HCloud") } if serverType == nil { msg := fmt.Sprintf("failed to get server type %q", string(s.scope.HCloudMachine.Spec.Type)) - return nil, msg, errServerTypeNotFound + conditions.MarkFalse( + s.scope.HCloudMachine, + infrav1.ServerCreateSucceededCondition, + infrav1.ServerTypeNotFoundReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) } // query for an existing image by label @@ -1337,7 +1295,7 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud images, err := s.scope.HCloudClient.ListImages(ctx, listOpts) if err != nil { - return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by label in HCloud") + return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by label in HCloud") } // query for an existing image by name. @@ -1347,7 +1305,7 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud } imagesByName, err := s.scope.HCloudClient.ListImages(ctx, listOpts) if err != nil { - return nil, "", handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by name in HCloud") + return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by name in HCloud") } images = append(images, imagesByName...) @@ -1356,15 +1314,27 @@ func (s *Service) getServerImage(ctx context.Context, imageName string) (*hcloud msg := fmt.Sprintf("image is ambiguous - %d images have name %s", len(images), imageName) record.Warn(s.scope.HCloudMachine, "ImageNameAmbiguous", msg) - return nil, msg, errImageAmbiguous + conditions.MarkFalse(s.scope.HCloudMachine, + infrav1.ServerCreateSucceededCondition, + infrav1.ImageAmbiguousReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) } if len(images) == 0 { msg := fmt.Sprintf("no image found with name %s", s.scope.HCloudMachine.Spec.ImageName) record.Warn(s.scope.HCloudMachine, "ImageNotFound", msg) - return nil, msg, errImageNotFound + conditions.MarkFalse(s.scope.HCloudMachine, + infrav1.ServerCreateSucceededCondition, + infrav1.ImageNotFoundReason, + clusterv1.ConditionSeverityError, + "%s", msg, + ) + return nil, fmt.Errorf("%s: %w", msg, errServerCreateNotPossible) } - return images[0], "", nil + return images[0], nil } func (s *Service) handleServerStatusOff(ctx context.Context, server *hcloud.Server) (res reconcile.Result, err error) { From 3736da1ee57e4a11f34801461333f9a05e88f832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 10:46:56 +0200 Subject: [PATCH 06/12] linting. --- test/helpers/envtest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go index 9f863b095..ae725fcdf 100644 --- a/test/helpers/envtest.go +++ b/test/helpers/envtest.go @@ -224,7 +224,7 @@ func NewTestEnvironment() *TestEnvironment { func wrapEnvtestStartError(err error) error { if err == nil { - return err + return nil } errText := err.Error() From 4f3040bdf6141d3513d0f124f7676f4d6106cbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 11:01:38 +0200 Subject: [PATCH 07/12] Fix envtest error comment wording --- test/helpers/envtest.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go index ae725fcdf..b9dc8841d 100644 --- a/test/helpers/envtest.go +++ b/test/helpers/envtest.go @@ -227,10 +227,13 @@ func wrapEnvtestStartError(err error) error { return nil } + // Check whether the error is about missing binaries in $PATH, for example: + // + // unable to start control plane itself: failed to start the controlplane. + // retried 5 times: exec: "etcd": executable file not found in $PATH" errText := err.Error() if !strings.Contains(errText, "$PATH") { - // This is not: unable to start control plane itself: failed to start the controlplane. - // retried 5 times: exec: "etcd": executable file not found in $PATH + // This looks like another error, so do not add the hint. return err } From d93a20cd5ad0c49b6e3d5e9fee5edb1d30dcc39e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 13:36:58 +0200 Subject: [PATCH 08/12] test: add condition Eventually helper --- controllers/hcloudmachine_controller_test.go | 30 +++------ test/helpers/conditions.go | 68 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 test/helpers/conditions.go diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index 38542e8c1..8f49d1e96 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -37,6 +37,7 @@ import ( infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" "github.com/syself/cluster-api-provider-hetzner/pkg/utils" + "github.com/syself/cluster-api-provider-hetzner/test/helpers" ) func TestIgnoreInsignificantHCloudMachineStatusUpdates(t *testing.T) { @@ -514,27 +515,14 @@ var _ = Describe("HCloudMachineReconciler", func() { It("checks that ImageNotFound is visible in conditions if image does not exist", func() { Eventually(func() error { - if err := testEnv.Get(ctx, key, hcloudMachine); err != nil { - return err - } - - c := conditions.Get(hcloudMachine, infrav1.ServerCreateSucceededCondition) - if c == nil { - return fmt.Errorf("%s not set", infrav1.ServerCreateSucceededCondition) - } - - if c.Status != corev1.ConditionFalse || c.Reason != infrav1.ImageNotFoundReason { - return fmt.Errorf( - "expected %s to be False with reason %q, got status=%q reason=%q message=%q", - infrav1.ServerCreateSucceededCondition, - infrav1.ImageNotFoundReason, - c.Status, - c.Reason, - c.Message, - ) - } - - return nil + return helpers.ConditionFalseWithReasonAtKey( + ctx, + testEnv, + key, + hcloudMachine, + infrav1.ServerCreateSucceededCondition, + infrav1.ImageNotFoundReason, + ) }, timeout, interval).Should(Succeed()) }) }) diff --git a/test/helpers/conditions.go b/test/helpers/conditions.go new file mode 100644 index 000000000..1c58229d7 --- /dev/null +++ b/test/helpers/conditions.go @@ -0,0 +1,68 @@ +/* +Copyright 2022 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 helpers + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type conditionedObject interface { + client.Object + conditions.Getter +} + +func conditionFalseWithReason(getter conditions.Getter, condition clusterv1.ConditionType, reason string) error { + if !conditions.Has(getter, condition) { + return fmt.Errorf("%s not set", condition) + } + + objectCondition := conditions.Get(getter, condition) + if objectCondition.Status != corev1.ConditionFalse || objectCondition.Reason != reason { + return fmt.Errorf( + "expected %s to be False with reason %q, got status=%q reason=%q message=%q", + condition, + reason, + objectCondition.Status, + objectCondition.Reason, + objectCondition.Message, + ) + } + + return nil +} + +// ConditionFalseWithReasonAtKey fetches an object and checks that a condition is false with the expected reason. +func ConditionFalseWithReasonAtKey( + ctx context.Context, + reader client.Reader, + key client.ObjectKey, + obj conditionedObject, + condition clusterv1.ConditionType, + reason string, +) error { + if err := reader.Get(ctx, key, obj); err != nil { + return err + } + + return conditionFalseWithReason(obj, condition, reason) +} From 5e10227b1392a0f6a2abf1a0ce23368119bf855a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 14:23:11 +0200 Subject: [PATCH 09/12] Revert "test: add condition Eventually helper" This reverts commit d93a20cd5ad0c49b6e3d5e9fee5edb1d30dcc39e. --- controllers/hcloudmachine_controller_test.go | 30 ++++++--- test/helpers/conditions.go | 68 -------------------- 2 files changed, 21 insertions(+), 77 deletions(-) delete mode 100644 test/helpers/conditions.go diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index 8f49d1e96..38542e8c1 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -37,7 +37,6 @@ import ( infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" "github.com/syself/cluster-api-provider-hetzner/pkg/utils" - "github.com/syself/cluster-api-provider-hetzner/test/helpers" ) func TestIgnoreInsignificantHCloudMachineStatusUpdates(t *testing.T) { @@ -515,14 +514,27 @@ var _ = Describe("HCloudMachineReconciler", func() { It("checks that ImageNotFound is visible in conditions if image does not exist", func() { Eventually(func() error { - return helpers.ConditionFalseWithReasonAtKey( - ctx, - testEnv, - key, - hcloudMachine, - infrav1.ServerCreateSucceededCondition, - infrav1.ImageNotFoundReason, - ) + if err := testEnv.Get(ctx, key, hcloudMachine); err != nil { + return err + } + + c := conditions.Get(hcloudMachine, infrav1.ServerCreateSucceededCondition) + if c == nil { + return fmt.Errorf("%s not set", infrav1.ServerCreateSucceededCondition) + } + + if c.Status != corev1.ConditionFalse || c.Reason != infrav1.ImageNotFoundReason { + return fmt.Errorf( + "expected %s to be False with reason %q, got status=%q reason=%q message=%q", + infrav1.ServerCreateSucceededCondition, + infrav1.ImageNotFoundReason, + c.Status, + c.Reason, + c.Message, + ) + } + + return nil }, timeout, interval).Should(Succeed()) }) }) diff --git a/test/helpers/conditions.go b/test/helpers/conditions.go deleted file mode 100644 index 1c58229d7..000000000 --- a/test/helpers/conditions.go +++ /dev/null @@ -1,68 +0,0 @@ -/* -Copyright 2022 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 helpers - -import ( - "context" - "fmt" - - corev1 "k8s.io/api/core/v1" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type conditionedObject interface { - client.Object - conditions.Getter -} - -func conditionFalseWithReason(getter conditions.Getter, condition clusterv1.ConditionType, reason string) error { - if !conditions.Has(getter, condition) { - return fmt.Errorf("%s not set", condition) - } - - objectCondition := conditions.Get(getter, condition) - if objectCondition.Status != corev1.ConditionFalse || objectCondition.Reason != reason { - return fmt.Errorf( - "expected %s to be False with reason %q, got status=%q reason=%q message=%q", - condition, - reason, - objectCondition.Status, - objectCondition.Reason, - objectCondition.Message, - ) - } - - return nil -} - -// ConditionFalseWithReasonAtKey fetches an object and checks that a condition is false with the expected reason. -func ConditionFalseWithReasonAtKey( - ctx context.Context, - reader client.Reader, - key client.ObjectKey, - obj conditionedObject, - condition clusterv1.ConditionType, - reason string, -) error { - if err := reader.Get(ctx, key, obj); err != nil { - return err - } - - return conditionFalseWithReason(obj, condition, reason) -} From 94d2bd5671a21d3ab2e681fef04f340c4bfc0c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 15:04:01 +0200 Subject: [PATCH 10/12] reverted changes to tests which are not needed. --- controllers/hcloudmachine_controller_test.go | 26 +++----------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index 38542e8c1..751da28a7 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -513,29 +513,9 @@ var _ = Describe("HCloudMachineReconciler", func() { }) It("checks that ImageNotFound is visible in conditions if image does not exist", func() { - Eventually(func() error { - if err := testEnv.Get(ctx, key, hcloudMachine); err != nil { - return err - } - - c := conditions.Get(hcloudMachine, infrav1.ServerCreateSucceededCondition) - if c == nil { - return fmt.Errorf("%s not set", infrav1.ServerCreateSucceededCondition) - } - - if c.Status != corev1.ConditionFalse || c.Reason != infrav1.ImageNotFoundReason { - return fmt.Errorf( - "expected %s to be False with reason %q, got status=%q reason=%q message=%q", - infrav1.ServerCreateSucceededCondition, - infrav1.ImageNotFoundReason, - c.Status, - c.Reason, - c.Message, - ) - } - - return nil - }, timeout, interval).Should(Succeed()) + Eventually(func() bool { + return isPresentAndFalseWithReason(key, hcloudMachine, infrav1.ServerCreateSucceededCondition, infrav1.ImageNotFoundReason) + }, timeout, interval).Should(BeTrue()) }) }) }) From 8ab0d332d1e3050dc5ac813f937c4120a9fe280b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 16:21:44 +0200 Subject: [PATCH 11/12] remove not needed code. --- pkg/services/hcloud/server/server.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index 41ee559cc..b4447919b 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -252,17 +252,6 @@ func (s *Service) handleBootStateUnset(ctx context.Context) (reconcile.Result, e return reconcile.Result{}, nil } - if errors.Is(err, errServerCreateNotPossible) { - err = fmt.Errorf("createServerFromImageNameOrURL failed: %w", err) - s.scope.Error(err, "") - if !conditions.IsFalse(hm, infrav1.ServerCreateSucceededCondition) { - // Preserve a more specific condition reason already set by lower-level helpers. - conditions.MarkFalse(hm, infrav1.ServerCreateSucceededCondition, - "ServerCreateNotPossible", clusterv1.ConditionSeverityWarning, - "%s", err.Error()) - } - return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil - } return reconcile.Result{}, fmt.Errorf("failed to create server: %w", err) } From 91c28c1b411f6d870047ff289ea535973d4dc4cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=BCttler?= Date: Tue, 14 Apr 2026 16:46:52 +0200 Subject: [PATCH 12/12] added this to ensure last typo does not happen again. --- pkg/services/hcloud/server/server.go | 6 ++++- pkg/services/hcloud/server/server_test.go | 27 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/pkg/services/hcloud/server/server.go b/pkg/services/hcloud/server/server.go index b4447919b..dc3741480 100644 --- a/pkg/services/hcloud/server/server.go +++ b/pkg/services/hcloud/server/server.go @@ -251,7 +251,11 @@ func (s *Service) handleBootStateUnset(ctx context.Context) (reconcile.Result, e ) return reconcile.Result{}, nil } - + if errors.Is(err, errServerCreateNotPossible) { + err = fmt.Errorf("createServerFromImageNameOrURL failed: %w", err) + s.scope.Error(err, "") + return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil + } return reconcile.Result{}, fmt.Errorf("failed to create server: %w", err) } diff --git a/pkg/services/hcloud/server/server_test.go b/pkg/services/hcloud/server/server_test.go index 624c8d2a5..b9b6f8d69 100644 --- a/pkg/services/hcloud/server/server_test.go +++ b/pkg/services/hcloud/server/server_test.go @@ -1229,6 +1229,33 @@ var _ = Describe("Reconcile", func() { Expect(isPresentAndFalseWithReason(service.scope.HCloudMachine, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason)).To(BeTrue()) }) + It("requeues for 5 minutes when server creation is not possible while creating a server", func() { + By("setting the bootstrap data") + err = testEnv.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrapsecret", + Namespace: testNs.Name, + }, + Data: map[string][]byte{ + "value": []byte("dummy-bootstrap-data"), + }, + }) + Expect(err).To(BeNil()) + + service.scope.Machine.Spec.Bootstrap.DataSecretName = ptr.To("bootstrapsecret") + + By("ensuring that the mock hcloud client returns no server type") + hcloudClient.On("GetServerType", mock.Anything, mock.Anything).Return(nil, nil).Once() + + By("calling reconcile") + res, err := service.Reconcile(ctx) + Expect(err).To(BeNil()) + Expect(res).To(Equal(reconcile.Result{RequeueAfter: 5 * time.Minute})) + + By("ensuring the server type not found condition is set") + Expect(isPresentAndFalseWithReason(service.scope.HCloudMachine, infrav1.ServerCreateSucceededCondition, infrav1.ServerTypeNotFoundReason)).To(BeTrue()) + }) + It("sets condition HCloudCredentialsInvalid when HCloud API returns 'unauthorized' error while finding server", func() { By("setting the bootstrap data") err = testEnv.Create(ctx, &corev1.Secret{