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

Fix debug for Helm on Windows #4872

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

briandealwis
Copy link
Member

@briandealwis briandealwis commented Oct 7, 2020

Fixes: #4862
Related to: #4732

Description
To support debug in our Helm deployer, we create a Skaffold command-line that we encode in the SKAFFOLD_CMDLINE environment variable, something like:

[]string{"filter", "--debugging", "--kube-context", "kubecontext", "--build-artifacts", "buildfile", "--kubeconfig", "kubeconfig"}

We're just using strings.Join() to build up this command-line:

env["SKAFFOLD_CMDLINE"] = strings.Join(cmdLine, " ")

But in Windows, the buildfile has backslashes in the name. And we parse our this command-line using shell.Split() which interprets backslashes:

parsed, err := shell.Split(cmdLine)

And so the directory separators are being stripped out and the path name is not found.

@briandealwis briandealwis requested a review from a team as a code owner October 7, 2020 21:19
@google-cla google-cla bot added the cla: yes label Oct 7, 2020
@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

Merging #4872 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4872      +/-   ##
==========================================
+ Coverage   71.86%   71.88%   +0.02%     
==========================================
  Files         356      356              
  Lines       12218    12218              
==========================================
+ Hits         8780     8783       +3     
+ Misses       2786     2785       -1     
+ Partials      652      650       -2     
Impacted Files Coverage Δ
pkg/skaffold/deploy/helm/helm.go 74.61% <100.00%> (ø)
pkg/skaffold/docker/image.go 80.75% <0.00%> (+1.40%) ⬆️

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 da65fa5...fd5c78a. Read the comment docs.

@aurelien-baumann-lacapitale

If it helps, I can build it locally and test it on Windows.

@briandealwis
Copy link
Member Author

Please do @aurelien-baumann-lacapitale!

@gsquared94 gsquared94 added the kokoro:force-run forces a kokoro re-run on a PR label Oct 8, 2020
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Oct 8, 2020
@aurelien-baumann-lacapitale

@briandealwis it works! Thank you 🎉👌

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏼

@briandealwis briandealwis merged commit 70c72f9 into GoogleContainerTools:master Oct 9, 2020
@briandealwis briandealwis deleted the i4862 branch October 9, 2020 18:13
@alisters
Copy link

alisters commented Oct 27, 2020

@aurelien-baumann-lacapitale - this is a long shot, but how did you build on windows ? I'd like to also test this fix (to see if this fix's cloud code in intellij not attaching), but hopefully to be able to provide some future fixes too. I'm a Go newbie, and it's been 20 years since i wrote Makefiles so my attempts to build skaffold in powershell and cmd.exe are failing miserably....

What target did you execute ?
What shell/platform did you build in/on ? WSL ? (I see there is cross compile make target rules)
Can skaffold even be built on cmd or powershell ? I see it runs some shell scripts to generate some code so i'm guessing not ?

One word answers are fine ! :-)

For anyone else that finds this i managed to cross compile in WSL, producing a binary for windows by following this guide - https://www.digitalocean.com/community/tutorials/how-to-build-go-executables-for-multiple-platforms-on-ubuntu-16-04.

env GOOS=windows GOARCH=amd64 make

Then renamed the binary skaffold.exe and copied into an appropriate location.

Tested that the fix worked, cloud code attached successfully, and now see this has been merged and released in 1.16.

@aurelien-baumann-lacapitale

Hey @alisters you seem to have cracked it alright. Sorry for my non-reactivity on that one 🤭

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.

Debug doesn't work with Helm on Windows
6 participants