Adds support for `debug` for Helm #4732

briandealwis · 2020-08-28T04:08:44Z

Fixes: #2350
Related: #4704

Description

This PR adds support for skaffold debug for Helm deploys. It requires Helm 3.1.0 or later as the implementation requires using Helm 3.1's --post-renderer flag.

Basically the implementation is in two parts:

It adds a new hidden command skaffold filter, which supports --build-artifacts=xxx and --debugging. This command takes in a set of Kubernetes manifests on stdin and performs a set of transforms on the images listed in build-artifacts, or all images found if no images are provided.
it causes the Helm deployer to run helm with Skaffold as a post-renderer. It uses the SKAFFOLD_CMDLINE facility from Add SKAFFOLD_CMDLINE environment variable to pass command-line #4704 to cause Skaffold to run with filter --debugging --build-artifacts=xxx, such that Skaffold then debug-ifies the Helm-produced manifests

User facing changes (remove if N/A)

Helm is now supported by skaffold debug

codecov · 2020-08-28T04:18:08Z

Codecov Report

Merging #4732 into master will decrease coverage by 0.16%.
The diff coverage is 51.32%.

@@            Coverage Diff             @@
##           master    #4732      +/-   ##
==========================================
- Coverage   73.85%   73.68%   -0.17%     
==========================================
  Files         347      348       +1     
  Lines       13796    13899     +103     
==========================================
+ Hits        10189    10242      +53     
- Misses       2972     3014      +42     
- Partials      635      643       +8

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/flags.go	`86.36% <ø> (ø)`
pkg/skaffold/debug/debug.go	`43.07% <0.00%> (-2.09%)`	⬇️
pkg/skaffold/deploy/kubectl/cli.go	`88.63% <ø> (ø)`
cmd/skaffold/app/cmd/filter.go	`26.08% <26.08%> (ø)`
pkg/skaffold/deploy/helm.go	`78.16% <68.42%> (-0.73%)`	⬇️
cmd/skaffold/app/cmd/cmd.go	`68.57% <100.00%> (+0.22%)`	⬆️
pkg/skaffold/util/util.go	`86.58% <100.00%> (+0.50%)`	⬆️

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 10275c6...64519ba. Read the comment docs.

…ug mode

cmd/skaffold/app/cmd/debug.go

gsquared94 · 2020-09-02T14:26:20Z

cmd/skaffold/app/cmd/debug.go

@@ -37,14 +46,70 @@ func NewCmdDebug() *cobra.Command {
 		WithLongDescription("Similar to `dev`, but configures the pipeline for debugging.").
 		WithCommonFlags().
 		WithHouseKeepingMessages().
+		WithFlags(func(f *pflag.FlagSet) {
+			// --filter and --build-artifacts are used to support debug for Helm and are internal
+			f.BoolVar(&filtering, "filter", false, `Filter manifests from stdin for debugging similar to "skaffold debug".`)


nit: since skaffold debug --filter only writes the modified manifests to StdOut and doesn't do anything else should the flag be called filter-only or render-only or something which makes it obvious?

docs/content/en/docs/workflows/debug.md

pkg/skaffold/deploy/helm.go

briandealwis · 2020-09-11T21:07:53Z

cmd/skaffold/app/cmd/flags.go

@@ -254,15 +254,15 @@ var flagRegistry = []Flag{
 		Value:         &opts.KubeContext,
 		DefValue:      "",
 		FlagAddMethod: "StringVar",
-		DefinedOn:     []string{"build", "debug", "delete", "deploy", "dev", "run"},
+		DefinedOn:     []string{"build", "debug", "delete", "deploy", "dev", "run", "filter"},


This feels weird, but we do derive the local docker daemon from the kube-context (e.g., minikube)

briandealwis · 2020-09-11T21:09:47Z

Move debug --filter into a new command as filter --debugging
Adds tests for TestHelmDeploy —
Fix fun bug in HelmDeployer.binVer() where it wasn't caching the helm version if the minor had a 0

nkubala · 2020-09-15T00:04:18Z

this is really close, looks like you have one file that needs a rebase. I also see this "error" when running locally after ending my session with ctrl+c:

Error: uninstallation completed with 1 error(s): uninstall: Failed to purge the release: release: not found

but when I monitor my workloads in cloud console, things are being created/deleted correctly so nothing seems to actually be going wrong, and skaffold is exiting with code 0. any idea why that's happening?

briandealwis · 2020-09-15T15:14:17Z

@nkubala which version of Helm are you using?

$ cd examples/helm-deployment
$ helm version
version.BuildInfo{Version:"v3.3.1", GitCommit:"249e5215cde0c3fa72e27eb7a30e8d55c9696144", GitTreeState:"clean", GoVersion:"go1.14.7"}
$ skaffold debug --port-forward
[...]
Starting deploy...
Helm release skaffold-helm not installed. Installing...
NAME: skaffold-helm
LAST DEPLOYED: Tue Sep 15 11:07:37 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
Waiting for deployments to stabilize...
 - deployment/skaffold-helm: waiting for rollout to finish: 0 of 1 updated replicas are available...
 - deployment/skaffold-helm is ready.
Deployments stabilized in 5.99460414s
Press Ctrl+C to exit
Not watching for changes...
Port forwarding pod/skaffold-helm-7f4866968f-fv6l8 in namespace default, remote port 56268 -> address 127.0.0.1 port 56268
[install-go-support] Installing runtime debugging support files in /dbg
[install-go-support] Installation complete
[skaffold-helm] API server listening at: [::]:56268
[skaffold-helm] Hello world! 0
[skaffold-helm] Hello world! 1
^CCleaning up...
release "skaffold-helm" uninstalled

Tried Helm 3.1.2 and don't see anything different.

nkubala · 2020-09-15T19:57:27Z

sorry I meant to put that in there but forgot to copy-paste :)

➜  helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"dirty", GoVersion:"go1.14.3"}

briandealwis · 2020-09-18T17:41:33Z

@nkubala I tried against GKE with Helm 3.2.2 (admittedly not 3.2.4) and I can't reproduce.

nkubala · 2020-09-18T19:13:58Z

yeah i tried again on a different cluster and on minikube and i can't get it to happen - something is weird with the cluster i was testing on. don't worry about it

briandealwis requested review from loosebazooka and a team as code owners August 28, 2020 04:08

briandealwis requested a review from tstromberg August 28, 2020 04:08

pull-request-size bot added the size/L label Aug 28, 2020

googlebot added the cla: yes label Aug 28, 2020

briandealwis and others added 4 commits August 31, 2020 23:49

Add debug --filter to apply debug transforms to arbitrary manifest

4da8f78

Cause Helm to use skaffold debug --filter as a post-renderer in deb…

4d6e9d8

…ug mode

bow to golangci-lint

aa48c44

expose Mode() on deploy.Config

23bec07

briandealwis force-pushed the debug-helm branch from 8dece20 to 23bec07 Compare September 1, 2020 04:01

gsquared94 reviewed Sep 2, 2020

View reviewed changes

cmd/skaffold/app/cmd/debug.go Outdated Show resolved Hide resolved

gsquared94 reviewed Sep 2, 2020

View reviewed changes

nkubala reviewed Sep 8, 2020

View reviewed changes

docs/content/en/docs/workflows/debug.md Outdated Show resolved Hide resolved

pkg/skaffold/deploy/helm.go Outdated Show resolved Hide resolved

briandealwis added 6 commits September 10, 2020 17:13

Merge remote-tracking branch 'origin/master' into HEAD

51ad40d

fixup doc

57eb382

Add skaffold filter and move debug filtering to there

09df135

Add debug tests for TestHelmDeploy

6efadeb

fix year

26de11a

Ensure Helm SKAFFOLD_CMDLINE replaces any existing entries

0eb48e9

pull-request-size bot added size/XL and removed size/L labels Sep 11, 2020

briandealwis commented Sep 11, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

47216ac

merge with HEAD

64519ba

nkubala self-assigned this Sep 17, 2020

nkubala approved these changes Sep 18, 2020

View reviewed changes

nkubala merged commit 77244b8 into GoogleContainerTools:master Sep 18, 2020

This was referenced Sep 28, 2020

skaffold debug when using helm does not work when helm chart renders more than one manifest #4826

Closed

fix: skaffold debug does not work for helm deployments with multiple … #4827

Closed

briandealwis mentioned this pull request Sep 28, 2020

Fix skaffold filter to handle multiple yaml documents #4829

Merged

fmotrifork mentioned this pull request Oct 1, 2020

skaffold 1.15.0 fishworks/fish-food#989

Merged

briandealwis mentioned this pull request Oct 7, 2020

Fix debug for Helm on Windows #4872

Merged

briandealwis mentioned this pull request Oct 7, 2021

Remove need for Helm artifactsOverride by using Skaffold as a post-renderer #6706

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for `debug` for Helm #4732

Adds support for `debug` for Helm #4732

briandealwis commented Aug 28, 2020 •

edited

Loading

codecov bot commented Aug 28, 2020 •

edited

Loading

gsquared94 Sep 2, 2020

briandealwis Sep 11, 2020

briandealwis commented Sep 11, 2020

nkubala commented Sep 15, 2020

briandealwis commented Sep 15, 2020

nkubala commented Sep 15, 2020

briandealwis commented Sep 18, 2020

nkubala commented Sep 18, 2020

Adds support for debug for Helm #4732

Adds support for debug for Helm #4732

Conversation

briandealwis commented Aug 28, 2020 • edited Loading

codecov bot commented Aug 28, 2020 • edited Loading

Codecov Report

gsquared94 Sep 2, 2020

Choose a reason for hiding this comment

briandealwis Sep 11, 2020

Choose a reason for hiding this comment

briandealwis commented Sep 11, 2020

nkubala commented Sep 15, 2020

briandealwis commented Sep 15, 2020

nkubala commented Sep 15, 2020

briandealwis commented Sep 18, 2020

nkubala commented Sep 18, 2020

Adds support for `debug` for Helm #4732

Adds support for `debug` for Helm #4732

briandealwis commented Aug 28, 2020 •

edited

Loading

codecov bot commented Aug 28, 2020 •

edited

Loading