Merge pull request #5846 from sbueringer/pr-topology-managed-fields-m…

…ore-tests 🌱 ClusterClass: Add reconcile repeatedly test
kubernetes-sigs · Jan 14, 2022 · 58f834f · 58f834f
2 parents 6a3a91b + 62fa051
commit 58f834f
Show file tree

Hide file tree

Showing 5 changed files with 645 additions and 21 deletions.
diff --git a/internal/controllers/topology/cluster/mergepatch/managed_paths.go b/internal/controllers/topology/cluster/mergepatch/managed_paths.go
@@ -114,18 +114,27 @@ func toManagedFieldsMap(m map[string]interface{}, ignorePaths []contract.Path) m
 			continue
 		}
 
-		// If the field has nested values, process them.
-		nestedV := make(map[string]interface{})
+		// If the field has nested values (it is an object/map), process them.
 		if nestedM, ok := v.(map[string]interface{}); ok {
 			nestedIgnorePaths := make([]contract.Path, 0)
 			for _, i := range ignorePaths {
 				if i[0] == k && len(i) > 1 {
 					nestedIgnorePaths = append(nestedIgnorePaths, i[1:])
 				}
 			}
-			nestedV = toManagedFieldsMap(nestedM, nestedIgnorePaths)
+			nestedV := toManagedFieldsMap(nestedM, nestedIgnorePaths)
+
+			// Note: we are considering the object managed only if it is setting a value for one of the nested fields.
+			// This prevents the topology controller to become authoritative on all the empty maps generated due to
+			// how serialization works.
+			if len(nestedV) > 0 {
+				r[k] = nestedV
+			}
+			continue
 		}
-		r[k] = nestedV
+
+		// Otherwise, it is a "simple" field so mark it as managed
+		r[k] = make(map[string]interface{})
 	}
 	return r
 }

diff --git a/internal/controllers/topology/cluster/mergepatch/managed_paths_test.go b/internal/controllers/topology/cluster/mergepatch/managed_paths_test.go
@@ -191,6 +191,43 @@ func Test_ManagedFieldAnnotation(t *testing.T) {
 				{"spec", "kubeadmConfigSpec", "joinConfiguration"},
 			},
 		},
+		{
+			name: "Managed fields annotation ignore empty maps",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"kubeadmConfigSpec": map[string]interface{}{
+							"clusterConfiguration": map[string]interface{}{
+								"version": "v2.0.1",
+							},
+							"initConfiguration": map[string]interface{}{},
+						},
+					},
+				},
+			},
+			wantPaths: []contract.Path{
+				{"spec", "kubeadmConfigSpec", "clusterConfiguration", "version"},
+			},
+		},
+		{
+			name: "Managed fields annotation ignore empty maps - excluding ignore paths",
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"kubeadmConfigSpec": map[string]interface{}{
+							"clusterConfiguration": map[string]interface{}{
+								"version": "v2.0.1",
+							},
+							"initConfiguration": map[string]interface{}{},
+						},
+					},
+				},
+			},
+			ignorePaths: []contract.Path{
+				{"spec", "kubeadmConfigSpec", "clusterConfiguration", "version"},
+			},
+			wantPaths: []contract.Path{},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/internal/controllers/topology/cluster/mergepatch/mergepatch.go b/internal/controllers/topology/cluster/mergepatch/mergepatch.go
@@ -62,7 +62,7 @@ func NewHelper(original, modified client.Object, c client.Client, opts ...Helper
 	// changes to those paths are going to be considered authoritative.
 	managedPaths, err := getManagedPaths(original)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal original object to json")
+		return nil, errors.Wrap(err, "failed to get managed paths")
 	}
 	helperOptions.managedPaths = managedPaths
 

diff --git a/internal/controllers/topology/cluster/mergepatch/mergepatch_test.go b/internal/controllers/topology/cluster/mergepatch/mergepatch_test.go
@@ -801,15 +801,7 @@ func TestNewHelper(t *testing.T) {
 			wantPatch: []byte(fmt.Sprintf(
 				"{\"metadata\":{\"annotations\":{%q:%q}},\"spec\":{\"kubeadmConfigSpec\":{\"clusterConfiguration\":{\"controllerManager\":{\"extraArgs\":{\"enable-hostpath-provisioner\":null}}}}}}",
 				clusterv1.ClusterTopologyManagedFieldsAnnotation,
-				mustManagedFieldAnnotation(map[string]interface{}{
-					"kubeadmConfigSpec": map[string]interface{}{
-						"clusterConfiguration": map[string]interface{}{
-							"controllerManager": map[string]interface{}{
-								"extraArgs": map[string]interface{}{},
-							},
-						},
-					},
-				}),
+				mustManagedFieldAnnotation(map[string]interface{}{}),
 			)),
 		},
 		{
@@ -923,13 +915,7 @@ func TestNewHelper(t *testing.T) {
 			wantPatch: []byte(fmt.Sprintf(
 				"{\"metadata\":{\"annotations\":{%q:%q}},\"spec\":{\"kubeadmConfigSpec\":{\"clusterConfiguration\":{\"controllerManager\":{\"extraArgs\":{\"enable-hostpath-provisioner\":null}}}}}}",
 				clusterv1.ClusterTopologyManagedFieldsAnnotation,
-				mustManagedFieldAnnotation(map[string]interface{}{
-					"kubeadmConfigSpec": map[string]interface{}{
-						"clusterConfiguration": map[string]interface{}{
-							"controllerManager": map[string]interface{}{},
-						},
-					},
-				}),
+				mustManagedFieldAnnotation(map[string]interface{}{}),
 			)),
 		},