Skip to content

Commit

Permalink
[Hotfix] Always ignore command and args defined in Devfile contai…
Browse files Browse the repository at this point in the history
…ner components (#5639)

* Reproduce the issue via a test case with `odo dev`

* Always ignore `command` and `args` fields in Devfile container components

This is a hotfix for [1], until a proper fix is implemented, which would
consist in keeping using supervisord as pid1, and executing the command
with args "on the side" as a separate process.

[1] #5620
  • Loading branch information
rm3l authored Apr 8, 2022
1 parent 7c62587 commit 13bbc37
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 14 deletions.
22 changes: 14 additions & 8 deletions pkg/devfile/adapters/kubernetes/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ func UpdateContainersWithSupervisord(devfileObj devfileParser.DevfileObj, contai
// Check if the container belongs to a run command component
if container.Name == runCommand.Exec.Component {
// If the run component container has no entrypoint and arguments, override the entrypoint with supervisord
if len(container.Command) == 0 && len(container.Args) == 0 {
overrideContainerArgs(container)
}
overrideContainerArgs(container)

// Always mount the supervisord volume in the run component container
klog.V(2).Infof("Updating container %v with supervisord volume mounts", container.Name)
Expand Down Expand Up @@ -133,9 +131,7 @@ func UpdateContainersWithSupervisord(devfileObj devfileParser.DevfileObj, contai
// Check if the container belongs to a debug command component
if debugCommand.Exec != nil && container.Name == debugCommand.Exec.Component {
// If the debug component container has no entrypoint and arguments, override the entrypoint with supervisord
if len(container.Command) == 0 && len(container.Args) == 0 {
overrideContainerArgs(container)
}
overrideContainerArgs(container)

foundMountPath := false
for _, mounts := range container.VolumeMounts {
Expand Down Expand Up @@ -211,6 +207,16 @@ func UpdateContainersWithSupervisord(devfileObj devfileParser.DevfileObj, contai
// overrideContainerArgs overrides the container's entrypoint with supervisord
func overrideContainerArgs(container *corev1.Container) {
klog.V(2).Infof("Updating container %v entrypoint with supervisord", container.Name)
container.Command = append(container.Command, adaptersCommon.SupervisordBinaryPath)
container.Args = append(container.Args, "-c", adaptersCommon.SupervisordConfFile)
//TODO(#5620): overriding command and args, irrespective of whether the container had Command or Args defined in it.
// This is a hacky hotfix for https://github.com/redhat-developer/odo/issues/5620. Otherwise,
// odo does not work at all if Devfile container component defines a Command or an Args.
// As suggested in the issue, a proper fix will need to be implemented later on,
// for example by keeping using supervisord as pid1, and executing the command with args "on the side"
// as a separate process.
if len(container.Command) != 0 || len(container.Args) != 0 {
klog.V(4).Infof("original command and args for container %v intentionally ignored due to issue #5620",
container.Name)
}
container.Command = []string{adaptersCommon.SupervisordBinaryPath}
container.Args = []string{"-c", adaptersCommon.SupervisordConfFile}
}
16 changes: 10 additions & 6 deletions pkg/devfile/adapters/kubernetes/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,11 @@ func TestUpdateContainersWithSupervisord(t *testing.T) {
},
},
},
componentType: devfilev1.ContainerComponentType,
expectRunCommand: command,
isSupervisordEntrypoint: false,
componentType: devfilev1.ContainerComponentType,
expectRunCommand: command,
//TODO(#5620): Hotfix by ignoring Component Command and Args.
// A proper fix will need to be implemented later on.
isSupervisordEntrypoint: true,
wantErr: false,
},
{
Expand Down Expand Up @@ -269,9 +271,11 @@ func TestUpdateContainersWithSupervisord(t *testing.T) {
},
},
},
componentType: devfilev1.ContainerComponentType,
expectRunCommand: command,
isSupervisordEntrypoint: false,
componentType: devfilev1.ContainerComponentType,
expectRunCommand: command,
//TODO(#5620): Hotfix by ignoring Component Command and Args.
// A proper fix will need to be implemented later on.
isSupervisordEntrypoint: true,
wantErr: false,
},
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
schemaVersion: 2.0.0
metadata:
name: nodejs
version: 1.0.1
displayName: Node.js Runtime
description: Stack with Node.js 14
icon: https://nodejs.org/static/images/logos/nodejs-new-pantone-black.svg
tags: ['NodeJS', 'Express', 'ubi8']
projectType: 'nodejs'
language: 'javascript'
starterProjects:
- name: nodejs-starter
git:
remotes:
origin: 'https://github.com/odo-devfiles/nodejs-ex.git'
components:
- name: runtime
container:
image: registry.access.redhat.com/ubi8/nodejs-14:latest
command: ["tail"]
args: ["-f", "/dev/null"]
memoryLimit: 1024Mi
mountSources: true
endpoints:
- name: http-3000
targetPort: 3000
commands:
- id: install
exec:
component: runtime
commandLine: npm install
workingDir: ${PROJECT_SOURCE}
group:
kind: build
isDefault: true
- id: run
exec:
component: runtime
commandLine: npm start
workingDir: ${PROJECT_SOURCE}
group:
kind: run
isDefault: true
- id: debug
exec:
component: runtime
commandLine: npm run debug
workingDir: ${PROJECT_SOURCE}
group:
kind: debug
isDefault: true
- id: test
exec:
component: runtime
commandLine: npm test
workingDir: ${PROJECT_SOURCE}
group:
kind: test
isDefault: true
34 changes: 34 additions & 0 deletions tests/integration/devfile/cmd_dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/redhat-developer/odo/pkg/devfile/adapters/common"
segment "github.com/redhat-developer/odo/pkg/segment/context"
"github.com/redhat-developer/odo/pkg/util"

Expand Down Expand Up @@ -1248,4 +1249,37 @@ var _ = Describe("odo dev command tests", func() {
}
})
})

When("a container component defines a Command or Args", func() {
devfileCmpName := "nodejs"
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
helper.CopyExampleDevFile(
filepath.Join("source", "devfiles", "nodejs", "issue-5620-devfile-with-container-command-args.yaml"),
filepath.Join(commonVar.Context, "devfile.yaml"))
})

It("should run odo dev successfully (#5620)", func() {
devSession, stdoutBytes, stderrBytes, _, err := helper.StartDevMode()
Expect(err).ShouldNot(HaveOccurred())
defer devSession.Stop()
const errorMessage = "Failed to create the component:"
helper.DontMatchAllInOutput(string(stdoutBytes), []string{errorMessage})
helper.DontMatchAllInOutput(string(stderrBytes), []string{errorMessage})
//Check the processes managed by supervisord
for process, state := range map[string]string{"devrun": "RUNNING", "debugrun": "STOPPED"} {
commonVar.CliRunner.CheckCmdOpInRemoteDevfilePod(
commonVar.CliRunner.GetRunningPodNameByComponent(devfileCmpName, commonVar.Project),
"runtime",
commonVar.Project,
[]string{common.SupervisordBinaryPath, common.SupervisordCtlSubCommand, "status", process},
func(stdout string, err error) bool {
Expect(err).ShouldNot(HaveOccurred())
helper.MatchAllInOutput(stdout, []string{state})
return err == nil
},
)
}
})
})
})

0 comments on commit 13bbc37

Please sign in to comment.