* Improve CLI documentation
* feat: Allow workspace resources to attach multiple agents
This enables a "kubernetes_pod" to attach multiple agents that
could be for multiple services. Each agent is required to have
a unique name, so SSH syntax is:
`coder ssh <workspace>.<agent>`
A resource can have zero agents too, they aren't required.
* Add tree view
* Improve table UI
* feat: Allow workspace resources to attach multiple agents
This enables a "kubernetes_pod" to attach multiple agents that
could be for multiple services. Each agent is required to have
a unique name, so SSH syntax is:
`coder ssh <workspace>.<agent>`
A resource can have zero agents too, they aren't required.
* Rename `tunnel` to `skip-tunnel`
This command was `true` by default, which causes
a confusing user experience.
* Add disclaimer about editing templates
* Add help to template create
* Improve workspace create flow
* Add end-to-end test for config-ssh
* Improve testing of config-ssh
* Fix workspace list
* feat: Add support for VS Code and JetBrains Gateway via SSH
This fixes various bugs that made this not work:
- Incorrect max message size in `peer`
- Incorrect reader buffer size in `peer`
- Lack of SFTP support in `agent`
- Lack of direct-tcpip support in `agent`
- Misuse of command from session. It should always use the shell
- Blocking on SSH session, only allowing one at a time
Fixes#833 too.
* Fix config-ssh command with socat
* chore: Improve CI builds by caching Go modules
* Skip running with `race` on non-Linux systems
* Fix darwin file descriptor error
* Fix log after close
* Improve PostgreSQL test speeds
* Fix parallel connections with PostgreSQL tests
* Fix CI flake
* Separate test/go into PostgreSQL
* fix: Leaking yamux session after HTTP handler is closed
Closes#317. The httptest server cancels the context after the connection
is closed, but if a connection takes a long time to close, the request
would never end. This applies a context to the entire listener that cancels
on test cleanup.
After discussion with @bryphe-coder, reducing the parallel limit on
Windows is likely to reduce failures as well.
* Switch to windows-2022 to improve decompression
* Invalidate cache on matrix OS
* Initial agent
* fix: Use buffered reader in peer to fix ShortBuffer
This prevents a io.ErrShortBuffer from occurring when the byte
slice being read is smaller than the chunks sent from the opposite
pipe.
This makes sense for unordered connections, where transmission is
not guarunteed, but does not make sense for TCP-like connections.
We use a bufio.Reader when ordered to ensure data isn't lost.
* SSH server works!
* Start Windows support
* Something works
* Refactor pty package to support Windows spawn
* SSH server now works on Windows
* Fix non-Windows
* Fix Linux PTY render
* FIx linux build tests
* Remove agent and wintest
* Add test for Windows resize
* Fix linting errors
* Add Windows environment variables
* Add strings import
* Add comment for attrs
* Add goleak
* Add require import
This prevents a io.ErrShortBuffer from occurring when the byte
slice being read is smaller than the chunks sent from the opposite
pipe.
This makes sense for unordered connections, where transmission is
not guaranteed, but does not make sense for TCP-like connections.
We use a bufio.Reader when ordered to ensure data isn't lost.
* chore: Rename ProjectHistory to ProjectVersion
Version more accurately represents version storage. This
forks from the WorkspaceHistory name, but I think it's
easier to understand Workspace history.
* Rename files
* Standardize tests a bit more
* Remove Server struct from coderdtest
* Improve test coverage for workspace history
* Fix linting errors
* Fix coderd test leak
* Fix coderd test leak
* Improve workspace history logs
* Standardize test structure for codersdk
* Fix linting errors
* Fix WebSocket compression
* Update coderd/workspaces.go
Co-authored-by: Bryan <bryan@coder.com>
* Add test for listing project parameters
* Cache npm dependencies with setup node
* Remove windows npm cache key
Co-authored-by: Bryan <bryan@coder.com>
* feat: Add history middleware parameters
These will be used for streaming logs, checking status,
and other operations related to workspace and project
history.
* refactor: Move all HTTP routes to top-level struct
Nesting all structs behind their respective structures
is leaky, and promotes naming conflicts between handlers.
Our HTTP routes cannot have conflicts, so neither should
function naming.
* Add provisioner daemon routes
* Add periodic updates
* Skip pubsub if short
* Return jobs with WorkspaceHistory
* Add endpoints for extracting singular history
* The full end-to-end operation works
* fix: Disable compression for websocket dRPC transport (#145)
There is a race condition in the interop between the websocket and `dRPC`: https://github.com/coder/coder/runs/5038545709?check_suite_focus=true#step:7:117 - it seems both the websocket and dRPC feel like they own the `byte[]` being sent between them. This can lead to data races, in which both `dRPC` and the websocket are writing.
This is just tracking some experimentation to fix that race condition
## Run results: ##
- Run 1: peer test failure
- Run 2: peer test failure
- Run 3: `TestWorkspaceHistory/CreateHistory` - https://github.com/coder/coder/runs/5040858460?check_suite_focus=true#step:8:45
```
status code 412: The provided project history is running. Wait for it to complete importing!`
```
- Run 4: `TestWorkspaceHistory/CreateHistory` - https://github.com/coder/coder/runs/5040957999?check_suite_focus=true#step:7:176
```
workspacehistory_test.go:122:
Error Trace: workspacehistory_test.go:122
Error: Condition never satisfied
Test: TestWorkspaceHistory/CreateHistory
```
- Run 5: peer failure
- Run 6: Pass ✅
- Run 7: Peer failure
## Open Questions: ##
### Is `dRPC` or `websocket` at fault for the data race?
It looks like this condition is specifically happening when `dRPC` decides to [`SendError`]). This constructs a new byte payload from [`MarshalError`](https://github.com/storj/drpc/blob/f6e369438f636b47ee788095d3fc13062ffbd019/drpcwire/error.go#L15) - so `dRPC` has created this buffer and owns it.
From `dRPC`'s perspective, the callstack looks like this:
- [`sendPacket`](https://github.com/storj/drpc/blob/f6e369438f636b47ee788095d3fc13062ffbd019/drpcstream/stream.go#L253)
- [`writeFrame`](https://github.com/storj/drpc/blob/f6e369438f636b47ee788095d3fc13062ffbd019/drpcwire/writer.go#L65)
- [`AppendFrame`](https://github.com/storj/drpc/blob/f6e369438f636b47ee788095d3fc13062ffbd019/drpcwire/packet.go#L128)
- with finally the data race happening here:
```go
// AppendFrame appends a marshaled form of the frame to the provided buffer.
func AppendFrame(buf []byte, fr Frame) []byte {
...
out := buf
out = append(out, control). // <---------
```
This should be fine, since `dPRC` create this buffer, and is taking the byte buffer constructed from `MarshalError` and tacking a bunch of headers on it to create a proper frame.
Once `dRPC` is done writing, it _hangs onto the buffer and resets it here__: https://github.com/storj/drpc/blob/f6e369438f636b47ee788095d3fc13062ffbd019/drpcwire/writer.go#L73
However... the websocket implementation, once it gets the buffer, it runs a `statelessDeflate` [here](https://github.com/nhooyr/websocket/blob/8dee580a7f74cf1713400307b4eee514b927870f/write.go#L180), which compresses the buffer on the fly. This functionality actually [mutates the buffer in place](https://github.com/klauspost/compress/blob/a1a9cfc821f00faf2f5231beaa96244344d50391/flate/stateless.go#L94), which is where get our race.
In the case where the `byte[]` aren't being manipulated anywhere else, this compress-in-place operation would be safe, and that's probably the case for most over-the-wire usages. In this case, though, where we're plumbing `dRPC` -> websocket, they both are manipulating it (`dRPC` is reusing the buffer for the next `write`, and `websocket` is compressing on the fly).
### Why does cloning on `Read` fail?
Get a bunch of errors like:
```
2022/02/02 19:26:10 [WARN] yamux: frame for missing stream: Vsn:0 Type:0 Flags:0 StreamID:0 Length:0
2022/02/02 19:26:25 [ERR] yamux: Failed to read header: unexpected EOF
2022/02/02 19:26:25 [ERR] yamux: Failed to read header: unexpected EOF
2022/02/02 19:26:25 [WARN] yamux: frame for missing stream: Vsn:0 Type:0 Flags:0 StreamID:0 Length:0
```
# UPDATE:
We decided we could disable websocket compression, which would avoid the race because the in-place `deflate` operaton would no longer be run. Trying that out now:
- Run 1: ✅
- Run 2: https://github.com/coder/coder/runs/5042645522?check_suite_focus=true#step:8:338
- Run 3: ✅
- Run 4: https://github.com/coder/coder/runs/5042988758?check_suite_focus=true#step:7:168
- Run 5: ✅
* fix: Remove race condition with acquiredJobDone channel (#148)
Found another data race while running the tests: https://github.com/coder/coder/runs/5044320845?check_suite_focus=true#step:7:83
__Issue:__ There is a race in the p.acquiredJobDone chan - in particular, there can be a case where we're waiting on the channel to finish (in close) with <-p.acquiredJobDone, but in parallel, an acquireJob could've been started, which would create a new channel for p.acquiredJobDone. There is a similar race in `close(..)`ing the channel, which also came up in test runs.
__Fix:__ Instead of recreating the channel everytime, we can use `sync.WaitGroup` to accomplish the same functionality - a semaphore to make close wait for the current job to wrap up.
* fix: Bump up workspace history timeout (#149)
This is an attempted fix for failures like: https://github.com/coder/coder/runs/5043435263?check_suite_focus=true#step:7:32
Looking at the timing of the test:
```
t.go:56: 2022-02-02 21:33:21.964 [DEBUG] (terraform-provisioner) <provision.go:139> ran apply
t.go:56: 2022-02-02 21:33:21.991 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
t.go:56: 2022-02-02 21:33:22.050 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
t.go:56: 2022-02-02 21:33:22.090 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
t.go:56: 2022-02-02 21:33:22.140 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
t.go:56: 2022-02-02 21:33:22.195 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
t.go:56: 2022-02-02 21:33:22.240 [DEBUG] (provisionerd) <provisionerd.go:162> skipping acquire; job is already running
workspacehistory_test.go:122:
Error Trace: workspacehistory_test.go:122
Error: Condition never satisfied
Test: TestWorkspaceHistory/CreateHistory
```
It appears that the `terraform apply` job had just finished - with less than a second to spare until our `require.Eventually` completes - but there's still work to be done (ie, collecting the state files). So my suspicion is that terraform might, in some cases, exceed our 5s timeout.
Note that in the setup for this test - there is a similar project history wait that waits for 15s, so I borrowed that here.
In the future - we can look at potentially using a simple echo provider to exercise this in the unit test, in a way that is more reliable in terms of timing. I'll log an issue to track that.
Co-authored-by: Bryan <bryan@coder.com>
* fix: Synchronize peer logging with a channel
We were depending on the close mutex to properly
report connection state. This ensures the RTC
connection is properly closed before returning.
* Disable pion logging
* Remove buffer
* Try ICE servers
* Remove flushed
* Add diagram explaining handshake
* Fix candidate accept ordering
* Add debug logging to peerbroker
* Fix send ordering
* Lock adding ICE candidate
* Add test for negotiating out of order
* Reduce connection to a single negotiation channel
* Improve test times by pre-installing Terraform
* Lock remote session description being applied
* Organize conn
* Revert to multi-channel setup
* Properly close ICE gatherer
* Improve comments
* Try removing buffered candidates
* Buffer local and remote messages
* Log dTLS transport state
* Add pion logging
Having a mixture of abbreviations in the codebase reduces
clarity. Although opts is common for options, I'd rather
set a precedent of clarifying verbosity.
* chore: Update pion/ice fork to resolve goroutine leak
* Flush remote too
* Add logs for setting the description
* Try locking only on remote
* Remove local bufferring in favor of remote
* Remove unused flush func
* Set candidates flushed to true
* Defer flush until the end of negotiation
* Buffer ICE candidates
* Add comment clarifying channel buffer
* Flush after handshake
* Move away from fork
* Ignore pion/ice leaks
* chore: Buffer remote candidates like local
This was added for local candidates, and is required for remote
to prevent a race where they are added before a negotiation is
complete.
I removed the mutex earlier, because it would cause a different race.
I didn't realize the remote candidates wouldn't be buffered,
but with this change they are!
* Use local description instead
* Add logging for candidate flush
* Fix race with atomic bool
* Simplify locks
* Add mutex to flush
* Reset buffer
* Remove leak dependency to limit confusion
* Fix ordering
* Revert channel close
* Flush candidates after remote session description is set
* Bump up count to ensure race is fixed
* Use custom ICE dependency
* Fix data race
* Lower timeout to make for fast CI
* Add back mutex to prevent race
* Improve debug logging
* Lock on local description
* Flush local candidates uniquely
* Fix race
* Move mutex to prevent candidate send race
* Move lock to handshake so no race can occur
* Reduce timeout to improve test times
* Move unlock to defer
* Use flushed bool instead of checking remote
* chore: Fix race in collecting ICE Candidates
This logic was flawed previously. ICE Candidates could collect
before a negotiation was triggered, which led to a race where
candidates would be lost. Candidates can no longer be lost,
and we removed some code 😎.
* Add comment describing fix
* Use upstream dependency to fix goroutine leak
* Use upstream dependency to fix goroutine leak
* feat: Add organizations endpoint for users
This moves the /user endpoint to /users/me instead. This
will reduce code duplication.
This adds /users/<name>/organizations to list organizations
a user has access to. It doesn't contain the permissions a
user has over the organizations, but that will come in a future
contribution.
* Fix requested changes
* Fix tests
* Fix timeout
* Add test for UserOrgs
* Add test for userparam getting
* Add test for NoUser
* ci: Run tests using PostgreSQL database and mock
This allows us to use the mock database for quick iterative testing,
and have confidence from CI using a real PostgreSQL database.
PostgreSQL tests are only ran on Linux. They are *really* slow on MacOS
and Windows runners, and don't provide much additional confidence.
* Only run PostgreSQL tests once for speed
* Fix race condition of log after close
Not all resources were cleaned up immediately after a peer connection was
closed. DataChannels could have a goroutine exit after Close() prior to this.
* Fix comment
* chore: Fix golangci-lint configuration and patch errors
Due to misconfiguration of a linting rules directory, our linter has not been
working properly. This change fixes the configuration issue, and all remaining
linting errors.
* Fix race in peer logging
* Fix race and return
* Lock on bufferred amount low
* Fix mutex lock
* test: Increase disconnectTimeout to reduce test flakes
WebRTC uses UDP, which means a network connection is never open or closed. It uses timeouts to determine connection state; on a slow CI runner, these timeouts could be reached. Increasing this timeout should reduce flakes, but is unlikely to remove this flake entirely.
* Fix close after offline
* Run tests in parallel
* fix: Lock when obtaining a peer connection answer<->offer
This fixes a race in the peerbroker package where ICE candidates could be added before the connection was negotiated. This would result in the connection failing.
* Remove unnecessary log
* feat: Create broker for negotiating connections
WebRTC require an exchange of encryption keys and network hops to connect. This package pipes the exchange over gRPC. This will be used in all connecting clients and agents.
* Regenerate protobuf definition
* Cache Go build and test
* Fix gRPC language with dRPC
Co-authored-by: Bryan <bryan@coder.com>
Co-authored-by: Bryan <bryan@coder.com>
This package was pulled straight from github.com/coder/m. Nothing has been changed.
It will be used for networking clients<->workspaces, and coderd<->provisionerd.
Kyle Carberry