Refactor(defaults): centralize remove node defaults

[Srishti-j18]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Moves inline defaults out of tasks and into role defaults for remove_node. This reduces repeated | default(...) usage and aligns with #11822.

Which issue(s) this PR fixes:

Special notes for your reviewer:
No functional behavior change intended.

Does this PR introduce a user-facing change?:

NONE

[k8s-ci-robot]

Hi @Srishti-j18. 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.

[Srishti-j18]

This is already defined here:

kubespray/roles/kubespray_defaults/defaults/main/main.yml

Line 297 in bf3fef0

kube_override_hostname: "{{ inventory_hostname }}"

[Srishti-j18]

/kind cleanup

[VannTen]

Is that all of the unnecessary default usage left ? Otherwise let's not use the Fixes line to keep the issue open.
For network_facts, this should be handled in #12981 and the loadbalancer_apiserver should be fixed in #12897 . So let's drop those change for now

[Srishti-j18]

No, this PR only addresses a small slice. Sorry, I’ve removed the Fixes #11822 line, so the issue stays open.
I’ll also drop the network_facts changes (since those are already tracked).

I’ll try to help with the remaining defaults in this PR only. Could you please take a look at this comment?
#11822 (comment)

[VannTen]

/ok-to-test

[VannTen]

You just need to squash your commits and then we're good.

[Srishti-j18]

Yeah, I know. I'm migrating roles/download in this PR. Give me some time.

[VannTen]

I'm migrating roles/download in this PR

The download role is gonna substantially change in #12299 and #12937 ( as-in, full rewrite, in fact) ; I'd prefer not to touch it too much until this lands (which should in the coming month, there is not much left to do) since rebases can already take some time with large changes like this.

[Srishti-j18]

🥲
Could you please tell me what dir I can touch? Or should I only solve the remove nodes ?

[Srishti-j18]

Okay, I have removed all the other changes and only updated it for the remove node.

[VannTen]

It's generally easier to do such changes piece meal instead of one big PR, precisely for things like this. I don't have an exact list, but I think except download there isn't a big rewrite right now.

[VannTen]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Srishti-j18, VannTen

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [VannTen]

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

[Srishti-j18]

@VannTen
Also, I think roles/network_plugin/calico can be covered in this PR: #12443.

[Srishti-j18]

/retest-failed

[Srishti-j18]

/retest

[VannTen]

/retest-failed

[Srishti-j18]

I just rebased onto the master branch.

[VannTen]

Yeah that's normal the bot removes lgtm on any change. /lgtm