Skip to content

Commit

Permalink
Introduce ReconcileError with Transient and Terminal Error type
Browse files Browse the repository at this point in the history 
Remove RequeueAfterError(depricated in CAPI)
Remove repetative error setting on m3Machine
  • Loading branch information
@Sunnatillo
Sunnatillo committed Jul 12, 2023
1 parent 1a79f3f commit ecd4b65
Show file tree
Hide file tree
Showing 18 changed files with 336 additions and 325 deletions.
35 changes: 21 additions & 14 deletions baremetal/metal3data_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ func (m *DataManager) Reconcile(ctx context.Context) error {
m.clearError(ctx)

if err := m.createSecrets(ctx); err != nil {
if ok := errors.As(err, &hasRequeueAfterError); ok {
var reconcileError ReconcileError
if errors.As(err, &reconcileError) && reconcileError.IsTransient() {
return err
}
m.setError(ctx, errors.Cause(err).Error())
Expand Down Expand Up @@ -220,8 +221,9 @@ func (m *DataManager) createSecrets(ctx context.Context) error {
return errors.Wrapf(err, "Metal3Machine's owner Machine could not be retrieved")
}
if capiMachine == nil {
m.Log.Info("Waiting for Machine Controller to set OwnerRef on Metal3Machine")
return &RequeueAfterError{RequeueAfter: requeueAfter}
errMessage := "Waiting for Machine Controller to set OwnerRef on Metal3Machine"
m.Log.Info(errMessage)
return WithTransientError(errors.New(errMessage), requeueAfter)
}
m.Log.Info("Fetched Machine")

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

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

m.Log.Info("done allocating addresses", "addresses", addresses, "requeue", requeue)
if requeue {
return addresses, &RequeueAfterError{RequeueAfter: requeueAfter}
return addresses, WithTransientError(nil, requeueAfter)
}
return addresses, nil
}
Expand Down Expand Up @@ -620,7 +624,8 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
return reconciledClaim{m3Claim: ipClaim}, nil
}

if ok := errors.As(err, &hasRequeueAfterError); !ok {
var reconcileError ReconcileError
if !errors.As(err, &reconcileError) {
return reconciledClaim{m3Claim: ipClaim}, err
}

Expand Down Expand Up @@ -651,15 +656,15 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
return reconciledClaim{m3Claim: ipClaim}, err
}
if bmh == nil {
return reconciledClaim{m3Claim: ipClaim}, &RequeueAfterError{RequeueAfter: requeueAfter}
return reconciledClaim{m3Claim: ipClaim}, WithTransientError(nil, requeueAfter)
}
m.Log.Info("Fetched BMH")

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

Expand All @@ -683,7 +688,7 @@ func (m *DataManager) ensureM3IPClaim(ctx context.Context, poolRef corev1.TypedL
}
err = createObject(ctx, m.client, ipClaim)
if err != nil {
if ok := errors.As(err, &hasRequeueAfterError); !ok {
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
return reconciledClaim{m3Claim: ipClaim}, err
}
}
Expand Down Expand Up @@ -783,7 +788,8 @@ func (m *DataManager) releaseAddressFromM3Pool(ctx context.Context, poolRef core
}
ipClaim, err = fetchM3IPClaim(ctx, m.client, m.Log, m.Data.Name+"-"+poolRef.Name, m.Data.Namespace)
if err != nil {
if ok := errors.As(err, &hasRequeueAfterError); !ok {
var reconcileError ReconcileError
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
return err
}
return nil
Expand Down Expand Up @@ -1373,7 +1379,7 @@ func (m *DataManager) getM3Machine(ctx context.Context, m3dt *infrav1.Metal3Data

if err := m.client.Get(ctx, claimNamespacedName, capm3DataClaim); err != nil {
if apierrors.IsNotFound(err) {
return nil, &RequeueAfterError{RequeueAfter: requeueAfter}
return nil, WithTransientError(nil, requeueAfter)
}
return nil, err
}
Expand Down Expand Up @@ -1413,8 +1419,9 @@ func fetchM3IPClaim(ctx context.Context, cl client.Client, mLog logr.Logger,
}
if err := cl.Get(ctx, metal3ClaimName, metal3IPClaim); err != nil {
if apierrors.IsNotFound(err) {
mLog.Info("Address claim not found, requeuing")
return nil, &RequeueAfterError{RequeueAfter: requeueAfter}
errMessage := "Address claim not found, requeuing"
mLog.Info(errMessage)
return nil, WithTransientError(errors.New(errMessage), requeueAfter)
}
err := errors.Wrap(err, "Failed to get address claim")
return nil, err
Expand Down
26 changes: 13 additions & 13 deletions baremetal/metal3data_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.expectError || tc.expectRequeue {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -190,9 +190,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.expectError || tc.expectRequeue {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
return
}
Expand Down Expand Up @@ -579,9 +579,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.expectError || tc.expectRequeue {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -672,9 +672,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.expectError || tc.expectRequeue {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -1051,9 +1051,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.expectError || tc.expectRequeue {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -1239,7 +1239,7 @@ var _ = Describe("Metal3Data manager", func() {
)
if tc.expectError {
if tc.m3dt != nil {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).To(HaveOccurred())
}
Expand Down Expand Up @@ -3342,9 +3342,9 @@ var _ = Describe("Metal3Data manager", func() {
if tc.ExpectError || tc.ExpectRequeue {
Expect(err).To(HaveOccurred())
if tc.ExpectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down
5 changes: 3 additions & 2 deletions baremetal/metal3datatemplate_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,11 @@ func (m *DataTemplateManager) createData(ctx context.Context,
}

// Create the Metal3Data object. If we get a conflict (that will set
// HasRequeueAfterError), then requeue to retrigger the reconciliation with
// TransientType ReconcileError), then requeue to retrigger the reconciliation with
// the new state
if err := createObject(ctx, m.client, dataObject); err != nil {
if ok := errors.As(err, &requeueAfterError); !ok {
var reconcileError ReconcileError
if !(errors.As(err, &reconcileError) && reconcileError.IsTransient()) {
dataClaim.Status.ErrorMessage = pointer.String("Failed to create associated Metal3Data object")
}
return indexes, err
Expand Down
8 changes: 4 additions & 4 deletions baremetal/metal3datatemplate_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ var _ = Describe("Metal3DataTemplate manager", func() {
if tc.expectRequeue || tc.expectError {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -625,9 +625,9 @@ var _ = Describe("Metal3DataTemplate manager", func() {
if tc.expectRequeue || tc.expectError {
Expect(err).To(HaveOccurred())
if tc.expectRequeue {
Expect(err).To(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).To(BeAssignableToTypeOf(ReconcileError{}))
} else {
Expect(err).NotTo(BeAssignableToTypeOf(&RequeueAfterError{}))
Expect(err).NotTo(BeAssignableToTypeOf(ReconcileError{}))
}
} else {
Expect(err).NotTo(HaveOccurred())
Expand Down
Loading

0 comments on commit ecd4b65

Please sign in to comment.