-
Notifications
You must be signed in to change notification settings - Fork 928
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
base: main
Are you sure you want to change the base?
Conversation
3506fa2
to
06d714c
Compare
cafacc7
to
cfcef3c
Compare
I had some trouble getting the example working with my barebones deployment, and I think it's because of the docker-in-docker requirement I'm going to have to come back for some more testing |
@@ -1,4 +1,4 @@ | |||
# Dev containers | |||
# Envbuilder Dev Containers |
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.
Why does it lead with Envbuilder
?
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 need to decide what to do with this set of docs - I re-titled them and made some quick changes to sections to be envbuilder-specific
we aren't removing envbuilder with this PR, so I want to strip these down as much as possible, but don't want to remove and redirect the docs just yet.
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.
Should we add some sort of notice to this page that users should pick the new devcontainer approach instead of envbuilder?
docs/admin/templates/extending-templates/dev-containers-envbuilder.md
Outdated
Show resolved
Hide resolved
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 didn't quite get through everything, but here's some initial feedback.
|
||
```terraform | ||
resource "coder_devcontainer" "my-repository" { | ||
resource "coder_devcontainer" "project" { |
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 glad you used the same name (project
) here and in the workspace folder. We should potentially call it out that this name is how users will be connecting to the devcontainer (e.g. 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.
I'm trying to standardize the examples and configs we use so that someone using a combination of docs can have a linear/consistent experience. At some point, I think it'd be good to have a heavily commented set of examples that we can use both for teaching and for a docs style guide.
For now, I just added a comment on this line
base_dir = "/home/coder/project" | ||
} | ||
|
||
resource "coder_devcontainer" "project" { |
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.
It's unfortunate folder_name
can't be used here. 😔 I'm really of two-minds using this for the sub-agent name when present but it is what it is.
We support devcontainer.json
customization to change the name on a per-project basis.
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 really have that documented, but I think we should def add it in the next iteration
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.
adding: it would be great as part of or in addition to the Agent naming
section
|
||
### Disable dev containers integration | ||
|
||
To disable the dev containers integration in your workspace, add the `CODER_AGENT_DEVCONTAINERS_ENABLE` environment variable to your existing `coder_agent` block: |
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 is incorrect, the env must be set before the agent is started, i.e. on the Docker container. We could potentially implement this behavior but it's currently not supported (/cc @DanielleMaywood thoughts?)
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.
changed to
To disable the dev containers integration in your workspace, set the `CODER_AGENT_DEVCONTAINERS_ENABLE= "false"` environment variable.
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.
Coder automatically forwards any port declared in `appPort`, `forwardPorts`, | ||
or exposed by `docker-compose.yml`. |
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.
Is this correct @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.
Nope, called this out in another section I believe.
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
|
||
## Complete Template Example | ||
For example, if the path is `/home/coder/some/project` and `project` is taken, then the agent is `some-project`. |
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 hope I'm reading #18513 right
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.
Yep that's exactly right
Uh oh!
There was an error while loading. Please reload this page.