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

Change default repositories for Helm v2 #8901

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

technosophos
Copy link
Member

Per the discussion in the Helm call, this PR is part of the final set of changes to Helm 2. It does the following:

  • Init now initializes stable to charts.helm.sh/stable
  • There is a flag to disable this behavior and use the old repo
  • Every command checks to see if stable or incubator is pointed at the Google buckets. If so, Helm issues a warning.

Obviously this is a breaking change. All of the Helm core maintainers, chart maintainers, and org maintainers have been notified of this, and we agree that it is necessary because Google is going to decommission the existing charts repo.

Note that I have NOT automatically converted existing repository URLs to the new one. I will do that in a separate PR because the core maintainers did not reach a consensus as to whether or not that was appropriate.

NOTE: Race detection on unit tests has been disabled because there is a known issue with current versions of Go and macOS Catalina.

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
@technosophos technosophos added this to the 2.17.0 milestone Oct 15, 2020
@technosophos technosophos self-assigned this Oct 15, 2020
@helm-bot helm-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 15, 2020
@mattfarina mattfarina added the v2.x Issues and Pull Requests related to the major version v2 label Oct 15, 2020
@mattfarina
Copy link
Collaborator

@technosophos the error is the one that always catches me... gotta run make docs.

@technosophos
Copy link
Member Author

Oh yeah... Helm 2... :-|

Copy link
Contributor

@hickeyma hickeyma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be helpful for users, thanks @technosophos. Just small comment inline and few things as follows:

Do you mind checking in the following files as I think there is still usage of the old storage that might need to be updated: ./docs/using_helm.md, ./cmd/helm/installer/init_test.go and ./pkg/repo/index_test.go

Is this working as expected or should the add also check for the old storage?

$ ./bin/helm repo add old https://kubernetes-charts.storage.googleapis.com
"old" has been added to your repositories

$ ./bin/helm repo list
NAME    	URL                                             
stable  	https://charts.helm.sh/stable                   
local   	http://127.0.0.1:8879/charts                    
jetstack	https://charts.jetstack.io                      
old     	https://kubernetes-charts.storage.googleapis.com

$ ./bin/helm repo remove stable
"stable" has been removed from your repositories

$ ./bin/helm repo add stable https://kubernetes-charts.storage.googleapis.com
"stable" has been added to your repositories

$ ./bin/helm repo update
WARNING: "kubernetes-charts.storage.googleapis.com" is deprecated for "stable" and will be deleted Nov. 13, 2020.
WARNING: You must switch to "https://charts.helm.sh/stable"
Hang tight while we grab the latest from your chart repositories...
...Skip local chart repository
...Successfully got an update from the "jetstack" chart repository
...Successfully got an update from the "stable" chart repository
...Successfully got an update from the "old" chart repository
Update Complete.

$ ./bin/helm install --name mysql-1 stable/mysql
WARNING: "kubernetes-charts.storage.googleapis.com" is deprecated for "stable" and will be deleted Nov. 13, 2020.
WARNING: You must switch to "https://charts.helm.sh/stable"
NAME:   mysql-1
LAST DEPLOYED: Mon Oct 19 11:30:35 2020
NAMESPACE: default
STATUS: DEPLOYED
[...]

cmd/helm/helm.go Outdated Show resolved Hide resolved
@technosophos
Copy link
Member Author

@hickeyma In last Thursday's call, someone remarked that we should be able to add the old repos under a different name, so people could work with the new and the old repos for a bit (I don't know why... but it was one of the goals I wrote down in my notes from that meeting). I supported the behavior by warning on stable/incubator, but allowing for other names like "old"

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
@technosophos
Copy link
Member Author

I can also backport the error-generating code to Helm 2, but my understanding from the last meeting was that we didn't want to do that for Helm 2. Just Helm 3.

Copy link
Collaborator

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@technosophos technosophos merged commit 62d6e40 into helm:dev-v2 Oct 19, 2020
@technosophos technosophos deleted the v2/new-repositories branch October 19, 2020 20:44
onap-github pushed a commit to onap/integration that referenced this pull request Jan 15, 2021
Location of the default Helm chart repository changed
from: https://kubernetes-charts.storage.googleapis.com
to:   https://charts.helm.sh/stable

This change has been addressed by Helm 2.17 release [1] but recommended
Helm version for Guilin is 2.16.10 which still requires manual override.

[1] helm/helm#8901

Issue-ID: ONAPARC-551
Change-Id: I63d94e37f639a213cff38c2e92166c41f29d1a9c
Signed-off-by: Pawel Wieczorek <p.wieczorek2@samsung.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. v2.x Issues and Pull Requests related to the major version v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants