-
Notifications
You must be signed in to change notification settings - Fork 21
feat: auto tolerate daemonsets with MAP #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| # MutatingAdmissionPolicyBinding binds the policy to the ConfigMap parameter | ||
| apiVersion: admissionregistration.k8s.io/v1alpha1 | ||
| kind: MutatingAdmissionPolicyBinding | ||
| metadata: | ||
| name: inject-daemonset-readiness-tolerations-binding | ||
| spec: | ||
| policyName: inject-daemonset-readiness-tolerations | ||
| # Reference the ConfigMap containing toleration data | ||
| paramRef: | ||
| name: readiness-taints | ||
| namespace: nrr-system | ||
| parameterNotFoundAction: Deny | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, in my local testing with the PR changes, it is blocking creation of new daemonsets. It works if a user creates node-readiness-rules first and then daemonsets, but the other way around blocks the creation of new daemonsets. I tested the behaviour by disabling the empty configmap file in the kustomization.yaml, because I was thinking why do we need to have an empty configmap file, if the configmap creation is taken care of by the controller itself - https://github.com/kubernetes-sigs/node-readiness-controller/pull/117/changes#diff-b857080def3b2fecd3967ca35b1bdc29564dda0b064fa902fa24efcd55471899R721-R727 But the thing is - the controller logic only kicks in when a user create a new node-readiness-rule. The presence of the empty configmap file at the same time of creating MAP Policy/Policy-Binding fixes it. ❯ kubectl get node nrr-test-worker -o jsonpath={.spec.taints} | jq .
[
{
"effect": "NoSchedule",
"key": "readiness.k8s.io/NetworkReady",
"value": "pending"
}
]
// applying MAP policy/policy-binding without empty configmap
❯ kubectl apply -k config/admission-policy
mutatingadmissionpolicy.admissionregistration.k8s.io/inject-daemonset-readiness-tolerations created
mutatingadmissionpolicybinding.admissionregistration.k8s.io/inject-daemonset-readiness-tolerations-binding created
❯ kubectl get cm -n nrr-system
NAME DATA AGE
kube-root-ca.crt 1 41s
// daemonset creation is blocked
❯ kubectl apply -f test-daemonset.yaml
Error from server (Forbidden): error when creating "test-daemonset.yaml": policy 'inject-daemonset-readiness-tolerations' with binding 'inject-daemonset-readiness-tolerations-binding' denied request: failed to configure binding: no params found for policy binding with `Deny` parameterNotFoundAction
// creating a new nrr rule, which will trigger in turn configmap creation/patch per PR changes
❯ kubectl apply -f examples/cni-readiness/network-readiness-rule.yaml
nodereadinessrule.readiness.node.x-k8s.io/network-readiness-rule created
❯ kubectl get cm readiness-taints -n nrr-system -o jsonpath={.data}
{"taint-keys":"readiness.k8s.io/NetworkReady"}
// now daemonset creation works
❯ kubectl apply -f test-daemonset.yaml
daemonset.apps/test-ds created |
||
| matchResources: | ||
| namespaceSelector: {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| # ConfigMap that stores readiness tolerations | ||
| # This will be populated/updated by the NodeReadinessRule controller | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: readiness-taints | ||
| namespace: nrr-system | ||
| data: | ||
| # Store each toleration key separately for easier CEL access | ||
| # Format: key1=readiness.k8s.io/NetworkReady,key2=readiness.k8s.io/StorageReady | ||
| taint-keys: "" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| apiVersion: kustomize.config.k8s.io/v1beta1 | ||
| kind: Kustomization | ||
|
|
||
| resources: | ||
| - configmap.yaml | ||
| - policy.yaml | ||
| - binding.yaml | ||
|
|
||
| labels: | ||
| - pairs: | ||
| app.kubernetes.io/name: nrrcontroller | ||
| app.kubernetes.io/component: admission-policy |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,71 @@ | ||||||
| --- | ||||||
| # MutatingAdmissionPolicy for automatic DaemonSet toleration injection | ||||||
| # Reads taint keys from a ConfigMap parameter resource | ||||||
| # Requires: MutatingAdmissionPolicy feature enabled in the cluster | ||||||
| apiVersion: admissionregistration.k8s.io/v1alpha1 | ||||||
pehlicd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| kind: MutatingAdmissionPolicy | ||||||
| metadata: | ||||||
| name: inject-daemonset-readiness-tolerations | ||||||
| spec: | ||||||
| failurePolicy: Fail | ||||||
|
|
||||||
| # Define what this policy watches | ||||||
| matchConstraints: | ||||||
| resourceRules: | ||||||
| - apiGroups: ["apps"] | ||||||
| apiVersions: ["v1"] | ||||||
| operations: ["CREATE", "UPDATE"] | ||||||
| resources: ["daemonsets"] | ||||||
|
|
||||||
| # Reference the ConfigMap that contains toleration data | ||||||
| paramKind: | ||||||
| apiVersion: v1 | ||||||
| kind: ConfigMap | ||||||
|
|
||||||
| # Variables for CEL expressions | ||||||
| variables: | ||||||
| # Check if opt-out annotation is set | ||||||
| - name: optedOut | ||||||
| expression: | | ||||||
| has(object.metadata.annotations) && | ||||||
| object.metadata.annotations.exists(k, k == "readiness.k8s.io/auto-tolerate" && object.metadata.annotations[k] == "false") | ||||||
| # Get existing tolerations (empty array if none) | ||||||
| - name: existingTolerations | ||||||
| expression: | | ||||||
| has(object.spec.template.spec.tolerations) ? | ||||||
| object.spec.template.spec.tolerations : [] | ||||||
| # Get taint keys from ConfigMap and parse to array | ||||||
| - name: taintKeys | ||||||
| expression: | | ||||||
| ("taint-keys" in params.data) && params.data["taint-keys"] != "" ? | ||||||
| params.data["taint-keys"].split(",") : [] | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
currently, if someone intentionally add a empty taint key to the configmap our MAP policy will create a matching empty-key wildcard toleration For example ❯ kubectl patch configmap readiness-taints -n nrr-system --type=merge -p '{"data":{"taint-keys":"readiness.k8s.io/NetworkReady,,readiness.k8s.io/StorageReady"}}'
configmap/readiness-taints patched
// notice the two commas which basically mean an empty taint key
❯ kubectl get configmap -n nrr-system readiness-taints -o jsonpath='{.data.taint-keys}'
readiness.k8s.io/NetworkReady,,readiness.k8s.io/StorageReady
// the above empty taint, injected an empty-key wildcard toleration on the ds
❯ kubectl get ds test-ds -o jsonpath='{.spec.template.spec.tolerations[*]}' | jq
{
"effect": "NoSchedule",
"key": "readiness.k8s.io/NetworkReady",
"operator": "Exists"
}
{
"effect": "NoSchedule",
"operator": "Exists"
}
{
"effect": "NoSchedule",
"key": "readiness.k8s.io/StorageReady",
"operator": "Exists"
} |
||||||
| # Create tolerations from taint keys (as plain maps since CEL has issues with complex types) | ||||||
| - name: tolerationsToInject | ||||||
| expression: | | ||||||
| variables.taintKeys | ||||||
| .filter(key, !variables.existingTolerations.exists(t, t.key == key)) | ||||||
| .map(key, { | ||||||
| "key": key, | ||||||
| "operator": "Exists", | ||||||
| "effect": "NoSchedule" | ||||||
| }) | ||||||
| # Apply mutations | ||||||
| mutations: | ||||||
| - patchType: JSONPatch | ||||||
| jsonPatch: | ||||||
| expression: | | ||||||
| !variables.optedOut && size(variables.tolerationsToInject) > 0 ? | ||||||
| [ | ||||||
| JSONPatch{ | ||||||
| op: has(object.spec.template.spec.tolerations) ? "replace" : "add", | ||||||
| path: "/spec/template/spec/tolerations", | ||||||
| value: variables.existingTolerations + variables.tolerationsToInject | ||||||
| } | ||||||
| ] : [] | ||||||
| # Never reinvoke this policy | ||||||
| reinvocationPolicy: Never | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # MutatingAdmissionPolicy for DaemonSet Toleration Injection | ||
|
|
||
| This document describes how to deploy and use the MutatingAdmissionPolicy-based approach for automatically injecting readiness tolerations into DaemonSets. | ||
|
|
||
| ## Overview | ||
|
|
||
| The MutatingAdmissionPolicy approach uses Kubernetes's native admission control mechanism with CEL (Common Expression Language) to inject tolerations **without running a webhook server**. This provides a simpler, more declarative alternative to the webhook-based approach. | ||
|
|
||
| ## Requirements | ||
|
|
||
| > [!IMPORTANT] | ||
| > MutatingAdmissionPolicy is needed to be enabled in the cluster. | ||
|
|
||
| - Feature gate: `MutatingAdmissionPolicy=true` | ||
| - Runtime config: `admissionregistration.k8s.io/v1alpha1=true` | ||
pehlicd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| - `kubectl` configured to access your cluster | ||
| - NodeReadinessRule CRDs installed | ||
|
|
||
| ## Architecture | ||
|
|
||
| ``` | ||
| User applies DaemonSet | ||
| ↓ | ||
| API Server evaluates CEL policy | ||
| ↓ | ||
| Fetches Tolerations ConfigMap which contains the tolerations to be injected | ||
| ↓ | ||
| Injects tolerations (if applicable) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesnt tell what is applicable. could we also add details on the annotation requirement? |
||
| ↓ | ||
| DaemonSet created with tolerations | ||
| ``` | ||
|
|
||
| ## Deployment | ||
|
|
||
| ### Option 1: Using kustomize | ||
|
|
||
| ```bash | ||
| # Install CRDs first | ||
| make install | ||
|
|
||
| # Deploy the admission policy | ||
| kubectl apply -k config/admission-policy | ||
| ``` | ||
|
|
||
| ### Option 2: Direct kubectl apply | ||
|
|
||
| ```bash | ||
| # Install CRDs first | ||
| make install | ||
|
|
||
| # Deploy policy and binding | ||
| kubectl apply -f config/admission-policy/policy.yaml | ||
| kubectl apply -f config/admission-policy/binding.yaml | ||
| ``` | ||
|
Comment on lines
+44
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm. this looks redundant. also, dont we need to show updating the config-map and how we maintain it with readiness-taints introduced later? |
||
Uh oh!
There was an error while loading. Please reload this page.