Skip to content

Remove support for Helm 2 #5019

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Remove support for Helm 2 #5019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions pkg/skaffold/deploy/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,22 @@ func NewDeployer(cfg kubectl.Config, labels map[string]string) *Deployer {
}
}

func (h *Deployer) checkMinVersion(v semver.Version) error {
if v.LT(helm3Version) {
return fmt.Errorf("skaffold requires Helm version 3.0.0-beta.0 or greater")
}
return nil
}

// Deploy deploys the build results to the Kubernetes cluster
func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]string, error) {
hv, err := h.binVer(ctx)
if err != nil {
return nil, fmt.Errorf(versionErrorString, err)
}
if err = h.checkMinVersion(hv); err != nil {
return nil, err
}

logrus.Infof("Deploying with helm v%s ...", hv)

Expand Down Expand Up @@ -187,15 +197,14 @@ func (h *Deployer) Dependencies() ([]string, error) {

lockFiles := []string{
"Chart.lock",
"requirements.lock",
}

// We can always add a dependency if it is not contained in our chartDepsDirs.
// However, if the file is in our chartDepsDir, we can only include the file
// if we are not running the helm dep build phase, as that modifies files inside
// the chartDepsDir and results in an infinite build loop.
// We additionally exclude ChartFile.lock (Helm 3) and requirements.lock (Helm 2)
// since they also get modified on helm dep build phase
// We additionally exclude ChartFile.lock,
// since it also gets modified during a `helm dep build`.
isDep := func(path string, info walk.Dirent) (bool, error) {
if info.IsDir() {
return false, nil
Expand Down Expand Up @@ -233,6 +242,9 @@ func (h *Deployer) Cleanup(ctx context.Context, out io.Writer) error {
if err != nil {
return fmt.Errorf(versionErrorString, err)
}
if err = h.checkMinVersion(hv); err != nil {
return err
}

for _, r := range h.Releases {
releaseName, err := util.ExpandEnvTemplate(r.Name, nil)
Expand All @@ -246,9 +258,7 @@ func (h *Deployer) Cleanup(ctx context.Context, out io.Writer) error {
}

args := []string{"delete", releaseName}
if hv.LT(helm3Version) {
args = append(args, "--purge")
} else if namespace != "" {
if namespace != "" {
args = append(args, "--namespace", namespace)
}
if err := h.exec(ctx, out, false, nil, args...); err != nil {
Expand All @@ -264,18 +274,16 @@ func (h *Deployer) Render(ctx context.Context, out io.Writer, builds []build.Art
if err != nil {
return fmt.Errorf(versionErrorString, err)
}
if err = h.checkMinVersion(hv); err != nil {
return err
}

renderedManifests := new(bytes.Buffer)

for _, r := range h.Releases {
args := []string{"template", r.ChartPath}

if hv.GTE(helm3Version) {
// Helm 3 requires the name to be before the chart path
args = append(args[:1], append([]string{r.Name}, args[1:]...)...)
} else {
args = append(args, "--name", r.Name)
}
args = append(args[:1], append([]string{r.Name}, args[1:]...)...)

for _, vf := range r.ValuesFiles {
args = append(args, "--values", vf)
Expand Down Expand Up @@ -399,7 +407,7 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, r latest.He
return nil, err
}

if err := h.exec(ctx, ioutil.Discard, false, nil, getArgs(helmVersion, releaseName, opts.namespace)...); err != nil {
if err := h.exec(ctx, ioutil.Discard, false, nil, getArgs(releaseName, opts.namespace)...); err != nil {
color.Yellow.Fprintf(out, "Helm release %s not installed. Installing...\n", releaseName)

opts.upgrade = false
Expand Down Expand Up @@ -458,7 +466,7 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, r latest.He
return nil, fmt.Errorf("install: %w", err)
}

b, err := h.getRelease(ctx, helmVersion, releaseName, opts.namespace)
b, err := h.getRelease(ctx, releaseName, opts.namespace)
if err != nil {
return nil, fmt.Errorf("get release: %w", err)
}
Expand All @@ -468,15 +476,15 @@ 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
func (h *Deployer) getRelease(ctx context.Context, releaseName string, namespace string) (bytes.Buffer, error) {
// 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.

opts := backoff.NewExponentialBackOff()
opts.MaxElapsedTime = 4 * time.Second
var b bytes.Buffer

err := backoff.Retry(
func() error {
if err := h.exec(ctx, &b, false, nil, getArgs(helmVersion, releaseName, namespace)...); err != nil {
if err := h.exec(ctx, &b, false, nil, getArgs(releaseName, namespace)...); err != nil {
logrus.Debugf("unable to get release: %v (may retry):\n%s", err, b.String())
return err
}
Expand Down Expand Up @@ -543,9 +551,6 @@ func installArgs(r latest.HelmRelease, builds []build.Artifact, valuesSet map[st
}
} else {
args = append(args, "install")
if o.helmVersion.LT(helm3Version) {
args = append(args, "--name")
}
args = append(args, o.releaseName)
args = append(args, o.flags...)
}
Expand Down Expand Up @@ -686,13 +691,10 @@ func sortKeys(m map[string]string) []string {
}

// getArgs calculates the correct arguments to "helm get"
func getArgs(v semver.Version, releaseName string, namespace string) []string {
args := []string{"get"}
if v.GTE(helm3Version) {
args = append(args, "all")
if namespace != "" {
args = append(args, "--namespace", namespace)
}
func getArgs(releaseName string, namespace string) []string {
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.

.... 😲

args = append(args, "--namespace", namespace)
}
return append(args, releaseName)
}
Expand Down
Loading