Skip to content

Commit ed4d92a

Browse files
killianmuldoonkillianmuldoon
committed
Remove generated names from error messages to reduce reconciliation
Signed-off-by: killianmuldoon <[email protected]>
1 parent c3e0ec6 commit ed4d92a

File tree

2 files changed

+86
-15
lines changed

2 files changed

+86
-15
lines changed

‎internal/controllers/topology/cluster/reconcile_state.go

+37-15
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
184184

185185
cpInfraRef, err := contract.ControlPlane().MachineTemplate().InfrastructureRef().Get(s.Desired.ControlPlane.Object)
186186
if err != nil {
187-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate})
187+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate})
188188
}
189189

190190
// Create or update the MachineInfrastructureTemplate of the control plane.
@@ -198,13 +198,13 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
198198
},
199199
)
200200
if err != nil {
201-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate})
201+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.InfrastructureMachineTemplate})
202202
}
203203

204204
// The controlPlaneObject.Spec.machineTemplate.infrastructureRef has to be updated in the desired object
205205
err = contract.ControlPlane().MachineTemplate().InfrastructureRef().Set(s.Desired.ControlPlane.Object, refToUnstructured(cpInfraRef))
206206
if err != nil {
207-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object})
207+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object})
208208
}
209209
}
210210

@@ -227,7 +227,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
227227
},
228228
},
229229
}); err != nil {
230-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object})
230+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object})
231231
}
232232

233233
// If the ControlPlane has defined a current or desired MachineHealthCheck attempt to reconcile it.
@@ -242,7 +242,7 @@ func (r *Reconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope)
242242

243243
// Reconcile the current and desired state of the MachineHealthCheck.
244244
if err := r.reconcileMachineHealthCheck(ctx, s.Current.ControlPlane.MachineHealthCheck, s.Desired.ControlPlane.MachineHealthCheck); err != nil {
245-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.MachineHealthCheck})
245+
return err
246246
}
247247
}
248248
return nil
@@ -403,21 +403,21 @@ func (r *Reconciler) createMachineDeployment(ctx context.Context, cluster *clust
403403
cluster: cluster,
404404
desired: md.InfrastructureMachineTemplate,
405405
}); err != nil {
406-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.Object})
406+
return errors.Wrapf(err, "failed to create %s", md.Object.Kind)
407407
}
408408

409409
bootstrapCtx, _ := log.WithObject(md.BootstrapTemplate).Into(ctx)
410410
if err := r.reconcileReferencedTemplate(bootstrapCtx, reconcileReferencedTemplateInput{
411411
cluster: cluster,
412412
desired: md.BootstrapTemplate,
413413
}); err != nil {
414-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.Object})
414+
return errors.Wrapf(err, "failed to create %s", md.Object.Kind)
415415
}
416416

417417
log = log.WithObject(md.Object)
418418
log.Infof(fmt.Sprintf("Creating %s", tlog.KObj{Obj: md.Object}))
419419
if err := r.Client.Create(ctx, md.Object.DeepCopy()); err != nil {
420-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.Object})
420+
return createErrorWithoutObjectName(err, md.Object)
421421
}
422422
r.recorder.Eventf(cluster, corev1.EventTypeNormal, createEventReason, "Created %q", tlog.KObj{Obj: md.Object})
423423

@@ -429,7 +429,7 @@ func (r *Reconciler) createMachineDeployment(ctx context.Context, cluster *clust
429429
*ownerReferenceTo(md.Object),
430430
})
431431
if err := r.reconcileMachineHealthCheck(ctx, nil, md.MachineHealthCheck); err != nil {
432-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.MachineHealthCheck})
432+
return err
433433
}
434434
}
435435
return nil
@@ -448,7 +448,7 @@ func (r *Reconciler) updateMachineDeployment(ctx context.Context, cluster *clust
448448
templateNamePrefix: infrastructureMachineTemplateNamePrefix(cluster.Name, mdTopologyName),
449449
compatibilityChecker: check.ObjectsAreCompatible,
450450
}); err != nil {
451-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: currentMD.Object})
451+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: currentMD.Object})
452452
}
453453

454454
bootstrapCtx, _ := log.WithObject(desiredMD.BootstrapTemplate).Into(ctx)
@@ -460,13 +460,13 @@ func (r *Reconciler) updateMachineDeployment(ctx context.Context, cluster *clust
460460
templateNamePrefix: bootstrapTemplateNamePrefix(cluster.Name, mdTopologyName),
461461
compatibilityChecker: check.ObjectsAreInTheSameNamespace,
462462
}); err != nil {
463-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: currentMD.Object})
463+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: currentMD.Object})
464464
}
465465

466466
// Patch MachineHealthCheck for the MachineDeployment.
467467
if desiredMD.MachineHealthCheck != nil || currentMD.MachineHealthCheck != nil {
468468
if err := r.reconcileMachineHealthCheck(ctx, currentMD.MachineHealthCheck, desiredMD.MachineHealthCheck); err != nil {
469-
return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: desiredMD.MachineHealthCheck})
469+
return errors.Wrapf(err, "failed to reconcile %s", tlog.KObj{Obj: desiredMD.MachineHealthCheck})
470470
}
471471
}
472472

@@ -587,7 +587,7 @@ func (r *Reconciler) reconcileReferencedObject(ctx context.Context, in reconcile
587587
return errors.Wrapf(err, "failed to create a copy of %s with the managed field annotation", tlog.KObj{Obj: in.desired})
588588
}
589589
if err := r.Client.Create(ctx, desiredWithManagedFieldAnnotation); err != nil {
590-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: desiredWithManagedFieldAnnotation})
590+
return createErrorWithoutObjectName(err, desiredWithManagedFieldAnnotation)
591591
}
592592
r.recorder.Eventf(in.cluster, corev1.EventTypeNormal, createEventReason, "Created %q", tlog.KObj{Obj: in.desired})
593593
return nil
@@ -664,7 +664,7 @@ func (r *Reconciler) reconcileReferencedTemplate(ctx context.Context, in reconci
664664
return errors.Wrapf(err, "failed to create a copy of %s with the managed field annotation", tlog.KObj{Obj: in.desired})
665665
}
666666
if err := r.Client.Create(ctx, desiredWithManagedFieldAnnotation); err != nil {
667-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: desiredWithManagedFieldAnnotation})
667+
return createErrorWithoutObjectName(err, desiredWithManagedFieldAnnotation)
668668
}
669669
r.recorder.Eventf(in.cluster, corev1.EventTypeNormal, createEventReason, "Created %q", tlog.KObj{Obj: in.desired})
670670
return nil
@@ -716,7 +716,7 @@ func (r *Reconciler) reconcileReferencedTemplate(ctx context.Context, in reconci
716716
return errors.Wrapf(err, "failed to create a copy of %s with the managed field annotation", tlog.KObj{Obj: in.desired})
717717
}
718718
if err := r.Client.Create(ctx, desiredWithManagedFieldAnnotation); err != nil {
719-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: desiredWithManagedFieldAnnotation})
719+
return createErrorWithoutObjectName(err, desiredWithManagedFieldAnnotation)
720720
}
721721
r.recorder.Eventf(in.cluster, corev1.EventTypeNormal, createEventReason, "Created %q as a replacement for %q (template rotation)", tlog.KObj{Obj: in.desired}, in.ref.Name)
722722

@@ -727,3 +727,25 @@ func (r *Reconciler) reconcileReferencedTemplate(ctx context.Context, in reconci
727727

728728
return nil
729729
}
730+
731+
// createErrorWithoutObjectName removes the name of the object from the error message. As each new Create call involves an
732+
// object with a unique generated name each error appears to be a different error. As the errors are being surfaced in a condition
733+
// on the Cluster, the name is removed here to prevent each creation error from triggering a new reconciliation.
734+
func createErrorWithoutObjectName(err error, obj client.Object) error {
735+
var statusError *apierrors.StatusError
736+
if errors.As(err, &statusError) {
737+
out := ""
738+
if statusError.Status().Details != nil {
739+
for _, cause := range statusError.Status().Details.Causes {
740+
out = fmt.Sprintf("%s %s: %s: %s", out, cause.Type, cause.Field, cause.Message)
741+
}
742+
out = fmt.Sprintf("%s.%s: %s", statusError.Status().Details.Kind, statusError.Status().Details.Group, out)
743+
}
744+
745+
// Replace the statusError message with the constructed message.
746+
statusError.ErrStatus.Message = fmt.Sprintf("failed to create: %s", out)
747+
return statusError
748+
}
749+
// If this isn't a StatusError return a more generic error with the object details.
750+
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: obj})
751+
}

‎internal/controllers/topology/cluster/reconcile_state_test.go

+49
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package cluster
1919
import (
2020
"context"
2121
"fmt"
22+
"net/http"
2223
"regexp"
2324
"testing"
2425
"time"
@@ -2081,3 +2082,51 @@ func TestReconciler_reconcileMachineHealthCheck(t *testing.T) {
20812082
})
20822083
}
20832084
}
2085+
2086+
func Test_createErrorWithoutObjectName(t *testing.T) {
2087+
originalError := &apierrors.StatusError{
2088+
ErrStatus: metav1.Status{
2089+
Status: metav1.StatusFailure,
2090+
Code: http.StatusUnprocessableEntity,
2091+
Reason: metav1.StatusReasonInvalid,
2092+
Message: "DockerMachineTemplate.infrastructure.cluster.x-k8s.io \"docker-template-one\" is invalid: spec.template.spec.preLoadImages: Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2093+
Details: &metav1.StatusDetails{
2094+
Group: "infrastructure.cluster.x-k8s.io",
2095+
Kind: "DockerMachineTemplate",
2096+
Name: "docker-template-one",
2097+
Causes: []metav1.StatusCause{
2098+
{
2099+
Type: "FieldValueInvalid",
2100+
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2101+
Field: "spec.template.spec.preLoadImages",
2102+
},
2103+
},
2104+
},
2105+
}}
2106+
wantError := &apierrors.StatusError{
2107+
ErrStatus: metav1.Status{
2108+
Status: metav1.StatusFailure,
2109+
Code: http.StatusUnprocessableEntity,
2110+
Reason: metav1.StatusReasonInvalid,
2111+
// The only difference between the two objects should be in the Message section.
2112+
Message: "failed to create DockerMachineTemplate.infrastructure.cluster.x-k8s.io: FieldValueInvalid: spec.template.spec.preLoadImages: Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\".",
2113+
Details: &metav1.StatusDetails{
2114+
Group: "infrastructure.cluster.x-k8s.io",
2115+
Kind: "DockerMachineTemplate",
2116+
Name: "docker-template-one",
2117+
Causes: []metav1.StatusCause{
2118+
{
2119+
Type: "FieldValueInvalid",
2120+
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2121+
Field: "spec.template.spec.preLoadImages",
2122+
},
2123+
},
2124+
},
2125+
},
2126+
}
2127+
t.Run("Transform a create error correctly", func(t *testing.T) {
2128+
g := NewWithT(t)
2129+
err := createErrorWithoutObjectName(originalError, nil)
2130+
g.Expect(err).To(Equal(wantError), cmp.Diff(err, wantError))
2131+
})
2132+
}

0 commit comments

Comments
 (0)