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.
This commit is contained in:
Andrea Nodari 2021-04-18 13:55:15 +02:00
parent c10dd884c4
commit 5b666a61a1
3 changed files with 76 additions and 13 deletions

View File

@ -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") return nil, fmt.Errorf("the server could not properly serve the CR subresources")
} }
if subresources == nil || subresources.Scale == nil { if subresources == nil || subresources.Scale == nil {
replicasPathInCustomResource[schema.GroupVersion{Group: crd.Spec.Group, Version: v.Name}.String()] = nil
continue continue
} }
path := fieldpath.Path{} path := fieldpath.Path{}

View File

@ -69,8 +69,8 @@ func (h *ScaleHandler) ToSubresource() ([]metav1.ManagedFieldsEntry, error) {
t := map[string]*metav1.Time{} t := map[string]*metav1.Time{}
for manager, versionedSet := range managed.Fields() { for manager, versionedSet := range managed.Fields() {
path, ok := h.mappings[string(versionedSet.APIVersion())] path, ok := h.mappings[string(versionedSet.APIVersion())]
// Drop the field if the APIVersion of the managed field is unknown // Skip the entry if the APIVersion is unknown
if !ok { if !ok || path == nil {
continue continue
} }
@ -110,13 +110,15 @@ func (h *ScaleHandler) ToParent(scaleEntries []metav1.ManagedFieldsEntry) ([]met
for manager, versionedSet := range parentFields { for manager, versionedSet := range parentFields {
// Get the main resource "replicas" path // Get the main resource "replicas" path
path, ok := h.mappings[string(versionedSet.APIVersion())] 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 { if !ok {
continue continue
} }
// If the parent entry does not have the replicas path, just keep it as it is // If the parent entry does not have the replicas path or it is nil, just
if !versionedSet.Set().Has(path) { // 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 f[manager] = versionedSet
t[manager] = decodedParentEntries.Times()[manager] t[manager] = decodedParentEntries.Times()[manager]
continue continue

View File

@ -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{ parent: []metav1.ManagedFieldsEntry{
{ {
Manager: "test", Manager: "test",
@ -637,6 +637,7 @@ func TestTransformingManagedFieldsToParent(t *testing.T) {
func TestTransformingManagedFieldsToParentMultiVersion(t *testing.T) { func TestTransformingManagedFieldsToParentMultiVersion(t *testing.T) {
tests := []struct { tests := []struct {
desc string desc string
groupVersion schema.GroupVersion
mappings ResourcePathMappings mappings ResourcePathMappings
parent []metav1.ManagedFieldsEntry parent []metav1.ManagedFieldsEntry
subresource []metav1.ManagedFieldsEntry subresource []metav1.ManagedFieldsEntry
@ -644,6 +645,7 @@ func TestTransformingManagedFieldsToParentMultiVersion(t *testing.T) {
}{ }{
{ {
desc: "multi-version", desc: "multi-version",
groupVersion: schema.GroupVersion{Group: "apps", Version: "v1"},
mappings: ResourcePathMappings{ mappings: ResourcePathMappings{
"apps/v1": fieldpath.MakePathOrDie("spec", "the-replicas"), "apps/v1": fieldpath.MakePathOrDie("spec", "the-replicas"),
"apps/v2": fieldpath.MakePathOrDie("spec", "not-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 { for _, test := range tests {
t.Run(test.desc, func(t *testing.T) { t.Run(test.desc, func(t *testing.T) {
handler := NewScaleHandler( handler := NewScaleHandler(
test.parent, test.parent,
schema.GroupVersion{Group: "apps", Version: "v1"}, test.groupVersion,
test.mappings, test.mappings,
) )
parentEntries, err := handler.ToParent(test.subresource) parentEntries, err := handler.ToParent(test.subresource)