-
Notifications
You must be signed in to change notification settings - Fork 929
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.
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.
1. In the `devcontainers-cli` module block, add: | ||
```terraform | ||
depends_on = [module.nodejs] |
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.
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 have any success with depends_on
in my testing either
my goal was to ship this as a doc folks can use as a standalone, but that's going to have to come with a future iteration, so I'm going to remove the <details>
expand
|
||
```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 |
Uh oh!
There was an error while loading. Please reload this page.