fix: correctly rewrite debug container entrypoint and bind host port #6682
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6682 +/- ##
==========================================
- Coverage 70.48% 69.77% -0.71%
==========================================
Files 515 525 +10
Lines 23150 24009 +859
==========================================
+ Hits 16317 16753 +436
- Misses 5776 6157 +381
- Partials 1057 1099 +42
Continue to review full report at Codecov.
|
nkubala
force-pushed
the
docker-debug-fix
branch
from
461a6b7 to
f27a4ab
Compare
pkg/skaffold/deploy/docker/deploy.go
Outdated
| } | ||
| } | ||
|
|
||
| bindings, err := d.portManager.getPorts(artifact.ImageName, d.resources, containerCfg, debugBindings) |
There was a problem hiding this comment.
Aside: I think users will need the ability to specify additional ports to expose for our docker deployer. It might be worth turning the []string into an array of objects sooner than later.
There was a problem hiding this comment.
yeah the configuration definitely needs improvement, a []string was basically an MVP to get things working. thanks for pointing this out though, will keep a note of it
done. PTAL @briandealwis |
|
Test failed due to timeout which is fixed on master |
Two issues fixed in this change:
CMDwas being rewritten with the debugging entrypoint. This was erroneous as it wasn't actually intercepting the entrypoint for the container. This change makes it so the actualENTRYPOINTis being rewritten in the application container.DebugManagerto theDeployer, which passes them to thePortManagerto ensure no collisions go undetected. This also ensures the correct events are issued through the API, so the clients can successfully attached to the exposed debugger.Verified using the Cloud Code Hello World Sample App.