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

Install chart from registry #6982

Closed
JulienBreux opened this issue Nov 15, 2019 · 27 comments · Fixed by #9782
Closed

Install chart from registry #6982

JulienBreux opened this issue Nov 15, 2019 · 27 comments · Fixed by #9782
Labels
feature v3.x Issues and Pull Requests related to the major version v3

Comments

@JulienBreux
Copy link

JulienBreux commented Nov 15, 2019

Hi dream team community!

This is a little feature request: Install chart from registry

Before

helm registry login
helm chart pull ...
helm chart export ... ./charts
helm install ... /charts/my-chart

After

helm install gcr.io/my-project/my-chart@sha256:xxx --from-registry

— or —

helm chart install my-chart@sha256:xxx

Now

I need your comments. And I want to know if you want help to create this?

/label feature

@bacongobbler
Copy link
Member

Hi @JulienBreux!

Yes, we would be happy to accept a PR to support this. Internally, charts are fetched from APIs (such as the chart repository API) using a getter interface. We don't have one available for the OCI spec.

Let us know if you'd like to implement this functionality.

See also #6593

@bacongobbler bacongobbler added feature v3.x Issues and Pull Requests related to the major version v3 labels Nov 15, 2019
@JulienBreux
Copy link
Author

JulienBreux commented Nov 15, 2019

Thanks @bacongobbler for your reply.

Let's go!

But a question, you prefer helm install with this supported interface or helm chart install?
Because the commands in helm chart and helm registry are marked as "experimental"?

@bacongobbler
Copy link
Member

bacongobbler commented Nov 15, 2019

My first impression would be to implement as helm chart install for the time being until it's considered a stable feature. Let's circle back once a PoC has been implemented.

Before we get too ahead of ourselves, I'd look at implementing an internal getter that supports the OCI distribution specification. That way the groundwork can be laid out first. Have a look at internal/experimental/registry for inspiration. The work should probably live under internal/experimental/registry/getter.

@jdolitsky
Copy link
Contributor

This is much needed. What's unclear is how we shall determine if the ref is to be treated as a 1.) local dir 2.) remote repo-based chart 3.) locally cached/remote registry-based chart

We could require a prefix such as oci:// but that seems like too much

Alternatively, we could require that a tag be provided. Then we can check for the : (colon) character. This is probably my preference, since registries dont yet support some search mechanism that would enable us to auto-determine the latest semver tag anyway (must say mysite.io/mychart:1.2.3 vs. just mysite.io/mychart.

The --from-registry as you suggested @JulienBreux would certainly work, but I think we should be able to do without.

@scones
Copy link

scones commented Dec 16, 2019

We could require a prefix such as oci:// but that seems like too much

i'd suggest the git method: oci@some.regist.ry/some-chart (implicit latest)

for the actual complete install command, i'd strongly suggest combining it with the normal install (if only to avoid possibile logic duplication):

helm upgrade \
  some-name \
  oci@some.regist.ry/some-chart \
  --username my-user \
  --password my-password \
  --install \
  --set some-key=some-value \
  --namespace some-namespace \
  --wait

this would also shrink down adjustment efforts for anyone using the repo way. the HELM_EXPERIMENTAL_OCI check would have to be on that specific chart source option then.

unfortunately i don't know go :(

edit: mixed upgrade/install up

@maxsxu
Copy link

maxsxu commented Jun 28, 2020

This feature is very needed!
I thought this has already been implemented, I'm using helm v3.2.4 now, but it seems not.

And I noticed that you've already created a PR before but not been merged. So, what I've missed? @JulienBreux

@bacongobbler
Copy link
Member

See #6593.

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Nov 20, 2020
@pre
Copy link

pre commented Nov 20, 2020

/remove stale

@jdolitsky jdolitsky removed the Stale label Nov 20, 2020
@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Feb 20, 2021
@will-beta
Copy link

/remove stale

@bacongobbler
Copy link
Member

I believe this has been implemented in #8843. Can you please follow the documentation and check back with your results?

@github-actions github-actions bot removed the Stale label Feb 21, 2021
@anderius
Copy link

anderius commented Feb 21, 2021

@bacongobbler I can't find the documentation, but I tried the following using helm 3.5.2:

helm upgrade --install --atomic name oci://xxx.azurecr.io/helm/chart-name:1.2.3
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x1609ec3]

goroutine 1 [running]:
helm.sh/helm/v3/internal/experimental/registry.(*Client).PullChart(0x0, 0xc0008f03e0, 0xc0008f03e0, 0x0, 0x0)
	/home/circleci/helm.sh/helm/internal/experimental/registry/client.go:156 +0x183
helm.sh/helm/v3/pkg/getter.(*OCIGetter).get(0xc00088bea0, 0xc0008a3940, 0x32, 0xc00056b7b0, 0xc0008a3940, 0x32)
	/home/circleci/helm.sh/helm/pkg/getter/ocigetter.go:52 +0xc5
helm.sh/helm/v3/pkg/getter.(*OCIGetter).Get(0xc00088bea0, 0xc0008a3940, 0x32, 0xc00090b9e0, 0x4, 0x6, 0xc00088bea0, 0x0, 0x0)
	/home/circleci/helm.sh/helm/pkg/getter/ocigetter.go:36 +0x77

Is this the correct way to use install/upgrade with the oci repositories?

@bacongobbler
Copy link
Member

Try omitting the tag in the repository name. If you need a specific version, try specifying it with the --version flag.

@irons
Copy link

irons commented Feb 24, 2021

I'm seeing the same error, using helm 3.5.2 with Azure container registry. When I use the --version flag:

$ helm install oci://ouracr.azurecr.io/helm/chart-name --version 0.1.140 --dry-run --generate-name
Error: failed to download "oci://ouracr.azurecr.io/helm/chart-name" at version "0.1.140" (hint: running `helm repo update` may help)

helm pull succeeds with the same arguments:

$ helm pull oci://ouracr.azurecr.io/helm/chart-name --version 0.1.140 
0.1.140: Pulling from ouracr.azurecr.io/helm/chart-name

@will-beta
Copy link

Try omitting the tag in the repository name. If you need a specific version, try specifying it with the --version flag.

Hi, could you elaborate the differences between these two values?

@the-scott-hand
Copy link

same problem for me in helm v3.5.3. doing the pull works:
helm pull oci://my-registry/my-chart --version 1.0.0
but doing the install does not
helm install test -n test oci://my-registry/my-chart --version 1.0.0
gives the "failed to download" message despite identical paths in the two commands.

using the oci://my-registry/my-chart:1.0.0 syntax on either gives the nvalid memory address or nil pointer dereference message as mentioned.

for now i will just pull and then install from local as a two step process but would love to know if/when this has been fixed

@bacongobbler
Copy link
Member

bacongobbler commented Mar 31, 2021

This is because helm pull was implemented, but not helm install. #8843 only implemented the helm pull and helm dependency update side of things.

That would also explain the reason why @anderius was running into issues with helm upgrade --install (regardless if the arguments provided were invalid).

@the-scott-hand
Copy link

Makes sense thanks! Does this also extend to a helm install where Chart.yaml dependencies are oci://whatever ?
I'm finding that when I run helm dep up my charts get pulled into the chart dir, but when i helm install my chart locally it claims only my oci deps are missing from the charts dir(despite having just run helm dep up successfully and the files being present)

@bacongobbler
Copy link
Member

bacongobbler commented Mar 31, 2021

Does this also extend to a helm install where Chart.yaml dependencies are oci://whatever ?

No. As mentioned before, helm dependency update support was provided in #8843. It's even called out in the original PR subject.

If you're having issues with OCI dependencies and helm dependency update, please open a separate ticket. Thanks.

@will-beta
Copy link

same problem for me in helm v3.5.3. doing the pull works:
helm pull oci://my-registry/my-chart --version 1.0.0
but doing the install does not
helm install test -n test oci://my-registry/my-chart --version 1.0.0
gives the "failed to download" message despite identical paths in the two commands.

using the oci://my-registry/my-chart:1.0.0 syntax on either gives the nvalid memory address or nil pointer dereference message as mentioned.

for now i will just pull and then install from local as a two step process but would love to know if/when this has been fixed

For me, doing the pull while using the tag instead of the version works.
Wondering the differences between these two. What if they differs?
@bacongobbler could you help elaborate this? thanks in advance.

@emmaroberts-nbs
Copy link

Any progress on implementing helm install from an oci registry?

@koalalorenzo
Copy link

Any update? :) Would be nice to have this and maybe also support in the terraform helm_release resource :D

@bacongobbler
Copy link
Member

See #9782.

@koalalorenzo
Copy link

@JulienBreux why was this closed? 🤔

@JulienBreux
Copy link
Author

JulienBreux commented Jul 26, 2021

@JulienBreux why was this closed? 🤔

Because a PR was opened to resolve this issue and implement this feature.

Ref. #9782

@lawliet89
Copy link

@JulienBalestra shouldn't the issue be closed when the PR is reviewed and merged in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature v3.x Issues and Pull Requests related to the major version v3
Projects
None yet
Development

Successfully merging a pull request may close this issue.