[JENKINS-39748] provide Image.exec() to avoid disabling ENTRYPOINT in the image #78
Conversation
…le ENTRYPOINT in the image
hogarthj
force-pushed
the
jenkins_37987
branch
from
e2850e0
to
35ef45a
Compare
hogarthj
changed the title
|
Any chance this could be reviewed and merged? |
| @@ -53,7 +53,7 @@ | |||
|
|
|||
| /** | |||
| * Simple docker client for Pipeline. | |||
| * | |||
| * | |||
There was a problem hiding this comment.
Please avoid making extraneous diff hunks, particular whitespace changes. You should be able to configure popular editors to not touch lines of code unless and until you edit them.
| @@ -125,6 +125,17 @@ class Docker implements Serializable { | |||
| } | |||
| } | |||
|
|
|||
| public <V> V exec(String args = '', Closure<V> body) { | |||
| docker.node { | |||
| if (docker.script.sh(script: "docker inspect -f . ${id}", returnStatus: true) != 0) { | |||
| WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); | ||
| } | ||
| }); | ||
| story.addStep(new Statement() { |
| @@ -116,7 +116,11 @@ public String run(@Nonnull EnvVars launchEnv, @Nonnull String image, @CheckForNu | |||
| argb.add("-e"); | |||
| argb.addMasked(variable.getKey()+"="+variable.getValue()); | |||
| } | |||
| argb.add("--entrypoint").add(entrypoint).add(image); | |||
| if (!entrypoint.equals("--")) { | |||
There was a problem hiding this comment.
Better to make entrypoint be @CheckForNull.
|
|
||
| container = dockerClient.run(env, step.image, step.args, ws, volumes, volumesFromContainers, envReduced, dockerClient.whoAmI(), /* expected to hang until killed */ "cat"); | ||
| if (step.entrypoint == null) { | ||
| step.entrypoint = "cat"; |
There was a problem hiding this comment.
See the developer guide: better to initialize the field with the "cat" value, have the setter call Util.fixEmpty, and have Image.exec specify entrypoint: ''. Also be sure to adjust WithContainerStep/config.jelly to specify
<f:entry field="entrypoint" title="…">
<f:textbox default="cat"/> <!-- or ${descriptor.defaultEntrypoint}, if defined -->
<f:entry>and add help-entrypoint.html.
But anyway better would probably be to change this to a boolean field, since
- there is no use case for doing anything other than leaving the image along, or overriding its entrypoint to go to sleep
- there is a good reason to consider replacing
catwith something likesleep infinity(except with a more POSIX-compliant argument)
The special techniques for step default values are the same for boolean parameters, but if you pick the semantics so that the default is in fact false, it is somewhat easier to write:
<f:checkbox/>rather than
<f:checkbox default="true"/>and a simpler form in the Java class.
|
Alternately, JENKINS-40170. |
There were comments on JENKINS-37987 that the change to overriding ENTRYPOINT on Image.inside() (#63) broke workflows.
On the way to JENKINS-26179 this was a fairly simple change whilst heading towards a Container.exec()
This adds a new Image.exec() {} which behaves identically to Image.inside() {} with the one exception that it does not override ENTRYPOINT so images that rely on ENTRYPOINT not being messed with can still be used.