-
Notifications
You must be signed in to change notification settings - Fork 943
docs: add comprehensive dev containers documentation with examples #18582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3506fa2
to
06d714c
Compare
cafacc7
to
cfcef3c
Compare
moved to "ready for review" while I tech test the remaining examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm slightly concerned at how many things were subtly wrong and that I haven't caught them all.
```console | ||
coder ssh --container keen_dijkstra my-workspace | ||
coder ssh --container my-container-name my-workspace | ||
``` | ||
|
||
Remember to replace: | ||
|
||
- `my-container-name` with the dev container agent name. | ||
- `my-workspace` with your workspace's name. | ||
|
||
> [!NOTE] | ||
> | ||
> SSH access is not yet compatible with the `coder config-ssh` command for use | ||
> with OpenSSH. You would need to manually modify your SSH config to include the | ||
> `--container` flag in the `ProxyCommand`. | ||
> Starting with Coder v2.24.0, `coder config-ssh` works with dev containers. | ||
> If you’re using an older Coder version, add `--container <name>` to the | ||
> `ProxyCommand` entry in your SSH config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this section could be worded better. It gives priority to the old pattern using a hidden CLI flag that was only made available during early access.
I believe we should instead focus entirely on how to do it with the current agent-based approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
I think this whole doc needs a review, but I'm leaning towards doing that as a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still feel like this needs addressing.
docs/user-guides/devcontainers/troubleshooting-dev-containers.md
Outdated
Show resolved
Hide resolved
docs/user-guides/devcontainers/troubleshooting-dev-containers.md
Outdated
Show resolved
Hide resolved
docs/user-guides/devcontainers/troubleshooting-dev-containers.md
Outdated
Show resolved
Hide resolved
docs/admin/templates/extending-templates/dev-containers-envbuilder.md
Outdated
Show resolved
Hide resolved
docs/admin/templates/extending-templates/dev-containers-envbuilder.md
Outdated
Show resolved
Hide resolved
docs/admin/templates/extending-templates/dev-containers-envbuilder.md
Outdated
Show resolved
Hide resolved
docs/admin/templates/extending-templates/advanced-dev-containers.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Danielle Maywood <danielle@themaywoods.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for another spam of review comments 😅 I still feel like there is some outdated/confusing/incorrect information present.
| Build engine | `@devcontainers/cli` + Docker | Envbuilder transforms the workspace image | | ||
| Runs separate Docker container | Yes (parent workspace + child container) | No (modifies the parent container) | | ||
| Multiple Dev Containers per workspace | Yes | No | | ||
| Rebuild when `devcontainer.json` changes | Yes (auto-prompt) | Limited (requires full workspace rebuild) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mafredri what do we think on the wording of auto-prompt
here? We don't technically prompt the user they can rebuild, the UI just indicates the dev container is outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it feels a bit off. Perhaps "(user initiated, change detection)".
source = "dev.registry.coder.com/modules/devcontainers-cli/coder" | ||
agent_id = coder_agent.main.id | ||
} | ||
resource "coder_devcontainer" "project" { # `project` in this example is how users will connect to the dev container: `ssh://project.<workspace>.me.coder` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like project
is the name because of the terraform resource name, but I think we should make it more clear it is because of the workspace folder path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the comment could be removed. (FYI for Edward, the resource name did play a role in the past, but not in the code that was released in GA.)
} | ||
``` | ||
|
||
Alternatively, install the devcontainer CLI manually in your base image. | ||
Alternatively, install `devcontainer/cli` manually in your base image: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devcontainers/cli
otherwise this looks a little confusing
|
||
```terraform | ||
resource "coder_devcontainer" "my-repository" { | ||
resource "coder_devcontainer" "project" { # `project` in this example is how users will connect to the dev container: `ssh://project.<workspace>.me.coder` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as earlier suggestions
|
||
<!-- nolint:MD028/no-blanks-blockquote --> | ||
This step is optional, but it ensures that the project is present before the dev container starts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way this is worded feels a little confusing as it makes me believe I can just skip this step?
- Integrated IDE experience in dev containers with VS Code | ||
- Direct service access in dev containers | ||
- Limited SSH access to dev containers | ||
- Automatic discovery of `.devcontainer` configs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
## Rebuild prompt does not appear | ||
|
||
1. Confirm that you saved `devcontainer.json` in the correct repo path detected by Coder. | ||
1. Check agent logs for `devcontainer build` errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what devcontainer build
is referring to here.
```console | ||
coder ssh --container keen_dijkstra my-workspace | ||
coder ssh --container my-container-name my-workspace | ||
``` | ||
|
||
Remember to replace: | ||
|
||
- `my-container-name` with the dev container agent name. | ||
- `my-workspace` with your workspace's name. | ||
|
||
> [!NOTE] | ||
> | ||
> SSH access is not yet compatible with the `coder config-ssh` command for use | ||
> with OpenSSH. You would need to manually modify your SSH config to include the | ||
> `--container` flag in the `ProxyCommand`. | ||
> Starting with Coder v2.24.0, `coder config-ssh` works with dev containers. | ||
> If you’re using an older Coder version, add `--container <name>` to the | ||
> `ProxyCommand` entry in your SSH config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still feel like this needs addressing.
@@ -34,39 +41,22 @@ You can open your dev container directly in VS Code by: | |||
2. Using the Coder CLI with the container flag: | |||
|
|||
```console | |||
coder open vscode --container keen_dijkstra my-workspace | |||
coder open vscode --container my-container-name my-workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spotted here as well.
``` | ||
|
||
You can forward these ports to your local machine using: | ||
Coder automatically forwards any port declared in `appPort` or `forwardPorts`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is correct (cc @mafredri)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel there may be too much feedback here and issues/suggestions (especially those that need to be fixed in multiple places) will easily be missed. Not sure how we should tackle this problem but I do see a few issues still present from previous feedback rounds too and with this many changes in one PR, it's also going to be very hard to verify that all the necessary changes are made.
|
||
Run independent dev containers in the same workspace so each component appears as its own agent. | ||
|
||
In this example, there are two: `frontend` and `backend`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should mention here that the names come from the project name (i.e. folder they are cloned into).
|
||
agent_id = coder_agent.main.id | ||
url = "https://github.com/your-org/frontend.git" | ||
base_dir = "/home/coder/frontend" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will result in /home/coder/frontend/frontend
, we should drop the base_dir
for simplicity since it defaults to $HOME
.
resource "coder_devcontainer" "backend" { | ||
count = data.coder_workspace.me.start_count | ||
agent_id = coder_agent.main.id | ||
workspace_folder = "/home/coder/backend/${module.git-clone-backend[0].folder_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspace_folder = "/home/coder/backend/${module.git-clone-backend[0].folder_name}" | |
workspace_folder = module.git-clone-backend[0].repo_dir |
Simplification.
``` | ||
|
||
Each dev container appears as a separate agent, so developers can connect to any | ||
component in the workspace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a better suggestion, but "component" feels a bit off for me and it's feels like we're calling dev containers both dev containers and components.
data "coder_parameter" "enable_frontend" { | ||
type = "bool" | ||
name = "Enable frontend container" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data "coder_parameter" "enable_frontend" { | |
type = "bool" | |
name = "Enable frontend container" | |
data "coder_parameter" "autostart_frontend" { | |
type = "bool" | |
name = "Autostart frontend container" |
I suggest alternative terminology here so that we don't accidentally suggest that the exclusion of resource "coder_devcontainer" "frontend"
can prevent the devcontainer from showing up in the UI if started manually.
### Slow builds | ||
|
||
- Allocate more CPU/RAM. | ||
- Use image caching or pre-build common images. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A local (Docker) registry mirror can also help.
Also, we should be cautious with the term pre-build
here. What are we referring to? Prebuilt Docker image? Prebuilds in Coder? devcontainer build
?
1. The integration builds and starts the dev container based on the | ||
configuration. | ||
1. The integration detects repositories with a `.devcontainer` directory or a `devcontainer.json` file. | ||
1. The integration builds (or rebuilds) and starts the dev container based on the configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. The integration builds (or rebuilds) and starts the dev container based on the configuration. | |
1. The integration starts the dev container based on the configuration. |
"Builds" may add a bit of confusion here. Also, please specify what "configuration" means here. I'm assuming terraform but below "configuration" refers to devcontainer.json
.
- Integrated IDE experience in dev containers with VS Code | ||
- Direct service access in dev containers | ||
- Limited SSH access to dev containers | ||
- Automatic discovery of `.devcontainer` configs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Why reword this and mention .devcontainer
which is the folder and is not required, the main party is devcontainer.json
.
Also the statement is kind of not true (requires that bit in terraform or manually starting the devcontainer), but we might want to go here some day (see coder/internal#711)
## Rebuild prompt does not appear | ||
|
||
1. Confirm that you saved `devcontainer.json` in the correct repo path detected by Coder. | ||
1. Check agent logs for `devcontainer build` errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't ever explicitly run devcontainer build
currently. I don't think we should highlight this command.
``` | ||
|
||
You can forward these ports to your local machine using: | ||
Coder automatically forwards any port declared in `appPort` or `forwardPorts`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weren't we supposed to fix this already?
I can incorporate all the feedback and suggestions from @DanielleMaywood's and your review, but I think we're butting up against my limited dev containers knowledge and experience, and might need to approach this differently. Does it make more sense to you both if I keep making changes here, or do you think we're at the point where it makes more sense to open a new PR that's less of an overhaul of the existing doc to add the new functionality and examples there? |
changing approach - replacement PR to follow |
Uh oh!
There was an error while loading. Please reload this page.