This PR focuses on optimizing go-test CI times on Windows. It:
- backs the `$RUNNER_TEMP` directory with a RAM disk. This directory is
used by actions like cache, setup-go, and setup-terraform as a staging
area
- backs `GOCACHE`, `GOMODCACHE`, and `GOPATH` with a RAM disk
- backs `$GITHUB_WORKSPACE` with a RAM disk - that's where the
repository is checked out
- uses preinstalled Go on Windows runners
- starts using the depot Windows runner
From what I've seen, these changes bring test times down to be on par
with Linux and macOS. The biggest improvement comes from backing
frequently accessed paths with RAM disks. The C drive is surprisingly
slow - I ran some performance tests with
[fio](https://fio.readthedocs.io/en/latest/fio_doc.html#) where I tested
IOPS on many small files, and the RAM disk was 100x faster.
Additionally, the depot runners seem to have more consistent performance
than the ones provided by GitHub.
Database transactions hold onto connections, and `pubsub.Publish` tries
to acquire a connection of its own. If the latter is called within a
transaction, this can lead to connection exhaustion.
I plan two follow-ups to this PR:
1. Make connection counts tuneable
https://github.com/coder/coder/blob/main/cli/server.go#L2360-L2376
We will then be able to write tests showing how connection exhaustion
occurs.
2. Write a linter/ruleguard to prevent `pubsub.Publish` from being
called within a transaction.
---------
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Currently we don't have a way to get insight into Postgres connections
being exhausted.
By using the prometheus' [`DBStats`
collector](https://github.com/prometheus/client_golang/blob/main/prometheus/collectors/dbstats_collector.go),
we get some insight out-of-the-box.
```
# HELP go_sql_idle_connections The number of idle connections.
# TYPE go_sql_idle_connections gauge
go_sql_idle_connections{db_name="coder"} 1
# HELP go_sql_in_use_connections The number of connections currently in use.
# TYPE go_sql_in_use_connections gauge
go_sql_in_use_connections{db_name="coder"} 2
# HELP go_sql_max_idle_closed_total The total number of connections closed due to SetMaxIdleConns.
# TYPE go_sql_max_idle_closed_total counter
go_sql_max_idle_closed_total{db_name="coder"} 112
# HELP go_sql_max_idle_time_closed_total The total number of connections closed due to SetConnMaxIdleTime.
# TYPE go_sql_max_idle_time_closed_total counter
go_sql_max_idle_time_closed_total{db_name="coder"} 0
# HELP go_sql_max_lifetime_closed_total The total number of connections closed due to SetConnMaxLifetime.
# TYPE go_sql_max_lifetime_closed_total counter
go_sql_max_lifetime_closed_total{db_name="coder"} 0
# HELP go_sql_max_open_connections Maximum number of open connections to the database.
# TYPE go_sql_max_open_connections gauge
go_sql_max_open_connections{db_name="coder"} 10
# HELP go_sql_open_connections The number of established connections both in use and idle.
# TYPE go_sql_open_connections gauge
go_sql_open_connections{db_name="coder"} 3
# HELP go_sql_wait_count_total The total number of connections waited for.
# TYPE go_sql_wait_count_total counter
go_sql_wait_count_total{db_name="coder"} 28
# HELP go_sql_wait_duration_seconds_total The total time blocked waiting for a new connection.
# TYPE go_sql_wait_duration_seconds_total counter
go_sql_wait_duration_seconds_total{db_name="coder"} 0.086936235
```
`go_sql_wait_count_total` is the metric I'm most interested in gaining,
but the others are also very useful.
Changing the prefix is easy (`prometheus.WrapRegistererWithPrefix`), but
getting rid of the `go_` segment is not quite so easy. I've kept the
changeset small for now.
**NOTE:** I imported a library to determine the database name from the
given conn string. It's [not as
simple](https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING)
as one might hope. The database name is used for the `db_name` label.
---------
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Refactor WebSocket error handling to ensure that errors are only set
when the current socket ref matches the active one. This prevents
unnecessary error messages when the WebSocket connection closes
unexpectedly
This solves the problem of showing error messages because of React
Strict mode rendering the page twice and opening 2 websocket
connections.
This change documents the early access dev containers integration and
how to enable it, what features are available and what limitations exist
at the time of writing.
---------
Co-authored-by: EdwardAngert <17991901+EdwardAngert@users.noreply.github.com>
Don't specify the template version for a delete transition, because the
prebuilt workspace may have been created using an older template
version.
If the template version isn't explicitly set, the builder will
automatically use the version from the last workspace build - which is
the desired behavior.
Fixes https://github.com/coder/internal/issues/604
Fixes a data race in `agentscripts.Runner` where a concurrent `Execute()` call races with `Init()`. We hit this race during shut down, which is not synchronized against starting up.
In this PR I've chosen to add synchronization to the `Runner` rather than try to synchronize the calls in the agent. When we close down the agent, it's OK to just throw an error if we were never initialized with a startup script---we don't want to wait for it since that requires an active connection to the control plane.
Adds @spikecurtis and @johnstcn as CODEOWNERS of the provisioner protocol files. These need to be versioned, so we need some human review over changes.
PR contains:
- fix for claiming & deleting prebuilds with immutable params
- unit test for claiming scenario
- unit test for deletion scenario
The parameter resolver was failing when deleting/claiming prebuilds
because a value for a previously-used parameter was provided to the
resolver, but since the value was unchanged (it's coming from the
preset) it failed in the resolver. The resolver was missing a check to
see if the old value != new value; if the values match then there's no
mutation of an immutable parameter.
---------
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
The regular network info file creation code also calls `Mkdirall`.
Wasn't picked up in manual testing as I already had the `/net` folder in
my VSCode.
Wasn't picked up in automated testing because we use an in-memory FS,
which for some reason does this implicitly.
Fix https://github.com/coder/internal/issues/594
**Notice:**
This is a temporary solution to get the devcontainers feature released.
Maybe a better solution, to avoid pulling the API every 10 seconds, is
to implement a websocket connection to get updates on containers.
Closes https://github.com/coder/vscode-coder/issues/447
Closes https://github.com/coder/jetbrains-coder/issues/543
Closes https://github.com/coder/coder-jetbrains-toolbox/issues/21
This PR adds Coder Connect support to `coder ssh --stdio`.
When connecting to a workspace, if `--force-new-tunnel` is not passed, the CLI will first do a DNS lookup for `<agent>.<workspace>.<owner>.<hostname-suffix>`. If an IP address is returned, and it's within the Coder service prefix, the CLI will not create a new tailnet connection to the workspace, and instead dial the SSH server running on port 22 on the workspace directly over TCP.
This allows IDE extensions to use the Coder Connect tunnel, without requiring any modifications to the extensions themselves.
Additionally, `using_coder_connect` is added to the `sshNetworkStats` file, which the VS Code extension (and maybe Jetbrains?) will be able to read, and indicate to the user that they are using Coder Connect.
One advantage of this approach is that running `coder ssh --stdio` on an offline workspace with Coder Connect enabled will have the CLI wait for the workspace to build, the agent to connect (and optionally, for the startup scripts to finish), before finally connecting using the Coder Connect tunnel.
As a result, `coder ssh --stdio` has the overhead of looking up the workspace and agent, and checking if they are running. On my device, this meant `coder ssh --stdio <workspace>` was approximately a second slower than just connecting to the workspace directly using `ssh <workspace>.coder` (I would assume anyone serious about their Coder Connect usage would know to just do the latter anyway).
To ensure this doesn't come at a significant performance cost, I've also benchmarked this PR.
<details>
<summary>Benchmark</summary>
## Methodology
All tests were completed on `dev.coder.com`, where a Linux workspace running in AWS `us-west1` was created.
The machine running Coder Desktop (the 'client') was a Windows VM running in the same AWS region and VPC as the workspace.
To test the performance of specifically the SSH connection, a port was forwarded between the client and workspace using:
```
ssh -p 22 -L7001:localhost:7001 <host>
```
where `host` was either an alias for an SSH ProxyCommand that called `coder ssh`, or a Coder Connect hostname.
For latency, [`tcping`](https://www.elifulkerson.com/projects/tcping.php) was used against the forwarded port:
```
tcping -n 100 localhost 7001
```
For throughput, [`iperf3`](https://iperf.fr/iperf-download.php) was used:
```
iperf3 -c localhost -p 7001
```
where an `iperf3` server was running on the workspace on port 7001.
## Test Cases
### Testcase 1: `coder ssh` `ProxyCommand` that bicopies from Coder Connect
This case tests the implementation in this PR, such that we can write a config like:
```
Host codercliconnect
ProxyCommand /path/to/coder ssh --stdio workspace
```
With Coder Connect enabled, `ssh -p 22 -L7001:localhost:7001 codercliconnect` will use the Coder Connect tunnel. The results were as follows:
**Throughput, 10 tests, back to back:**
- Average throughput across all tests: 788.20 Mbits/sec
- Minimum average throughput: 731 Mbits/sec
- Maximum average throughput: 871 Mbits/sec
- Standard Deviation: 38.88 Mbits/sec
**Latency, 100 RTTs:**
- Average: 0.369ms
- Minimum: 0.290ms
- Maximum: 0.473ms
### Testcase 2: `ssh` dialing Coder Connect directly without a `ProxyCommand`
This is what we assume to be the 'best' way to use Coder Connect
**Throughput, 10 tests, back to back:**
- Average throughput across all tests: 789.50 Mbits/sec
- Minimum average throughput: 708 Mbits/sec
- Maximum average throughput: 839 Mbits/sec
- Standard Deviation: 39.98 Mbits/sec
**Latency, 100 RTTs:**
- Average: 0.369ms
- Minimum: 0.267ms
- Maximum: 0.440ms
### Testcase 3: `coder ssh` `ProxyCommand` that creates its own Tailnet connection in-process
This is what normally happens when you run `coder ssh`:
**Throughput, 10 tests, back to back:**
- Average throughput across all tests: 610.20 Mbits/sec
- Minimum average throughput: 569 Mbits/sec
- Maximum average throughput: 664 Mbits/sec
- Standard Deviation: 27.29 Mbits/sec
**Latency, 100 RTTs:**
- Average: 0.335ms
- Minimum: 0.262ms
- Maximum: 0.452ms
## Analysis
Performing a two-tailed, unpaired t-test against the throughput of testcases 1 and 2, we find a P value of `0.9450`. This suggests the difference between the data sets is not statistically significant. In other words, there is a 94.5% chance that the difference between the data sets is due to chance.
## Conclusion
From the t-test, and by comparison to the status quo (regular `coder ssh`, which uses gvisor, and is noticeably slower), I think it's safe to say any impact on throughput or latency by the `ProxyCommand` performing a bicopy against Coder Connect is negligible. Users are very much unlikely to run into performance issues as a result of using Coder Connect via `coder ssh`, as implemented in this PR.
Less scientifically, I ran these same tests on my home network with my Sydney workspace, and both throughput and latency were consistent across testcases 1 and 2.
</details>
* Refactors toolsdk.Tools to remove opaque `map[string]any` argument in
favour of typed args structs.
* Refactors toolsdk.Tools to remove opaque passing of dependencies via
`context.Context` in favour of a tool dependencies struct.
* Adds panic recovery and clean context middleware to all tools.
* Adds `GenericTool` implementation to allow keeping `toolsdk.All` with
uniform type signature while maintaining type information in handlers.
* Adds stricter checks to `patchWorkspaceAgentAppStatus` handler.
There were too many ways to configure the agentcontainers API resulting
in inconsistent behavior or features not being enabled. This refactor
introduces a control flag for enabling or disabling the containers API.
When disabled, all implementations are no-op and explicit endpoint
behaviors are defined. When enabled, concrete implementations are used
by default but can be overridden by passing options.
* Updates default Coder prompt.
* Skips the directions to report tasks if the pre-requisites are not
available (agent token and app slug).
* Adds the capability to override the default Coder prompt via
`CODER_MCP_CLAUDE_CODER_PROMPT`.
I've been bit in the past by hard to deduce bugs caused by circular
dependencies within TS projects. On a hunch that this could be
contributing to some flaky tests I've used the tool
[dpdm](https://github.com/acrazing/dpdm) to find and remove them.
This PR does the following:
- Move around exports/create new files to remove any non-type circular
depencies
- Add dpdm as a dev dependency and create the `check:circular-depency`
pnpm script
Fix https://github.com/coder/coder/issues/16409
Since the provisioner timings action is not strongly typed, but it is
typed as a generic string, and we are not using
`noUncheckedIndexedAccess`, we can miss some of the actions returned
from the API, causing type errors. To avoid that, I changed the code to
be extra safe by adding `undefined` into the return type.
Fix https://github.com/coder/coder/issues/17188
We forgot to filter the scripts by `run_on_start`, since we only
calculate timings in the start phase, which was causing the miss match
between the expected script timings count, and the loop in the refetch
logic.
While I think this fix is enough for now, I think the server should be
responsible to telling the client when to stop fetching. It could be a
simple attribute such as `done: false | true` or a websocket endpoint as
suggested by @dannykopping
[here](https://github.com/coder/coder/issues/17188#issuecomment-2788235333).
Hugo Dutka