From 8e4b5c849b67b3a12dbd63391a4e75234382ba2c Mon Sep 17 00:00:00 2001 From: Andrea Nodari Date: Sun, 4 Apr 2021 19:05:45 +0200 Subject: [PATCH] Do not add managed fields if a scale entry doesn't own replicas This happens when a request changes the .status.replicas but not .spec.replicas --- .../handlers/fieldmanager/scalehandler.go | 3 +++ .../handlers/fieldmanager/scalehandler_test.go | 12 ++---------- test/integration/apiserver/apply/scale_test.go | 18 +++++++++--------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler.go index 114cdb1845d..4f7296b88ed 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler.go @@ -135,6 +135,9 @@ func (h *ScaleHandler) ToParent(scaleEntries []metav1.ManagedFieldsEntry) ([]met } for manager, versionedSet := range scaleFields { + if !versionedSet.Set().Has(replicasPathInScale) { + continue + } newVersionedSet := fieldpath.NewVersionedSet( fieldpath.NewSet(h.mappings[h.groupVersion.String()]), fieldpath.APIVersion(h.groupVersion.String()), diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler_test.go index 6ea8a8c7649..291f32fb35b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/scalehandler_test.go @@ -463,7 +463,7 @@ func TestTransformingManagedFieldsToParent(t *testing.T) { Operation: metav1.ManagedFieldsOperationApply, APIVersion: "apps/v1", FieldsType: "FieldsV1", - FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:replicas":{},"f:selector":{}}}`)}, + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:selector":{}}}`)}, }, }, subresource: []metav1.ManagedFieldsEntry{ @@ -472,7 +472,7 @@ func TestTransformingManagedFieldsToParent(t *testing.T) { Operation: metav1.ManagedFieldsOperationUpdate, APIVersion: "autoscaling/v1", FieldsType: "FieldsV1", - FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:another":{}}}`)}, + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:status":{"f:replicas":{}}}`)}, Subresource: "scale", }, }, @@ -484,14 +484,6 @@ func TestTransformingManagedFieldsToParent(t *testing.T) { FieldsType: "FieldsV1", FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:selector":{}}}`)}, }, - { - Manager: "scale", - Operation: metav1.ManagedFieldsOperationUpdate, - APIVersion: "apps/v1", - FieldsType: "FieldsV1", - FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:replicas":{}}}`)}, - Subresource: "scale", - }, }, }, { diff --git a/test/integration/apiserver/apply/scale_test.go b/test/integration/apiserver/apply/scale_test.go index ad0db889f7b..834af8741d1 100644 --- a/test/integration/apiserver/apply/scale_test.go +++ b/test/integration/apiserver/apply/scale_test.go @@ -97,7 +97,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Failed to create object using apply: %v", err) } - obj := retrieveObject(t, client, test) + obj := retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 1) assertReplicasOwnership(t, obj, "apply_test") @@ -115,7 +115,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Failed to scale object: %v", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 5) assertReplicasOwnership(t, obj, "scale_test") @@ -145,7 +145,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Error force-updating: %v", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 1) assertReplicasOwnership(t, obj, "apply_test") @@ -180,7 +180,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Error updating object by applying scale and forcing: %v ", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 17) assertReplicasOwnership(t, obj, "apply_scale") @@ -197,7 +197,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Error replacing object: %v", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 7) assertReplicasOwnership(t, obj, "replace_test") @@ -214,7 +214,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Error updating object: %v", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 7) assertReplicasOwnership(t, obj, "replace_test", "co_owning_test") @@ -231,7 +231,7 @@ func TestScaleAllResources(t *testing.T) { if err != nil { t.Fatalf("Error scaling object: %v", err) } - obj = retrieveObject(t, client, test) + obj = retrieveObject(t, client, test.path, test.resource) assertReplicasValue(t, obj, 5) assertReplicasOwnership(t, obj, "scale_test") }) @@ -379,10 +379,10 @@ func validV1ReplicationController() string { }` } -func retrieveObject(t *testing.T, client clientset.Interface, test scaleTest) *unstructured.Unstructured { +func retrieveObject(t *testing.T, client clientset.Interface, prefix, resource string) *unstructured.Unstructured { t.Helper() - urlPath := path.Join(test.path, "namespaces", "default", test.resource, "test") + urlPath := path.Join(prefix, "namespaces", "default", resource, "test") bytes, err := client.CoreV1().RESTClient().Get().AbsPath(urlPath).DoRaw(context.TODO()) if err != nil { t.Fatalf("Failed to retrieve object: %v", err)