Merge pull request #106388 from alexzielenski/ssa-ignore-nonsemantic-changes

Ignore non-semantic changes to objects

Kubernetes-commit: 8bc12f24e64ae045eb6770524388301cce6b7429

go.mod

@@ -39,7 +39,7 @@ require (
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	gopkg.in/square/go-jose.v2 v2.2.2
 	k8s.io/api v0.0.0-20220725160253-f6f0d0e54216
-	k8s.io/apimachinery v0.0.0-20220722160307-915d89afb4cb
+	k8s.io/apimachinery v0.0.0-20220726200055-965218438260
 	k8s.io/client-go v0.0.0-20220722162234-b2097e607c19
 	k8s.io/component-base v0.0.0-20220726160733-a30295a8559b
 	k8s.io/klog/v2 v2.70.1
@@ -120,7 +120,7 @@ require (
 
 replace (
 	k8s.io/api => k8s.io/api v0.0.0-20220725160253-f6f0d0e54216
-	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20220722160307-915d89afb4cb
+	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20220726200055-965218438260
 	k8s.io/client-go => k8s.io/client-go v0.0.0-20220722162234-b2097e607c19
 	k8s.io/component-base => k8s.io/component-base v0.0.0-20220726160733-a30295a8559b
 )

go.sum

@@ -959,8 +959,8 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 k8s.io/api v0.0.0-20220725160253-f6f0d0e54216 h1:lK3Ih8q6XH8zLGPKkAx0CDs3WYWiQuK3NxL/W7LtqSw=
 k8s.io/api v0.0.0-20220725160253-f6f0d0e54216/go.mod h1:esLTNaRjCQz5pCDYxvulIzMUPDaw9bOuAQ9d6ndNdvs=
-k8s.io/apimachinery v0.0.0-20220722160307-915d89afb4cb h1:CkEjGDsYVgTZod3JtHw5+t2LG2ZoEQpGlvjdQTiiosA=
-k8s.io/apimachinery v0.0.0-20220722160307-915d89afb4cb/go.mod h1:E6C2QnwpsJTXktTwPqd4nAJ/xmsdi1AHqDD0iPB61j0=
+k8s.io/apimachinery v0.0.0-20220726200055-965218438260 h1:Yec9pKZ5AYLiwe5G9piTsqNR7/V57T/9dL/ISMGZRQM=
+k8s.io/apimachinery v0.0.0-20220726200055-965218438260/go.mod h1:E6C2QnwpsJTXktTwPqd4nAJ/xmsdi1AHqDD0iPB61j0=
 k8s.io/client-go v0.0.0-20220722162234-b2097e607c19 h1:MlhjUMLx4h1NsbY/bO3noaY+s6Wtt21SpqNrKZDDHfo=
 k8s.io/client-go v0.0.0-20220722162234-b2097e607c19/go.mod h1:7TyeX2vy8G+EisTteawMgmRjEf47thI1Qgfa93IkyQA=
 k8s.io/component-base v0.0.0-20220726160733-a30295a8559b h1:+Sr/CAv5xYeHBxwjOo7co4ytWysYS4PfY6FZUVzIkfs=

pkg/endpoints/handlers/fieldmanager/equality.go

@@ -0,0 +1,180 @@
+/*
+Copyright 2021 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
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"reflect"
+	"strconv"
+	"time"
+
+	"k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/conversion"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/endpoints/metrics"
+	"k8s.io/klog/v2"
+)
+
+func determineAvoidNoopTimestampUpdatesEnabled() bool {
+	if avoidNoopTimestampUpdatesString, exists := os.LookupEnv("KUBE_APISERVER_AVOID_NOOP_SSA_TIMESTAMP_UPDATES"); exists {
+		if ret, err := strconv.ParseBool(avoidNoopTimestampUpdatesString); err == nil {
+			return ret
+		} else {
+			klog.Errorf("failed to parse envar KUBE_APISERVER_AVOID_NOOP_SSA_TIMESTAMP_UPDATES: %v", err)
+		}
+	}
+
+	// enabled by default
+	return true
+}
+
+var (
+	avoidNoopTimestampUpdatesEnabled = determineAvoidNoopTimestampUpdatesEnabled()
+)
+
+var avoidTimestampEqualities = func() conversion.Equalities {
+	var eqs = equality.Semantic.Copy()
+
+	err := eqs.AddFunc(
+		func(a, b metav1.ManagedFieldsEntry) bool {
+			// Two objects' managed fields are equivalent if, ignoring timestamp,
+			//	the objects are deeply equal.
+			a.Time = nil
+			b.Time = nil
+			return reflect.DeepEqual(a, b)
+		},
+	)
+
+	if err != nil {
+		panic(err)
+	}
+
+	return eqs
+}()
+
+// IgnoreManagedFieldsTimestampsTransformer reverts timestamp updates
+// if the non-managed parts of the object are equivalent
+func IgnoreManagedFieldsTimestampsTransformer(
+	_ context.Context,
+	newObj runtime.Object,
+	oldObj runtime.Object,
+) (res runtime.Object, err error) {
+	if !avoidNoopTimestampUpdatesEnabled {
+		return newObj, nil
+	}
+
+	outcome := "unequal_objects_fast"
+	start := time.Now()
+	err = nil
+	res = nil
+
+	defer func() {
+		if err != nil {
+			outcome = "error"
+		}
+
+		metrics.RecordTimestampComparisonLatency(outcome, time.Since(start))
+	}()
+
+	// If managedFields modulo timestamps are unchanged
+	//		and
+	//	rest of object is unchanged
+	//		then
+	//	revert any changes to timestamps in managed fields
+	//		(to prevent spurious ResourceVersion bump)
+	//
+	// Procecure:
+	// Do a quicker check to see if just managed fields modulo timestamps are
+	//	unchanged. If so, then do the full, slower check.
+	//
+	// In most cases which actually update the object, the managed fields modulo
+	//	timestamp check will fail, and we will be able to return early.
+	//
+	// In other cases, the managed fields may be exactly the same,
+	// 	except for timestamp, but the objects are the different. This is the
+	//	slow path which checks the full object.
+	oldAccessor, err := meta.Accessor(oldObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire accessor for oldObj: %v", err)
+	}
+
+	accessor, err := meta.Accessor(newObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to acquire accessor for newObj: %v", err)
+	}
+
+	oldManagedFields := oldAccessor.GetManagedFields()
+	newManagedFields := accessor.GetManagedFields()
+
+	if len(oldManagedFields) != len(newManagedFields) {
+		// Return early if any managed fields entry was added/removed.
+		// We want to retain user expectation that even if they write to a field
+		// whose value did not change, they will still result as the field
+		// manager at the end.
+		return newObj, nil
+	} else if len(newManagedFields) == 0 {
+		// This transformation only makes sense when managedFields are
+		// non-empty
+		return newObj, nil
+	}
+
+	// This transformation only makes sense if the managed fields has at least one
+	// changed timestamp; and are otherwise equal. Return early if there are no
+	// changed timestamps.
+	allTimesUnchanged := true
+	for i, e := range newManagedFields {
+		if !e.Time.Equal(oldManagedFields[i].Time) {
+			allTimesUnchanged = false
+			break
+		}
+	}
+
+	if allTimesUnchanged {
+		return newObj, nil
+	}
+
+	// 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 !avoidTimestampEqualities.DeepEqualWithNilDifferentFromEmpty(oldManagedFields, newManagedFields) {
+		return newObj, nil
+	}
+
+	if avoidTimestampEqualities.DeepEqualWithNilDifferentFromEmpty(newObj, oldObj) {
+		// Remove any changed timestamps, so that timestamp is not the only
+		// change seen by etcd.
+		//
+		// newManagedFields is known to be exactly pairwise equal to
+		// oldManagedFields except for timestamps.
+		//
+		// Simply replace possibly changed new timestamps with their old values.
+		for idx := 0; idx < len(oldManagedFields); idx++ {
+			newManagedFields[idx].Time = oldManagedFields[idx].Time
+		}
+
+		accessor.SetManagedFields(newManagedFields)
+		outcome = "equal_objects"
+		return newObj, nil
+	}
+
+	outcome = "unequal_objects_slow"
+	return newObj, nil
+}

pkg/endpoints/handlers/patch.go

@@ -659,8 +659,13 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti
 		return obj, nil
 	}
 
+	transformers := []rest.TransformFunc{p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer}
+	if scope.FieldManager != nil {
+		transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
+	}
+
 	wasCreated := false
-	p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer)
+	p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, transformers...)
 	requestFunc := func() (runtime.Object, error) {
 		// Pass in UpdateOptions to override UpdateStrategy.AllowUpdateOnCreate
 		options := patchToUpdateOptions(p.options)

pkg/endpoints/handlers/update.go

@@ -191,6 +191,15 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 			})
 		}
 
+		// Ignore changes that only affect managed fields
+		// timestamps. FieldManager can't know about changes
+		// like normalized fields, defaulted fields and other
+		// mutations.
+		// Only makes sense when SSA field manager is being used
+		if scope.FieldManager != nil {
+			transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
+		}
+
 		createAuthorizerAttributes := authorizer.AttributesRecord{
 			User:            userInfo,
 			ResourceRequest: true,

pkg/endpoints/metrics/metrics.go

@@ -238,6 +238,18 @@ var (
 		[]string{"source", "status"},
 	)
 
+	requestTimestampComparisonDuration = compbasemetrics.NewHistogramVec(
+		&compbasemetrics.HistogramOpts{
+			Name:           "apiserver_request_timestamp_comparison_time",
+			Help:           "Time taken for comparison of old vs new objects in UPDATE or PATCH requests",
+			Buckets:        []float64{0.0001, 0.0003, 0.001, 0.003, 0.01, 0.03, 0.1, 0.3, 1.0, 5.0},
+			StabilityLevel: compbasemetrics.ALPHA,
+		},
+		// Path the code takes to reach a conclusion:
+		// i.e. unequalObjectsFast, unequalObjectsSlow, equalObjectsSlow
+		[]string{"code_path"},
+	)
+
 	metrics = []resettableCollector{
 		deprecatedRequestGauge,
 		requestCounter,
@@ -256,6 +268,7 @@ var (
 		requestFilterDuration,
 		requestAbortsTotal,
 		requestPostTimeoutTotal,
+		requestTimestampComparisonDuration,
 	}
 
 	// these are the valid request methods which we report in our metrics. Any other request methods
@@ -366,6 +379,10 @@ func RecordFilterLatency(ctx context.Context, name string, elapsed time.Duration
 	requestFilterDuration.WithContext(ctx).WithLabelValues(name).Observe(elapsed.Seconds())
 }
 
+func RecordTimestampComparisonLatency(codePath string, elapsed time.Duration) {
+	requestTimestampComparisonDuration.WithLabelValues(codePath).Observe(elapsed.Seconds())
+}
+
 func RecordRequestPostTimeout(source string, status string) {
 	requestPostTimeoutTotal.WithLabelValues(source, status).Inc()
 }