Skip to content

Commit

Permalink
🌱 Always create a Condition, if createServer failed (hcloud) (#1428)
Browse files Browse the repository at this point in the history
* 🌱 Always create a Condition, if createServer failed (hcloud)

* marking flaky test hetznerBaremetalMachine

failed run:
  https://github.com/syself/cluster-api-provider-hetzner/actions/runs/10319716274/job/28568676272?pr=1429

re-running helped:
  https://github.com/syself/cluster-api-provider-hetzner/actions/runs/10316952420

* simplify according to #1414
  • Loading branch information
guettli authored Aug 10, 2024
1 parent 0fad3b1 commit 705c1bd
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 44 deletions.
8 changes: 2 additions & 6 deletions api/v1beta1/conditions_const.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,10 @@ const (
ImageNotFoundReason = "ImageNotFound"
// ImageAmbiguousReason indicates that there are multiple images with the required properties.
ImageAmbiguousReason = "ImageAmbiguous"
// ServerLimitExceededReason indicates that server resource rate limit is hit.
ServerLimitExceededReason = "ServerLimitExceeded"
// ServerTypeNotFoundReason indicates that server type could not be found.
ServerTypeNotFoundReason = "ServerTypeNotFound"
// ServerResourceUnavailableReason indicates that server resource is unavailable.
ServerResourceUnavailableReason = "ServerResourceUnavailable"
// ServerPlacementErrorReason indicates placement error while creating server.
ServerPlacementErrorReason = "ServerPlacementError"
// ServerCreateFailedReason indicates that server could not get created.
ServerCreateFailedReason = "ServerCreateFailedReason"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion controllers/hetznerbaremetalremediation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ var _ = Describe("HetznerBareMetalRemediationReconciler", func() {
Expect(testEnv.Cleanup(ctx, hetznerBareMetalRemediation, hetznerBaremetalMachine)).To(Succeed())
})

It("should not remediate if no annotations is present in the hetznerBaremetalMachine", func() {
It("should not remediate if no annotations is present in the hetznerBaremetalMachine (flaky)", func() {
Expect(testEnv.Create(ctx, hetznerBaremetalMachine)).To(Succeed())
Expect(testEnv.Create(ctx, hetznerBareMetalRemediation)).To(Succeed())

Expand Down
48 changes: 12 additions & 36 deletions pkg/services/hcloud/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,49 +450,25 @@ func (s *Service) createServer(ctx context.Context) (*hcloud.Server, error) {
// Create the server
server, err := s.scope.HCloudClient.CreateServer(ctx, opts)
if err != nil {
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "CreateServer")
if hcloud.IsError(err, hcloud.ErrorCodeResourceLimitExceeded) {
conditions.MarkFalse(
s.scope.HCloudMachine,
infrav1.ServerCreateSucceededCondition,
infrav1.ServerLimitExceededReason,
clusterv1.ConditionSeverityError,
err.Error(),
)

err = errors.Join(errServerCreateNotPossible, err)
}

if hcloud.IsError(err, hcloud.ErrorCodeResourceUnavailable) {
conditions.MarkFalse(
s.scope.HCloudMachine,
infrav1.ServerCreateSucceededCondition,
infrav1.ServerResourceUnavailableReason,
clusterv1.ConditionSeverityWarning,
err.Error(),
)

err = errors.Join(errServerCreateNotPossible, err)
}

if hcloud.IsError(err, hcloud.ErrorCodePlacementError) {
conditions.MarkFalse(
s.scope.HCloudMachine,
infrav1.ServerCreateSucceededCondition,
infrav1.ServerPlacementErrorReason,
clusterv1.ConditionSeverityWarning,
err.Error(),
)

err = errors.Join(errServerCreateNotPossible, err)
if hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "CreateServer") {
// RateLimit was reached. Condition and Event got already created.
return nil, fmt.Errorf("failed to create HCloud server %s: %w", s.scope.HCloudMachine.Name, err)
}

// No condition was set yet. Set a general condition to false.
conditions.MarkFalse(
s.scope.HCloudMachine,
infrav1.ServerCreateSucceededCondition,
infrav1.ServerCreateFailedReason,
clusterv1.ConditionSeverityWarning,
err.Error(),
)
record.Warnf(s.scope.HCloudMachine,
"FailedCreateHCloudServer",
"Failed to create HCloud server %s: %s",
s.scope.Name(),
err,
)
err = errors.Join(errServerCreateNotPossible, err)
return nil, fmt.Errorf("failed to create HCloud server %s: %w", s.scope.HCloudMachine.Name, err)
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/services/hcloud/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ type runtimeObjectWithConditions interface {
}

// HandleRateLimitExceeded handles rate limit exceeded errors.
func HandleRateLimitExceeded(obj runtimeObjectWithConditions, err error, functionName string) {
// Returns true if RateLimit is reached.
func HandleRateLimitExceeded(obj runtimeObjectWithConditions, err error, functionName string) bool {
if hcloud.IsError(err, hcloud.ErrorCodeRateLimitExceeded) {
msg := fmt.Sprintf("exceeded hcloud rate limit with calling function %q", functionName)
conditions.MarkFalse(
Expand All @@ -80,5 +81,7 @@ func HandleRateLimitExceeded(obj runtimeObjectWithConditions, err error, functio
msg,
)
record.Warnf(obj, "RateLimitExceeded", msg)
return true
}
return false
}

0 comments on commit 705c1bd

Please sign in to comment.