Skip to content

Commit

Permalink
Add K8s honourEntrypoint option
Browse files Browse the repository at this point in the history
By default K8s launcher uses the  `command` pod attribute
to run the task and therefore overrides the container entrypoint.

When the option `k8s.honourEntrypoint` is set to true, the launcher
uses instead the pod `args` attribute without altering the container
entrypoint.
  • Loading branch information
pditommaso committed Apr 2, 2022
1 parent cd95e29 commit bfbfe24
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 8 deletions.
14 changes: 14 additions & 0 deletions modules/nextflow/src/main/groovy/nextflow/k8s/K8sConfig.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,20 @@ class K8sConfig implements Map<String,Object> {
target.storageSubPath
}

/**
* Whenever the pod should honour the entrypoint defined by the image (default: false)
*
* @return When {@code false} the launcher script is launched by using pod `command` attributes which
* overrides the entrypoint point eventually defined by the image.
*
* When {@code true} the launcher is launched via the pod `args` attribute, without altering the
* container entrypoint (it does however require to have a bash shell as the image entrypoint)
*
*/
boolean honourEntrypoint() {
return target.honourEntrypoint
}

/**
* @return the path where the workflow is launched and the user data is stored
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,27 @@ class K8sTaskHandler extends TaskHandler {
}
}

protected boolean honourEntrypoint() {
return executor.getK8sConfig().honourEntrypoint()
}

protected Map newSubmitRequest0(TaskRun task, String imageName) {

final fixOwnership = builder.fixOwnership()
final cmd = new ArrayList(new ArrayList(BashWrapperBuilder.BASH)) << TaskRun.CMD_RUN
final launcher = new ArrayList(new ArrayList(BashWrapperBuilder.BASH)) << TaskRun.CMD_RUN
final taskCfg = task.getConfig()
// by default launcher scripts use `command` and therefore overrides the container entrypoint
// when `honourEntrypoint` is true use `args` instead of `command` to keep using the container entrypoint
final entry = honourEntrypoint()
final cmd = entry ? null : launcher
final args = entry ? launcher : null

final clientConfig = client.config
final builder = new PodSpecBuilder()
.withImageName(imageName)
.withPodName(getSyntheticPodName(task))
.withCommand(cmd)
.withArgs(args)
.withWorkDir(task.workDir)
.withNamespace(clientConfig.namespace)
.withServiceAccount(clientConfig.serviceAccount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class PodSpecBuilder {

List<String> command = []

List<String> args = new ArrayList<>()

Map<String,String> labels = [:]

Map<String,String> annotations = [:]
Expand Down Expand Up @@ -130,11 +132,19 @@ class PodSpecBuilder {
}

PodSpecBuilder withCommand( cmd ) {
if( cmd==null ) return this
assert cmd instanceof List || cmd instanceof CharSequence, "Missing or invalid K8s command parameter: $cmd"
this.command = cmd instanceof List ? cmd : ['/bin/bash','-c', cmd.toString()]
return this
}

PodSpecBuilder withArgs( args ) {
if( args==null ) return this
assert args instanceof List || args instanceof CharSequence, "Missing or invalid K8s args parameter: $args"
this.args = args instanceof List ? args : ['/bin/bash','-c', args.toString()]
return this
}

PodSpecBuilder withCpus( Integer cpus ) {
this.cpus = cpus
return this
Expand Down Expand Up @@ -283,7 +293,7 @@ class PodSpecBuilder {
Map build() {
assert this.podName, 'Missing K8s podName parameter'
assert this.imageName, 'Missing K8s imageName parameter'
assert this.command, 'Missing K8s command parameter'
assert this.command || this.args, 'Missing K8s command parameter'

final restart = this.restart ?: 'Never'

Expand All @@ -303,12 +313,12 @@ class PodSpecBuilder {
if( this.memory )
res.memory = this.memory

final container = [
name: this.podName,
image: this.imageName,
command: this.command
]
final container = [ name: this.podName, image: this.imageName ]
if( this.command )
container.command = this.command
if( this.args )
container.args = args

if( this.workDir )
container.put('workingDir', workDir)

Expand Down
14 changes: 14 additions & 0 deletions modules/nextflow/src/test/groovy/nextflow/k8s/K8sConfigTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -336,4 +336,18 @@ class K8sConfigTest extends Specification {
then:
cfg.getPodOptions().getImagePullPolicy() == 'always'
}

def 'should set honour entrypoint setting'( ) {

when:
def cfg = new K8sConfig([:])
then:
!cfg.honourEntrypoint()

when:
cfg = new K8sConfig( honourEntrypoint: true )
then:
cfg.honourEntrypoint()

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,118 @@ class K8sTaskHandlerTest extends Specification {
PodSpecBuilder.VOLUMES.set(0)
}

def 'should return a new pod with args' () {
given:
def WORK_DIR = Paths.get('/some/work/dir')
def config = Mock(TaskConfig)
def task = Mock(TaskRun)
def client = Mock(K8sClient)
def builder = Mock(K8sWrapperBuilder)
def handler = Spy(K8sTaskHandler)
handler.builder = builder
handler.client = client
Map result

when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> true
1 * handler.getPodOptions() >> new PodOptions()
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getLabels(task) >> [:]
1 * handler.getAnnotations() >> [:]
1 * handler.getContainerMounts() >> []
1 * task.getContainer() >> 'debian:latest'
1 * task.getWorkDir() >> WORK_DIR
1 * task.getConfig() >> config
1 * config.getCpus() >> 0
1 * config.getMemory() >> null
1 * client.getConfig() >> new ClientConfig()
result == [ apiVersion: 'v1',
kind: 'Pod',
metadata: [
name:'nf-123',
namespace:'default'
],
spec: [
restartPolicy:'Never',
containers:[
[name:'nf-123',
image:'debian:latest',
args:['/bin/bash', '-ue','.command.run'],
workingDir:'/some/work/dir']
]
]
]

when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-foo'
1 * handler.getLabels(task) >> [sessionId:'xxx']
1 * handler.getAnnotations() >> [evict: 'false']
1 * handler.getPodOptions() >> new PodOptions()
1 * handler.getContainerMounts() >> []
1 * builder.fixOwnership() >> true
1 * handler.getOwner() >> '501:502'
1 * task.getContainer() >> 'debian:latest'
1 * task.getWorkDir() >> WORK_DIR
1 * task.getConfig() >> config
1 * config.getCpus() >> 1
1 * config.getMemory() >> null
1 * client.getConfig() >> new ClientConfig()
result == [ apiVersion: 'v1',
kind: 'Pod',
metadata: [name:'nf-foo', namespace:'default', labels: [sessionId: 'xxx'], annotations: [evict: 'false']],
spec: [
restartPolicy:'Never',
containers:[
[name:'nf-foo',
image:'debian:latest',
command:['/bin/bash', '-ue','.command.run'],
workingDir:'/some/work/dir',
resources:[ requests: [cpu:1], limits:[cpu:1] ],
env: [ [name:'NXF_OWNER', value:'501:502'] ]
]
]
]
]


when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-abc'
1 * handler.getLabels(task) >> [:]
1 * handler.getAnnotations() >> [:]
1 * handler.getPodOptions() >> new PodOptions()
1 * handler.getContainerMounts() >> []
1 * task.getContainer() >> 'user/alpine:1.0'
1 * task.getWorkDir() >> WORK_DIR
1 * task.getConfig() >> config
1 * config.getCpus() >> 4
1 * config.getMemory() >> MemoryUnit.of('16GB')
1 * client.getConfig() >> new ClientConfig(namespace: 'namespace-x')
result == [ apiVersion: 'v1',
kind: 'Pod',
metadata: [name:'nf-abc', namespace:'namespace-x' ],
spec: [
restartPolicy:'Never',
containers:[
[name:'nf-abc',
image:'user/alpine:1.0',
command:['/bin/bash', '-ue', '.command.run'],
workingDir:'/some/work/dir',
resources:[ requests: [cpu:4, memory:'16384Mi'], limits:[cpu:4, memory:'16384Mi'] ]
]
]
]
]

}

def 'should return a new pod request with no storage' () {
given:
def WORK_DIR = Paths.get('/some/work/dir')
Expand All @@ -63,6 +175,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getPodOptions() >> new PodOptions()
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getLabels(task) >> [foo: 'bar', hello: 'world']
Expand Down Expand Up @@ -96,6 +209,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-foo'
1 * handler.getLabels(task) >> [sessionId:'xxx']
1 * handler.getAnnotations() >> [evict: 'false']
Expand Down Expand Up @@ -130,6 +244,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-abc'
1 * handler.getLabels(task) >> [:]
1 * handler.getAnnotations() >> [:]
Expand Down Expand Up @@ -175,6 +290,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getPodOptions() >> new PodOptions()
1 * handler.getLabels(task) >> [:]
Expand Down Expand Up @@ -225,6 +341,7 @@ class K8sTaskHandlerTest extends Specification {
result = handler.newSubmitRequest(task)
then:
1 * client.getConfig() >> new ClientConfig()
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getLabels(task) >> [:]
1 * handler.getAnnotations() >> [:]
Expand Down Expand Up @@ -285,6 +402,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getContainerMounts() >> []
1 * handler.getLabels(task) >> [:]
Expand Down Expand Up @@ -327,6 +445,7 @@ class K8sTaskHandlerTest extends Specification {
when:
result = handler.newSubmitRequest(task)
then:
1 * handler.honourEntrypoint() >> false
1 * handler.getSyntheticPodName(task) >> 'nf-123'
1 * handler.getContainerMounts() >> ['/tmp', '/data']
1 * handler.getLabels(task) >> [:]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,61 @@ class PodSpecBuilderTest extends Specification {

}

def 'should create pod spec with args' () {

when:
def spec = new PodSpecBuilder()
.withPodName('foo')
.withImageName('busybox')
.withWorkDir('/some/work/dir')
.withArgs(['echo', 'hello'])
.build()

then:
spec == [ apiVersion: 'v1',
kind: 'Pod',
metadata: [name:'foo', namespace:'default'],
spec: [
restartPolicy:'Never',
containers:[
[name:'foo',
image:'busybox',
args:['echo', 'hello'],
workingDir:'/some/work/dir'
]
]
]
]

}

def 'should create pod spec with args string' () {

when:
def spec = new PodSpecBuilder()
.withPodName('foo')
.withImageName('busybox')
.withWorkDir('/some/work/dir')
.withArgs('echo foo')
.build()

then:
spec == [ apiVersion: 'v1',
kind: 'Pod',
metadata: [name:'foo', namespace:'default'],
spec: [
restartPolicy:'Never',
containers:[
[name:'foo',
image:'busybox',
args:['/bin/bash', '-c', 'echo foo'],
workingDir:'/some/work/dir'
]
]
]
]

}

def 'should set namespace, labels and annotations' () {

Expand Down

0 comments on commit bfbfe24

Please sign in to comment.