Remove the Docker in Docker feature configuration #129
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These four removed options are currently the default configuration for the feature. If we need to override the defaults in future, then we should do that. Reference: https://github.com/devcontainers/features/blob/f5787eed01022f177475a99084327e023a84ddaf/src/docker-in-docker/README.md These explicit options were added in ef7549f and that in turn was because Visual Studio Code added them in when creating the dev container configuration. For context, * `moby` specifies whether use Moby instead of Docker CE * `version` specifies the Moby/Docker CE version * `azureDnsAutoDetection` sets the DNS server for `dockerd` if running in Azure (Actions runners are hosted in Azure, not sure if this setting makes any difference for our use) * `installDockerBuildx`: installs Docker Buildx (which is used by Docker Buildkit); not convinced there's anywhere we use this for OpenSAFELY CLI, but it shouldn't do any harm to have it installed
This setting came from it being added via Visual Studio Code in ef7549f. The latest is currently v2. Also don't know of any reason why we currently would want to pin this: I'm not aware that we use Docker Compose whether in our research-template setup, or in tools like OpenSAFELY CLI.
|
The one thing from this I learned is that we're using Moby and not Docker CE. I'm not sure that matters too much as the two are closely related, but we could explicitly choose Docker if we like. |
|
My suggested minimal acceptance criteria for such a change are that the following work as expected:
|
iaindillingham
approved these changes
May 29, 2024
StevenMaude
deleted the
steve/remove-explicit-docker-in-docker-configuration
branch
StevenMaude
added a commit
to opensafely/ehrql-tutorial
that referenced
this pull request
Aug 14, 2024
We dropped this for the research-template. See opensafely/research-template#129.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #127.
This PR removes the Docker in Docker dev container feature configuration to simplify the
devcontainer.json.Much of the configuration simply restates the feature defaults.
This was all created by Visual Studio Code originally; see ef7549f.
See the commit messages for more detail.