Skip to content

Fix port-forwarding not being triggered.#1433

Merged
dgageot merged 1 commit intoGoogleContainerTools:masterfrom
bmcustodio:bmcstdio/port-forward
Dec 31, 2018
Merged

Fix port-forwarding not being triggered.#1433
dgageot merged 1 commit intoGoogleContainerTools:masterfrom
bmcustodio:bmcstdio/port-forward

Conversation

@bmcustodio
Copy link
Copy Markdown
Contributor

@bmcustodio bmcustodio commented Dec 29, 2018

While running skaffold dev to deploy a pod containing some port definitions, I noticed that port-forwardings were not being triggered. Turns out this was happening because...

  • ADDED events are currently ignored by the port forwarder's pod watcher; and
  • The pod I was deploying was already in the Running phase when the watcher is established; and
  • No further events (i.e. of type MODIFIED) were received for that pod (so as to trigger forwarding).

My proposal is that the port forwarder's pod watcher starts taking ADDED events into account.

Signed-off-by: Bruno Miguel Custodio <brunomcustodio@gmail.com>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 29, 2018

Codecov Report

Merging #1433 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1433      +/-   ##
==========================================
- Coverage   44.56%   44.53%   -0.03%     
==========================================
  Files         112      112              
  Lines        4593     4596       +3     
==========================================
  Hits         2047     2047              
- Misses       2339     2342       +3     
  Partials      207      207
Impacted Files Coverage Δ
pkg/skaffold/kubernetes/port_forward.go 41.48% <0%> (-1.37%) ⬇️

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 182b762...d2445b1. Read the comment docs.

Copy link
Copy Markdown
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

Thanks - looks like I made a bad assumption

@dgageot dgageot merged commit 60fb2e5 into GoogleContainerTools:master Dec 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants