Fix/eliminate default filter 11822 by AdeshDeshmukh · Pull Request #13136 · kubernetes-sigs/kubespray

AdeshDeshmukh · 2026-03-31T13:17:50Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

This PR refactors how default values are handled across several roles in kubespray.

Currently, multiple task files use the | d(default_value) filter inline, resulting in the same defaults being repeated throughout the codebase. This PR moves those defaults to their respective defaults/main.yml files, making defaults easier to maintain and override from inventory.

Specifically:

Defines node_labels: {} in node-label role defaults instead of using | d({}) in the task
Defines kubeadm_patch_default_type: strategic in kubeadm_common role defaults instead of hardcoding | d('strategic') in the task
Normalizes | d(None) to | default(None) in download.yml for consistency with Ansible best practices

This improves code readability and ensures each default is defined in a single location.

Which issue(s) this PR fixes:

Fixes #11822

Special notes for your reviewer:

This is an incremental change as suggested in the issue. Some uses of | d() remain where they handle Ansible built-in variable fallbacks (e.g., ansible_host vs ansible_ssh_host in bastion-ssh-config) or conditional stat checks.

Does this PR introduce a user-facing change?:

NONE

This PR adds documentation explaining best practices for managing Kubespray custom configurations in a separate git repository with minimal files. Resolves issue 10645

Define defaults in roles/defaults/main.yml instead of using the | d() filter throughout task files. - Add node_labels default to roles/kubernetes/node-label/defaults/main.yml - Add kubeadm_patch_default_type to roles/kubernetes/kubeadm_common/defaults/main.yml - Update node-label task to use node_labels directly without filter - Update kubeadm_common task to use kubeadm_patch_default_type variable - Change | d(None) to | default(None) in download.yml for consistency Closes kubernetes-sigs#11822 Signed-off-by: AdeshDeshmukh <adeshkd123@gmail.com>

k8s-ci-robot · 2026-03-31T13:17:54Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

linux-foundation-easycla · 2026-03-31T13:17:57Z

The committers listed above are authorized under a signed CLA.

✅ login: AdeshDeshmukh / name: AdeshDeshmukh (13882b7, 659f258, b039269)

k8s-ci-robot · 2026-03-31T13:18:01Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AdeshDeshmukh
Once this PR has been reviewed and has the lgtm label, please assign mzaian for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2026-03-31T13:18:02Z

Hi @AdeshDeshmukh. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

VannTen

Some comments.

The docs updates looks unrelated ? They seem to stand on their own, so maybe split them off in another PR ?

VannTen · 2026-04-02T08:56:52Z

 kubeadm_patches: []
+# Default patch type for kubeadm patches when not explicitly specified
+kubeadm_patch_default_type: strategic
+


This one can stays as a inline default.
The patches themselves are user-definable, but adding an additional knob would only save one line par patch and add some unpredictability ; better to keep the hardcoded default patch type for this.

VannTen · 2026-04-02T08:58:15Z

      {{ etcd_binary_checksum if (etcd_deployment_type == 'host')
-      else etcd_digest_checksum | d(None) }}
+      else etcd_digest_checksum | default(None) }}
    url: "{{ etcd_download_url }}"


While default is more explicit than d, please revert this one, the history churn isn't worth it.

Copilot

Pull request overview

This PR aims to reduce repeated inline default/d() filters in Kubespray roles by moving defaults into role defaults/ files, and it also adds documentation for a “minimal inventory” workflow.

Changes:

Introduces a node_labels default in the node-label role and simplifies the task loop accordingly.
Updates various download checksum templating expressions in kubespray_defaults.
Adds a new “Minimal Inventory Guide” doc and links it from Getting Started + sidebar.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
roles/kubespray_defaults/defaults/main/download.yml	Changes checksum templating for many container downloads (None fallback handling).
roles/kubernetes/node-label/tasks/main.yml	Removes inline `{}` default and `is defined` check for `node_labels`.
roles/kubernetes/node-label/defaults/main.yml	Adds `node_labels: {}` default and documentation comment/examples.
roles/kubernetes/kubeadm_common/defaults/main.yml	Touches defaults file (but does not currently add the described new default).
docs/getting_started/minimal-inventory.md	Adds a new guide describing a minimal inventory + submodule workflow.
docs/getting_started/getting-started.md	Adds a link to the Minimal Inventory guide.
docs/_sidebar.md	Adds Minimal Inventory page to the docs sidebar.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T13:41:11Z

    repo: "{{ cilium_image_repo }}"
    tag: "{{ cilium_image_tag }}"
-    checksum: "{{ cilium_digest_checksum | default(None) }}"
+    checksum: "{{ cilium_digest_checksum | d(None) }}"


The PR description says this change should normalize | d(None) to | default(None) for Ansible best practices, but this file currently uses the d(None) alias. If the goal is consistency/readability, consider switching back to default(None) (or update the PR description if d() is intentional).

Copilot · 2026-04-03T13:41:11Z

+# Node labels to be applied to nodes
+# Format: dictionary of key=value pairs


This defaults file is missing the YAML document start marker (---). With the repo’s .yamllint config extending the default rules, document-start is enforced and this file will fail linting unless it begins with ---.

Copilot · 2026-04-03T13:41:11Z

 ---
 kubeadm_patches_dir: "{{ kube_config_dir }}/patches"
 kubeadm_patches: []
+


The PR description mentions introducing a kubeadm_patch_default_type: strategic default to avoid hardcoding | d('strategic') in tasks, but this variable isn’t defined here. As a result, the tasks still rely on an inline default filter. Add the variable (and then update the task to use it) to complete the refactor.

Copilot · 2026-04-03T13:41:12Z

+
+```bash
+cd kubespray
+ansible-playbook -i ../inventory/mycluster cluster.yml -b -v


The multi-line ansible-playbook example splits the command across two lines without a line-continuation (\). Copy/pasting this will execute --private-key=... as a separate command and fail; keep it on one line or add \ at the end of the first line.

Suggested change

ansible-playbook -i ../inventory/mycluster cluster.yml -b -v

ansible-playbook -i ../inventory/mycluster cluster.yml -b -v \

AdeshDeshmukh added 2 commits March 31, 2026 18:23

docs: Add minimal inventory configuration guide

b039269

This PR adds documentation explaining best practices for managing Kubespray custom configurations in a separate git repository with minimal files. Resolves issue 10645

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 31, 2026

k8s-ci-robot requested review from mzaian and yankay March 31, 2026 13:17

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 31, 2026

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 31, 2026

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 31, 2026

VannTen reviewed Apr 2, 2026

View reviewed changes

refactor: address review feedback, keep only node-label fix

13882b7

Copilot AI review requested due to automatic review settings April 3, 2026 13:37

Copilot started reviewing on behalf of AdeshDeshmukh April 3, 2026 13:37 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/eliminate default filter 11822#13136

Fix/eliminate default filter 11822#13136
AdeshDeshmukh wants to merge 3 commits intokubernetes-sigs:masterfrom
AdeshDeshmukh:fix/eliminate-default-filter-11822

AdeshDeshmukh commented Mar 31, 2026

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

linux-foundation-easycla bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

VannTen left a comment

Uh oh!

VannTen Apr 2, 2026

Uh oh!

VannTen Apr 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		# Node labels to be applied to nodes
		# Format: dictionary of key=value pairs

	ansible-playbook -i ../inventory/mycluster cluster.yml -b -v
	ansible-playbook -i ../inventory/mycluster cluster.yml -b -v \

Conversation

AdeshDeshmukh commented Mar 31, 2026

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

linux-foundation-easycla bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

k8s-ci-robot commented Mar 31, 2026

Uh oh!

VannTen left a comment

Choose a reason for hiding this comment

Uh oh!

VannTen Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

VannTen Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

linux-foundation-easycla bot commented Mar 31, 2026 •

edited

Loading