Update to pack 0.15 and add debug support for CNB Platform API 0.4 #5064
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5064 +/- ##
==========================================
+ Coverage 72.30% 72.33% +0.03%
==========================================
Files 368 368
Lines 12927 12964 +37
==========================================
+ Hits 9347 9378 +31
- Misses 2890 2893 +3
- Partials 690 693 +3
Continue to review full report at Codecov.
|
briandealwis
force-pushed
the
cnb-0.4
branch
from
31126b6 to
e7dac68
Compare
| // indicates an image mis-configuration as we should have resolved the the | ||
| // CNB_PROCESS_TYPE (if specified) or `web`. | ||
| logrus.Warnf("no CNB launch found for %s/%s", ic.artifact, processType) | ||
| logrus.Warnf("no CNB launch found for %s", ic.artifact) |
There was a problem hiding this comment.
would it possible to debug a container if CNB launch is not found?
There was a problem hiding this comment.
No. At this point we have an CNB launcher entrypoint (either /cnb/lifecycle/launcher or /cnb/process/xxx), but no corresponding process found. So we don't have a command-line should be executed.
Basically we'll return, the debug transforms won't be able to figure anything out, and the container / pod will remain un-touched. And the execution of the container will then fail as no process definition will be found.
| // the original form. | ||
| // Script-style launches support referencing environment variables since they are expanded by the shell. | ||
| // | ||
| // Configuring the launch depends on the CNB Platform API version being used, which is determined by |
There was a problem hiding this comment.
Thanks for the details, it was a little bit easier to understand the code.
There was a problem hiding this comment.
Going through this was painful :-)
briandealwis
force-pushed
the
cnb-0.4
branch
from
e7dac68 to
015adfa
Compare
briandealwis
changed the title
briandealwis
force-pushed
the
cnb-0.4
branch
from
7b018cd to
18169aa
Compare
briandealwis
dismissed
tejal29’s stale review
Integration tests are failing; marking as draft and fixing
briandealwis
force-pushed
the
cnb-0.4
branch
from
6fa5ef4 to
ebd77a5
Compare
Tests were failing as I wasn't rewriting the entrypoint, so that Platform 0.4-style process executables (/cnb/process/web) continued through and the debug transformers wouldn't recognize the executable. - fix adjustCommandLine() and add tests - fix testutil.isNil to handle function pointers - narrow isCNBImage() to only allow entrypoint to be 1-element array and add tests - go style nits (rename isCnbImage -> isCNBImage, godoc style)
briandealwis
force-pushed
the
cnb-0.4
branch
from
ebd77a5 to
b288b3c
Compare
|
Marking TestDevPortForwardDeletePod to be skipped #4733. |
There was a problem hiding this comment.
this LGTM, its essentially an improved version of similar changes I made in #5038. thanks for the detailed comments @briandealwis
Fixes: #4796
Description
Update to pack 0.15.0, and update
skaffold debugto support the Cloud Native Buildpacks Platform API 0.4, the default for the CNBpack≥ 0.13.0 and lifecycle ≥ 0.9. Platform API 0.4 changes how processes are requested in an image:/cnb/lifecycle/launcher, and the process is set via aCNB_PROCESS_TYPEenvironment variable (defaulting towebif not present)/cnb/process/xxxwhich are symlinks to/cnb/lifecycle/launcher. When executed, they cause the corresponding process to be run. The default process is the entrypoint (e.g.,/cnb/process/web).The Platform API version is set via the new
CNB_PLATFORM_APIenvironment variable.User facing changes (remove if N/A)
(This doesn't actually have any user-facing changes as to this point we still link with pack 0.12 and so create images with CNB Platform API 0.3.)