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

Debug should report CNB_APP_DIR as working directory for buildpacks images #4326

Merged
merged 5 commits into from
Jun 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/skaffold/debug/cnb.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,16 @@ func updateForCNBImage(container *v1.Container, ic imageConfiguration, transform
return ContainerDebugConfiguration{}, "", fmt.Errorf("buildpacks metadata has no processes")
}

// The buildpacks launcher is retained as the entrypoint
// The CNB launcher is retained as the entrypoint.
ic, rewriter := adjustCommandLine(m, ic)

// The CNB launcher uses CNB_APP_DIR (defaults to /workspace) and ignores the image's working directory.
if appDir := ic.env["CNB_APP_DIR"]; appDir != "" {
ic.workingDir = appDir
} else {
ic.workingDir = "/workspace"
}

c, img, err := transformer(container, ic)

// Only rewrite the container.Args if set: some transforms only alter env vars,
Expand Down
30 changes: 27 additions & 3 deletions pkg/skaffold/debug/cnb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func TestUpdateForCNBImage(t *testing.T) {
input imageConfiguration
shouldErr bool
expected v1.Container
config ContainerDebugConfiguration
}{
{
description: "error when missing build.metadata",
Expand All @@ -65,48 +66,70 @@ func TestUpdateForCNBImage(t *testing.T) {
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, arguments: []string{"--", "web", "arg1", "arg2"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"--", "web", "arg1", "arg2"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "defaults to web process when no process type",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "resolves to default 'web' process",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "CNB_PROCESS_TYPE=web",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "web"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"webProcess", "webArg1", "webArg2"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "CNB_PROCESS_TYPE=diag",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "diag"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"diagProcess"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "CNB_PROCESS_TYPE=direct",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, env: map[string]string{"CNB_PROCESS_TYPE": "direct"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"--", "command", "cmdArg1"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "script command-line",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, arguments: []string{"python main.py"}, labels: map[string]string{"io.buildpacks.build.metadata": mdJSON}},
shouldErr: false,
expected: v1.Container{Args: []string{"python main.py"}},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "no process and no args",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdndJSON}},
shouldErr: false,
expected: v1.Container{},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "launcher ignores image's working dir",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdndJSON}, workingDir: "/workdir"},
shouldErr: false,
expected: v1.Container{},
config: ContainerDebugConfiguration{WorkingDir: "/workspace"},
},
{
description: "CNB_APP_DIR used if set",
input: imageConfiguration{entrypoint: []string{"/cnb/lifecycle/launcher"}, labels: map[string]string{"io.buildpacks.build.metadata": mdndJSON}, env: map[string]string{"CNB_APP_DIR": "/appDir"}, workingDir: "/workdir"},
shouldErr: false,
expected: v1.Container{},
config: ContainerDebugConfiguration{WorkingDir: "/appDir"},
},
}
for _, test := range tests {
Expand All @@ -115,17 +138,18 @@ func TestUpdateForCNBImage(t *testing.T) {
testutil.Run(t, test.description+" (args changed)", func(t *testutil.T) {
argsChangedTransform := func(c *v1.Container, ic imageConfiguration) (ContainerDebugConfiguration, string, error) {
c.Args = ic.arguments
return ContainerDebugConfiguration{}, "", nil
return ContainerDebugConfiguration{WorkingDir: ic.workingDir}, "", nil
}
copy := v1.Container{}
_, _, err := updateForCNBImage(&copy, test.input, argsChangedTransform)
c, _, err := updateForCNBImage(&copy, test.input, argsChangedTransform)
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, copy)
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.config, c)
})

// Test that when the arguments are left unchanged, that the container is unchanged
testutil.Run(t, test.description+" (args unchanged)", func(t *testutil.T) {
argsUnchangedTransform := func(c *v1.Container, ic imageConfiguration) (ContainerDebugConfiguration, string, error) {
return ContainerDebugConfiguration{}, "", nil
return ContainerDebugConfiguration{WorkingDir: ic.workingDir}, "", nil
}

copy := v1.Container{}
Expand Down
5 changes: 4 additions & 1 deletion pkg/skaffold/debug/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import (
)

// ContainerDebugConfiguration captures debugging information for a specific container.
// This structure is serialized out and included in the pod metadata.
type ContainerDebugConfiguration struct {
// Artifact is the corresponding artifact's image name used in the skaffold.yaml
Artifact string `json:"artifact,omitempty"`
Expand Down Expand Up @@ -222,7 +223,9 @@ func transformPodSpec(metadata *metav1.ObjectMeta, podSpec *v1.PodSpec, retrieve
// `err != nil` means that the container did not or could not be transformed
if configuration, requiredImage, err := transformContainer(container, imageConfig, portAlloc); err == nil {
configuration.Artifact = imageConfig.artifact
configuration.WorkingDir = imageConfig.workingDir
if configuration.WorkingDir == "" {
configuration.WorkingDir = imageConfig.workingDir
}
configurations[container.Name] = configuration
if len(requiredImage) > 0 {
logrus.Infof("%q requires debugging support image %q", container.Name, requiredImage)
Expand Down