Skip to content

Commit a08192f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #7051 from killianmuldoon/fix/control-plane-reconcile-error
🐛 Strip control plane creation error of unique name
2 parents a661c90 + 51971d7 commit a08192f

File tree

2 files changed

+172
-51
lines changed

2 files changed

+172
-51
lines changed

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -804,22 +804,34 @@ func (r *Reconciler) reconcileReferencedTemplate(ctx context.Context, in reconci
804804
func createErrorWithoutObjectName(err error, obj client.Object) error {
805805
var statusError *apierrors.StatusError
806806
if errors.As(err, &statusError) {
807+
var msg string
807808
if statusError.Status().Details != nil {
808809
var causes []string
809810
for _, cause := range statusError.Status().Details.Causes {
810811
causes = append(causes, fmt.Sprintf("%s: %s: %s", cause.Type, cause.Field, cause.Message))
811812
}
812-
var msg string
813813
if len(causes) > 0 {
814814
msg = fmt.Sprintf("failed to create %s.%s: %s", statusError.Status().Details.Kind, statusError.Status().Details.Group, strings.Join(causes, " "))
815815
} else {
816816
msg = fmt.Sprintf("failed to create %s.%s", statusError.Status().Details.Kind, statusError.Status().Details.Group)
817817
}
818-
// Replace the statusError message with the constructed message.
819818
statusError.ErrStatus.Message = msg
820819
return statusError
821820
}
821+
822+
if statusError.Status().Message != "" {
823+
if obj != nil {
824+
msg = fmt.Sprintf("failed to create %s", obj.GetObjectKind().GroupVersionKind().GroupKind().String())
825+
} else {
826+
msg = "failed to create object"
827+
}
828+
}
829+
statusError.ErrStatus.Message = msg
830+
return statusError
822831
}
823832
// If this isn't a StatusError return a more generic error with the object details.
824-
return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: obj})
833+
if obj != nil {
834+
return errors.Errorf("failed to create %s", obj.GetObjectKind().GroupVersionKind().GroupKind().String())
835+
}
836+
return errors.New("failed to create object")
825837
}

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

Lines changed: 157 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/google/go-cmp/cmp"
2727
. "github.com/onsi/gomega"
28+
"github.com/pkg/errors"
2829
corev1 "k8s.io/api/core/v1"
2930
apierrors "k8s.io/apimachinery/pkg/api/errors"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -2722,54 +2723,6 @@ func TestReconciler_reconcileMachineHealthCheck(t *testing.T) {
27222723
}
27232724
}
27242725

2725-
func Test_createErrorWithoutObjectName(t *testing.T) {
2726-
originalError := &apierrors.StatusError{
2727-
ErrStatus: metav1.Status{
2728-
Status: metav1.StatusFailure,
2729-
Code: http.StatusUnprocessableEntity,
2730-
Reason: metav1.StatusReasonInvalid,
2731-
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\"",
2732-
Details: &metav1.StatusDetails{
2733-
Group: "infrastructure.cluster.x-k8s.io",
2734-
Kind: "DockerMachineTemplate",
2735-
Name: "docker-template-one",
2736-
Causes: []metav1.StatusCause{
2737-
{
2738-
Type: "FieldValueInvalid",
2739-
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2740-
Field: "spec.template.spec.preLoadImages",
2741-
},
2742-
},
2743-
},
2744-
}}
2745-
wantError := &apierrors.StatusError{
2746-
ErrStatus: metav1.Status{
2747-
Status: metav1.StatusFailure,
2748-
Code: http.StatusUnprocessableEntity,
2749-
Reason: metav1.StatusReasonInvalid,
2750-
// The only difference between the two objects should be in the Message section.
2751-
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\"",
2752-
Details: &metav1.StatusDetails{
2753-
Group: "infrastructure.cluster.x-k8s.io",
2754-
Kind: "DockerMachineTemplate",
2755-
Name: "docker-template-one",
2756-
Causes: []metav1.StatusCause{
2757-
{
2758-
Type: "FieldValueInvalid",
2759-
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2760-
Field: "spec.template.spec.preLoadImages",
2761-
},
2762-
},
2763-
},
2764-
},
2765-
}
2766-
t.Run("Transform a create error correctly", func(t *testing.T) {
2767-
g := NewWithT(t)
2768-
err := createErrorWithoutObjectName(originalError, nil)
2769-
g.Expect(err).To(Equal(wantError), cmp.Diff(err, wantError))
2770-
})
2771-
}
2772-
27732726
// prepareControlPlaneBluePrint deep-copies and returns the input scope and sets
27742727
// the given namespace to all relevant objects.
27752728
func prepareControlPlaneBluePrint(in *scope.ControlPlaneBlueprint, namespace string) *scope.ControlPlaneBlueprint {
@@ -2872,3 +2825,159 @@ func prepareCluster(in *clusterv1.Cluster, namespace string) *clusterv1.Cluster
28722825
}
28732826
return c
28742827
}
2828+
2829+
func Test_createErrorWithoutObjectName(t *testing.T) {
2830+
detailsError := &apierrors.StatusError{
2831+
ErrStatus: metav1.Status{
2832+
Status: metav1.StatusFailure,
2833+
Code: http.StatusUnprocessableEntity,
2834+
Reason: metav1.StatusReasonInvalid,
2835+
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\"",
2836+
Details: &metav1.StatusDetails{
2837+
Group: "infrastructure.cluster.x-k8s.io",
2838+
Kind: "DockerMachineTemplate",
2839+
Name: "docker-template-one",
2840+
Causes: []metav1.StatusCause{
2841+
{
2842+
Type: "FieldValueInvalid",
2843+
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2844+
Field: "spec.template.spec.preLoadImages",
2845+
},
2846+
},
2847+
},
2848+
},
2849+
}
2850+
expectedDetailsError := &apierrors.StatusError{
2851+
ErrStatus: metav1.Status{
2852+
Status: metav1.StatusFailure,
2853+
Code: http.StatusUnprocessableEntity,
2854+
Reason: metav1.StatusReasonInvalid,
2855+
// The only difference between the two objects should be in the Message section.
2856+
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\"",
2857+
Details: &metav1.StatusDetails{
2858+
Group: "infrastructure.cluster.x-k8s.io",
2859+
Kind: "DockerMachineTemplate",
2860+
Name: "docker-template-one",
2861+
Causes: []metav1.StatusCause{
2862+
{
2863+
Type: "FieldValueInvalid",
2864+
Message: "Invalid value: \"array\": spec.template.spec.preLoadImages in body must be of type string: \"array\"",
2865+
Field: "spec.template.spec.preLoadImages",
2866+
},
2867+
},
2868+
},
2869+
},
2870+
}
2871+
NoCausesDetailsError := &apierrors.StatusError{
2872+
ErrStatus: metav1.Status{
2873+
Status: metav1.StatusFailure,
2874+
Code: http.StatusUnprocessableEntity,
2875+
Reason: metav1.StatusReasonInvalid,
2876+
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\"",
2877+
Details: &metav1.StatusDetails{
2878+
Group: "infrastructure.cluster.x-k8s.io",
2879+
Kind: "DockerMachineTemplate",
2880+
Name: "docker-template-one",
2881+
},
2882+
},
2883+
}
2884+
expectedNoCausesDetailsError := &apierrors.StatusError{
2885+
ErrStatus: metav1.Status{
2886+
Status: metav1.StatusFailure,
2887+
Code: http.StatusUnprocessableEntity,
2888+
Reason: metav1.StatusReasonInvalid,
2889+
// The only difference between the two objects should be in the Message section.
2890+
Message: "failed to create DockerMachineTemplate.infrastructure.cluster.x-k8s.io",
2891+
Details: &metav1.StatusDetails{
2892+
Group: "infrastructure.cluster.x-k8s.io",
2893+
Kind: "DockerMachineTemplate",
2894+
Name: "docker-template-one",
2895+
},
2896+
},
2897+
}
2898+
noDetailsError := &apierrors.StatusError{
2899+
ErrStatus: metav1.Status{
2900+
Status: metav1.StatusFailure,
2901+
Code: http.StatusUnprocessableEntity,
2902+
Reason: metav1.StatusReasonInvalid,
2903+
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\"",
2904+
},
2905+
}
2906+
expectedNoDetailsError := &apierrors.StatusError{
2907+
ErrStatus: metav1.Status{
2908+
Status: metav1.StatusFailure,
2909+
Code: http.StatusUnprocessableEntity,
2910+
Reason: metav1.StatusReasonInvalid,
2911+
// The only difference between the two objects should be in the Message section.
2912+
Message: "failed to create TestControlPlane.controlplane.cluster.x-k8s.io",
2913+
},
2914+
}
2915+
expectedObjectNilError := &apierrors.StatusError{
2916+
ErrStatus: metav1.Status{
2917+
Status: metav1.StatusFailure,
2918+
Code: http.StatusUnprocessableEntity,
2919+
Reason: metav1.StatusReasonInvalid,
2920+
// The only difference between the two objects should be in the Message section.
2921+
Message: "failed to create object",
2922+
},
2923+
}
2924+
nonStatusError := errors.New("an unexpected error with unknown information inside")
2925+
expectedNonStatusError := errors.New("failed to create TestControlPlane.controlplane.cluster.x-k8s.io")
2926+
expectedNilObjectNonStatusError := errors.New("failed to create object")
2927+
tests := []struct {
2928+
name string
2929+
input error
2930+
expected error
2931+
obj client.Object
2932+
}{
2933+
{
2934+
name: "Remove name from status error with details",
2935+
input: detailsError,
2936+
expected: expectedDetailsError,
2937+
obj: builder.TestControlPlane("default", "cp1").Build(),
2938+
},
2939+
{
2940+
name: "Remove name from status error with details but no causes",
2941+
input: NoCausesDetailsError,
2942+
expected: expectedNoCausesDetailsError,
2943+
obj: builder.TestControlPlane("default", "cp1").Build(),
2944+
},
2945+
{
2946+
name: "Remove name from status error with no details",
2947+
input: noDetailsError,
2948+
expected: expectedNoDetailsError,
2949+
obj: builder.TestControlPlane("default", "cp1").Build(),
2950+
},
2951+
{
2952+
name: "Remove name from status error with nil object",
2953+
input: noDetailsError,
2954+
expected: expectedObjectNilError,
2955+
obj: nil,
2956+
},
2957+
{
2958+
name: "Remove name from status error with nil object",
2959+
input: noDetailsError,
2960+
expected: expectedObjectNilError,
2961+
obj: nil,
2962+
},
2963+
{
2964+
name: "Replace message of non status error",
2965+
input: nonStatusError,
2966+
expected: expectedNonStatusError,
2967+
obj: builder.TestControlPlane("default", "cp1").Build(),
2968+
},
2969+
{
2970+
name: "Replace message of non status error with nil object",
2971+
input: nonStatusError,
2972+
expected: expectedNilObjectNonStatusError,
2973+
obj: nil,
2974+
},
2975+
}
2976+
for _, tt := range tests {
2977+
t.Run(tt.name, func(t *testing.T) {
2978+
g := NewWithT(t)
2979+
err := createErrorWithoutObjectName(tt.input, tt.obj)
2980+
g.Expect(err.Error()).To(Equal(tt.expected.Error()))
2981+
})
2982+
}
2983+
}

0 commit comments

Comments
 (0)