+
Skip to content

Conversation

JulianFlesch
Copy link
Contributor

@JulianFlesch JulianFlesch commented May 14, 2025

Description

Addresses the suggestions in issue 3466 to move away from gitpod and towards devcontainer for Github codespaces.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@JulianFlesch JulianFlesch changed the title [DRAFT] Issues/3466 gitpod [DRAFT] Issues/3466 Move away from Gitpod May 14, 2025
@JulianFlesch
Copy link
Contributor Author

Note: This still uses (a newer) gitpod base image and thus also the gitpod user.

@JulianFlesch JulianFlesch marked this pull request as ready for review May 20, 2025 10:13
@JulianFlesch JulianFlesch changed the title [DRAFT] Issues/3466 Move away from Gitpod Issues/3466 Move away from Gitpod May 20, 2025
Copy link

codecov bot commented May 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.74%. Comparing base (15aeedd) to head (285a2a1).
⚠️ Report is 174 commits behind head on dev.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

"image": "nfcore/gitpod:latest",
"remoteUser": "gitpod",
"runArgs": ["--privileged"],
"image": "nfcore/devcontainer", // ToDo: Add version hash once available
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use the latest one, since this file is updated right before a tools release, we won't have the hash available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true, renovate bot would always be lacking behind ...
I'll change to latest for now, Updating the version here would make the push workflow a lot more complicated.

"""Test that all pipeline template files are included in a pipeline customisation group."""
template_features_yml = load_features_yaml()
base_required_files = [
".devcontainer/setup.sh",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not mandatory, it should be added to the template_features.yml file (it's already there). Was it added to base_required_files for a different reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was one of the tests failing after the template_features.yml update, which was fixed by updating the snapshots. I think this was left by accident and I'll remove it 👍

@mahesh-panchal
Copy link
Member

Hi, I'm only just back from vacation. What should I test and review at the moment, and what has been tested?

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Aug 22, 2025

Welcome back and thanks for having a look @mahesh-panchal

This PR changed quite a bit, but this update still sums it up well: #3569 (comment)

The main things I could use feedback on:

  1. Are all required usecases covered by this devcontainer?
  2. Do you agree with or have a strong opinion on the choice of base image?
    The latest change to using the miniconda image was due to different python version interfering in the final container, as discussed here
    (EDIT: IMHO now certainly the most optimal way to install conda and the correct python version and all other requirements)
  3. Do you have an opinion on the devcontainer features used and how they play with the Dockerfile? The decision to have a Dockerfile again was mainly to be able to copy the repo at the correct ref when running our Github workflows and install the intended state/version of tools. (EDIT: Changed to minimal number of features)
  4. Specifically the local nextflow feature. I will investigate this further, but I believe this adds quite some size to the container potentially due to changing in how we install nf-test (09a1bb8, 4c758ba) which moved from using their install link to conda due to frequent download failures (docker image build irregularly fails due to 403 from nf-test download #3684). (EDIT: large size of nextflow/nf-test feature was adressed in most recent changes)
  5. Confirmation that the new build workflows (for dev and main) are functioning not just from my fork?

EDITED after 6c1596c changes

@JulianFlesch JulianFlesch dismissed mashehu’s stale review August 29, 2025 13:35

Review comments have all been addressed and are outdated now

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Sep 1, 2025

Final changes to address the size of the resulting devcontainer

Problem 1: The features added a lot of size (roughly 300MB each. >1GB for the local nextflow feature!)
Problem 2: Build dependencies / caches were not cleaned up well enough

Changes:

  • Nextflow, nf-test and apptainer is moved back to the Dockerfile
  • A two-stage build is used in the Dockerfile to improve the space usage.

Here are some numbers about the Dockerfile (.devcontainer/build-devcontainer/Dockerfile) based on manual builds:

  • Size of Dockerfile before improvements: 4.24GB
  • Size of Dockerfile with cleanups fce08cf: 3.86GB
  • Size of Dockerfile with nextflow/nf-test install 04c14dd: 5.54GB
  • Size of Dockerfile with nextflow/nf-test install and 2stage build 5fb202d: 4.62GB
  • Size of Dockerfile with nextflow/nf-test/apptainer install and 2stage build b4335b2: 4.82GB

And the resulting devcontainer (.devcontainer/build-devcontainer/devcontainer.json):

  • Devcontainer size before improvements: 7.82GB
  • Devcontainer size after 6c1596c: 5.21GB

  • Devcontainer size after 285a2a1: 4.11GB

@mashehu
Copy link
Contributor

mashehu commented Sep 2, 2025

Good investigation. 4gb still seems a lot too me. But nothing we have to fix in this PR. But maybe make an issue about it.

@mahesh-panchal
Copy link
Member

I got this error when opening this PR in Codespaces.


=================================================================================
2025-09-02 07:52:05.549Z: Configuration starting...
2025-09-02 07:52:05.560Z: Cloning...

=================================================================================
2025-09-02 07:52:06.549Z: Creating container...
2025-09-02 07:52:06.594Z: $ devcontainer up --id-label Type=codespaces --workspace-folder /var/lib/docker/codespacemount/workspace/nf-core-tools --mount type=bind,source=/.codespaces/agent/mount/cache,target=/vscode --user-data-folder /var/lib/docker/codespacemount/.persistedshare --container-data-folder .vscode-remote/data/Machine --container-system-data-folder /var/vscode-remote --log-level trace --log-format json --update-remote-user-uid-default never --mount-workspace-git-root false --omit-config-remote-env-from-metadata --skip-non-blocking-commands --skip-post-create --config "/var/lib/docker/codespacemount/workspace/nf-core-tools/.devcontainer/devcontainer.json" --override-config /root/.codespaces/shared/merged_devcontainer.json --default-user-env-probe loginInteractiveShell --container-session-data-folder /workspaces/.codespaces/.persistedshare/devcontainers-cli/cache --secrets-file /root/.codespaces/shared/user-secrets-envs.json
2025-09-02 07:52:06.726Z: @devcontainers/cli 0.80.0. Node.js v18.20.6. linux 6.8.0-1030-azure x64.
2025-09-02 07:52:09.211Z: Error: Command failed: docker inspect --type image nfcore/devcontainer:latest
2025-09-02 07:52:09.211Z: {"outcome":"error","message":"Command failed: docker inspect --type image nfcore/devcontainer:latest","description":"An error occurred setting up the container."}
2025-09-02 07:52:09.212Z:     at w6 (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:467:1253)
2025-09-02 07:52:09.212Z:     at ax (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:467:997)
2025-09-02 07:52:09.213Z:     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
2025-09-02 07:52:09.213Z:     at async Y6 (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:484:3842)
2025-09-02 07:52:09.215Z:     at async BC (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:484:4957)
2025-09-02 07:52:09.215Z:     at async p7 (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:665:202)
2025-09-02 07:52:09.215Z:     at async d7 (/.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:664:14804)
2025-09-02 07:52:09.215Z:     at async /.codespaces/agent/bin/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:484:1188
2025-09-02 07:52:09.218Z: devcontainer process exited with exit code 1

====================================== ERROR ====================================
2025-09-02 07:52:09.221Z: Failed to create container.
=================================================================================
2025-09-02 07:52:09.221Z: Error: Command failed: docker inspect --type image nfcore/devcontainer:latest
2025-09-02 07:52:09.224Z: Error code: 1302 (UnifiedContainersErrorFatalCreatingContainer)

====================================== ERROR ====================================
2025-09-02 07:52:09.232Z: Container creation failed.
=================================================================================
2025-09-02 07:52:09.281Z: 

and it opened a recovery container instead.

@mahesh-panchal
Copy link
Member

To test on the nf-core modules repo, which files are the important ones I should transfer to a branch?

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Sep 2, 2025

@mahesh-panchal The error is because the image is not yet pushed to Dockerhub. This will work once the first build action (after a pull to main) is run.
For now, you have to run the devcontainer build command for .devcontainer/build-devcontainer/devcontainer.json manually:

$ devcontainer build --workspace-folder . --image-name nfcore/devcontainer:latest --config .devcontainer/build-devcontainer/devcontainer.json

The devcontainer .devcontainer/devcontainer.json can then be run locally. For testing in codespaces, you need to upload the manually built devcontainer to a registry and change the "image": declaration in the config .devcontainer/devcontainer.json.

EDIT: I built and pushed our base devcontainer image (.devcontainer/build-devcontainer/devcontainer.json) to a registry: fljulian/nfcore-devcontainer:latest. You can use this in .devcontainer/devcontainer.json for testing instead.

EDIT2: You can test this repo in codespaces here: https://github.com/JulianFlesch/nf-core-tools/tree/testing/issue-3266-gitpod based on the image mentioned above

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Sep 2, 2025

To test on the nf-core modules repo, which files are the important ones I should transfer to a branch?

The files .devcontainer/devcontainer.json and .devcontainer/setup.sh I would assume.
Some testing was done for this issue (nf-core/modules#8769) on an earlier version of this PR.

EDIT: Here is a draft PR: nf-core/modules#8983

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Sep 2, 2025

@mashehu on final inspection, the size actually looks really good:

# Run from within Github codespaces after pushing the locally build image to dockerhub:
$ docker images 
REPOSITORY                     TAG       IMAGE ID       CREATED          SIZE
fljulian/nfcore-devcontainer   latest    9e8cf202ae44   19 minutes ago   2.76GB

I don't think I locally accounted for layer compression, but dockerhub took care of that for us 👍

@JulianFlesch JulianFlesch merged commit 78b790e into nf-core:dev Sep 3, 2025
116 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

8 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载