Skip to content

Refactor download (file) #12299

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

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

VannTen
Copy link
Contributor

@VannTen VannTen commented Jun 12, 2025

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.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
I'm working on this still, so expect heavy rebasing and force push. I think the design of the role will stay like this though.

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

VannTen added 7 commits June 7, 2025 16:28
This is the same logic as system_packages: we simply let jinja expand
our variables to a list of boolean and filter on this.
Delegate to everything to download_delegate.
group hosts by "delegate" then loops on the delegate with extract on
hostvars.

This shortcircuit the loop earlier than using selectattr on the whole
hostvars on every host.
Small efficiency gains when downloading, around 2-3s worse when
everything is already there.
@k8s-ci-robot
Copy link
Contributor

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 k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 12, 2025
@k8s-ci-robot
Copy link
Contributor

[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

Needs approval from an approver in each of these files:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 12, 2025
@k8s-ci-robot k8s-ci-robot requested review from ErikJiang and mzaian June 12, 2025 14:42
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 12, 2025
@VannTen
Copy link
Contributor Author

VannTen commented Jun 12, 2025 via email

VannTen added 2 commits June 15, 2025 16:10
Some stuff used on localhost (skopeo for download images, for instances)
may need the machine architecture and similar things, so collect facts
for localhost as well
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2025
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants