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

Support helm 3 #2142

Closed
corneliusweig opened this issue May 16, 2019 · 37 comments · Fixed by #3738
Closed

Support helm 3 #2142

corneliusweig opened this issue May 16, 2019 · 37 comments · Fixed by #3738

Comments

@corneliusweig
Copy link
Contributor

Expected behavior

Skaffold should work with helm 3. The pre-release for helm3 was published today (May 16).

As long as Skaffold does not support helm3, there should be an error message with a recommendation to downgrade.

Actual behavior

When trying the helm-deployment example, skaffold run failed with

(...)
Starting deploy...
Error: release: not found
Helm release skaffold-helm not installed. Installing...
Error: unknown flag: --name
WARN[0000] error retrieving helm deployment info: Error: release: not found 
Pruning images...
WARN[0000] builder cleanup: pruning images: Error response from daemon: conflict: unable to delete 890f17d42fc2 (cannot be forced) - image is being used by running container e0f8aa619e83 
Cleaning up...
Error: unknown flag: --purge
Cleanup complete in 48.575768ms
FATA[0000] exiting dev mode because first run failed: deploy failed: deploying skaffold-helm: exit status 1

So it seems that helm 3 does not support the --name flag anymore.

Information

@nkubala
Copy link
Contributor

nkubala commented May 17, 2019

thanks @corneliusweig, yeah I'll try playing around with this more next week and see what needs to be done. I'm all for trying to support it, just need to see what kind of changes we need to make.

@bjoberg
Copy link

bjoberg commented Jun 27, 2019

Hey, is Helm 3 support in the road map?

@tejal29 tejal29 added priority/p2 May take a couple of releases priority/p1 High impact feature/bug. and removed priority/p2 May take a couple of releases labels Aug 21, 2019
@mikesplain
Copy link

We've done some testing with skaffold and helm 3, the most important changes in terms of standard run / delete workflow are removing the --name flag from the install and --purge flag from delete. In a private fork we've added a flag to switch between helm 3 and helm 2 mode but not sure the right way to implement this upstream.

We also made it configurable to switch helm binary names since, for testing we map helm => helm v2.14.3 and helm3 => v3.0.0-beta.3.

If anyone has pointers on whether a flag to switch between helm 2 and helm 3 modes is amenable in the short term, that'd be great.

@AndiDog
Copy link
Contributor

AndiDog commented Sep 11, 2019

Maybe change the manifest? Deprecate the key helm: and instead use helm2:/helm3:.

Or make a field helm:version mandatory after warning about the change for a few releases.

@Multiply
Copy link
Contributor

@mikesplain Can the private repository be shared? We're very much interested in testing helm3 with skaffold. Currently we're manually running some scripts prior to running skaffold locally, since we're already on helm3 for local development.

@ktarplee
Copy link

ktarplee commented Sep 17, 2019

@mikesplain Do you have a fork of skaffold with your changes to support helm v3? I have one I will release shortly unless you can make yours public. I don't want to muddy the waters with multiple approaches.

Seems to me that modifying the skaffold.yaml file in anyway to support helm v3 is the wrong approach. One team might develop/use the same project with helm v2 and another with helm v3. No need to change the skaffold.yaml file. Skaffold can simply detect helm v3 with helm version --short and then skaffold can do the right thing. I am doing this in my fork.

@igoooor
Copy link

igoooor commented Nov 14, 2019

with helm 3 official release, you can get skaffold run to work by adding

    flags:
      upgrade:
        - "--install"

But this is only a temporary fix until skaffold fully supports helm 3.

What is still not working at the moment is to delete a release as the syntax changed.

@snickell
Copy link
Contributor

Is there a wip effort (or even a branch?) to fix this on the googly side that we can't see? If not, I might be up to take a pass at this.

@ktarplee
Copy link

The notable differences with helm v3 that I have noticed and fixed in my skaffold fork are:

  1. "helm install" syntax changed
  2. "helm get" syntax changed. This is used in skaffold to check if the release already exists (for install vs upgrade)
  3. "helm delete" does not have --purge

My fork went the path of making the helm deployer work for both helm v3 and helm v2. It looks like the maintainers would rather have a new/separate deployer for helm v3.

@bjornmagnusson
Copy link

Is there any work going on for getting this implemented?

@code-is-art
Copy link

code-is-art commented Nov 30, 2019

The notable differences with helm v3 that I have noticed and fixed in my skaffold fork are:

  1. "helm install" syntax changed
  2. "helm get" syntax changed. This is used in skaffold to check if the release already exists (for install vs upgrade)
  3. "helm delete" does not have --purge

My fork went the path of making the helm deployer work for both helm v3 and helm v2. It looks like the maintainers would rather have a new/separate deployer for helm v3.

This fork was working for the most part unless one was using helm with a namespace, then it would fail. I have never looked at a single line of go before but I am a good coder so I adjusted some things and it is working correctly for me now. If anyone experience with go would like to give it a go do it up. I am attaching the helm.go file if anyone is interested.
helm.go.zip
BTW I built it from source for osx and have only used it with Skaffold dev command so far so I could have missed some things

@ktarplee
Copy link

ktarplee commented Dec 4, 2019

I updated my fork to incorporate @cloudtagger changes to support namespaces properly.

@Romeh
Copy link

Romeh commented Dec 23, 2019

any news when skaffold will be officially supporting helm 3 ?

@hipstern
Copy link

hipstern commented Jan 3, 2020

Is there a way to use skaffold 1.1 with helm 3 at this point in time?

@TBBle
Copy link
Contributor

TBBle commented Jan 3, 2020

@igoooor's fix above will make Skaffold work with Helm 3 for installing, but it won't delete the release at the end and you'll have to do it manually (with helm uninstall).

@ktarplee
Copy link

ktarplee commented Jan 3, 2020

Is there a way to use skaffold 1.1 with helm 3 at this point in time?

I updated my skaffold fork to v1.1.0. The only change is support ffor helm 3 (see earlier in this ticket).

@kvokka
Copy link

kvokka commented Jan 6, 2020

This should resolve #3476 since Helm3 finally has this fix helm/helm#2060 (comment)

@eulersson
Copy link

Is there a way to use skaffold 1.1 with helm 3 at this point in time?

I updated my skaffold fork to v1.1.0. The only change is support ffor helm 3 (see earlier in this ticket).

I am using it @ktarplee and works like a charm! Thanks :)

@kvokka
Copy link

kvokka commented Jan 6, 2020

@nkubala is it possible to ship @ktarplee fix? Helm 3 was released a few months ago, and this will allow to remove add the ability to support actual version.

This is a blocker, and it feels like not for me only.

Thank you

@ekhaydarov
Copy link

@ktarplee or anyone else. How do you compile the skaffold repo? the binary built in @ktarplee skaffold fork does not work on a mac and it doesnt seem as staightforward to do a simple go build on that repo

Thanks

@eulersson
Copy link

@ktarplee or anyone else. How do you compile the skaffold repo? the binary built in @ktarplee skaffold fork does not work on a mac and it doesnt seem as staightforward to do a simple go build on that repo

Thanks

Just clone his repository, change directory to the root of the project and make.

It builds it to out/skaffold

@ktarplee
Copy link

I updated the helm v3 fork to v1.2.0 of skaffold and added mac and linux binaries this time.

@balchua
Copy link

balchua commented Feb 5, 2020

Is there a windows binary? 😁

@aboyett
Copy link

aboyett commented Feb 5, 2020

I'm not @ktarplee but I've got CI setup over on my Gitlab account[1] to cross-build binaries for linux-amd64, darwin-amd64, and windows-amd64. Note, the Windows binaries are untested as I don't develop on Windows.

v1.3.1-helm3.agb1 is the latest state of @ktarplee's patches against v1.2.0 forward ported to v1.3.1

[1] https://gitlab.com/aboyett/skaffold/-/releases

@balchua
Copy link

balchua commented Feb 5, 2020

@aboyett thanks, i will give this a try. 👍

@wohckcin
Copy link

wohckcin commented Feb 10, 2020

I personally don't understand why it's so difficult for the official upstream to get helmv3 supported. @ktarplee A big shout out for your fork. Thanks for your great work.

@TBBle
Copy link
Contributor

TBBle commented Feb 14, 2020

To deliver the feature to upstream, either the people who've created the patches need to put them up for a PR, or someone has to redo the work from scratch. Which one will happen really depends on whether the original patches were signed-off sufficiently and if anyone's willing to actually create and shepherd the PR, and whether the changes actually match the upstream design or other requirements, e.g., It looks like the maintainers would rather have a new/separate deployer for helm v3. although I don't see a source for that information.

@Gary-Mansell
Copy link

@TBBle

See here for the PR that @ktarplee had originally opened with his changes and the closing comment referencing a new deployer for v3.

@TBBle
Copy link
Contributor

TBBle commented Feb 14, 2020

Ah, thank you. I didn't realise closed PRs don't show up in the "linked PR list" on the right. >_<

@ktarplee
Copy link

Updated my fork that includes helm v3 support rebased on skaffold v1.4.0. It includes mac and linux binaries.

@nkubala
Copy link
Contributor

nkubala commented Feb 26, 2020

hey all, just wanted to give an update from the team here.

we pretty hastily closed #2900 a few months ago without a full discussion on our implementation plan for this, and I think that was a little unfair. IMO if we're going to close a PR that's been sent to us, we should be providing feedback on what we'll be doing instead, but I think even before going that far we should be trying to give guidance on how to massage the PR into something we're inclined to accept. we didn't do either of those things in this case, so I wanted to apologize for that.

also, with an issue as pressing and with as much community support as this one, we owe it to the community to at the very least be more communicative about our processes so users know what to expect. truthfully, a combination of organizational changes and some reprioritization of work caused this one to slip away from us for too long. given how many people we had asking for this and even offering to help out, I think that's unacceptable on our part. we should do better, and going forward we will.

with all that said, this is now our #1 issue and I promise it'll ship as soon as it's ready. committing to milestones is always risky, but I'm hoping this will go out with our next release. stay tuned on slack and twitter, we'll be sure and notify people there as soon as it goes out.

special thanks to @ktarplee for maintaining the fork and keeping people satisfied while we get this upstreamed 🙏

@Romeh
Copy link

Romeh commented Mar 5, 2020

@nkubala thx for covering that , can u please share with us which release version will include helm3 support ?

@Gary-Mansell
Copy link

@Romeh from a thread on the kubernetes/skaffold slack channel the next release should contain helm 3 support and the ETA is this Thursday so I guess very soon! 👀

@nkubala
Copy link
Contributor

nkubala commented Mar 5, 2020

@Gary-Mansell right you are, I'll be doing the release in just a few hours :)

@davidfernandezm
Copy link

@nkubala could you confirm if skaffold debug will work with helm as of #2350?

@ekhaydarov
Copy link

FYI skaffold dev still has issues with recreation and needs the --force=false flag applied

@TBBle
Copy link
Contributor

TBBle commented Mar 12, 2020

@ekhaydarov That's a Helm issue, that skaffold is triggering with its default --force=true. See #3798 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.