From 8d40ba73fba3608e76531b1b33230856a58256ed Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Thu, 19 Jan 2023 09:37:12 -0800 Subject: [PATCH 1/3] fieldmanager: Move structured benchmarks to their own file --- .../handlers/fieldmanager/bench_test.go | 307 ++++++++++++++++++ .../fieldmanager/fieldmanager_test.go | 277 ---------------- 2 files changed, 307 insertions(+), 277 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/bench_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/bench_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/bench_test.go new file mode 100644 index 00000000000..56a383aec01 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/bench_test.go @@ -0,0 +1,307 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fieldmanager_test + +import ( + "fmt" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanagertest" + "sigs.k8s.io/yaml" +) + +func BenchmarkNewObject(b *testing.B) { + tests := []struct { + gvk schema.GroupVersionKind + obj []byte + }{ + { + gvk: schema.FromAPIVersionAndKind("v1", "Pod"), + obj: getObjectBytes("pod.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Node"), + obj: getObjectBytes("node.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), + obj: getObjectBytes("endpoints.yaml"), + }, + } + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + b.Fatalf("Failed to add to scheme: %v", err) + } + for _, test := range tests { + b.Run(test.gvk.Kind, func(b *testing.B) { + f := fieldmanagertest.NewTestFieldManager(fakeTypeConverter, test.gvk) + + decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) + newObj, err := runtime.Decode(decoder, test.obj) + if err != nil { + b.Fatalf("Failed to parse yaml object: %v", err) + } + objMeta, err := meta.Accessor(newObj) + if err != nil { + b.Fatalf("Failed to get object meta: %v", err) + } + objMeta.SetManagedFields([]metav1.ManagedFieldsEntry{ + { + Manager: "default", + Operation: "Update", + APIVersion: "v1", + }, + }) + appliedObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := yaml.Unmarshal(test.obj, &appliedObj.Object); err != nil { + b.Fatalf("Failed to parse yaml object: %v", err) + } + b.Run("Update", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + if err := f.Update(newObj, "fieldmanager_test"); err != nil { + b.Fatal(err) + } + f.Reset() + } + }) + b.Run("UpdateTwice", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + if err := f.Update(newObj, "fieldmanager_test"); err != nil { + b.Fatal(err) + } + if err := f.Update(newObj, "fieldmanager_test_2"); err != nil { + b.Fatal(err) + } + f.Reset() + } + }) + b.Run("Apply", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + if err := f.Apply(appliedObj, "fieldmanager_test", false); err != nil { + b.Fatal(err) + } + f.Reset() + } + }) + b.Run("UpdateApply", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + if err := f.Update(newObj, "fieldmanager_test"); err != nil { + b.Fatal(err) + } + if err := f.Apply(appliedObj, "fieldmanager_test", false); err != nil { + b.Fatal(err) + } + f.Reset() + } + }) + }) + } +} + +func toUnstructured(b *testing.B, o runtime.Object) *unstructured.Unstructured { + u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(o) + if err != nil { + b.Fatalf("Failed to unmarshal to json: %v", err) + } + return &unstructured.Unstructured{Object: u} +} + +func BenchmarkConvertObjectToTyped(b *testing.B) { + tests := []struct { + gvk schema.GroupVersionKind + obj []byte + }{ + { + gvk: schema.FromAPIVersionAndKind("v1", "Pod"), + obj: getObjectBytes("pod.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Node"), + obj: getObjectBytes("node.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), + obj: getObjectBytes("endpoints.yaml"), + }, + } + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + b.Fatalf("Failed to add to scheme: %v", err) + } + + for _, test := range tests { + b.Run(test.gvk.Kind, func(b *testing.B) { + decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) + structured, err := runtime.Decode(decoder, test.obj) + if err != nil { + b.Fatalf("Failed to parse yaml object: %v", err) + } + b.Run("structured", func(b *testing.B) { + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, err := fakeTypeConverter.ObjectToTyped(structured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + } + }) + }) + + unstructured := toUnstructured(b, structured) + b.Run("unstructured", func(b *testing.B) { + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, err := fakeTypeConverter.ObjectToTyped(unstructured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + } + }) + }) + }) + } +} + +func BenchmarkCompare(b *testing.B) { + tests := []struct { + gvk schema.GroupVersionKind + obj []byte + }{ + { + gvk: schema.FromAPIVersionAndKind("v1", "Pod"), + obj: getObjectBytes("pod.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Node"), + obj: getObjectBytes("node.yaml"), + }, + { + gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), + obj: getObjectBytes("endpoints.yaml"), + }, + } + + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + b.Fatalf("Failed to add to scheme: %v", err) + } + + for _, test := range tests { + b.Run(test.gvk.Kind, func(b *testing.B) { + decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) + structured, err := runtime.Decode(decoder, test.obj) + if err != nil { + b.Fatal(err) + } + tv1, err := fakeTypeConverter.ObjectToTyped(structured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + tv2, err := fakeTypeConverter.ObjectToTyped(structured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + + b.Run("structured", func(b *testing.B) { + b.ReportAllocs() + for n := 0; n < b.N; n++ { + _, err = tv1.Compare(tv2) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + } + }) + + unstructured := toUnstructured(b, structured) + utv1, err := fakeTypeConverter.ObjectToTyped(unstructured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + utv2, err := fakeTypeConverter.ObjectToTyped(unstructured) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + b.Run("unstructured", func(b *testing.B) { + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, err = utv1.Compare(utv2) + if err != nil { + b.Errorf("Error in ObjectToTyped: %v", err) + } + } + }) + }) + }) + } +} + +func BenchmarkRepeatedUpdate(b *testing.B) { + f := fieldmanagertest.NewTestFieldManager(fakeTypeConverter, schema.FromAPIVersionAndKind("v1", "Pod")) + podBytes := getObjectBytes("pod.yaml") + + var obj *corev1.Pod + if err := yaml.Unmarshal(podBytes, &obj); err != nil { + b.Fatalf("Failed to parse yaml object: %v", err) + } + obj.Spec.Containers[0].Image = "nginx:latest" + objs := []*corev1.Pod{obj} + obj = obj.DeepCopy() + obj.Spec.Containers[0].Image = "nginx:4.3" + objs = append(objs, obj) + + appliedObj := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := yaml.Unmarshal(podBytes, &appliedObj.Object); err != nil { + b.Fatalf("error decoding YAML: %v", err) + } + + err := f.Apply(appliedObj, "fieldmanager_apply", false) + if err != nil { + b.Fatal(err) + } + + if err := f.Update(objs[1], "fieldmanager_1"); err != nil { + b.Fatal(err) + } + + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + err := f.Update(objs[n%len(objs)], fmt.Sprintf("fieldmanager_%d", n%len(objs))) + if err != nil { + b.Fatal(err) + } + f.Reset() + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index f91f947a788..ed25c8ae3e7 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go @@ -27,14 +27,12 @@ import ( "testing" "time" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" yamlutil "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanagertest" @@ -303,281 +301,6 @@ func TestApplyNewObject(t *testing.T) { } } -func BenchmarkNewObject(b *testing.B) { - tests := []struct { - gvk schema.GroupVersionKind - obj []byte - }{ - { - gvk: schema.FromAPIVersionAndKind("v1", "Pod"), - obj: getObjectBytes("pod.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Node"), - obj: getObjectBytes("node.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), - obj: getObjectBytes("endpoints.yaml"), - }, - } - scheme := runtime.NewScheme() - if err := corev1.AddToScheme(scheme); err != nil { - b.Fatalf("Failed to add to scheme: %v", err) - } - for _, test := range tests { - b.Run(test.gvk.Kind, func(b *testing.B) { - f := fieldmanagertest.NewTestFieldManager(fakeTypeConverter, test.gvk) - - decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) - newObj, err := runtime.Decode(decoder, test.obj) - if err != nil { - b.Fatalf("Failed to parse yaml object: %v", err) - } - objMeta, err := meta.Accessor(newObj) - if err != nil { - b.Fatalf("Failed to get object meta: %v", err) - } - objMeta.SetManagedFields([]metav1.ManagedFieldsEntry{ - { - Manager: "default", - Operation: "Update", - APIVersion: "v1", - }, - }) - appliedObj := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := yaml.Unmarshal(test.obj, &appliedObj.Object); err != nil { - b.Fatalf("Failed to parse yaml object: %v", err) - } - b.Run("Update", func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := f.Update(newObj, "fieldmanager_test"); err != nil { - b.Fatal(err) - } - f.Reset() - } - }) - b.Run("UpdateTwice", func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := f.Update(newObj, "fieldmanager_test"); err != nil { - b.Fatal(err) - } - if err := f.Update(newObj, "fieldmanager_test_2"); err != nil { - b.Fatal(err) - } - f.Reset() - } - }) - b.Run("Apply", func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := f.Apply(appliedObj, "fieldmanager_test", false); err != nil { - b.Fatal(err) - } - f.Reset() - } - }) - b.Run("UpdateApply", func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for n := 0; n < b.N; n++ { - if err := f.Update(newObj, "fieldmanager_test"); err != nil { - b.Fatal(err) - } - if err := f.Apply(appliedObj, "fieldmanager_test", false); err != nil { - b.Fatal(err) - } - f.Reset() - } - }) - }) - } -} - -func toUnstructured(b *testing.B, o runtime.Object) *unstructured.Unstructured { - u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(o) - if err != nil { - b.Fatalf("Failed to unmarshal to json: %v", err) - } - return &unstructured.Unstructured{Object: u} -} - -func BenchmarkConvertObjectToTyped(b *testing.B) { - tests := []struct { - gvk schema.GroupVersionKind - obj []byte - }{ - { - gvk: schema.FromAPIVersionAndKind("v1", "Pod"), - obj: getObjectBytes("pod.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Node"), - obj: getObjectBytes("node.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), - obj: getObjectBytes("endpoints.yaml"), - }, - } - scheme := runtime.NewScheme() - if err := corev1.AddToScheme(scheme); err != nil { - b.Fatalf("Failed to add to scheme: %v", err) - } - - for _, test := range tests { - b.Run(test.gvk.Kind, func(b *testing.B) { - decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) - structured, err := runtime.Decode(decoder, test.obj) - if err != nil { - b.Fatalf("Failed to parse yaml object: %v", err) - } - b.Run("structured", func(b *testing.B) { - b.ReportAllocs() - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - _, err := fakeTypeConverter.ObjectToTyped(structured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - } - }) - }) - - unstructured := toUnstructured(b, structured) - b.Run("unstructured", func(b *testing.B) { - b.ReportAllocs() - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - _, err := fakeTypeConverter.ObjectToTyped(unstructured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - } - }) - }) - }) - } -} - -func BenchmarkCompare(b *testing.B) { - tests := []struct { - gvk schema.GroupVersionKind - obj []byte - }{ - { - gvk: schema.FromAPIVersionAndKind("v1", "Pod"), - obj: getObjectBytes("pod.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Node"), - obj: getObjectBytes("node.yaml"), - }, - { - gvk: schema.FromAPIVersionAndKind("v1", "Endpoints"), - obj: getObjectBytes("endpoints.yaml"), - }, - } - - scheme := runtime.NewScheme() - if err := corev1.AddToScheme(scheme); err != nil { - b.Fatalf("Failed to add to scheme: %v", err) - } - - for _, test := range tests { - b.Run(test.gvk.Kind, func(b *testing.B) { - decoder := serializer.NewCodecFactory(scheme).UniversalDecoder(test.gvk.GroupVersion()) - structured, err := runtime.Decode(decoder, test.obj) - if err != nil { - b.Fatal(err) - } - tv1, err := fakeTypeConverter.ObjectToTyped(structured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - tv2, err := fakeTypeConverter.ObjectToTyped(structured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - - b.Run("structured", func(b *testing.B) { - b.ReportAllocs() - for n := 0; n < b.N; n++ { - _, err = tv1.Compare(tv2) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - } - }) - - unstructured := toUnstructured(b, structured) - utv1, err := fakeTypeConverter.ObjectToTyped(unstructured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - utv2, err := fakeTypeConverter.ObjectToTyped(unstructured) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - b.Run("unstructured", func(b *testing.B) { - b.ReportAllocs() - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - _, err = utv1.Compare(utv2) - if err != nil { - b.Errorf("Error in ObjectToTyped: %v", err) - } - } - }) - }) - }) - } -} - -func BenchmarkRepeatedUpdate(b *testing.B) { - f := fieldmanagertest.NewTestFieldManager(fakeTypeConverter, schema.FromAPIVersionAndKind("v1", "Pod")) - podBytes := getObjectBytes("pod.yaml") - - var obj *corev1.Pod - if err := yaml.Unmarshal(podBytes, &obj); err != nil { - b.Fatalf("Failed to parse yaml object: %v", err) - } - obj.Spec.Containers[0].Image = "nginx:latest" - objs := []*corev1.Pod{obj} - obj = obj.DeepCopy() - obj.Spec.Containers[0].Image = "nginx:4.3" - objs = append(objs, obj) - - appliedObj := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := yaml.Unmarshal(podBytes, &appliedObj.Object); err != nil { - b.Fatalf("error decoding YAML: %v", err) - } - - err := f.Apply(appliedObj, "fieldmanager_apply", false) - if err != nil { - b.Fatal(err) - } - - if err := f.Update(objs[1], "fieldmanager_1"); err != nil { - b.Fatal(err) - } - - b.ReportAllocs() - b.ResetTimer() - for n := 0; n < b.N; n++ { - err := f.Update(objs[n%len(objs)], fmt.Sprintf("fieldmanager_%d", n%len(objs))) - if err != nil { - b.Fatal(err) - } - f.Reset() - } -} - func TestApplyFailsWithManagedFields(t *testing.T) { f := fieldmanagertest.NewTestFieldManager(fakeTypeConverter, schema.FromAPIVersionAndKind("v1", "Pod")) From 577f3d8c9da461da93d4a0b66ca16b2a84f6a4d6 Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Thu, 19 Jan 2023 09:38:04 -0800 Subject: [PATCH 2/3] fieldmanager: Copy LastAppliedAnnotation to remove dependency on corev1 --- .../handlers/fieldmanager/fieldmanager_test.go | 2 +- .../handlers/fieldmanager/internal/lastapplied.go | 11 ++++++++--- .../fieldmanager/internal/lastappliedmanager.go | 3 +-- .../fieldmanager/internal/lastappliedupdater.go | 5 ++--- .../fieldmanager/internal/lastappliedupdater_test.go | 12 ++++++------ 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index ed25c8ae3e7..b61965bd4fd 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go @@ -817,7 +817,7 @@ func getLastApplied(obj runtime.Object) (string, error) { return "", fmt.Errorf("no annotations on obj: %v", obj) } - lastApplied, ok := annotations[corev1.LastAppliedConfigAnnotation] + lastApplied, ok := annotations[internal.LastAppliedConfigAnnotation] if !ok { return "", fmt.Errorf("expected last applied annotation, but got none for object: %v", obj) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastapplied.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastapplied.go index 5264c82772f..b00b6b8298a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastapplied.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastapplied.go @@ -19,12 +19,17 @@ package internal import ( "fmt" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/runtime" ) +// LastAppliedConfigAnnotation is the annotation used to store the previous +// configuration of a resource for use in a three way diff by UpdateApplyAnnotation. +// +// This is a copy of the corev1 annotation since we don't want to depend on the whole package. +const LastAppliedConfigAnnotation = "kubectl.kubernetes.io/last-applied-configuration" + // SetLastApplied sets the last-applied annotation the given value in // the object. func SetLastApplied(obj runtime.Object, value string) error { @@ -36,9 +41,9 @@ func SetLastApplied(obj runtime.Object, value string) error { if annotations == nil { annotations = map[string]string{} } - annotations[corev1.LastAppliedConfigAnnotation] = value + annotations[LastAppliedConfigAnnotation] = value if err := apimachineryvalidation.ValidateAnnotationsSize(annotations); err != nil { - delete(annotations, corev1.LastAppliedConfigAnnotation) + delete(annotations, LastAppliedConfigAnnotation) } accessor.SetAnnotations(annotations) return nil diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedmanager.go index 88675630aae..3f6cf88210c 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedmanager.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedmanager.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -100,7 +99,7 @@ func (f *lastAppliedManager) allowedConflictsFromLastApplied(liveObj runtime.Obj if annotations == nil { return nil, fmt.Errorf("no last applied annotation") } - var lastApplied, ok = annotations[corev1.LastAppliedConfigAnnotation] + var lastApplied, ok = annotations[LastAppliedConfigAnnotation] if !ok || lastApplied == "" { return nil, fmt.Errorf("no last applied annotation") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater.go index 0d9320730a1..06e6c5d8ce5 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater.go @@ -19,7 +19,6 @@ package internal import ( "fmt" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -78,7 +77,7 @@ func hasLastApplied(obj runtime.Object) bool { if annotations == nil { return false } - lastApplied, ok := annotations[corev1.LastAppliedConfigAnnotation] + lastApplied, ok := annotations[LastAppliedConfigAnnotation] return ok && len(lastApplied) > 0 } @@ -92,7 +91,7 @@ func buildLastApplied(obj runtime.Object) (string, error) { // Remove the annotation from the object before encoding the object var annotations = accessor.GetAnnotations() - delete(annotations, corev1.LastAppliedConfigAnnotation) + delete(annotations, LastAppliedConfigAnnotation) accessor.SetAnnotations(annotations) lastApplied, err := runtime.Encode(unstructured.UnstructuredJSONScheme, obj) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go index 35d3267fb4e..181f4943c5a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go @@ -114,7 +114,7 @@ func TestLargeLastApplied(t *testing.T) { Name: "large-update-test-cm", Namespace: "default", Annotations: map[string]string{ - corev1.LastAppliedConfigAnnotation: "nonempty", + internal.LastAppliedConfigAnnotation: "nonempty", }, }, Data: map[string]string{"k": "v"}, @@ -129,7 +129,7 @@ func TestLargeLastApplied(t *testing.T) { Name: "large-update-test-cm", Namespace: "default", Annotations: map[string]string{ - corev1.LastAppliedConfigAnnotation: "nonempty", + internal.LastAppliedConfigAnnotation: "nonempty", }, }, Data: map[string]string{"k": "v"}, @@ -153,7 +153,7 @@ func TestLargeLastApplied(t *testing.T) { Name: "large-update-test-cm", Namespace: "default", Annotations: map[string]string{ - corev1.LastAppliedConfigAnnotation: "nonempty", + internal.LastAppliedConfigAnnotation: "nonempty", }, }, Data: map[string]string{"k": "v"}, @@ -174,7 +174,7 @@ func TestLargeLastApplied(t *testing.T) { Name: "large-update-test-cm", Namespace: "default", Annotations: map[string]string{ - corev1.LastAppliedConfigAnnotation: "nonempty", + internal.LastAppliedConfigAnnotation: "nonempty", }, }, Data: map[string]string{"k": "v"}, @@ -220,7 +220,7 @@ func TestLargeLastApplied(t *testing.T) { if annotations == nil { t.Errorf("No annotations on obj: %v", f.Live()) } - lastApplied, ok := annotations[corev1.LastAppliedConfigAnnotation] + lastApplied, ok := annotations[internal.LastAppliedConfigAnnotation] if ok || len(lastApplied) > 0 { t.Errorf("Expected no last applied annotation, but got last applied with length: %d", len(lastApplied)) } @@ -238,7 +238,7 @@ func getLastApplied(obj runtime.Object) (string, error) { return "", fmt.Errorf("no annotations on obj: %v", obj) } - lastApplied, ok := annotations[corev1.LastAppliedConfigAnnotation] + lastApplied, ok := annotations[internal.LastAppliedConfigAnnotation] if !ok { return "", fmt.Errorf("expected last applied annotation, but got none for object: %v", obj) } From bc0962ad809bc5cd0951a1d643f6769459665711 Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Thu, 19 Jan 2023 10:48:46 -0800 Subject: [PATCH 3/3] fieldmanager: Use unstructured rather than built-in types to remove dependency --- .../internal/lastappliedupdater_test.go | 156 ++++++++++-------- .../internal/skipnonapplied_test.go | 10 +- 2 files changed, 94 insertions(+), 72 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go index 181f4943c5a..d4870634e70 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/lastappliedupdater_test.go @@ -17,13 +17,12 @@ limitations under the License. package internal_test import ( + "encoding/json" "fmt" "strings" "testing" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -100,91 +99,114 @@ spec: func TestLargeLastApplied(t *testing.T) { tests := []struct { name string - oldObject *corev1.ConfigMap - newObject *corev1.ConfigMap + oldObject *unstructured.Unstructured + newObject *unstructured.Unstructured }{ { name: "old object + new object last-applied annotation is too big", - oldObject: &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "large-update-test-cm", - Namespace: "default", - Annotations: map[string]string{ - internal.LastAppliedConfigAnnotation: "nonempty", - }, - }, - Data: map[string]string{"k": "v"}, - }, - newObject: func() *corev1.ConfigMap { - cfg := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "large-update-test-cm", - Namespace: "default", - Annotations: map[string]string{ - internal.LastAppliedConfigAnnotation: "nonempty", - }, - }, - Data: map[string]string{"k": "v"}, + oldObject: func() *unstructured.Unstructured { + u := &unstructured.Unstructured{} + err := json.Unmarshal([]byte(` +{ + "metadata": { + "name": "large-update-test-cm", + "namespace": "default", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "nonempty" + } + }, + "apiVersion": "v1", + "kind": "ConfigMap", + "data": { + "k": "v" + } +}`), &u) + if err != nil { + panic(err) + } + return u + }(), + newObject: func() *unstructured.Unstructured { + u := &unstructured.Unstructured{} + err := json.Unmarshal([]byte(` +{ + "metadata": { + "name": "large-update-test-cm", + "namespace": "default", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "nonempty" + } + }, + "apiVersion": "v1", + "kind": "ConfigMap", + "data": { + "k": "v" + } +}`), &u) + if err != nil { + panic(err) } for i := 0; i < 9999; i++ { unique := fmt.Sprintf("this-key-is-very-long-so-as-to-create-a-very-large-serialized-fieldset-%v", i) - cfg.Data[unique] = "A" + unstructured.SetNestedField(u.Object, "A", "data", unique) } - return cfg + return u }(), }, { name: "old object + new object annotations + new object last-applied annotation is too big", - oldObject: func() *corev1.ConfigMap { - cfg := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "large-update-test-cm", - Namespace: "default", - Annotations: map[string]string{ - internal.LastAppliedConfigAnnotation: "nonempty", - }, - }, - Data: map[string]string{"k": "v"}, + oldObject: func() *unstructured.Unstructured { + u := &unstructured.Unstructured{} + err := json.Unmarshal([]byte(` +{ + "metadata": { + "name": "large-update-test-cm", + "namespace": "default", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "nonempty" + } + }, + "apiVersion": "v1", + "kind": "ConfigMap", + "data": { + "k": "v" + } +}`), &u) + if err != nil { + panic(err) } for i := 0; i < 2000; i++ { unique := fmt.Sprintf("this-key-is-very-long-so-as-to-create-a-very-large-serialized-fieldset-%v", i) - cfg.Data[unique] = "A" + unstructured.SetNestedField(u.Object, "A", "data", unique) } - return cfg + return u }(), - newObject: func() *corev1.ConfigMap { - cfg := &corev1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "large-update-test-cm", - Namespace: "default", - Annotations: map[string]string{ - internal.LastAppliedConfigAnnotation: "nonempty", - }, - }, - Data: map[string]string{"k": "v"}, + newObject: func() *unstructured.Unstructured { + u := &unstructured.Unstructured{} + err := json.Unmarshal([]byte(` +{ + "metadata": { + "name": "large-update-test-cm", + "namespace": "default", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "nonempty" + } + }, + "apiVersion": "v1", + "kind": "ConfigMap", + "data": { + "k": "v" + } +}`), &u) + if err != nil { + panic(err) } for i := 0; i < 2000; i++ { unique := fmt.Sprintf("this-key-is-very-long-so-as-to-create-a-very-large-serialized-fieldset-%v", i) - cfg.Data[unique] = "A" - cfg.ObjectMeta.Annotations[unique] = "A" + unstructured.SetNestedField(u.Object, "A", "data", unique) + unstructured.SetNestedField(u.Object, "A", "metadata", "annotations", unique) } - return cfg + return u }(), }, } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/skipnonapplied_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/skipnonapplied_test.go index cdd01f4091b..0ba7b0845fa 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/skipnonapplied_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/skipnonapplied_test.go @@ -17,10 +17,10 @@ limitations under the License. package internal_test import ( + "encoding/json" "strings" "testing" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -78,10 +78,10 @@ func TestUpdateBeforeFirstApply(t *testing.T) { ) }) - updatedObj := &corev1.Pod{} - updatedObj.Kind = "Pod" - updatedObj.APIVersion = "v1" - updatedObj.ObjectMeta.Labels = map[string]string{"app": "my-nginx"} + updatedObj := &unstructured.Unstructured{} + if err := json.Unmarshal([]byte(`{"kind": "Pod", "apiVersion": "v1", "metadata": {"labels": {"app": "my-nginx"}}}`), updatedObj); err != nil { + t.Fatalf("Failed to unmarshal object: %v", err) + } if err := f.Update(updatedObj, "fieldmanager_test_update"); err != nil { t.Fatalf("failed to update object: %v", err)