From 98acbc3692a54444c7b186aa98157b0de54ea00f Mon Sep 17 00:00:00 2001 From: Mike Spreitzer Date: Wed, 12 Jan 2022 23:57:34 -0500 Subject: [PATCH 1/2] Make TestApplyResetFields exhibit surprising object --- .../apiserver/apply/reset_fields_test.go | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/test/integration/apiserver/apply/reset_fields_test.go b/test/integration/apiserver/apply/reset_fields_test.go index 2a0fed6cb97..1ee5400d3da 100644 --- a/test/integration/apiserver/apply/reset_fields_test.go +++ b/test/integration/apiserver/apply/reset_fields_test.go @@ -19,6 +19,7 @@ package apiserver import ( "context" "encoding/json" + "fmt" "reflect" "strings" "testing" @@ -268,25 +269,23 @@ func TestApplyResetFields(t *testing.T) { // skip checking for conflicts on resources // that will never have conflicts if _, ok = noConflicts[mapping.Resource.Resource]; !ok { + var objRet *unstructured.Unstructured + // reapply second object to the spec endpoint // that should fail with a conflict - _, err = dynamicClient. + objRet, err = dynamicClient. Resource(mapping.Resource). Namespace(namespace). Apply(context.TODO(), name, obj2, metav1.ApplyOptions{FieldManager: "fieldmanager2"}) - if err == nil || !strings.Contains(err.Error(), "conflict") { - t.Fatalf("expected conflict, got error %v", err) - } + expectConflict(t, objRet, err, dynamicClient, mapping.Resource, namespace, name, "spec") // reapply first object to the status endpoint // that should fail with a conflict - _, err = dynamicClient. + objRet, err = dynamicClient. Resource(mapping.Resource). Namespace(namespace). ApplyStatus(context.TODO(), name, &obj1, metav1.ApplyOptions{FieldManager: "fieldmanager1"}) - if err == nil || !strings.Contains(err.Error(), "conflict") { - t.Fatalf("expected conflict, got error %v", err) - } + expectConflict(t, objRet, err, dynamicClient, mapping.Resource, namespace, name, "status") } // cleanup @@ -298,3 +297,31 @@ func TestApplyResetFields(t *testing.T) { } } } + +func expectConflict(t *testing.T, objRet *unstructured.Unstructured, err error, dynamicClient dynamic.Interface, resource schema.GroupVersionResource, namespace, name, where string) { + if err != nil && strings.Contains(err.Error(), "conflict") { + return + } + which := "returned" + var gotten string + var err2 error + if objRet == nil { + which = "subsequently fetched" + objRet, err2 = dynamicClient. + Resource(resource). + Namespace(namespace). + Get(context.TODO(), name, metav1.GetOptions{}) + if err2 != nil { + gotten = fmt.Sprintf("", err2) + } + } + if gotten == "" { + marshBytes, marshErr := json.Marshal(objRet) + if marshErr == nil { + gotten = string(marshBytes) + } else { + gotten = fmt.Sprintf("", objRet, marshErr) + } + } + t.Fatalf("expected conflict in %s, but instead got error %v; %s object is %s", where, err, which, gotten) +} From d3081b378ad971244f35448983ebd0c91e186344 Mon Sep 17 00:00:00 2001 From: Mike Spreitzer Date: Mon, 29 Aug 2022 23:02:29 -0400 Subject: [PATCH 2/2] Revise according to review --- .../apiserver/apply/reset_fields_test.go | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/test/integration/apiserver/apply/reset_fields_test.go b/test/integration/apiserver/apply/reset_fields_test.go index 1ee5400d3da..3f8c699649c 100644 --- a/test/integration/apiserver/apply/reset_fields_test.go +++ b/test/integration/apiserver/apply/reset_fields_test.go @@ -277,7 +277,10 @@ func TestApplyResetFields(t *testing.T) { Resource(mapping.Resource). Namespace(namespace). Apply(context.TODO(), name, obj2, metav1.ApplyOptions{FieldManager: "fieldmanager2"}) - expectConflict(t, objRet, err, dynamicClient, mapping.Resource, namespace, name, "spec") + err = expectConflict(objRet, err, dynamicClient, mapping.Resource, namespace, name) + if err != nil { + t.Fatalf("Did not get expected conflict in spec of %s %s/%s: %v", mapping.Resource, namespace, name, err) + } // reapply first object to the status endpoint // that should fail with a conflict @@ -285,7 +288,10 @@ func TestApplyResetFields(t *testing.T) { Resource(mapping.Resource). Namespace(namespace). ApplyStatus(context.TODO(), name, &obj1, metav1.ApplyOptions{FieldManager: "fieldmanager1"}) - expectConflict(t, objRet, err, dynamicClient, mapping.Resource, namespace, name, "status") + err = expectConflict(objRet, err, dynamicClient, mapping.Resource, namespace, name) + if err != nil { + t.Fatalf("Did not get expected conflict in status of %s %s/%s: %v", mapping.Resource, namespace, name, err) + } } // cleanup @@ -298,30 +304,29 @@ func TestApplyResetFields(t *testing.T) { } } -func expectConflict(t *testing.T, objRet *unstructured.Unstructured, err error, dynamicClient dynamic.Interface, resource schema.GroupVersionResource, namespace, name, where string) { +func expectConflict(objRet *unstructured.Unstructured, err error, dynamicClient dynamic.Interface, resource schema.GroupVersionResource, namespace, name string) error { if err != nil && strings.Contains(err.Error(), "conflict") { - return + return nil } which := "returned" - var gotten string - var err2 error + // something unexpected is going on here, let's not assume that objRet==nil if any only if err!=nil if objRet == nil { which = "subsequently fetched" + var err2 error objRet, err2 = dynamicClient. Resource(resource). Namespace(namespace). Get(context.TODO(), name, metav1.GetOptions{}) if err2 != nil { - gotten = fmt.Sprintf("", err2) + return fmt.Errorf("instead got error %w, and failed to Get object: %v", err, err2) } } - if gotten == "" { - marshBytes, marshErr := json.Marshal(objRet) - if marshErr == nil { - gotten = string(marshBytes) - } else { - gotten = fmt.Sprintf("", objRet, marshErr) - } + marshBytes, marshErr := json.Marshal(objRet) + var gotten string + if marshErr == nil { + gotten = string(marshBytes) + } else { + gotten = fmt.Sprintf("", objRet, marshErr) } - t.Fatalf("expected conflict in %s, but instead got error %v; %s object is %s", where, err, which, gotten) + return fmt.Errorf("instead got error %w; %s object is %s", err, which, gotten) }