Fix docker hack

.github/workflows/run_tests.yml

@@ -53,18 +53,19 @@ jobs:
         python -c "import openproblems"
 
     - name: Build Docker images
+      if: "!startsWith(github.ref, 'refs/heads/main')"
       run: |
         cd workflow
-        snakemake -j $(grep -c processor /proc/cpuinfo) docker_build
+        snakemake -j $(nproc) docker_build
         cd ..
 
-    - name: Push Docker images
-      if: startsWith(github.ref, 'refs/heads/main')
+    - name: Build and push Docker images
+      if: "startsWith(github.ref, 'refs/heads/main')"
       env:
         DOCKER_PASSWORD: ${{ secrets.DOCKER_PASSWORD }}
       run: |
         cd workflow
-        snakemake -j $(grep -c processor /proc/cpuinfo) docker_push
+        snakemake -j $(nproc) docker_build docker_push
         cd ..
         echo "CHANGED=`git diff --exit-code > /dev/null && echo false || echo true`" >> $GITHUB_ENV
 
@@ -342,7 +343,7 @@ jobs:
         AWS_DEFAULT_REGION: us-west-2
         NXF_DEFAULT_DSL: 1
       run: |
-        RUN_NAME="$(echo "$BRANCH" | sed "s/[^a-z]//g")_$(git rev-parse --short HEAD)_${GITHUB_RUN_ATTEMPT}"
+        RUN_NAME="$(echo "$BRANCH" | sed "s/[^a-zA-Z0-9]//g")_$(git rev-parse --short HEAD)_${GITHUB_RUN_ATTEMPT}"
         cd /mnt/openproblems-nextflow/cwd/${BRANCH}
         nextflow run \
         -revision v1.2 \

workflow/Snakefile

@@ -26,18 +26,12 @@ rule docker_pull:
 rule docker_refresh:
     input: tools.refresh_images
 
-# this is a dirty hack
-if "docker_build" in sys.argv:
-	ruleorder: build_docker_image > refresh_docker_image
-else:
-	ruleorder: refresh_docker_image > build_docker_image
-
 rule refresh_docker_image:
     input:
         dockerfile = "{}/{{image}}/refresh.Dockerfile".format(tools.IMAGES_DIR),
+        requirements = tools.docker_refresh_requirements,
     output:
-        temp(touch("{}/{{image}}/.docker_refresh".format(tools.IMAGES_DIR))),
-        temp(touch("{}/{{image}}/.docker_update".format(tools.IMAGES_DIR)))
+        temp(touch("{}/{{image}}/.docker_refresh".format(tools.IMAGES_DIR)))
     params:
         sourcedir = os.path.dirname(tools.SCRIPTS_DIR),
         user = "singlecellopenproblems",
@@ -69,13 +63,18 @@ RUN cd /usr/src/singlecellopenproblems && sudo git clean -fxdq
 RUN sudo pip install --no-cache-dir --editable /usr/src/singlecellopenproblems
 ' > {output}"""
 
+rule update_docker_image:
+    input:
+        tools.docker_update_requirements
+    output:
+        temp(touch("{}/{{image}}/.docker_update".format(tools.IMAGES_DIR)))
+
 rule build_docker_image:
     input:
         dockerfile = "{}/{{image}}/Dockerfile".format(tools.IMAGES_DIR),
-        requirements = tools.docker_requirements,
+        requirements = tools.docker_build_requirements,
     output:
-        temp(touch("{}/{{image}}/.docker_build".format(tools.IMAGES_DIR))),
-        temp(touch("{}/{{image}}/.docker_update".format(tools.IMAGES_DIR)))
+        temp(touch("{}/{{image}}/.docker_build".format(tools.IMAGES_DIR)))
     params:
         sourcedir = os.path.dirname(tools.SCRIPTS_DIR),
         user = "singlecellopenproblems",
@@ -100,7 +99,7 @@ rule login_docker:
 
 rule push_docker_image:
     input:
-        build = tools.docker_push_requirements,
+        build = "{}/{{image}}/.docker_update".format(tools.IMAGES_DIR),
         login = ".docker_login",
     output:
         temp(touch("{}/{{image}}/.docker_push".format(tools.IMAGES_DIR)))

workflow/snakemake_tools.py

@@ -48,15 +48,6 @@ def build_type(wildcards):
     return "github_actions" if "GITHUB_ACTIONS" in os.environ else "local"
 
 
-def image_markers(wildcards):
-    """Get the appropriate marker for each image."""
-    return [
-        docker_image_marker(image)
-        for image in os.listdir(IMAGES_DIR)
-        if os.path.isdir(os.path.join(IMAGES_DIR, image))
-    ]
-
-
 def push_images(wildcards):
     """Get Docker push timestamp for all images."""
     images = _images(".docker_push")
@@ -321,21 +312,24 @@ def docker_image_marker(image, refresh=True):
     """Get the file to be created to ensure Docker image exists from the image name."""
     docker_path = os.path.join(IMAGES_DIR, image)
     # possible outputs
-    docker_refresh = os.path.join(docker_path, ".docker_update")
+    docker_refresh = os.path.join(docker_path, ".docker_refresh")
     dockerfile = os.path.join(docker_path, "Dockerfile")
     no_change = docker_refresh if refresh else dockerfile
     no_change_text = "refreshing source code only" if refresh else "no change"
     docker_build = os.path.join(docker_path, ".docker_build")
 
-    # inputs to conditional logic
-    local_imagespec_changed = docker_imagespec_changed(image, dockerfile)
-    local_codespec_changed = not version_not_changed()
-    if local_codespec_changed:
+    if "docker_build" in sys.argv:
+        # building everything from scratch
+        requirement_file = docker_build
+    elif not version_not_changed():
         print(
             "Code version changed: {}".format(openproblems.__version__), file=sys.stderr
         )
-    if local_imagespec_changed or local_codespec_changed:
-        # spec has changed, let's rebuild
+        # codebase has changed, let's rebuild
+        print("{}: rebuilding".format(image), file=sys.stderr)
+        requirement_file = docker_build
+    elif docker_imagespec_changed(image, dockerfile):
+        # image has changed, let's rebuild
         print("{}: rebuilding".format(image), file=sys.stderr)
         requirement_file = docker_build
     elif docker_image_exists(image, local=True) or docker_image_exists(
@@ -352,48 +346,46 @@ def docker_image_marker(image, refresh=True):
     return requirement_file
 
 
-def _docker_requirements(image, include_self=False, refresh=True):
+def _docker_requirements(image, refresh=True):
     """Get all files to ensure a Docker image is up to date from the image name."""
     docker_dir = os.path.join(IMAGES_DIR, image)
-    dockerfile = os.path.join(docker_dir, "Dockerfile")
-    requirements = [dockerfile]
+    requirements = []
+    if not refresh:
+        dockerfile = os.path.join(docker_dir, "Dockerfile")
+        requirements.append(dockerfile)
     requirements.extend(
         [
             os.path.join(docker_dir, f)
             for f in os.listdir(docker_dir)
             if f.endswith("requirements.txt")
         ]
     )
-    if include_self:
-        requirements.append(docker_image_marker(image, refresh=refresh))
     base_image = _docker_base(image)
     if base_image is not None:
-        requirements.extend(
-            _docker_requirements(base_image, include_self=True, refresh=refresh)
-        )
+        requirements.append(os.path.join(IMAGES_DIR, base_image, ".docker_update"))
     return requirements
 
 
-def docker_requirements(wildcards):
+def docker_build_requirements(wildcards):
     """Get all files to ensure a Docker image is up to date from wildcards."""
-    return _docker_requirements(wildcards.image)
+    return _docker_requirements(wildcards.image, refresh=False)
 
 
 def docker_update_requirements(wildcards):
+    """Check if we need to refresh or build a docker image."""
+    return docker_image_marker(wildcards.image, refresh=True)
+
+
+def docker_refresh_requirements(wildcards):
     """Get all files to ensure a Docker image is built and up to date from wildcards."""
-    return _docker_requirements(wildcards.image, include_self=True, refresh=True)
+    return _docker_requirements(wildcards.image, refresh=True)
 
 
 def docker_push_requirements(wildcards):
     """Get all files to ensure a Docker image is ready to push from wildcards."""
     return _docker_requirements(wildcards.image, include_self=True, refresh=False)
 
 
-def docker_push(wildcards):
-    """Get the file to be created to ensure Docker image exists from wildcards."""
-    return docker_image_marker(docker_image_name(wildcards))
-
-
 def docker_command(wildcards, output):
     """Get the Docker command to be run given a set of wildcards."""
     image = docker_image_name(wildcards)