Skip to content

Commit 90fda08

Browse files
metal3-io-botmetal3-io-bot
authored
Merge pull request #1089 from Nordix/backport-improve-error-handling-to-release-1.4/sunnat
✨Introduce ReconcileError with Transient and Terminal Error type
2 parents e54256e + babf27a commit 90fda08

18 files changed

+336
-325
lines changed

‎baremetal/metal3data_manager.go‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ func (m *DataManager) Reconcile(ctx context.Context) error {
113113
m.clearError(ctx)
114114

115115
if err := m.createSecrets(ctx); err != nil {
116-
if ok := errors.As(err, &hasRequeueAfterError); ok {
116+
var reconcileError ReconcileError
117+
if errors.As(err, &reconcileError) && reconcileError.IsTransient() {
117118
return err
118119
}
119120
m.setError(ctx, errors.Cause(err).Error())
@@ -220,8 +221,9 @@ func (m *DataManager) createSecrets(ctx context.Context) error {
220221
return errors.Wrapf(err, "Metal3Machine's owner Machine could not be retrieved")
221222
}
222223
if capiMachine == nil {
223-
m.Log.Info("Waiting for Machine Controller to set OwnerRef on Metal3Machine")
224-
return &RequeueAfterError{RequeueAfter: requeueAfter}
224+
errMessage := "Waiting for Machine Controller to set OwnerRef on Metal3Machine"
225+
m.Log.Info(errMessage)
226+
return WithTransientError(errors.New(errMessage), requeueAfter)
225227
}
226228
m.Log.Info("Fetched Machine")
227229

@@ -231,7 +233,9 @@ func (m *DataManager) createSecrets(ctx context.Context) error {
231233
return err
232234
}
233235
if bmh == nil {
234-
return &RequeueAfterError{RequeueAfter: requeueAfter}
236+
errMessage := "Waiting for BareMetalHost to become available"
237+
m.Log.Info(errMessage)
238+
return WithTransientError(errors.New(errMessage), requeueAfter)
235239
}
236240
m.Log.Info("Fetched BMH")
237241

@@ -328,7 +332,7 @@ type reconciledClaim struct {
328332
// getAddressesFromPool allocates IP addresses from all IP pools referenced by a [Metal3DataTemplate].
329333
// It does so by creating IP claims for each referenced pool. It will check whether the claim was fulfilled
330334
// and return a map containing all pools and addresses. If some claims are not fulfilled yet, it will
331-
// return a [RequeueAfterError], indicating that some addresses were not fully allocated yet.
335+
// return a Transient type ReconcileError, indicating that some addresses were not fully allocated yet.
332336
func (m *DataManager) getAddressesFromPool(ctx context.Context,
333337
m3dt infrav1.Metal3DataTemplate,
334338
) (map[string]addressFromPool, error) {
@@ -388,7 +392,7 @@ func (m *DataManager) getAddressesFromPool(ctx context.Context,
388392

389393
m.Log.Info("done allocating addresses", "addresses", addresses, "requeue", requeue)
390394
if requeue {
391-
return addresses, &RequeueAfterError{RequeueAfter: requeueAfter}
395+
return addresses, WithTransientError(nil, requeueAfter)
392396
}
393397
return addresses, nil
394398
}
@@ -620,7 +624,8 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
620624
return reconciledClaim{m3Claim: ipClaim}, nil
621625
}
622626

623-
if ok := errors.As(err, &hasRequeueAfterError); !ok {
627+
var reconcileError ReconcileError
628+
if !errors.As(err, &reconcileError) {
624629
return reconciledClaim{m3Claim: ipClaim}, err
625630
}
626631

@@ -651,15 +656,15 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
651656
return reconciledClaim{m3Claim: ipClaim}, err
652657
}
653658
if bmh == nil {
654-
return reconciledClaim{m3Claim: ipClaim}, &RequeueAfterError{RequeueAfter: requeueAfter}
659+
return reconciledClaim{m3Claim: ipClaim}, WithTransientError(nil, requeueAfter)
655660
}
656661
m.Log.Info("Fetched BMH")
657662

658663
ipClaim, err = fetchM3IPClaim(ctx, m.client, m.Log, bmh.Name+"-"+poolRef.Name, m.Data.Namespace)
659664
if err == nil {
660665
return reconciledClaim{m3Claim: ipClaim}, nil
661666
}
662-
if ok := errors.As(err, &hasRequeueAfterError); !ok {
667+
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
663668
return reconciledClaim{m3Claim: ipClaim}, err
664669
}
665670

@@ -683,7 +688,7 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
683688
}
684689
err = createObject(ctx, m.client, ipClaim)
685690
if err != nil {
686-
if ok := errors.As(err, &hasRequeueAfterError); !ok {
691+
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
687692
return reconciledClaim{m3Claim: ipClaim}, err
688693
}
689694
}
@@ -783,7 +788,8 @@ func (m *DataManager) releaseAddressFromM3Pool(ctx context.Context, poolRef core
783788
}
784789
ipClaim, err = fetchM3IPClaim(ctx, m.client, m.Log, m.Data.Name+"-"+poolRef.Name, m.Data.Namespace)
785790
if err != nil {
786-
if ok := errors.As(err, &hasRequeueAfterError); !ok {
791+
var reconcileError ReconcileError
792+
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
787793
return err
788794
}
789795
return nil
@@ -1373,7 +1379,7 @@ func (m *DataManager) getM3Machine(ctx context.Context, m3dt *infrav1.Metal3Data
13731379

13741380
if err := m.client.Get(ctx, claimNamespacedName, capm3DataClaim); err != nil {
13751381
if apierrors.IsNotFound(err) {
1376-
return nil, &RequeueAfterError{RequeueAfter: requeueAfter}
1382+
return nil, WithTransientError(nil, requeueAfter)
13771383
}
13781384
return nil, err
13791385
}
@@ -1413,8 +1419,9 @@ func fetchM3IPClaim(ctx context.Context, cl client.Client, mLog logr.Logger,
14131419
}
14141420
if err := cl.Get(ctx, metal3ClaimName, metal3IPClaim); err != nil {
14151421
if apierrors.IsNotFound(err) {
1416-
mLog.Info("Address claim not found, requeuing")
1417-
return nil, &RequeueAfterError{RequeueAfter: requeueAfter}
1422+
errMessage := "Address claim not found, requeuing"
1423+
mLog.Info(errMessage)
1424+
return nil, WithTransientError(errors.New(errMessage), requeueAfter)
14181425
}
14191426
err := errors.Wrap(err, "Failed to get address claim")
14201427
return nil, err

‎baremetal/metal3data_manager_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ var _ = Describe("Metal3Data manager", func() {
109109
if tc.expectError || tc.expectRequeue {
110110
Expect(err).To(HaveOccurred())
111111
if tc.expectRequeue {
112-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
112+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
113113
} else {
114-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
114+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
115115
}
116116
} else {
117117
Expect(err).NotTo(HaveOccurred())
@@ -190,9 +190,9 @@ var _ = Describe("Metal3Data manager", func() {
190190
if tc.expectError || tc.expectRequeue {
191191
Expect(err).To(HaveOccurred())
192192
if tc.expectRequeue {
193-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
193+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
194194
} else {
195-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
195+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
196196
}
197197
return
198198
}
@@ -579,9 +579,9 @@ var _ = Describe("Metal3Data manager", func() {
579579
if tc.expectError || tc.expectRequeue {
580580
Expect(err).To(HaveOccurred())
581581
if tc.expectRequeue {
582-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
582+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
583583
} else {
584-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
584+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
585585
}
586586
} else {
587587
Expect(err).NotTo(HaveOccurred())
@@ -672,9 +672,9 @@ var _ = Describe("Metal3Data manager", func() {
672672
if tc.expectError || tc.expectRequeue {
673673
Expect(err).To(HaveOccurred())
674674
if tc.expectRequeue {
675-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
675+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
676676
} else {
677-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
677+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
678678
}
679679
} else {
680680
Expect(err).NotTo(HaveOccurred())
@@ -1051,9 +1051,9 @@ var _ = Describe("Metal3Data manager", func() {
10511051
if tc.expectError || tc.expectRequeue {
10521052
Expect(err).To(HaveOccurred())
10531053
if tc.expectRequeue {
1054-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
1054+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
10551055
} else {
1056-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
1056+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
10571057
}
10581058
} else {
10591059
Expect(err).NotTo(HaveOccurred())
@@ -1239,7 +1239,7 @@ var _ = Describe("Metal3Data manager", func() {
12391239
)
12401240
if tc.expectError {
12411241
if tc.m3dt != nil {
1242-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
1242+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
12431243
} else {
12441244
Expect(err).To(HaveOccurred())
12451245
}
@@ -3342,9 +3342,9 @@ var _ = Describe("Metal3Data manager", func() {
33423342
if tc.ExpectError || tc.ExpectRequeue {
33433343
Expect(err).To(HaveOccurred())
33443344
if tc.ExpectRequeue {
3345-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
3345+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
33463346
} else {
3347-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
3347+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
33483348
}
33493349
} else {
33503350
Expect(err).NotTo(HaveOccurred())

‎baremetal/metal3datatemplate_manager.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,10 +353,11 @@ func (m *DataTemplateManager) createData(ctx context.Context,
353353
}
354354

355355
// Create the Metal3Data object. If we get a conflict (that will set
356-
// HasRequeueAfterError), then requeue to retrigger the reconciliation with
356+
// TransientType ReconcileError), then requeue to retrigger the reconciliation with
357357
// the new state
358358
if err := createObject(ctx, m.client, dataObject); err != nil {
359-
if ok := errors.As(err, &requeueAfterError); !ok {
359+
var reconcileError ReconcileError
360+
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
360361
dataClaim.Status.ErrorMessage = pointer.String("Failed to create associated Metal3Data object")
361362
}
362363
return indexes, err

‎baremetal/metal3datatemplate_manager_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,9 @@ var _ = Describe("Metal3DataTemplate manager", func() {
262262
if tc.expectRequeue || tc.expectError {
263263
Expect(err).To(HaveOccurred())
264264
if tc.expectRequeue {
265-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
265+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
266266
} else {
267-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
267+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
268268
}
269269
} else {
270270
Expect(err).NotTo(HaveOccurred())
@@ -625,9 +625,9 @@ var _ = Describe("Metal3DataTemplate manager", func() {
625625
if tc.expectRequeue || tc.expectError {
626626
Expect(err).To(HaveOccurred())
627627
if tc.expectRequeue {
628-
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
628+
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
629629
} else {
630-
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
630+
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
631631
}
632632
} else {
633633
Expect(err).NotTo(HaveOccurred())

0 commit comments

Comments
 (0)