POC: using MutatingAdmissionPolicy to schedule CS pods

This change documents a proof-of-concept of configuring the node
placement of Config Sync system pods using MutatingAdmissionPolicy.

This works out of the box, but is currently an alpha api and is behind a
feature gate. It is targeted for beta in k8s 1.34.

Author: sdowell

e2e/nomostest/clusters/kind.go

@@ -22,6 +22,7 @@ import (
 	"sync"
 	"time"
 
+	"k8s.io/apiserver/pkg/features"
 	"kpt.dev/configsync/e2e"
 	"kpt.dev/configsync/e2e/nomostest/docker"
 	"kpt.dev/configsync/e2e/nomostest/taskgroup"
@@ -155,6 +156,12 @@ func createKindCluster(p *cluster.Provider, name, kcfgPath string) error {
 					fmt.Sprintf(`[plugins."io.containerd.grpc.v1.cri".registry.mirrors."localhost:%d"]
   endpoint = ["http://%s:%d"]`, docker.RegistryPort, docker.RegistryName, docker.RegistryPort),
 				},
+				FeatureGates: map[string]bool{
+					string(features.MutatingAdmissionPolicy): true,
+				},
+				RuntimeConfig: map[string]string{
+					"admissionregistration.k8s.io/v1alpha1": "true",
+				},
 				// Enable ValidatingAdmissionWebhooks in the Kind cluster, as these
 				// are disabled by default.
 				// Also mount etcd to tmpfs for memory-backed storage.
@@ -166,7 +173,7 @@ etcd:
     dataDir: /tmp/etcd
 apiServer:
   extraArgs:
-    "enable-admission-plugins": "ValidatingAdmissionWebhook"`,
+    "enable-admission-plugins": "ValidatingAdmissionWebhook,MutatingAdmissionPolicy,ValidatingAdmissionPolicy"`,
 				},
 			}),
 			// Retain nodes for debugging logs.

e2e/nomostest/config_sync.go

@@ -1398,6 +1398,23 @@ func podHasReadyCondition(conditions []corev1.PodCondition) bool {
 	return false
 }
 
+// ValidatePodByLabel validates the all Pods matching the provided label pass the
+// provided list of predicates.
+func ValidatePodByLabel(nt *NT, label, value string, predicates ...testpredicates.Predicate) error {
+	newPods := &corev1.PodList{}
+	if err := nt.KubeClient.List(newPods, client.InNamespace(configmanagement.ControllerNamespace), client.MatchingLabels{label: value}); err != nil {
+		return err
+	}
+	tg := taskgroup.New()
+	for _, pod := range newPods.Items {
+		po := pod
+		tg.Go(func() error {
+			return nt.Validate(po.Name, po.Namespace, &corev1.Pod{}, predicates...)
+		})
+	}
+	return tg.Wait()
+}
+
 // NewPodReady checks if the new created pods are ready.
 // It also checks if the new children pods that are managed by the pods are ready.
 func NewPodReady(nt *NT, labelName, currentLabel, childLabel string, oldCurrentPods, oldChildPods []corev1.Pod) error {

e2e/nomostest/testpredicates/predicates.go

@@ -224,6 +224,28 @@ func HasExactlyLabelKeys(wantKeys ...string) Predicate {
 	}
 }
 
+// HasExactlyNodeAffinity ensures the Pod has the provided nodeAffinity
+func HasExactlyNodeAffinity(nodeAffinity *corev1.NodeAffinity) Predicate {
+	return func(o client.Object) error {
+		if o == nil {
+			return ErrObjectNotFound
+		}
+		pod, ok := o.(*corev1.Pod)
+		if !ok {
+			return WrongTypeErr(pod, &corev1.Pod{})
+		}
+		gotAffinity := &corev1.NodeAffinity{}
+		if pod.Spec.Affinity != nil && pod.Spec.Affinity.NodeAffinity != nil {
+			gotAffinity = pod.Spec.Affinity.NodeAffinity
+		}
+		if !equality.Semantic.DeepEqual(nodeAffinity, gotAffinity) {
+			return fmt.Errorf("expected %s to have spec.affinity.nodeAffinity: %s, but got %s",
+				kinds.ObjectSummary(pod), log.AsJSON(nodeAffinity), log.AsJSON(gotAffinity))
+		}
+		return nil
+	}
+}
+
 // HasExactlyImage ensures a container has the expected image.
 func HasExactlyImage(containerName, expectImageName, expectImageTag, expectImageDigest string) Predicate {
 	return func(o client.Object) error {

e2e/testcases/mutating_admission_policy_test.go

@@ -0,0 +1,77 @@
+package e2e
+
+import (
+	"path/filepath"
+	"testing"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	"kpt.dev/configsync/e2e/nomostest"
+	"kpt.dev/configsync/e2e/nomostest/ntopts"
+	"kpt.dev/configsync/e2e/nomostest/syncsource"
+	nomostesting "kpt.dev/configsync/e2e/nomostest/testing"
+	"kpt.dev/configsync/e2e/nomostest/testpredicates"
+	"kpt.dev/configsync/pkg/api/configsync"
+	"kpt.dev/configsync/pkg/core/k8sobjects"
+	"kpt.dev/configsync/pkg/kinds"
+	"kpt.dev/configsync/pkg/reconcilermanager"
+)
+
+// This test currently requires KinD because MutatingAdmissionPolicy is alpha
+// and requires a feature gate.
+func TestMutatingAdmissionPolicy(t *testing.T) {
+	nt := nomostest.New(t, nomostesting.Reconciliation2,
+		ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID, ntopts.Unstructured),
+		ntopts.RequireKind(t))
+	rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID)
+
+	mapFile := filepath.Join(".", "..", "..", "examples", "mutating-admission-policies", "config-sync-node-placement.yaml")
+	nt.T.Cleanup(func() {
+		nt.Must(nt.Shell.Kubectl("delete", "--ignore-not-found", "-f", mapFile))
+	})
+	nt.Must(nt.Shell.Kubectl("apply", "-f", mapFile))
+	// TODO: is there a way to wait for MutatingAdmissionPolicy readiness? (it doesn't appear so)
+	// sleep hack to give time to propagate
+	time.Sleep(5 * time.Second)
+
+	// expected nodeAffinity from the example MutatingAdmissionPolicy yaml
+	exampleNodeAffinity := &corev1.NodeAffinity{
+		PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{
+			{
+				Weight: 1,
+				Preference: corev1.NodeSelectorTerm{
+					MatchExpressions: []corev1.NodeSelectorRequirement{
+						{
+							Key:      "another-node-label-key",
+							Operator: corev1.NodeSelectorOpIn,
+							Values:   []string{"another-node-label-value"},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	// bounce reconciler-manager Pod and verify the nodeAffinity is applied by MAP
+	nt.Must(nomostest.ValidatePodByLabel(nt, "app", reconcilermanager.ManagerName,
+		testpredicates.HasExactlyNodeAffinity(&corev1.NodeAffinity{})))
+	nt.T.Log("Replacing the reconciler-manager Pod to validate nodeAffinity is added")
+	nomostest.DeletePodByLabel(nt, "app", reconcilermanager.ManagerName, false)
+	nt.Must(nomostest.ValidatePodByLabel(nt, "app", reconcilermanager.ManagerName,
+		testpredicates.HasExactlyNodeAffinity(exampleNodeAffinity)))
+
+	// update the RootSync to trigger a Deployment change, verify new reconciler Pod has nodeAffinity
+	rootSync := nomostest.RootSyncObjectV1Beta1FromRootRepo(nt, nomostest.DefaultRootSyncID.Name)
+	rootSync.Spec.Git.Dir = "foo"
+	nt.Must(nt.KubeClient.Apply(rootSync))
+	nt.Must(rootSyncGitRepo.Add("foo/ns.yaml", k8sobjects.NamespaceObject("test-map-ns")))
+	nt.Must(rootSyncGitRepo.CommitAndPush("add foo-ns under foo/ dir"))
+	nt.Must(nt.WatchForSync(kinds.RootSyncV1Beta1(), rootSync.Name, configsync.ControllerNamespace,
+		&syncsource.GitSyncSource{
+			ExpectedCommit:    rootSyncGitRepo.MustHash(t),
+			ExpectedDirectory: "foo",
+		}))
+	nt.Must(nt.Validate("test-map-ns", "", &corev1.Namespace{}))
+	nt.Must(nomostest.ValidatePodByLabel(nt, "app", reconcilermanager.Reconciler,
+		testpredicates.HasExactlyNodeAffinity(exampleNodeAffinity)))
+}

examples/mutating-admission-policies/README.md

@@ -0,0 +1,21 @@
+# Scheduling Config Sync system pods using MutatingAdmissionPolicies
+
+[MutatingAdmissionPolicy] can be used to configure the way the Config Sync's
+system Pods are scheduled.
+
+The [example in this directory](./config-sync-node-placement.yaml) demonstrates
+how to inject `nodeAffinity` into all Pods in the `config-management-system` Namespace.
+
+Note that this is just a demonstrative example, and different `matchConstraints` and
+`mutations` can be applied depending on the use case.
+
+Caveats:
+
+- MutatingAdmissionPolicy is currently in alpha and requires feature gate enablement. It is [targeted to enter beta in k8s 1.34].
+- MutatingAdmissionPolicy does not update existing Pods. Pods created before the policy was applied must be updated/recreated.
+
+
+
+
+[MutatingAdmissionPolicy]: https://kubernetes.io/docs/reference/access-authn-authz/mutating-admission-policy/
+[targeted to enter beta in k8s 1.34]: https://github.com/kubernetes/enhancements/issues/3962

examples/mutating-admission-policies/config-sync-node-placement.yaml

@@ -0,0 +1,56 @@
+apiVersion: admissionregistration.k8s.io/v1alpha1
+kind: MutatingAdmissionPolicy
+metadata:
+  name: "configsync-nodeplacement"
+spec:
+  # This example uniformly applies the nodeAffinity to *all* Pods in config-management-system.
+  # Different matchConstraints can be used for more granular mutation.
+  matchConstraints:
+    namespaceSelector:
+      matchLabels:
+        kubernetes.io/metadata.name: config-management-system
+    resourceRules:
+    - apiGroups:   [""]
+      apiVersions: ["v1"]
+      operations:  ["CREATE", "UPDATE"]
+      resources:   ["pods"]
+  failurePolicy: Fail
+  reinvocationPolicy: IfNeeded
+  mutations:
+  # Simple example of adding nodeAffinity: https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/
+  # Similar mutations can be applied for nodeSelector/tolerations/etc
+  - patchType: "JSONPatch"
+    jsonPatch:
+      expression: >
+        [
+          JSONPatch{
+            op: "add", path: "/spec/affinity",
+            value: Object.spec.affinity{
+              nodeAffinity: Object.spec.affinity.nodeAffinity{
+                preferredDuringSchedulingIgnoredDuringExecution: [
+                  Object.spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution{
+                    weight: 1,
+                    preference: Object.spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution.preference{
+                      matchExpressions: [
+                        Object.spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution.preference.matchExpressions{
+                          key: "another-node-label-key",
+                          operator: "In",
+                          values: [
+                            "another-node-label-value"
+                          ]
+                        }
+                      ]
+                    }
+                  }
+                ]
+              }
+            }
+          }
+        ]
+---
+apiVersion: admissionregistration.k8s.io/v1alpha1
+kind: MutatingAdmissionPolicyBinding
+metadata:
+  name: "configsync-nodeplacement"
+spec:
+  policyName: "configsync-nodeplacement"