Decouple kubespray-defaults from download

[VannTen]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

This moves all defaults variable contained into the download role into kubespray-defaults, thus avoiding the need for the
kubespray-defaults role to add a dependency on dowload only to load variable (and doing a lot of nothing as described in the linked issue).

I tested this on a 3-master 3-etcd 9-nodes cluster, where it takes around 5 minutes less (this is not the exact same patch, because my cluster is in 1.25)
This was running upgrade-cluster.yml, with no real changes (the cluster was already at the target version).

With the patch

vendredi 17 novembre 2023  10:18:04 +0100 (0:00:00.405)       0:52:34.727 ***** 
=============================================================================== 
bootstrap-os : Install libselinux python package ----------------------- 53.40s
etcd : Gen_certs | Write etcd member/admin and kube_control_plane clinet certs to other etcd nodes -- 20.02s
bootstrap-os : Enable RHEL 8 repos ------------------------------------- 15.10s
kubernetes-apps/ansible : Kubernetes Apps | Start Resources ------------ 10.79s
kubernetes-apps/ansible : Kubernetes Apps | Lay Down CoreDNS templates -- 10.64s
kubernetes/control-plane : kubeadm | Upgrade other masters ------------- 10.61s
kubernetes/control-plane : kubeadm | Upgrade other masters ------------- 10.56s
kubernetes/control-plane : kubeadm | Upgrade first master -------------- 10.50s
kubernetes/preinstall : Install packages requirements ------------------- 8.51s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 7.72s
kubernetes/preinstall : Ensure kube-bench parameters are set ------------ 7.52s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 7.39s
kubernetes/preinstall : Create kubernetes directories ------------------- 7.19s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 6.75s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 6.65s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 6.50s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 6.48s
network_plugin/calico : Calico | Create calico manifests ---------------- 6.40s
network_plugin/calico : Calico | Create calico manifests ---------------- 6.24s
bootstrap-os : Check RHEL subscription-manager status ------------------- 5.78s

Without the patch

vendredi 17 novembre 2023  11:17:27 +0100 (0:00:00.354)       0:57:04.881 ***** 
=============================================================================== 
etcd : Gen_certs | Write etcd member/admin and kube_control_plane clinet certs to other etcd nodes -- 20.47s
bootstrap-os : Enable RHEL 8 repos ------------------------------------- 14.92s
bootstrap-os : Install libselinux python package ----------------------- 13.18s
kubernetes-apps/ansible : Kubernetes Apps | Start Resources ------------ 11.35s
kubernetes/control-plane : kubeadm | Upgrade first master -------------- 10.58s
kubernetes/control-plane : kubeadm | Upgrade other masters ------------- 10.55s
kubernetes/control-plane : kubeadm | Upgrade other masters ------------- 10.54s
kubernetes-apps/ansible : Kubernetes Apps | Lay Down CoreDNS templates -- 10.47s
kubernetes/preinstall : Install packages requirements ------------------- 8.18s
kubernetes/preinstall : Ensure kube-bench parameters are set ------------ 7.71s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 7.25s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 7.06s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 6.85s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 6.60s
network_plugin/calico : Calico | Create calico manifests ---------------- 6.56s
container-engine/runc : runc | Uninstall runc package managed by package manager --- 6.53s
kubernetes/preinstall : Create kubernetes directories ------------------- 6.42s
container-engine/containerd : containerd | Remove any package manager controlled containerd package --- 6.35s
network_plugin/calico : Calico | Create calico manifests ---------------- 6.34s
etcd : Gen_certs | Gather etcd member/admin and kube_control_plane clinet certs from first etcd node --- 5.70s

Which issue(s) this PR fixes:

Fixes #9279

Special notes for your reviewer:

There is a lot of potential further cleanup, but I tried to keep this simple.
In particular:

• moving back vars which are only used in the download role itself into its defaults
• moving role-specific vars defined in download.yml into their respective roles (and refactor download to make that work).

Does this PR introduce a user-facing change?:

Decouple kubespray-defaults from download

[k8s-ci-robot]

Hi @VannTen. 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.

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/test-infra repository.

[yankay]

/ok-to-test

[MrFreezeex]

Wow, thank you for the improvement! Indeed there are most likely a good amount of performance improvement we could do in kubespray.

/lgtm

[VannTen]

Rebased to re-trigger the CI, sorry for the noise @MrFreezeex

[MrFreezeex]

No worries!
/lgtm

[VannTen]

I guess git is better at resolving conflicts than tide, because I had no merge conflicts while rebasing ^^.

[VannTen]

@floryut @yankay wdyt ? To me it seems like a pretty low-hanging fruit for speeding up the playbooks

[MrFreezeex]

(adding back my lgtm after the rebase)
/lgtm

[VannTen]

/assign @floryut

[VannTen]

/assign @liupeng0518
I've seen your reactions, care to approve on this ?
@MrFreezeex If you don't mind, would need another lgtm.

This ones is pretty "rebase annoying" even though git can solve the presumed conflicts with no problem...
(Hopefully the CI holds 🤞 )

[liupeng0518]

/approve

[MrFreezeex]

Hmmm I think the CI failure might be legit 🤔

EDIT: Just saw the linked issue about this failure 👍

[VannTen]

@MrFreezeex looks like CI will finally pass on this, can you review ? (Pretty please 🙏 )

[MrFreezeex]

Sure, thanks again!
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liupeng0518, MrFreezeex, VannTen

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [liupeng0518]

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