Skip to content
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

feat: Use default AllowDownload = true config #42

Closed
wants to merge 1 commit into from

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented Jan 23, 2025

This PR removes the custom configuration of AllowDownload = false to have the default of true.
This means the image will keep shipping the configured kubectl versions in the cache, but will also work if targetting different Kubernetes versions.

@viccuad viccuad requested a review from a team as a code owner January 23, 2025 10:39
@viccuad
Copy link
Member Author

viccuad commented Jan 23, 2025

Hi, Kubewarden maintainer here. We are currently consuming this image as-is.
Yet given that our users can install Kubewarden in different Rancher versions or SUSE products with old yet supported Kubernetes versions, and given that we are a CNCF project that must work on other Kubernetes versions, the AllowDownload = false falls flat for us.

We would still like to benefit from using the rancher/kuberlr-kubectl to deduplicate images in a full Rancher installation, hence this PR.

If this change is out of scope, Kubewarden's best bet may be either to fork the image and provide a layer on top with AllowDownload = true configuration, or to maintain our own image as we were doing.

Please feel free to reach out of band or of course close this PR if this is out of scope for rancher/kuberlr-kubectl, felt easier to open a PR than an issue.

@mallardduck
Copy link
Member

Hey @viccuad - I'll need to do some testing around this change. That effort may take some time since we haven't fully sorted out automated testing for this project yet. The main reason we opted to set AllowDownload to false was to respect/enforce our expectation that each major version has a specific version of rancher (with specific k8s versions) that we satisfy in the image OOTB. The main motivator for this requirement is an easier mental model with the expected results for all Rancher clusters (both inside air-gaps and with access to internet).

That said, maybe we can have our cake and eat it too. As long as this doesn't need to change in the config that we prefer for Rancher, but can change at container deploy time - then I think that's how. Because the underlying kuberlr tool uses viper, I wonder if you can override the config value? From the viper docs it seems loading env is higher priority than config file values.

If not maybe that's an improvement we can provide at the upstream kuberlr project first, then use that improvement to solve this here. That seems like a strong possible solution to make this image more usable for kubewarden I think?

Even still, our images will have kubectl for k8s that match specific versions of rancher, so if that's undesirable I wonder if directly using kuberlr images is an option: https://github.com/flavio/kuberlr/pkgs/container/kuberlr Otherwise I think improving that at Kubrlr repo would be our best bet.

@viccuad
Copy link
Member Author

viccuad commented Jan 24, 2025

Because the underlying kuberlr tool uses viper, I wonder if you can override the config value?

This is a good approach, totally forgot about env vars. I think this should be the way to go. Opened flavio/kuberlr#153 in kuberlr upstream, closing this PR.

@viccuad viccuad closed this Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants