From 5b666a61a170f61c7e223085478b24a03612fa99 Mon Sep 17 00:00:00 2001 From: Andrea Nodari Date: Sun, 18 Apr 2021 13:55:15 +0200 Subject: [PATCH] Add nil path to mapping when a CR has no "scale" subresource This is to prevent the ScaleHandler to drop the entry. In this way entries just get ignored. --- .../pkg/apiserver/customresource_handler.go | 1 + .../handlers/fieldmanager/scalehandler.go | 12 +-- .../fieldmanager/scalehandler_test.go | 76 +++++++++++++++++-- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index d3585429b5f..5cf0fad6965 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -709,6 +709,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd return nil, fmt.Errorf("the server could not properly serve the CR subresources") } if subresources == nil || subresources.Scale == nil { + replicasPathInCustomResource[schema.GroupVersion{Group: crd.Spec.Group, Version: v.Name}.String()] = nil continue } path := fieldpath.Path{} 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 3b328ae9a54..d81383628c7 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 @@ -69,8 +69,8 @@ func (h *ScaleHandler) ToSubresource() ([]metav1.ManagedFieldsEntry, error) { t := map[string]*metav1.Time{} for manager, versionedSet := range managed.Fields() { path, ok := h.mappings[string(versionedSet.APIVersion())] - // Drop the field if the APIVersion of the managed field is unknown - if !ok { + // Skip the entry if the APIVersion is unknown + if !ok || path == nil { continue } @@ -110,13 +110,15 @@ func (h *ScaleHandler) ToParent(scaleEntries []metav1.ManagedFieldsEntry) ([]met for manager, versionedSet := range parentFields { // Get the main resource "replicas" path path, ok := h.mappings[string(versionedSet.APIVersion())] - // Drop the field if the APIVersion of the managed field is unknown + // Drop the entry if the APIVersion is unknown. if !ok { continue } - // If the parent entry does not have the replicas path, just keep it as it is - if !versionedSet.Set().Has(path) { + // If the parent entry does not have the replicas path or it is nil, just + // keep it as it is. The path is nil for Custom Resources without scale + // subresource. + if path == nil || !versionedSet.Set().Has(path) { f[manager] = versionedSet t[manager] = decodedParentEntries.Times()[manager] continue 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 ffed4839b1a..96d40e70d5a 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 @@ -578,7 +578,7 @@ func TestTransformingManagedFieldsToParent(t *testing.T) { }, }, { - desc: "drops other fields if the api version is unknown", + desc: "drops the entry if the api version is unknown", parent: []metav1.ManagedFieldsEntry{ { Manager: "test", @@ -636,14 +636,16 @@ func TestTransformingManagedFieldsToParent(t *testing.T) { func TestTransformingManagedFieldsToParentMultiVersion(t *testing.T) { tests := []struct { - desc string - mappings ResourcePathMappings - parent []metav1.ManagedFieldsEntry - subresource []metav1.ManagedFieldsEntry - expected []metav1.ManagedFieldsEntry + desc string + groupVersion schema.GroupVersion + mappings ResourcePathMappings + parent []metav1.ManagedFieldsEntry + subresource []metav1.ManagedFieldsEntry + expected []metav1.ManagedFieldsEntry }{ { - desc: "multi-version", + desc: "multi-version", + groupVersion: schema.GroupVersion{Group: "apps", Version: "v1"}, mappings: ResourcePathMappings{ "apps/v1": fieldpath.MakePathOrDie("spec", "the-replicas"), "apps/v2": fieldpath.MakePathOrDie("spec", "not-the-replicas"), @@ -699,13 +701,71 @@ func TestTransformingManagedFieldsToParentMultiVersion(t *testing.T) { }, }, }, + { + desc: "Custom resource without scale subresource, scaling a version with `scale`", + groupVersion: schema.GroupVersion{Group: "mygroup", Version: "v1"}, + mappings: ResourcePathMappings{ + "mygroup/v1": fieldpath.MakePathOrDie("spec", "the-replicas"), + "mygroup/v2": nil, + }, + parent: []metav1.ManagedFieldsEntry{ + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + APIVersion: "mygroup/v1", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:the-replicas":{},"f:selector":{}}}`)}, + }, + { + Manager: "test-other", + Operation: metav1.ManagedFieldsOperationApply, + APIVersion: "mygroup/v2", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:test-other":{}}}`)}, + }, + }, + subresource: []metav1.ManagedFieldsEntry{ + { + Manager: "scale", + Operation: metav1.ManagedFieldsOperationUpdate, + APIVersion: "autoscaling/v1", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:replicas":{}}}`)}, + Subresource: "scale", + }, + }, + expected: []metav1.ManagedFieldsEntry{ + { + Manager: "test", + Operation: metav1.ManagedFieldsOperationApply, + APIVersion: "mygroup/v1", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:selector":{}}}`)}, + }, + { + Manager: "test-other", + Operation: metav1.ManagedFieldsOperationApply, + APIVersion: "mygroup/v2", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:test-other":{}}}`)}, + }, + { + Manager: "scale", + Operation: metav1.ManagedFieldsOperationUpdate, + APIVersion: "mygroup/v1", + FieldsType: "FieldsV1", + FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec":{"f:the-replicas":{}}}`)}, + Subresource: "scale", + }, + }, + }, } for _, test := range tests { t.Run(test.desc, func(t *testing.T) { handler := NewScaleHandler( test.parent, - schema.GroupVersion{Group: "apps", Version: "v1"}, + test.groupVersion, test.mappings, ) parentEntries, err := handler.ToParent(test.subresource)