-
Notifications
You must be signed in to change notification settings - Fork 368
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
Panic when trying to download Helm chart from OCI registry without a version #5156
Comments
Looking at the stack trace, looks like Helm is choking on trying to figure out the chart version. Could you try with adding a |
Thanks for doing the research. |
That I'm not 100% sure yet. 😄 k0s uses Helm as a library so there has been some discrepancies between it's CLI usage and how it behaves as a library in the past. What I'm currently leaning towards is that when you use Helm via CLI and do NOT specify However, I must say that using |
As Jussi mentioned, the panic happens because of the missing version, but that's only part of it. In fact, I suspect the panic would happen for any version that isn't a valid semver. IIUC the course of events is as follows:
This only happens if the charts are downloaded via OCI, and only if the chart version is not a valid semver. So this is definitely a bug on k0s' side. The question is how to fix it:
Either way, a panic is obviously not a good outcome. |
Heh, I based my assumptions on the Helm CLI
So in this case the |
Looks like this is the code where helm determines the "latest" version. https://github.com/helm/helm/blob/2848e3db51b19a171c2459ea6253d2be92f090e2/pkg/downloader/chart_downloader.go#L155-L171 Hope this helps. |
I've created a PR in helm to safeguard from panics As for k0s, when downloading from an OCI registry, you need to pass in a client for this just like https://github.com/helm/helm/blob/dd35343988c8c2bea5ebd6d59db01bda1f8ed1c2/pkg/action/install.go#L764-L781 |
As it stands right now, I think k0s can only support fixed versions, not rolling tags. As Jussi said, this would introduce hard to predict behavior. There's for example no mechanism in k0s which would refresh a chart that has been pulled once, let alone a mechanism to do planned chart upgrades based on rolling tags. |
Would this mean that validation would be needed to error if a non-semver version string is provided? |
Something along those lines, yes. Not saying that rolling tags are impossible to support, but this needs some design, and probably a compelling use case, too. |
Before creating an issue, make sure you've checked the following:
Platform
Linux 5.14.0-362.24.2.el9_3.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Mar 30 14:11:54 EDT 2024 x86_64 GNU/Linux
NAME="AlmaLinux"
VERSION="9.3 (Shamrock Pampas Cat)"
ID="almalinux"
ID_LIKE="rhel centos fedora"
VERSION_ID="9.3"
PLATFORM_ID="platform:el9"
PRETTY_NAME="AlmaLinux 9.3 (Shamrock Pampas Cat)"
ANSI_COLOR="0;34"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:almalinux:almalinux:9::baseos"
HOME_URL="https://almalinux.org/"
DOCUMENTATION_URL="https://wiki.almalinux.org/"
BUG_REPORT_URL="https://bugs.almalinux.org/"
ALMALINUX_MANTISBT_PROJECT="AlmaLinux-9"
ALMALINUX_MANTISBT_PROJECT_VERSION="9.3"
REDHAT_SUPPORT_PRODUCT="AlmaLinux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.3"
Version
v1.31.1+k0s.1
Sysinfo
`k0s sysinfo`
What happened?
I tried to install OCI using the Helm extension and got an error.
Steps to reproduce
Expected behavior
No response
Actual behavior
No response
Screenshots and logs
Additional context
No response
The text was updated successfully, but these errors were encountered: