github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #125317 from jpbetz/fix-nop-empty-map

Browse Source 
Fix non-semantic apply requests to ignore empty maps
This commit is contained in:
Kubernetes Prow Robot
2024-06-24 16:48:39 -07:00
committed by GitHub
parent 0d17892740 f31afbbf8e
commit d5dd5b13a5
2 changed files with 240 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go
View File

@@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
"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/conversion"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/endpoints/metrics"
@@ -152,14 +153,20 @@ func IgnoreManagedFieldsTimestampsTransformer(
return newObj, nil
}
eqFn := equalities.DeepEqual
if _, ok := newObj.(*unstructured.Unstructured); ok {
// Use strict equality with unstructured
eqFn = equalities.DeepEqualWithNilDifferentFromEmpty
}
// This condition ensures the managed fields are always compared first. If
// this check fails, the if statement will short circuit. If the check
// succeeds the slow path is taken which compares entire objects.
if !equalities.DeepEqualWithNilDifferentFromEmpty(oldManagedFields, newManagedFields) {
if !eqFn(oldManagedFields, newManagedFields) {
return newObj, nil
}
if equalities.DeepEqualWithNilDifferentFromEmpty(newObj, oldObj) {
if eqFn(newObj, oldObj) {
// Remove any changed timestamps, so that timestamp is not the only
// change seen by etcd.
//

231
test/integration/apiserver/apply/apply_test.go
View File

@@ -244,6 +244,237 @@ func TestNoOpUpdateSameResourceVersion(t *testing.T) {
}
}
// TestNoOpApplyWithEmptyMap
func TestNoOpApplyWithEmptyMap(t *testing.T) {
client, closeFn := setup(t)
defer closeFn()
deploymentName := "no-op"
deploymentsResource := "deployments"
deploymentBytes := []byte(`{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": {
"name": "` + deploymentName + `",
"labels": {
"app": "nginx"
}
},
"spec": {
"replicas": 1,
"selector": {
"matchLabels": {
"app": "nginx"
}
},
"template": {
"metadata": {
"annotations": {},
"labels": {
"app": "nginx"
}
},
"spec": {
"containers": [{
"name": "nginx",
"image": "nginx:1.14.2",
"ports": [{
"containerPort": 80
}]
}]
}
}
}
}`)
_, err := client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource(deploymentsResource).
Name(deploymentName).
Body(deploymentBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
// This sleep is necessary to consistently produce different timestamps because the time field in managedFields has
// 1 second granularity and if both apply requests happen during the same second, this test would flake.
time.Sleep(1 * time.Second)
createdObject, err := client.AppsV1().RESTClient().Get().Namespace("default").Resource(deploymentsResource).Name(deploymentName).Do(context.TODO()).Get()
if err != nil {
t.Fatalf("Failed to retrieve created object: %v", err)
}
createdAccessor, err := meta.Accessor(createdObject)
if err != nil {
t.Fatalf("Failed to get meta accessor for created object: %v", err)
}
createdBytes, err := json.MarshalIndent(createdObject, "\t", "\t")
if err != nil {
t.Fatalf("Failed to marshal created object: %v", err)
}
// Test that we can apply the same object and don't change the RV
_, err = client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource(deploymentsResource).
Name(deploymentName).
Body(deploymentBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
updatedObject, err := client.AppsV1().RESTClient().Get().Namespace("default").Resource(deploymentsResource).Name(deploymentName).Do(context.TODO()).Get()
if err != nil {
t.Fatalf("Failed to retrieve updated object: %v", err)
}
updatedAccessor, err := meta.Accessor(updatedObject)
if err != nil {
t.Fatalf("Failed to get meta accessor for updated object: %v", err)
}
updatedBytes, err := json.MarshalIndent(updatedObject, "\t", "\t")
if err != nil {
t.Fatalf("Failed to marshal updated object: %v", err)
}
if createdAccessor.GetResourceVersion() != updatedAccessor.GetResourceVersion() {
t.Fatalf("Expected same resource version to be %v but got: %v\nold object:\n%v\nnew object:\n%v",
createdAccessor.GetResourceVersion(),
updatedAccessor.GetResourceVersion(),
string(createdBytes),
string(updatedBytes),
)
}
}
// TestApplyEmptyMarkerStructDifferentFromNil
func TestApplyEmptyMarkerStructDifferentFromNil(t *testing.T) {
client, closeFn := setup(t)
defer closeFn()
podName := "pod-with-empty-dir"
podsResource := "pods"
podBytesWithEmptyDir := []byte(`{
"apiVersion": "v1",
"kind": "Pod",
"metadata": {
"name": "` + podName + `"
},
"spec": {
"containers": [{
"name": "test-container-a",
"image": "test-image-one",
"volumeMounts": [{
"mountPath": "/cache",
"name": "cache-volume"
}],
}],
"volumes": [{
"name": "cache-volume",
"emptyDir": {}
}]
}
}`)
_, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource(podsResource).
Name(podName).
Body(podBytesWithEmptyDir).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
// This sleep is necessary to consistently produce different timestamps because the time field in managedFields has
// 1 second granularity and if both apply requests happen during the same second, this test would flake.
time.Sleep(1 * time.Second)
createdObject, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource(podsResource).Name(podName).Do(context.TODO()).Get()
if err != nil {
t.Fatalf("Failed to retrieve created object: %v", err)
}
createdAccessor, err := meta.Accessor(createdObject)
if err != nil {
t.Fatalf("Failed to get meta accessor for created object: %v", err)
}
createdBytes, err := json.MarshalIndent(createdObject, "\t", "\t")
if err != nil {
t.Fatalf("Failed to marshal created object: %v", err)
}
podBytesNoEmptyDir := []byte(`{
"apiVersion": "v1",
"kind": "Pod",
"metadata": {
"name": "` + podName + `"
},
"spec": {
"containers": [{
"name": "test-container-a",
"image": "test-image-one",
"volumeMounts": [{
"mountPath": "/cache",
"name": "cache-volume"
}],
}],
"volumes": [{
"name": "cache-volume"
}]
}
}`)
// Test that an apply with no emptyDir is recognized as distinct from an empty marker struct emptyDir.
_, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource(podsResource).
Name(podName).
Body(podBytesNoEmptyDir).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
updatedObject, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource(podsResource).Name(podName).Do(context.TODO()).Get()
if err != nil {
t.Fatalf("Failed to retrieve updated object: %v", err)
}
updatedAccessor, err := meta.Accessor(updatedObject)
if err != nil {
t.Fatalf("Failed to get meta accessor for updated object: %v", err)
}
updatedBytes, err := json.MarshalIndent(updatedObject, "\t", "\t")
if err != nil {
t.Fatalf("Failed to marshal updated object: %v", err)
}
if createdAccessor.GetResourceVersion() == updatedAccessor.GetResourceVersion() {
t.Fatalf("Expected different resource version to be %v but got: %v\nold object:\n%v\nnew object:\n%v",
createdAccessor.GetResourceVersion(),
updatedAccessor.GetResourceVersion(),
string(createdBytes),
string(updatedBytes),
)
}
}
func getRV(obj runtime.Object) (string, error) {
acc, err := meta.Accessor(obj)
if err != nil {