Improve Charliecloud registry handling (#5018) [ci fast]

Signed-off-by: Niklas Schandry <niklas@bio.lmu.de>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: Dr Marco Claudio De La Pierre <marco.delapierre@gmail.com>

docs/config.md

@@ -476,6 +476,7 @@ The following settings are available:
 ### Scope `charliecloud`
 
 The `charliecloud` scope controls how [Charliecloud](https://hpc.github.io/charliecloud/) containers are executed by Nextflow.
+If `charliecloud.writeFake` is unset / `false`, charliecloud will create a copy of the container in the process working directory.
 
 The following settings are available:
 
@@ -497,6 +498,15 @@ The following settings are available:
 `charliecloud.temp`
 : Mounts a path of your choice as the `/tmp` directory in the container. Use the special value `auto` to create a temporary directory each time a container is created.
 
+`charliecloud.registry`
+: The registry from where images are pulled. It should be only used to specify a private registry server. It should NOT include the protocol prefix i.e. `http://`.
+
+`charliecloud.writeFake`
+: Enable `writeFake` with charliecloud. This allows to run containers from storage in writeable mode, using overlayfs, see [charliecloud documentation](https://hpc.github.io/charliecloud/ch-run.html#ch-run-overlay) for details 
+
+`charliecloud.useSquash`
+: Create a temporary squashFS container image in the process work directory instead of a folder.
+
 Read the {ref}`container-charliecloud` page to learn more about how to use Charliecloud containers with Nextflow.
 
 (config-conda)=
@@ -2126,4 +2136,4 @@ Some features can be enabled using the `nextflow.enable` and `nextflow.preview`
 
 : *Experimental: may change in a future release.*
 
-: When `true`, enables {ref}`topic channels <channel-topic>` feature.
+: When `true`, enables {ref}`topic channels <channel-topic>` feature.

modules/nextflow/src/main/groovy/nextflow/container/CharliecloudBuilder.groovy

@@ -27,6 +27,7 @@ import groovy.util.logging.Slf4j
  * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
  * @author Patrick Hüther <patrick.huether@gmail.com>
  * @author Laurent Modolo <laurent.modolo@ens-lyon.fr>
+ * @author Niklas Schandry <niklas@bio.lmu.de>
  */
 @CompileStatic
 @Slf4j
@@ -165,4 +166,4 @@ class CharliecloudBuilder extends ContainerBuilder<CharliecloudBuilder> {
 
         return result
     }
-}
+}

modules/nextflow/src/main/groovy/nextflow/container/CharliecloudCache.groovy

@@ -34,6 +34,7 @@ import nextflow.util.Duration
  *
  * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
  * @author Patrick Hüther <patrick.huether@gmail.com>
+ * @author Niklas Schandry <niklas@bio.lmu.de>
  */
 @Slf4j
 @CompileStatic
@@ -79,11 +80,15 @@ class CharliecloudCache {
     String simpleName(String imageUrl) {
         def p = imageUrl.indexOf('://')
         def name = p != -1 ? imageUrl.substring(p+3) : imageUrl
-
+        
         // add registry
-        if( registry )
+        if( registry ) {
+            if( !registry.endsWith('/') ) {
+                registry += '/'
+            }
             name = registry + name
-
+        }
+
         name = name.replace(':','+').replace('/','%')
         return name 
     }
@@ -207,8 +212,9 @@ class CharliecloudCache {
         if( missingCacheDir )
             log.warn1 "Charliecloud cache directory has not been defined -- Remote image will be stored in the path: $targetPath.parent.parent -- Use the charliecloud.cacheDir config option or set the NXF_CHARLIECLOUD_CACHEDIR variable to specify a different location"
 
+
         log.info "Charliecloud pulling image $imageUrl [cache $targetPath]"
-
+            
         String cmd = "ch-image pull -s $targetPath.parent.parent $imageUrl > /dev/null"
         try {
             runCommand( cmd, targetPath )
@@ -296,4 +302,4 @@ class CharliecloudCache {
         return result
     }
 
-}
+}

modules/nextflow/src/main/groovy/nextflow/container/ContainerHandler.groovy

@@ -88,12 +88,15 @@ class ContainerHandler {
             return Escape.path(result)
         }
         if( engine == 'charliecloud' ) {
+            final normalizedImageName = normalizeCharliecloudImageName(imageName)
+            if( !config.isEnabled() || !normalizedImageName )
+                return normalizedImageName
             // 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
+            final result = requiresCaching ? createCharliecloudCache(this.config, normalizedImageName) : normalizedImageName
             return Escape.path(result)
         }
         // fallback to docker
@@ -271,4 +274,38 @@ class ContainerHandler {
         // prefix it with the `docker://` pseudo protocol used by apptainer to download it
         return "docker://${normalizeDockerImageName(img)}"
     }
-}
+
+    /**
+     * Normalize charliecloud image name resolving the absolute path
+     *
+     * @param imageName The container image name
+     * @return Image name in canonical format
+     */
+     @PackageScope
+     String normalizeCharliecloudImageName(String img) {
+        if( !img )
+            return null
+
+        // when starts with `/` it's an absolute image file path, just return it
+        if( img.startsWith("/") ) {
+            return img
+        }
+        // remove docker:// if present
+        if( img.startsWith("docker://") ) {
+            img = img.minus("docker://")
+        }
+        // if no tag, add :latest
+        if( !img.contains(':') ) {
+            img += ':latest'
+        }
+
+        // if it's the path of an existing image file return it
+        def imagePath = baseDir.resolve(img)
+        if( imagePath.exists() ) {
+            return imagePath.toString()
+        }
+
+        // in all other case it's supposed to be the name of an image
+        return "${normalizeDockerImageName(img)}"
+    }
+}

modules/nextflow/src/test/groovy/nextflow/container/CharliecloudCacheTest.groovy

@@ -29,7 +29,6 @@ import spock.lang.Unroll
  * @author Patrick Hüther <patrick.huether@gmail.com>
  */
 class CharliecloudCacheTest extends Specification {
-
     @Unroll
     def 'should return a simple name given an image url'() {
 
@@ -48,6 +47,21 @@ class CharliecloudCacheTest extends Specification {
         'foo:bar'                   | 'foo+bar'
     }
 
+    @Unroll
+    def 'should return a path with registry'() {
+
+        given:
+        def helper = new CharliecloudCache([registry: registry])
+
+        expect:
+        helper.simpleName(url) == expected
+
+        where:
+        url                      | registry   | expected
+        'foo:2.0'                | 'my.reg'   | 'my.reg%foo+2.0'
+        'foo:2.0'                | 'my.reg/'  | 'my.reg%foo+2.0'
+    }
+
     def 'should return the cache dir from the config file' () {
 
         given:

modules/nextflow/src/test/groovy/nextflow/container/ContainerHandlerTest.groovy

@@ -201,6 +201,35 @@ class ContainerHandlerTest extends Specification {
 
     }
 
+    @Unroll
+    def 'test normalize method for charliecloud' () {
+
+       given:
+        def n = new ContainerHandler([registry: registry])
+
+        expect:
+        n.normalizeCharliecloudImageName(image) == expected
+
+        where:
+        image                      | registry   | expected
+        null                       | null       | null
+        ''                         | null       | null
+        '/abs/path/bar.img'        | null       | '/abs/path/bar.img'
+        'docker://library/busybox' | null       | 'library/busybox:latest'
+        'shub://busybox'           | null       | 'shub://busybox'
+        'foo://busybox'            | null       | 'foo://busybox'
+        'foo'                      | null       | 'foo:latest'
+        'foo:2.0'                  | null       | 'foo:2.0'
+        'foo.img'                  | null       | 'foo.img:latest'
+        'quay.io/busybox'          | null       | 'quay.io/busybox:latest'
+        'http://reg.io/v1/alpine:latest'        | null       | 'http://reg.io/v1/alpine:latest'
+        'https://reg.io/v1/alpine:latest'       | null       | 'https://reg.io/v1/alpine:latest'
+        and:
+        '/abs/path/bar.img'        | 'my.reg'  | '/abs/path/bar.img'
+        'busybox'                  | 'my.reg'  | 'my.reg/busybox:latest'
+        'foo:2.0'                  | 'my.reg'  | 'my.reg/foo:2.0'
+    }
+
     @Unroll
     def 'test normalize method for singularity' () {
         given: