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

Remove support for Helm 2 #5019

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Nov 11, 2020

Fixes #4607

Helm 2 is reaching its end of life on November 13, 2020, after which date old chart repositories may no longer be available. To continue to align with the Helm project, this change removes support for Helm client versions older than 3.0.0-beta.0.

To get the most feature-rich and stable Skaffold experience, we recommend using Helm version 3.1 or greater.

@nkubala nkubala requested a review from a team as a code owner November 11, 2020 19:39
@nkubala nkubala requested a review from IsaacPD November 11, 2020 19:39
@google-cla google-cla bot added the cla: yes label Nov 11, 2020
@tejal29
Copy link
Contributor

tejal29 commented Nov 11, 2020

I just saw this on the helm EOL announcement blog,

To Be Removed
Docker image for Tiller stored in Google Container Registry |

  • We will distribute a Tiller image that will be made available at an alternative location which can be updated with helm init --tiller-image.

Until next release, we will be in spot where

  1. skaffold will allow deploying with helm2
  2. Users deployment might fail if tiller image is not present.

Can we add a FAQ, or something on how to pass --tiller-image in helm.flags.install until next skaffold release?

@@ -469,7 +477,7 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, r latest.He

// getRelease confirms that a release is visible to helm
func (h *Deployer) getRelease(ctx context.Context, helmVersion semver.Version, releaseName string, namespace string) (bytes.Buffer, error) {
// Retry, because under Helm 2, at least, a release may not be immediately visible
// Retry, because sometimes a release may not be immediately visible
Copy link
Contributor

Choose a reason for hiding this comment

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

from the comments, looks like the retry logic was explicitly added maybe for helm2. Is there a way to verify if this is needed for helm3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thought about it, but it can't hurt to leave i think. if it's not needed then it will just get bypassed, and it serves as a safeguard in case we do actually end up needing it.

args = append(args, "--namespace", namespace)
}
args := []string{"get", "all"}
if namespace != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

ah! looks like we passed the namespace arg only for helm2. :)
Wonder how helm3 users didn't hit this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.... 😲

@nkubala
Copy link
Contributor Author

nkubala commented Nov 11, 2020

Can we add a FAQ, or something on how to pass --tiller-image in helm.flags.install until next skaffold release?

i'm planning on making an announcement on slack about the deprecation, and what to do in a few scenarios - i could add this in there. i think that's probably enough? our docs won't get updated on the website until the release anyway (unless we did it manually), so not sure how many people would see it.

@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #5019 (6a3f3a0) into master (8caebb9) will increase coverage by 0.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5019      +/-   ##
==========================================
+ Coverage   72.17%   72.19%   +0.01%     
==========================================
  Files         365      365              
  Lines       12807    12822      +15     
==========================================
+ Hits         9244     9257      +13     
  Misses       2878     2878              
- Partials      685      687       +2     
Impacted Files Coverage Δ
pkg/skaffold/deploy/helm/helm.go 73.73% <94.11%> (-0.07%) ⬇️
pkg/skaffold/build/result.go 83.33% <0.00%> (-3.34%) ⬇️
pkg/skaffold/build/bazel/build.go 67.24% <0.00%> (-3.13%) ⬇️
pkg/skaffold/build/jib/maven.go 100.00% <0.00%> (ø)
pkg/skaffold/build/jib/gradle.go 100.00% <0.00%> (ø)
pkg/skaffold/color/formatter.go 100.00% <0.00%> (+10.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8caebb9...6a3f3a0. Read the comment docs.

@tejal29
Copy link
Contributor

tejal29 commented Nov 11, 2020

Can we add a FAQ, or something on how to pass --tiller-image in helm.flags.install until next skaffold release?

i'm planning on making an announcement on slack about the deprecation, and what to do in a few scenarios - i could add this in there. i think that's probably enough? our docs won't get updated on the website until the release anyway (unless we did it manually), so not sure how many people would see it.

I have ran the trigger for docs manually a couple of times when Skaffold Reference Yaml was broken.
We could do that again.

@nkubala nkubala merged commit be1c5e0 into GoogleContainerTools:master Nov 12, 2020
@nkubala nkubala deleted the helm-2-remove branch November 12, 2020 21:42
yuwenma pushed a commit to yuwenma/skaffold that referenced this pull request Nov 13, 2020
remove const var kustomizeFurtherGuidance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate Helm 2 Support
3 participants