Skip to content

Commit

Permalink
Fix Inspect command fails with Singularity
Browse files Browse the repository at this point in the history
Signed-off-by: Paolo Di Tommaso <[email protected]>
  • Loading branch information
pditommaso committed Oct 28, 2023
1 parent 171831e commit f5bb829
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 66 deletions.
12 changes: 2 additions & 10 deletions modules/nextflow/src/main/groovy/nextflow/cli/CmdInspect.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import com.beust.jcommander.Parameters
import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import nextflow.Session
import nextflow.container.inspect.ContainerInspectMode
import nextflow.container.inspect.ContainersInspector
import nextflow.exception.AbortOperationException
import nextflow.util.LoggerHelper
/**
* Implement `inspect` command
Expand Down Expand Up @@ -70,6 +70,7 @@ class CmdInspect extends CmdBase {

@Override
void run() {
ContainerInspectMode.activate(true)
// configure quiet mode
LoggerHelper.setQuiet(true)
// setup the target run command
Expand All @@ -89,8 +90,6 @@ class CmdInspect extends CmdBase {
}

protected void applyInspect(Session session) {
// disallow singularity w/o wave and freeze
checkSingularityConfig(session.config)
// disable wave await mode when running
if( session.config.wave instanceof Map )
checkWaveConfig(session.config.wave as Map)
Expand All @@ -106,11 +105,4 @@ class CmdInspect extends CmdBase {
wave.dryRun = !concretize
}

protected void checkSingularityConfig(Map config) {
final sing = config.navigate('singularity.enabled',false)
final wave = config.navigate('wave.enabled',false)
final freeze = config.navigate('wave.freeze',false)
if( sing && (!wave || !freeze) )
throw new AbortOperationException("Inspect command with Singularity requires enabling 'wave' and 'freeze' settings")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import java.util.regex.Pattern
import groovy.transform.CompileStatic
import groovy.transform.PackageScope
import groovy.util.logging.Slf4j
import nextflow.container.inspect.ContainerInspectMode
import nextflow.util.Escape
/**
* Helper class to normalise a container image name depending
Expand Down Expand Up @@ -67,7 +68,8 @@ class ContainerHandler {
if( !config.isEnabled() || !normalizedImageName )
return normalizedImageName
final requiresCaching = normalizedImageName =~ IMAGE_URL_PREFIX

if( ContainerInspectMode.active() && requiresCaching )
return imageName
final result = requiresCaching ? createSingularityCache(this.config, normalizedImageName) : normalizedImageName
return Escape.path(result)
}
Expand All @@ -76,14 +78,17 @@ class ContainerHandler {
if( !config.isEnabled() || !normalizedImageName )
return normalizedImageName
final requiresCaching = normalizedImageName =~ IMAGE_URL_PREFIX

if( ContainerInspectMode.active() && requiresCaching )
return imageName
final result = requiresCaching ? createApptainerCache(this.config, normalizedImageName) : normalizedImageName
return Escape.path(result)
}
if( engine == 'charliecloud' ) {
// if the imagename starts with '/' it's an absolute path
// otherwise we assume it's in a remote registry and pull it from there
final requiresCaching = !imageName.startsWith('/')
if( ContainerInspectMode.active() && requiresCaching )
return imageName
final result = requiresCaching ? createCharliecloudCache(this.config, imageName) : imageName
return Escape.path(result)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2013-2023, Seqera Labs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package nextflow.container.inspect

import groovy.transform.CompileStatic
/**
* Activate the container inspect mode
*
* @author Paolo Di Tommaso <[email protected]>
*/
@CompileStatic
class ContainerInspectMode {

private static boolean active

static boolean active() { return active }

static void activate(boolean value) { active=value }

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

package nextflow.cli

import nextflow.exception.AbortOperationException

import spock.lang.Specification
import spock.lang.Unroll
/**
Expand Down Expand Up @@ -51,41 +51,4 @@ class CmdInspectTest extends Specification {

}

def 'should allow singularity only with wave and freeze' () {
given:
def cmd = Spy(new CmdInspect())

when:
cmd.checkSingularityConfig([:])
then:
noExceptionThrown()

when:
cmd.checkSingularityConfig([singularity:[enabled:false]])
then:
noExceptionThrown()

when:
cmd.checkSingularityConfig([wave:[enabled:true, freeze:true]])
then:
noExceptionThrown()

when:
cmd.checkSingularityConfig([singularity:[enabled:true], wave:[enabled:true, freeze:true]])
then:
noExceptionThrown()

// singularity w/o wave and freeze is not allowed
when:
cmd.checkSingularityConfig([singularity:[enabled:true]])
then:
thrown(AbortOperationException)

when:
cmd.checkSingularityConfig([singularity:[enabled:true], wave:[enabled:true]])
then:
thrown(AbortOperationException)

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ class WaveAssets {
}


static void validateContainerRepo(String name) {
static void validateContainerName(String name) {
if( !name )
return
final scheme = StringUtils.getUrlProtocol(name)
if( scheme )
throw new IllegalArgumentException("Container repository should not start with URL like prefix - offending value: $name")
throw new IllegalArgumentException("Wave container request image cannot start with URL like prefix - offending value: $name")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import io.seqera.wave.plugin.exception.UnauthorizedException
import io.seqera.wave.plugin.packer.Packer
import nextflow.Session
import nextflow.SysEnv
import nextflow.container.inspect.ContainerInspectMode
import nextflow.container.resolver.ContainerInfo
import nextflow.fusion.FusionConfig
import nextflow.processor.Architecture
Expand Down Expand Up @@ -184,7 +185,7 @@ class WaveClient {
fingerprint: assets.fingerprint(),
freeze: config.freezeMode(),
format: assets.singularity ? 'sif' : null,
dryRun: config.dryRun()
dryRun: ContainerInspectMode.active()
)
}

Expand All @@ -208,7 +209,7 @@ class WaveClient {
towerEndpoint: tower.endpoint,
workflowId: tower.workflowId,
freeze: config.freezeMode(),
dryRun: config.dryRun(),
dryRun: ContainerInspectMode.active(),
)
return sendRequest(request)
}
Expand Down Expand Up @@ -489,7 +490,7 @@ class WaveClient {
final platform = dockerArch

// check is a valid container image
WaveAssets.validateContainerRepo(containerImage)
WaveAssets.validateContainerName(containerImage)
// read the container config and go ahead
final containerConfig = this.resolveContainerConfig(platform)
return new WaveAssets(
Expand Down Expand Up @@ -521,7 +522,7 @@ class WaveClient {
// get from cache or submit a new request
final response = cache.get(key, { sendRequest(assets) } as Callable )
if( config.freezeMode() ) {
if( response.buildId && !config.dryRun() ) {
if( response.buildId && !ContainerInspectMode.active() ) {
// await the image to be available when a new image is being built
awaitImage(response.targetImage)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,11 @@ class WaveConfig {
final private RetryOpts retryOpts
final private HttpOpts httpClientOpts
final private Boolean freezeMode
final private Boolean dryRunMode

WaveConfig(Map opts, Map<String,String> env=System.getenv()) {
this.enabled = opts.enabled
this.endpoint = (opts.endpoint?.toString() ?: env.get('WAVE_API_ENDPOINT') ?: DEF_ENDPOINT)?.stripEnd('/')
this.freezeMode = opts.freeze as Boolean
this.dryRunMode = opts.navigate('dryRun', false)
this.containerConfigUrl = parseConfig(opts, env)
this.tokensCacheMaxDuration = opts.navigate('tokens.cache.maxDuration', '30m') as Duration
this.condaOpts = opts.navigate('build.conda', Collections.emptyMap()) as CondaOpts
Expand Down Expand Up @@ -87,8 +85,6 @@ class WaveConfig {

boolean freezeMode() { return this.freezeMode }

boolean dryRun() { return this.dryRunMode }

boolean bundleProjectResources() { bundleProjectResources }

String buildRepository() { buildRepository }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,22 @@ class WaveAssetsTest extends Specification {

def 'should validate container name' () {
when:
WaveAssets.validateContainerRepo('ubuntu')
WaveAssets.validateContainerName('ubuntu')
then:
noExceptionThrown()

when:
WaveAssets.validateContainerRepo('ubuntu:latest')
WaveAssets.validateContainerName('ubuntu:latest')
then:
noExceptionThrown()

when:
WaveAssets.validateContainerRepo('quay.io/wtsicgp/nanoseq:3.3.0')
WaveAssets.validateContainerName('quay.io/wtsicgp/nanoseq:3.3.0')
then:
noExceptionThrown()

when:
WaveAssets.validateContainerRepo('docker://quay.io/wtsicgp/nanoseq:3.3.0')
WaveAssets.validateContainerName('docker://quay.io/wtsicgp/nanoseq:3.3.0')
then:
thrown(IllegalArgumentException)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import nextflow.Session
import nextflow.SysEnv
import nextflow.container.inspect.ContainerInspectMode
import nextflow.container.inspect.ContainersInspector
import nextflow.extension.FilesEx
import nextflow.file.FileHelper
import nextflow.processor.TaskRun
Expand Down Expand Up @@ -197,7 +199,8 @@ class WaveClientTest extends Specification {

def 'should create request object with dry-run mode' () {
given:
def session = Mock(Session) { getConfig() >> [wave:[dryRun:true]]}
ContainerInspectMode.activate(true)
def session = Mock(Session) { getConfig() >> [:]}
def IMAGE = 'foo:latest'
def wave = new WaveClient(session)

Expand All @@ -215,6 +218,9 @@ class WaveClientTest extends Specification {
and:
req.fingerprint == 'bd2cb4b32df41f2d290ce2366609f2ad'
req.timestamp instanceof String

cleanup:
ContainerInspectMode.activate(false)
}

def 'should create request object and platform' () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class WaveConfigTest extends Specification {
given:
def config = new WaveConfig([enabled: true])
expect:
config.toString() == 'WaveConfig(enabled:true, endpoint:https://wave.seqera.io, containerConfigUrl:[], tokensCacheMaxDuration:30m, condaOpts:CondaOpts(mambaImage=mambaorg/micromamba:1.5.1; basePackages=conda-forge::procps-ng, commands=null), spackOpts:SpackOpts(basePackages=null, commands=null), strategy:[container, dockerfile, conda, spack], bundleProjectResources:null, buildRepository:null, cacheRepository:null, retryOpts:RetryOpts(delay:450ms, maxDelay:1m 30s, maxAttempts:10, jitter:0.25), httpClientOpts:HttpOpts(), freezeMode:null, dryRunMode:false)'
config.toString() == 'WaveConfig(enabled:true, endpoint:https://wave.seqera.io, containerConfigUrl:[], tokensCacheMaxDuration:30m, condaOpts:CondaOpts(mambaImage=mambaorg/micromamba:1.5.1; basePackages=conda-forge::procps-ng, commands=null), spackOpts:SpackOpts(basePackages=null, commands=null), strategy:[container, dockerfile, conda, spack], bundleProjectResources:null, buildRepository:null, cacheRepository:null, retryOpts:RetryOpts(delay:450ms, maxDelay:1m 30s, maxAttempts:10, jitter:0.25), httpClientOpts:HttpOpts(), freezeMode:null)'
}

def 'should not allow invalid settinga' () {
Expand Down

0 comments on commit f5bb829

Please sign in to comment.