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 port forwarding on Windows #4373

Merged
merged 5 commits into from Jun 23, 2020

Conversation

gsquared94
Copy link
Collaborator

Fixes: #3679
Description

On Windows OS, kubectl portforward command launches child processes that do not get cleaned up on the dev loop rerun. This way open ports keep getting locked up.

This PR introduces kubectl.Cmd which embeds exec.Cmd. The Start and Run methods are overridden for Windows OS (using build tags) to wrap around a job object and the job object handle is closed on cancellation to ensure all processes get killed. For non windows OSes it behaves identical to exec.Cmd.

@gsquared94 gsquared94 requested a review from a team as a code owner June 22, 2020 23:15
@gsquared94 gsquared94 requested a review from nkubala June 22, 2020 23:15
@codecov
Copy link

codecov bot commented Jun 22, 2020

Codecov Report

Merging #4373 into master will decrease coverage by 0.02%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4373      +/-   ##
==========================================
- Coverage   71.72%   71.69%   -0.03%     
==========================================
  Files         324      325       +1     
  Lines       12500    12554      +54     
==========================================
+ Hits         8966     9001      +35     
- Misses       2966     2977      +11     
- Partials      568      576       +8     
Impacted Files Coverage Δ
...affold/kubernetes/portforward/kubectl_forwarder.go 60.97% <75.00%> (ø)
pkg/skaffold/kubectl/cli.go 100.00% <100.00%> (ø)
pkg/skaffold/kubectl/exec.go 100.00% <100.00%> (ø)
pkg/skaffold/deploy/util.go 60.97% <0.00%> (-2.92%) ⬇️
cmd/skaffold/app/cmd/deploy.go 66.66% <0.00%> (-1.91%) ⬇️
pkg/skaffold/deploy/kustomize.go 77.05% <0.00%> (-0.66%) ⬇️
pkg/skaffold/config/util.go 70.71% <0.00%> (-0.55%) ⬇️
pkg/skaffold/debug/debug.go 45.16% <0.00%> (ø)
pkg/skaffold/server/server.go 48.88% <0.00%> (ø)
pkg/skaffold/config/options.go 100.00% <0.00%> (ø)
... and 4 more

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 fa83c60...8efb966. Read the comment docs.

@tejal29 tejal29 self-assigned this Jun 22, 2020
@tejal29 tejal29 requested review from tejal29 and removed request for nkubala June 22, 2020 23:28
@gsquared94 gsquared94 added the kokoro:run runs the kokoro jobs on a PR label Jun 23, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jun 23, 2020
@gsquared94 gsquared94 merged commit d18604e into GoogleContainerTools:master Jun 23, 2020
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.

Port Forwarding does not work with Helm deployer on Windows.
4 participants