Refactor download (file)

[VannTen]

What type of PR is this?
/kind bug
/kind feature

What this PR does / why we need it:
This should fix multi-arch cluster download, and be much more scalable. (rough testing of the file part only:
-> around 1 minutes for 1000 fake hosts with download_delegate == localhost)
This is only the file part, I plan to handle containers images in a separate PR.

This is a full rewrite of the file part of the download role.
This massively improves the runtime of the playbooks (our CI jobs appears to go from 9 to 6 minutes), and should also fix or at least improve a lot the situation with multi-architecture clusters (but that will need the follow-up handling container for that usecase), because the downloads are now calculated per host, before deduplication to the host handling the download if there is delegation.

The improvement mostly comes from dropping the include_tasks: download_file.yml with a loop on download, dropping unnecessary tasks and pre-processing the loop (so, there should not be any skipped tasks, because they are removed of the list before being passed to loop).

The uniqueness of binaries rely on the filename, so using the correct variable inside the downloads variable (architecture / os_version / etc) should suffice.

This also removes all of the include used in various roles to reuse part of the download role. Now only the downloads role is responsible for getting artefacts, which should help with skipping with --tags and similar.

Which issue(s) this PR fixes:
Fixes #12403
Fixes #13086

Special notes for your reviewer:
There will be a follow-up to handle the container part as well, with the same ideas

After this is merged, and I'll also handle the container download part (which should be easier as a good chunk of the logic around delegation will be reused), I plan to default download_delegate to localhost.
This will be a breaking change, moreso than the existing one in the release-note, because it will change behavior, not just interface.

I wanted to keep it separate for that reason.

Much of the complexity of the new role is for handling download_delegate != inventory_hostname || 'localhost' which is the current default (delegate to first control plane).

NOTE: The release-note also take into account #12937 which will need to be merge for fully removing the listed variables.

I tried to keep this not too big, but unfortunately it touched on a lot of assumptions / weird stuff in Kubespray. If you see something which could be split out do not hesitate to point it out, I'll try to extract it in a new PR (or if you have the energy feel free to do it directly ^)

Does this PR introduce a user-facing change?:

action required
`download_run_once`, `download_localhost`, `download_force_cache` are removed.
Only `download_delegate` is used (this can be a different values for each host, though this is not recommended.
Binaries are only put on hosts at their actual place or if inventory_hostname == download_delegate.
Using limits now require to include `localhost` in the limit if download/file is not skipped (by tags).

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

[VannTen]

Also, I think the approach could make manage-offline.sh redundant (aka, just run the download tags on a non offline control node, copy the local_release_dir, done) I'm not super into that so might not be enough though

[tico88612]

These failing maybe are related with docker in conformance test.

[VannTen]

The trigger was support for K8s 1.36, it fails only after that (looking at gitlab pipelines history) Unless there are objections I'm gonna send a PR to put this in manual tests, and I'd like to try to merge both download PR before September or sooner. AFAIK there is only the semaphore issue (Ansible bug with async on semaphoreui) which has a workaround. The PR also fixes a number of bugs and we're kinda blocking other fix PR (incompatible ones) waiting for it. And if I could stop rebasing and handling conflict that'd be nice :) (I need to test the giraffe merge thing...)

[Payback159]

I test-merged this against #13397 and ran a full install + upgrade on 6 VMs. Two things I hit
while doing it, both about the control node becoming a provisioning target.

1. vagrant up hangs forever

playbooks/internal_facts.yml:12 gains localhost:

-  hosts: k8s_cluster:etcd:calico_rr
+  hosts: k8s_cluster:etcd:calico_rr:localhost

The shipped Vagrantfile runs Ansible with a global become (ansible.become = true, line 329)
and -e ansible_become_pass=vagrant (line 334). So the setup tasks now run on the control
node under sudo, with the VM's password. On a workstation whose sudo needs a password this
blocks indefinitely — no error, no timeout. I sat at

TASK [network_facts : Gather node IPs]

for 14 minutes twice before spotting

sudo -H -S -p "[sudo via ansible, key=…] password:" -u root /bin/sh -c
  '… /home/<me>/.ansible/tmp/ansible-tmp-…/AnsiballZ_setup.py'

(the controller's ~/.ansible/tmp and venv interpreter identify it as localhost, not a VM).

Setting ansible_become: false for localhost fixes it and the task drops to 0.94s.
Gathering facts about the control node doesn't need root, so become: false on that play
might be the cleaner fix.

CI wouldn't catch this — a control node running as root or with passwordless sudo just works.

2. tools_setup writes into /usr/local/bin on the control node

With become off, the next thing is:

TASK [tools_setup : Install tools | kubeadm]
fatal: [localhost]: FAILED! => {"msg": "Destination /usr/local/bin not writable"}

Which is by design — the README says the role sets up tools "on both the Ansible control node
and the managed node", and roles/download/container then calls {{ bin_dir }}/skopeo on
localhost. But bin_dir defaults to /usr/local/bin, so this is a new requirement worth
calling out: kubespray now needs root on the control node, and it will overwrite an existing
/usr/local/bin/kubeadm there. Neither is mentioned in the docs.

A separate control-node bin_dir (or documenting that it must be set) would avoid both.

Both were easy to work around locally (ansible_become: false + a writable bin_dir in
host_vars/localhost.yml), after which install and upgrade completed with failed=0.

[VannTen]

Thanks for the testing, I'll address those and the one in the other PR, time permitting.

…

-- Max Gautier

[tico88612]

By the way, how should I test this download refactor PR? It looks like some parts still need to be adjusted.

[kubernetes-prow]

PR needs rebase.

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.