From 25883df3994e1ca8939ae3447279449897fad181 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Sat, 28 Oct 2023 07:34:24 +0200 Subject: [PATCH] Fix Inspect command fails with Singularity Signed-off-by: Paolo Di Tommaso --- .../groovy/nextflow/cli/CmdInspect.groovy | 12 +----- .../container/ContainerHandler.groovy | 9 ++++- .../inspect/ContainerInspectMode.groovy | 35 +++++++++++++++++ .../groovy/nextflow/cli/CmdInspectTest.groovy | 39 +------------------ .../io/seqera/wave/plugin/WaveAssets.groovy | 4 +- .../io/seqera/wave/plugin/WaveClient.groovy | 9 +++-- .../wave/plugin/config/WaveConfig.groovy | 4 -- .../seqera/wave/plugin/WaveAssetsTest.groovy | 8 ++-- .../seqera/wave/plugin/WaveClientTest.groovy | 8 +++- .../wave/plugin/config/WaveConfigTest.groovy | 2 +- 10 files changed, 64 insertions(+), 66 deletions(-) create mode 100644 modules/nextflow/src/main/groovy/nextflow/container/inspect/ContainerInspectMode.groovy diff --git a/modules/nextflow/src/main/groovy/nextflow/cli/CmdInspect.groovy b/modules/nextflow/src/main/groovy/nextflow/cli/CmdInspect.groovy index 00c943e83b..c865c8aee8 100644 --- a/modules/nextflow/src/main/groovy/nextflow/cli/CmdInspect.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/cli/CmdInspect.groovy @@ -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 @@ -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 @@ -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) @@ -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") - } } diff --git a/modules/nextflow/src/main/groovy/nextflow/container/ContainerHandler.groovy b/modules/nextflow/src/main/groovy/nextflow/container/ContainerHandler.groovy index 2e446fc86c..aafa544913 100644 --- a/modules/nextflow/src/main/groovy/nextflow/container/ContainerHandler.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/container/ContainerHandler.groovy @@ -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 @@ -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) } @@ -76,7 +78,8 @@ 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) } @@ -84,6 +87,8 @@ class ContainerHandler { // 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) } diff --git a/modules/nextflow/src/main/groovy/nextflow/container/inspect/ContainerInspectMode.groovy b/modules/nextflow/src/main/groovy/nextflow/container/inspect/ContainerInspectMode.groovy new file mode 100644 index 0000000000..593736c62f --- /dev/null +++ b/modules/nextflow/src/main/groovy/nextflow/container/inspect/ContainerInspectMode.groovy @@ -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 + */ +@CompileStatic +class ContainerInspectMode { + + private static boolean active + + static boolean active() { return active } + + static void activate(boolean value) { active=value } + +} diff --git a/modules/nextflow/src/test/groovy/nextflow/cli/CmdInspectTest.groovy b/modules/nextflow/src/test/groovy/nextflow/cli/CmdInspectTest.groovy index 2e45bae5c8..c6ed93ca74 100644 --- a/modules/nextflow/src/test/groovy/nextflow/cli/CmdInspectTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/cli/CmdInspectTest.groovy @@ -17,7 +17,7 @@ package nextflow.cli -import nextflow.exception.AbortOperationException + import spock.lang.Specification import spock.lang.Unroll /** @@ -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) - - } - } diff --git a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveAssets.groovy b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveAssets.groovy index 2b7e6fb6e7..1810791a90 100644 --- a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveAssets.groovy +++ b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveAssets.groovy @@ -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") } } diff --git a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy index c913ef99fb..7e4b4f7487 100644 --- a/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy +++ b/plugins/nf-wave/src/main/io/seqera/wave/plugin/WaveClient.groovy @@ -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 @@ -184,7 +185,7 @@ class WaveClient { fingerprint: assets.fingerprint(), freeze: config.freezeMode(), format: assets.singularity ? 'sif' : null, - dryRun: config.dryRun() + dryRun: ContainerInspectMode.active() ) } @@ -208,7 +209,7 @@ class WaveClient { towerEndpoint: tower.endpoint, workflowId: tower.workflowId, freeze: config.freezeMode(), - dryRun: config.dryRun(), + dryRun: ContainerInspectMode.active(), ) return sendRequest(request) } @@ -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( @@ -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) } diff --git a/plugins/nf-wave/src/main/io/seqera/wave/plugin/config/WaveConfig.groovy b/plugins/nf-wave/src/main/io/seqera/wave/plugin/config/WaveConfig.groovy index 6e00733de2..5a47010a78 100644 --- a/plugins/nf-wave/src/main/io/seqera/wave/plugin/config/WaveConfig.groovy +++ b/plugins/nf-wave/src/main/io/seqera/wave/plugin/config/WaveConfig.groovy @@ -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 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 @@ -87,8 +85,6 @@ class WaveConfig { boolean freezeMode() { return this.freezeMode } - boolean dryRun() { return this.dryRunMode } - boolean bundleProjectResources() { bundleProjectResources } String buildRepository() { buildRepository } diff --git a/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveAssetsTest.groovy b/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveAssetsTest.groovy index 8ecf593e61..8844af0328 100644 --- a/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveAssetsTest.groovy +++ b/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveAssetsTest.groovy @@ -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) } diff --git a/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveClientTest.groovy b/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveClientTest.groovy index 3885ce2cf1..c0259af468 100644 --- a/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveClientTest.groovy +++ b/plugins/nf-wave/src/test/io/seqera/wave/plugin/WaveClientTest.groovy @@ -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 @@ -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) @@ -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' () { diff --git a/plugins/nf-wave/src/test/io/seqera/wave/plugin/config/WaveConfigTest.groovy b/plugins/nf-wave/src/test/io/seqera/wave/plugin/config/WaveConfigTest.groovy index 567fd0678d..a904258b2f 100644 --- a/plugins/nf-wave/src/test/io/seqera/wave/plugin/config/WaveConfigTest.groovy +++ b/plugins/nf-wave/src/test/io/seqera/wave/plugin/config/WaveConfigTest.groovy @@ -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' () {