Merge pull request #158 from apelisse/return-nil-on-non-change-apply

merge: Apply returns nil object if unchanged

Author: k8s-ci-robot

fieldpath/managers.go

@@ -58,6 +58,37 @@ func (v versionedSet) Applied() bool {
 // what version).
 type ManagedFields map[string]VersionedSet
 
+// Equals returns true if the two managedfields are the same, false
+// otherwise.
+func (lhs ManagedFields) Equals(rhs ManagedFields) bool {
+	if len(lhs) != len(rhs) {
+		return false
+	}
+
+	for manager, left := range lhs {
+		right, ok := rhs[manager]
+		if !ok {
+			return false
+		}
+		if left.APIVersion() != right.APIVersion() || left.Applied() != right.Applied() {
+			return false
+		}
+		if !left.Set().Equals(right.Set()) {
+			return false
+		}
+	}
+	return true
+}
+
+// Copy the list, this is mostly a shallow copy.
+func (lhs ManagedFields) Copy() ManagedFields {
+	copy := ManagedFields{}
+	for manager, set := range lhs {
+		copy[manager] = set
+	}
+	return copy
+}
+
 // Difference returns a symmetric difference between two Managers. If a
 // given user's entry has version X in lhs and version Y in rhs, then
 // the return value for that user will be from rhs. If the difference for

fieldpath/managers_test.go

@@ -30,7 +30,7 @@ var (
 	_P  = fieldpath.MakePathOrDie
 )
 
-func TestManagersDifference(t *testing.T) {
+func TestManagersEquals(t *testing.T) {
 	tests := []struct {
 		name string
 		lhs  fieldpath.ManagedFields
@@ -164,3 +164,123 @@ func TestManagersDifference(t *testing.T) {
 		})
 	}
 }
+
+func TestManagersDifference(t *testing.T) {
+	tests := []struct {
+		name  string
+		lhs   fieldpath.ManagedFields
+		rhs   fieldpath.ManagedFields
+		equal bool
+	}{
+		{
+			name:  "Empty sets",
+			equal: true,
+		},
+		{
+			name: "Same everything",
+			lhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			rhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			equal: true,
+		},
+		{
+			name: "Empty RHS",
+			lhs: fieldpath.ManagedFields{
+				"default": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			equal: false,
+		},
+		{
+			name: "Empty LHS",
+			rhs: fieldpath.ManagedFields{
+				"default": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			equal: false,
+		},
+		{
+			name: "Different managers",
+			lhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			rhs: fieldpath.ManagedFields{
+				"two": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			equal: false,
+		},
+		{
+			name: "Same manager, different version",
+			lhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("integer")),
+					"v1",
+					false,
+				),
+			},
+			rhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string"), _P("bool")),
+					"v2",
+					false,
+				),
+			},
+			equal: false,
+		},
+		{
+			name: "Set difference",
+			lhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("numeric"), _P("string")),
+					"v1",
+					false,
+				),
+			},
+			rhs: fieldpath.ManagedFields{
+				"one": fieldpath.NewVersionedSet(
+					_NS(_P("string"), _P("bool")),
+					"v1",
+					false,
+				),
+			},
+			equal: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf(test.name), func(t *testing.T) {
+			equal := test.lhs.Equals(test.rhs)
+			if test.equal && !equal {
+				difference := test.lhs.Difference(test.rhs)
+				t.Errorf("should be equal, but are different: %v", difference)
+			} else if !test.equal && equal {
+				t.Errorf("should not be equal, but they are")
+			}
+		})
+	}
+}

internal/fixture/state.go

@@ -150,9 +150,10 @@ func (s *State) ApplyObject(tv *typed.TypedValue, version fieldpath.APIVersion,
 	if err != nil {
 		return err
 	}
-	s.Live = new
 	s.Managers = managers
-
+	if new != nil {
+		s.Live = new
+	}
 	return nil
 }
 

merge/update.go

@@ -148,7 +148,8 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
 
 // Apply should be called when Apply is run, given the current object as
 // well as the configuration that is applied. This will merge the object
-// and return it.
+// and return it. If the object hasn't changed, nil is returned (the
+// managers can still have changed though).
 func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
 	managers = shallowCopyManagers(managers)
 	var err error
@@ -178,10 +179,13 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
 	if err != nil {
 		return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to prune fields: %v", err)
 	}
-	managers, _, err = s.update(liveObject, newObject, version, managers, manager, force)
+	managers, compare, err := s.update(liveObject, newObject, version, managers, manager, force)
 	if err != nil {
 		return nil, fieldpath.ManagedFields{}, err
 	}
+	if compare.IsSame() {
+		newObject = nil
+	}
 	return newObject, managers, nil
 }