opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-10-28 02:42:54 +08:00
Code Issues Packages Projects Releases Wiki Activity
7,186 Commits 479 Branches 412 Tags
3d707cbe5abd083700a50dcb4cca7e007e893f38
Commit Graph
100 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
Spike Curtis 1832a755e1 docs: describe AWS hard NAT (#13205) 
Documents what I've learned about getting direct connections on AWS.  Several customers have had issues.
 2024-05-08 20:29:12 +04:00
Spike Curtis dfd27f559e Revert "chore: fix build ci (#13164)" (#13180) 
This reverts commit 886a97b425.
 2024-05-06 13:13:24 +00:00
Spike Curtis deee9492e3 Revert "fix: install openrc service on alpine (#12294) (#12870)" (#13178) 
This reverts commit b20c63c185.
 2024-05-06 16:48:19 +04:00
Spike Curtis e76b595052 fix: use a native websocket.NetConn for agent RPC client (#13142) 
One cause of #13139 is a peculiar failure mode of `WebsocketNetConn` which causes it to return `context.Canceled` in some circumstances when the underlying websocket fails.  We have special processing for that error in the `agent.run()` routine, which is erroneously being triggered.

Since we don't actually need the returned context from `WebsocketNetConn`, we can simplify and just use the netConn from the `websocket` library directly.
 2024-05-06 15:00:34 +04:00
Spike Curtis d51c6912a7 fix: make handleManifest always signal dependents (#13141) 
Fixes #13139

Using a bare channel to signal dependent goroutines means that we can only signal success, not failure, which leads to deadlock if we fail in a way that doesn't cause the whole `apiConnRoutineManager` to tear down routines.

Instead, we use a new object called a `checkpoint` that signals success or failure, so that dependent routines get unblocked if the routine they depend on fails.
 2024-05-06 14:47:41 +04:00
Spike Curtis 2efb46a10e chore: remove superfluous context.Canceled handling (#13140) 
Removes a check for `context.Canceled` inside the `handleManifest` routine.  This checking is handled in the `apiConnRoutineManager`, so checking inside the handler is redundant.
 2024-05-06 14:33:16 +04:00
Spike Curtis 3de737fdc8 fix: start packet capture immediately on speedtest (#13128) 
I initially made this change when hacking wgengine to also capture wireguard packets going into the magicsock, so that we could capture the initial wireguard handshake. 

I don't think we should ship that additional capture logic, but... it seems generally useful to capture packets from the get go on speedtest, so that you can see disco and pings before the TCP speedtest session starts.
 2024-05-02 19:44:32 +04:00
Spike Curtis a231b5aef5 feat: add src_id and dst_id indexes to tailnet_tunnels (#12911) 
Fixes #12780

Adds indexes to the `tailnet_tunnels` table to speed up `GetTailnetTunnelPeerIDs` and `GetTailnetTunnelPeerBindings` queries, which match on `src_id` and `dst_id`.
 2024-04-11 10:05:53 +04:00
Spike Curtis 06eae954c9 fix: stop sending DeleteTailnetPeer when coordinator is unhealthy (#12925) 
fixes #12923

Prevents Coordinate peer connections from generating spurious database queries like DeleteTailnetPeer when the coordinator is unhealthy.

It does this by checking the health of the querier before accepting a connection, rather than unconditionally accepting it only for it to get swatted down later.
 2024-04-10 22:49:13 +04:00
Spike Curtis 5469011018 fix: stop logging session shutdown as warning (#12922) 
A customer hit like 200k of ErrSessionShutdown, which just dupes any errors we would have generated when shutting down the session for e.g. Ping failures.
 2024-04-10 11:50:46 +04:00
Spike Curtis 3b7380fa00 fix: fix race in assertWorkspaceLastUsedAtUpdated (#12899) 
fixes #12789

Stats are collected asynchronously with respect to sessions ending.  Flush repeatedly so that we pick up the collection if we missed it.
 2024-04-08 16:22:33 +04:00
Spike Curtis 3addf7ac5d fix: use latest coder/tailscale 
Bad merge in #12252 clobbered #12574
 2024-04-01 22:42:38 +04:00
Spike Curtis 51491fc01b fix(provisionersdk): change test to use bash script instead of binary echo (#12759) 
Just upgraded to macOS 14.4 and TestAgentScript/Run fails for me with error `signal: killed`.  I opened the test directory in a terminal and sure enough, when you execute the `echo` binary, it is immediately killed.  The binary has no extended attributes and is byte-identical to the one in `/bin/`.

This fix uses a different strategy: instead of copying the `echo` binary from the system around, we just copy a small bash script that _calls_ the `echo` command.
 2024-03-26 14:37:20 +04:00
Spike Curtis fe6def31eb feat: upgrade tailscale fork to set TCP options for perf (#12574) 
Upgrades our tailscale fork to include some TCP options tweaks for performance. c.f. https://github.com/coder/tailscale/pull/46
 2024-03-13 16:02:02 +04:00
Spike Curtis 51707446d0 fix: stop holding Pubsub mutex while calling pq.Listener (#12518) 
fixes #11950

https://github.com/coder/coder/issues/11950#issuecomment-1987756088 explains the bug

We were also calling into `Unlisten()` and `Close()` while holding the mutex.  I don't believe that `Close()` depends on the notification loop being unblocked, but it's hard to be sure, and the safest thing to do is assume it could block.

So, I added a unit test that fakes out `pq.Listener` and sends a bunch of notifies every time we call into it to hopefully prevent regression where we hold the mutex while calling into these functions.

It also removes the use of a `context.Context` to stop the PubSub -- it must be explicitly `Closed()`.  This simplifies a bunch of the logic, and is how we use the pubsub anyway.
 2024-03-12 09:44:12 +04:00
Spike Curtis b96f6b48a4 fix: ensure ssh cleanup happens on cmd error 
I noticed in my logs that sometimes `coder ssh` doesn't gracefully disconnect from the coordinator.

The cause is the `closerStack` construct we use in that function.  It has two paths to start closing things down:

1. explicit `close()` which we do in `defer`
2. context cancellation, which happens if the cli function returns an error

sometimes the ssh remote command returns an error, and this triggers context cancellation of the `closerStack`.  That is fine in and of itself, but we still want the explicit `close()` to wait until everything is closed before returning, since that's where we do cleanup, including the graceful disconnect.  Prior to this fix the `close()` just immediately exits if another goroutine is closing the stack.  Here we add a wait until everything is done.
 2024-03-07 17:26:49 +04:00
Spike Curtis 4e7beee102 feat: show tailnet peer diagnostics after coder ping (#12314) 
Beginnings of a solution to #12297 

Doesn't cover disco or definitively display whether we successfully connected to DERP, but shows some checklist diagnostics for connecting to an agent.

For this first PR, I just added it to `coder ping` to see how we like it, but could be incorporated into `coder ssh` _et al._ after a timeout.

```
$ coder ping dogfood2
p2p connection established in 147ms
pong from dogfood2 p2p via  95.217.xxx.yyy:42631  in 147ms
pong from dogfood2 p2p via  95.217.xxx.yyy:42631  in 140ms
pong from dogfood2 p2p via  95.217.xxx.yyy:42631  in 140ms
✔ preferred DERP region 999 (Council Bluffs, Iowa)
✔ sent local data to Coder networking coodinator
✔ received remote agent data from Coder networking coordinator
    preferred DERP 10013 (Europe Fly.io (Paris))
    endpoints: 95.217.xxx.yyy:42631, 95.217.xxx.yyy:37576, 172.17.0.1:37576, 172.20.0.10:37576
✔ Wireguard handshake 11s ago
```
 2024-02-27 22:04:46 +04:00
Spike Curtis b0afffbafb feat: use v2 API for agent metadata updates (#12281) 
Switches the agent to report metadata over the v2 API.

Fixes #10534
 2024-02-26 09:50:19 +04:00
Spike Curtis aa7a9f5cc4 feat: use v2 API for agent lifecycle updates (#12278) 
Agent uses the v2 API to post lifecycle updates.

Part of #10534
 2024-02-23 15:24:28 +04:00
Spike Curtis 4cc132cea0 feat: switch agent to use v2 API for sending logs (#12068) 
Changes the agent to use the new v2 API for sending logs, via the logSender component.

We keep the PatchLogs function around, but deprecate it so that we can test the v1 endpoint.
 2024-02-23 11:27:15 +04:00
Spike Curtis af3fdc68c3 chore: refactor agent routines that use the v2 API (#12223) 
In anticipation of needing the `LogSender` to run on a context that doesn't get immediately canceled when you `Close()` the agent, I've undertaken a little refactor to manage the goroutines that get run against the Tailnet and Agent API connection.

This handles controlling two contexts, one that gets canceled right away at the start of graceful shutdown, and another that stays up to allow graceful shutdown to complete.
 2024-02-23 11:04:23 +04:00
Spike Curtis da376549a3 fix: stop waiting for Agent in a goroutine in ssh test (#12268) 
Fixes race seen here: https://github.com/coder/coder/runs/21852483781

What happens is that the agent connects, completes the test, and then disconnects before the Eventually condition runs.  The waiter then times out because it's looking for a connected agent.

Then, since it's a `require` in a goroutine, that causes the `tGo` cleanup to hang and the whole test suite to timeout after 10 minutes.

Anyway, `agenttest.New` doesn't block, and we don't actually need to wait for the agent to connect, since a successful SSH session is evidence that it connected.
 2024-02-22 17:01:06 +04:00
Spike Curtis ab4cb66e00 feat: add WaitUntilEmpty to LogSender (#12159) 
We'll need this to be able to tell when all outstanding logs have been sent, as part of graceful shutdown.
 2024-02-20 11:11:31 +04:00
Spike Curtis 081e37d7d9 chore: move LogSender to agentsdk (#12158) 
Moves the LogSender to agentsdk and deprecates LogsSender based on the v1 API.
 2024-02-20 10:44:20 +04:00
Spike Curtis 2aff014e5d feat: add logSender for sending logs on agent v2 API (#12046) 
Adds a new subcomponent of the agent for queueing up logs until they can be sent over the Agent API.

Subsequent PR will change the agent to use this instead of the HTTP API for posting logs.

Relates to #10534
 2024-02-15 16:57:17 +04:00
Spike Curtis 627232eae9 fix: fix pgcoord to delete coordinator row last (#12155) 
Fixes #12141
Fixes #11750

PGCoord shutdown was uncoordinated, so an update at an inopportune time during shutdown would be rejected because the coordinator row was already deleted.

This PR ensures that the PGCoord subcomponents that write updates are shut down before we take down the heartbeats, which is responsible for deleting the coordinator row.
 2024-02-15 16:34:29 +04:00
Spike Curtis 2d0b9106c0 fix: change servertailnet to register the DERP dialer before setting DERP map (#12137) 
I noticed a possible race where tailnet.Conn can try to dial the embedded region before we've set our custom dialer that send the DERP in-memory.  This closes that race and adds a test case for servertailnet with no STUN and an embedded relay
 2024-02-15 10:51:12 +04:00
Spike Curtis 04991f425a fix: set node callback each time we reinit the coordinator in servertailnet (#12140) 
I think this will resolve #12136 but lets get a proper test at the system level before closing.

Before this change, we only register the node callback at start of day for the server tailnet.  If the coordinator changes, like we know happens when we are licensed for the PGCoordinator, we close the connection to the old coord, and open a new one to the new coord.

The callback is designed to direct the updates to the new coordinator, but there is nothing that specifically triggers it to fire after we connect to the new coordinator.

If we have STUN, then period re-STUNs will generally get it to fire eventually, but without STUN it we could go indefinitely without a callback.

This PR changes the servertailnet to re-register the callback each time we reconnect to the coordinator.  Registering a callback (even if it's the same callback) triggers an immediate call with our node information, so the new coordinator will have it.
 2024-02-14 20:45:31 +04:00
Spike Curtis 5a0d240bc3 feat: expose DERP server debug metrics (#12135) 
Adds some debug endpoints for looking into the DERP server.

The `api/v2/debug/derp/traffic` endpoint requires the `ss` utility to be present in order to function.  I have *not* added the `iproute2` package to our base image as it adds 11MB, so this endpoint won't be useful by default.  However, in a debugging situation, we could exec into the container and then `apk add iproute2`, or build a special debug image.

The `api/v2/debug/expvar` handler contains DERP metrics as well as commandline and memstats.

Example:

```
{
"alert_failed": 0,
"alert_generated": 0,
"cmdline": ["/Users/spike/repos/coder/build/coder_darwin_arm64","--global-config","/Users/spike/repos/coder/.coderv2","server","--http-address","0.0.0.0:3000","--swagger-enable","--access-url","http://127.0.0.1:3000","--dangerous-allow-cors-requests=true"],
"derp": {"accepts": 1, "average_queue_duration_ms": 0, "bytes_received": 0, "bytes_sent": 0, "counter_packets_dropped_reason": {"gone_disconnected": 0, "gone_not_here": 0, "queue_head": 0, "queue_tail": 0, "unknown_dest": 0, "unknown_dest_on_fwd": 0, "write_error": 0}, "counter_packets_dropped_type": {"disco": 0, "other": 0}, "counter_packets_received_kind": {"disco": 0, "other": 0}, "counter_tcp_rtt": {}, "counter_total_dup_client_conns": 0, "gauge_clients_local": 1, "gauge_clients_remote": 0, "gauge_clients_total": 1, "gauge_current_connections": 1, "gauge_current_dup_client_conns": 0, "gauge_current_dup_client_keys": 0, "gauge_current_file_descriptors": 0, "gauge_current_home_connections": 1, "gauge_memstats_sys0": 20874504, "gauge_watchers": 0, "got_ping": 0, "home_moves_in": 0, "home_moves_out": 0, "multiforwarder_created": 0, "multiforwarder_deleted": 0, "packet_forwarder_delete_other_value": 0, "packets_dropped": 0, "packets_forwarded_in": 0, "packets_forwarded_out": 0, "packets_received": 0, "packets_sent": 0, "peer_gone_disconnected_frames": 0, "peer_gone_not_here_frames": 0, "sent_pong": 0, "unknown_frames": 0, "version": "1.47.0-dev20240214-t64db8c604"},
"memstats": {"Alloc":286506256,"TotalAlloc":297594632,"Sys":310621512,"Lookups":0,"Mallocs":304204,"Frees":171570,"HeapAlloc":286506256,"HeapSys":294060032,"HeapIdle":3694592,"HeapInuse":290365440,"HeapReleased":3620864,"HeapObjects":132634,"StackInuse":3735552,"StackSys":3735552,"MSpanInuse":347256,"MSpanSys":358512,"MCacheInuse":9600,"MCacheSys":15600,"BuckHashSys":1469877,"GCSys":9434896,"OtherSys":1547043,"NextGC":551867656,"LastGC":1707892877408883000,"PauseTotalNs":1247000,"PauseNs":[200333,229375,239875,209542,106958,203792,57125,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],"PauseEnd":[1707892876217481000,1707892876219726000,1707892876222273000,1707892876226151000,1707892876234815000,1707892877398146000,1707892877408883000,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],"NumGC":7,"NumForcedGC":0,"GCCPUFraction":0.0022425810335762954,"EnableGC":true,"DebugGC":false,"BySize":[{"Size":0,"Mallocs":0,"Frees":0},{"Size":8,"Mallocs":14396,"Frees":9143},{"Size":16,"Mallocs":89090,"Frees":50507},{"Size":24,"Mallocs":40839,"Frees":24456},{"Size":32,"Mallocs":22404,"Frees":12379},{"Size":48,"Mallocs":51174,"Frees":23718},{"Size":64,"Mallocs":15406,"Frees":3501},{"Size":80,"Mallocs":6688,"Frees":2352},{"Size":96,"Mallocs":2567,"Frees":374},{"Size":112,"Mallocs":19371,"Frees":16883},{"Size":128,"Mallocs":2873,"Frees":1061},{"Size":144,"Mallocs":5600,"Frees":2742},{"Size":160,"Mallocs":2159,"Frees":622},{"Size":176,"Mallocs":454,"Frees":86},{"Size":192,"Mallocs":227,"Frees":128},{"Size":208,"Mallocs":1407,"Frees":732},{"Size":224,"Mallocs":1365,"Frees":1090},{"Size":240,"Mallocs":82,"Frees":48},{"Size":256,"Mallocs":310,"Frees":162},{"Size":288,"Mallocs":1945,"Frees":562},{"Size":320,"Mallocs":1200,"Frees":458},{"Size":352,"Mallocs":133,"Frees":33},{"Size":384,"Mallocs":582,"Frees":51},{"Size":416,"Mallocs":747,"Frees":200},{"Size":448,"Mallocs":113,"Frees":22},{"Size":480,"Mallocs":34,"Frees":21},{"Size":512,"Mallocs":951,"Frees":91},{"Size":576,"Mallocs":364,"Frees":122},{"Size":640,"Mallocs":532,"Frees":270},{"Size":704,"Mallocs":93,"Frees":39},{"Size":768,"Mallocs":83,"Frees":35},{"Size":896,"Mallocs":308,"Frees":175},{"Size":1024,"Mallocs":226,"Frees":122},{"Size":1152,"Mallocs":198,"Frees":100},{"Size":1280,"Mallocs":314,"Frees":171},{"Size":1408,"Mallocs":77,"Frees":47},{"Size":1536,"Mallocs":80,"Frees":54},{"Size":1792,"Mallocs":199,"Frees":107},{"Size":2048,"Mallocs":112,"Frees":48},{"Size":2304,"Mallocs":71,"Frees":32},{"Size":2688,"Mallocs":206,"Frees":81},{"Size":3072,"Mallocs":39,"Frees":15},{"Size":3200,"Mallocs":16,"Frees":7},{"Size":3456,"Mallocs":44,"Frees":29},{"Size":4096,"Mallocs":192,"Frees":83},{"Size":4864,"Mallocs":44,"Frees":25},{"Size":5376,"Mallocs":105,"Frees":43},{"Size":6144,"Mallocs":25,"Frees":5},{"Size":6528,"Mallocs":22,"Frees":7},{"Size":6784,"Mallocs":3,"Frees":0},{"Size":6912,"Mallocs":4,"Frees":2},{"Size":8192,"Mallocs":59,"Frees":10},{"Size":9472,"Mallocs":31,"Frees":12},{"Size":9728,"Mallocs":5,"Frees":2},{"Size":10240,"Mallocs":5,"Frees":0},{"Size":10880,"Mallocs":27,"Frees":11},{"Size":12288,"Mallocs":4,"Frees":1},{"Size":13568,"Mallocs":4,"Frees":2},{"Size":14336,"Mallocs":9,"Frees":2},{"Size":16384,"Mallocs":10,"Frees":2},{"Size":18432,"Mallocs":4,"Frees":2}]},
"warning_failed": 0,
"warning_generated": 0
}
```

If we find the DERP metrics useful we could consider how to include them in Prometheus scrapes based on the tailnet `varz` package.  That's for a later PR if at all.
 2024-02-14 15:11:45 +04:00
Spike Curtis 92b2e26a48 feat: send log limit exceeded in response, not error (#12078) 
When we exceed the db-imposed limit of logs, we need to communicate that back to the agent.  In v1 we did it with a 4xx-level HTTP status, but with dRPC, the errors are delivered as strings, which feels fragile to me for something we want to gracefully handle.

So, this PR adds the log limit exceeded as a field on the response message, and fixes the API handler to set it as appropriate instead of an error.
 2024-02-09 16:17:20 +04:00
Spike Curtis 1f5a6d59ba chore: consolidate websocketNetConn implementations (#12065) 
Consolidates websocketNetConn from multiple packages in favor of a central one in codersdk
 2024-02-09 11:39:08 +04:00
Spike Curtis 151aaadc23 fix: allow startup scripts larger than 32k (#12060) 
Fixes #12057 and adds a regression test.
 2024-02-07 22:26:42 +04:00
Spike Curtis 1cf4b62867 feat: change agent to use v2 API for reporting stats (#12024) 
Modifies the agent to use the v2 API to report its statistics, using the `statsReporter` subcomponent.
 2024-02-07 15:26:41 +04:00
Spike Curtis 213ae69bee fix: start timer before subscribing to avoid test race (#12031) 
Fixes #12030

This is a good example of the kind of thing I'd like to address with a time-testing lib.  The problem is that there is a race between the watchdog starting it's timer and the test incrementing the time.  What would make this easier is if the time-testing library could wait for and assert the call to start the timer before incrementing the time.
 2024-02-06 20:21:23 +04:00
Spike Curtis e09cd2c6bd feat: add watchdog to pubsub (#12011) 
adds a watchdog to our pubsub and runs it for Coder server.

If the watchdog times out, it triggers a graceful exit in `coder server` to give any provisioner jobs a chance to shut down.

c.f. #11950
 2024-02-06 16:58:45 +04:00
Spike Curtis c84a637116 fix: stop logging error on query canceled (#12017) 
Fixes flake seen here: https://github.com/coder/coder/actions/runs/7782340530/job/21218566449
 2024-02-06 08:43:34 +04:00
Spike Curtis e5ba586e30 fix: fix graceful disconnect in DialWorkspaceAgent (#11993) 
I noticed in testing that the CLI wasn't correctly sending the disconnect message when it shuts down, and thus agents are seeing this as a "lost" peer, rather than a "disconnected" one. 

What was happening is that we just used a single context for everything from the netconn to the RPCs, and when the context was canceled we failed to send the disconnect message due to canceled context.

So, this PR splits things into two contexts, with a graceful one set to last up to 1 second longer than the main one.
 2024-02-05 14:01:37 +04:00
Spike Curtis bb99cb7d2b chore: move FakeCoordinator to tailnettest (#11992) 
Moves FakeCoordinator to tailnettest since it's reused in testing multiple packages in this stack of PRs.
 2024-02-05 13:49:32 +04:00
Spike Curtis 646ac942b2 chore: rename FakeCoordinator for export (#11991) 
Part of a stack that fixes graceful disconnect from the CLI to tailnet.  I reuse FakeCoordinator in a test for graceful disconnects.
 2024-02-05 13:33:31 +04:00
Spike Curtis 1aa117b9ec chore: rename client Listen to ConnectRPC (#11916) 
ConnectRPC seems more appropriate for this function
 2024-02-01 14:44:11 +04:00
Spike Curtis d5a98cc6d7 fix: avoid race in TestPGPubsub_Metrics by using Eventually (#11973) 
Annoyingly, prometheus Registry collects metrics async, which is causing our test to be racy.  They also don't export enough from the Metric interface for us to replicate a synchronous collect, so we have to use Eventually to test.
 2024-02-01 12:10:19 +04:00
Spike Curtis 5a359d50dd feat: add metrics to PGPubsub (#11971) 
Adds prometheus metrics to PGPubsub for monitoring its health and performance in production.

Related to #11950 --- additional diagnostics to help figure out what's happening
 2024-02-01 11:25:03 +04:00
Spike Curtis 073d1f7078 chore: remove pingWebSocket since yamux runs keepalives (#11914) 
Since we run yamux over the websocket, we don't need to ping at the websocket layer because yamux has a 30 second keepalive mechanism enabled in the default config.
 2024-02-01 09:48:58 +04:00
Spike Curtis cc0dc103b6 chore: remove agentsdk client RPC() function (#11913) 
The RPC() function isn't called, since Listen() was modified to do this job.

Listen() has the right signature, since it returns a drpc.Conn, rather than the Agent API.  That's because tailnet v2 and agent v2 are separate APIs served over the same connection.

It might be clearer to rename `Listen()` to `RPC()` but I'll save that for a different PR.
 2024-02-01 08:22:12 +04:00
Spike Curtis eb03e4490a feat: add statsReporter for reporting stats on agent v2 API (#11920) 
Adds a new statsReporter subcomponent of the agent, which in a later PR will be used to report stats over the v2 API.

Refactors the logic a bit so that we can handle starting and stopping stats reporting if the agent API connection drops and reconnects.
 2024-02-01 08:21:01 +04:00
Spike Curtis b79785c86f feat: move agent v2 API connection monitoring to yamux layer (#11910) 
Moves monitoring of the agent v2 API connection to the yamux layer.

Present behavior monitors this at the websocket layer, and closes the websocket on completion. This can cause yamux to hit unexpected errors since the connection is closed underneath it.

This might be the cause of yamux errors that some customers are seeing

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/tCz4CxRU9jhAJ7zH8RTi/53b8b5ef-e9e5-44a5-b559-99c37c136071.png)

In any case, it's more graceful to close yamux first and let yamux close the underlying websocket.  That should limit yamux error logging to truly unexpected/error cases.

The only downside is that the yamux `Close()` doesn't accept a reason, so if the agent becomes outdated and we close the API connection, the agent just sees the connection close without a reason.  I'm not sure we log this at the agent anyway, but it would be nice.  I think more accurate logging on Coderd are more important.

I've also added some logging when the monitor disconnects for reasons other than the context being canceled (e.g. agent outdated, failed pings).
 2024-02-01 08:18:35 +04:00
Spike Curtis 13e214f7f1 feat: add logging to agent yamux session (#11912) 
Log yamux errors and warnings in the agent.
 2024-02-01 08:18:13 +04:00
Spike Curtis a34cada09a feat: add logging to pgPubsub (#11953) 
Should be helpful for #11950

Adds a logger to pgPubsub and logs various events, most especially connection and disconnection from postgres.
 2024-01-31 15:49:16 +04:00
Spike Curtis 1c8b803785 feat: add logging to pgcoord subscribe/unsubscribe (#11952) 
Adds logging to unsubscribing from peer and tunnel updates in pgcoordinator, since #11950 seems to be problem with these subscriptions
 2024-01-31 12:15:58 +04:00
Spike Curtis 520b12e1a2 fix: close MultiAgentConn when coordinator closes (#11941) 
Fixes an issue where a MultiAgentConn isn't closed properly when the coordinator it is connected to is closed.

Since servertailnet checks whether the conn is closed before reinitializing, it is important that we check this, otherwise servertailnet can get stuck if the coordinator closes (e.g. when we switch from AGPL to PGCoordinator after decoding a license).
 2024-01-31 00:38:19 +04:00
Spike Curtis 0fc177203e feat: use agent v2 API to update app health (#11889) 
Use the Agent v2 API to update App Health
 2024-01-30 11:35:12 +04:00
Spike Curtis 2599850e54 feat: use agent v2 API to post startup (#11877) 
Uses the v2 Agent API to post startup information.
 2024-01-30 11:23:28 +04:00
Spike Curtis da8bb1c198 feat: use agent v2 API to fetch manifest (#11832) 
Agent uses the v2 API to obtain the manifest, instead of the HTTP API.
 2024-01-30 10:11:28 +04:00
Spike Curtis 9cf4e7f15a fix: prevent agent_test.go from failing on error logs (#11909) 
We're failing tests on error logs like this: https://github.com/coder/coder/actions/runs/7706053882/job/21000984583

Unfortunately, the error we hit, when the underlying connection is closed, is unexported, so we can't specifically ignore it.

Part of the issue is that agent.Close() doesn't wait for these goroutines to complete before returning, so the test harness proceeds to close the connection. This looks to our product code like the network connection failing.  It would be possible to fix this, but just doesn't seem worth it for the extra insurance of catching other error logs in these tests.
 2024-01-30 10:04:01 +04:00
Spike Curtis d3983e4dba feat: add logging to client tailnet yamux (#11908) 
Adds logging to yamux when used for tailnet client connections, e.g. CLI and wsproxy.  This could be useful for debugging connection issues with tailnet v2 API.
 2024-01-30 09:58:59 +04:00
Spike Curtis 0eff646c31 chore: move proto to sdk conversion to agentsdk (#11831) 
`agentsdk` depends on `agent/proto` because it needs to get the version to dial.

Therefore, the conversion routines need to live in `agentsdk` so that we can convert to and from the Manifest.

I briefly considered refactoring the agent to only reference `proto.Manifest`, but decided against it because we might have multiple protocol versions in the future, its useful to have a protocol-independent data structure.
 2024-01-30 09:04:56 +04:00
Spike Curtis 1e8a9c09fe chore: remove legacy wsconncache (#11816) 
Fixes #8218

Removes `wsconncache` and related "is legacy?" functions and API calls that were used by it.

The only leftover is that Agents still use the legacy IP, so that back level clients or workspace proxies can dial them correctly.

We should eventually remove this: #11819
 2024-01-30 07:56:36 +04:00
Spike Curtis 13e24f21e4 feat: use Agent v2 API for Service Banner (#11806) 
Agent uses the v2 API for the service banner, rather than the v1 HTTP API.

One of several for #10534
 2024-01-30 07:44:47 +04:00
Spike Curtis 207328ca50 feat: use appearance.Fetcher in agentapi (#11770) 
This PR updates the Agent API to use the appearance.Fetcher, which is set by entitlement code in Enterprise coderd.

This brings the agentapi into compliance with the Enterprise feature.
 2024-01-29 21:22:50 +04:00
Spike Curtis b2bc3fff33 fix: wait for new template version before promoting (#11874) 
Fixes a test flake due to not waiting for the correct template version prior to promoting it.
 2024-01-29 19:29:56 +04:00
Spike Curtis bc4ae53261 chore: refactor Appearance to an interface callable by AGPL code (#11769) 
The new Agent API needs an interface for ServiceBanners, so this PR creates it and refactors the AGPL and Enterprise code to achieve it.

Before we depended on the fact that the HTTP endpoint was missing to serve an empty ServiceBanner on AGPL deployments, but that won't work with dRPC, so we need a real interface to call.
 2024-01-29 12:17:31 +04:00
Spike Curtis 37e9479815 fix: fix TestServiceBanners/Agent (#11768) 
The original test is bugged in that it

1. creates a new AGPL coderd with a new database, so no appearance is set in the DB.
2. overwrites the agentClient so the assertion after removing the license is against the AGPL coderd
 2024-01-29 11:56:33 +04:00
Spike Curtis f9fdd44510 feat: change codersdk to use tailnet v2 for DERPMap updates (#11736) 
fixes #10533


refactors `codersdk` workspace agent dialer to use a single websocket connection to the tailnet v2 API for both coordination and DERPMap updates, rather than separate websockets (and the v1 API for DERPMaps).
 2024-01-29 11:26:50 +04:00
Spike Curtis 4825b7ccd2 fix: use new context after t.Parallel in TestOAuthAppSecrets 
c.f. https://coder.com/blog/go-testing-contexts-and-t-parallel

fixes flakes like https://github.com/coder/coder/runs/20856469613
 2024-01-27 08:45:43 +04:00
Spike Curtis d6baa3cab0 fix: stop running tests that exec sh scripts in parallel (#11834) 
Ok, so my last attempt at a fix here failed

https://github.com/coder/coder/actions/runs/7666229961/job/20893608286

I have a new theory: it's not the `terraform` binary that's busy, it's actually `fake_cancel.sh` and it gets marked busy when we `exec` it from the script we write.

Use of `exec` also replaces the executing code in place, rather than starting a new process/shell, so that's why the error we get says `terraform` is busy.
 2024-01-26 19:22:35 +04:00
Spike Curtis 5cbb76b47a fix: stop spamming DERP map updates for equivalent maps (#11792) 
Fixes 2 related issues:

1. wsconncache had incorrect logic to test whether to send DERPMap updates, sending if the maps were equivalent, instead of if they were _not equivalent_.
2. configmaps used a bugged check to test equality between DERPMaps, since it contains a map and the map entries are serialized in random order. Instead, we avoid comparing the protobufs and instead depend on the existing function that compares `tailcfg.DERPMap`. This also has the effect of reducing the number of times we convert to and from protobuf.
 2024-01-24 16:27:15 +04:00
Spike Curtis f5dbc718a7 fix: accept agent RPC connection without version query parameter (#11790) 
Fixes an issue where Coder v2.7.1 agents connect to /api/v2/workspaceagents/me/rpc without a version query parameter
 2024-01-24 09:10:16 +04:00
Spike Curtis 059e533544 feat: agent uses Tailnet v2 API for DERPMap updates (#11698) 
Switches the Agent to use Tailnet v2 API to get DERPMap updates.

Subsequent PRs will do the same for the CLI (`codersdk`) and `wsproxy`.
 2024-01-23 14:42:07 +04:00
Spike Curtis 3e0e7f8739 feat: check agent API version on connection (#11696) 
fixes #10531

Adds a check for `version` on connection to the Agent API websocket endpoint.  This is primarily for future-proofing, so that up-level agents get a sensible error if they connect to a back-level Coderd.

It also refactors the location of the `CurrentVersion` variables, to be part of the `proto` packages, since the versions refer to the APIs defined therein.
 2024-01-23 14:27:49 +04:00
Spike Curtis eb12fd7d92 feat: make ServerTailnet set peers lost when it reconnects to the coordinator (#11682) 
Adds support to `ServerTailnet` to set all peers lost before attempting to reconnect to the coordinator. In practice, this only really affects `wsproxy` since coderd has a local connection to the coordinator that only goes down if we're shutting down or change licenses.
 2024-01-23 13:17:56 +04:00
Spike Curtis 5388a1b6d7 fix: use TSMP ping for reachability, not latency (#11749) 
Use TSMP ping for reachability, but leave Disco ping for when we call Ping() since we often use that to determine whether we have a direct connection.

Also adds unit tests to make sure Ping() returns direct connection vs DERP correctly.
 2024-01-22 17:37:15 +04:00
Spike Curtis 7ffd99cfe2 fix: use DiscoPing (partially reverts #11306) (#11744) 2024-01-22 12:40:21 +00:00
Spike Curtis 3d85cdfa11 feat: set peers lost when disconnected from coordinator (#11681) 
Adds support to Coordination to call SetAllPeersLost() when it is closed. This ensure that when we disconnect from a Coordinator, we set all peers lost.

This covers CoderSDK (CLI client) and Agent.  Next PR will cover MultiAgent (notably, `wsproxy`).
 2024-01-22 15:26:20 +04:00
Spike Curtis b7b936547d feat: add setAllPeersLost to the configMaps subcomponent (#11665) 
adds setAllPeersLost to the configMaps subcomponent of tailnet.Conn --- we'll call this when we disconnect from a coordinator so we'll eventually clean up peers if they disconnect while we are retrying the coordinator connection (or we don't succeed in reconnecting to the coordinator).
 2024-01-22 12:12:15 +04:00
Spike Curtis f01cab9894 feat: use tailnet v2 API for coordination (#11638) 
This one is huge, and I'm sorry.

The problem is that once I change `tailnet.Conn` to start doing v2 behavior, I kind of have to change it everywhere, including in CoderSDK (CLI), the agent, wsproxy, and ServerTailnet.

There is still a bit more cleanup to do, and I need to add code so that when we lose connection to the Coordinator, we mark all peers as LOST, but that will be in a separate PR since this is big enough!
 2024-01-22 11:07:50 +04:00
Spike Curtis 1f0e6ba6c6 fix: use raw syscalls to write binary we execute (#11684) 
Fixes flake seen here, I think

https://github.com/coder/coder/actions/runs/7565915337/job/20602500818

golang's file processing is complex, and in at least some cases it can return from a file.Close() call without having actually closed the file descriptor.

If we're holding open the file descriptor of an executable we just wrote, and try to execute it, it will fail with "text file busy" which is what we have seen.

So, to be extra sure, I've avoided the standard library and directly called the syscalls to open, write, and close the file we intend to use in the test.

I've also added some more logging so if it's some issue of multiple tests writing to the same location, the we might have a chance to see it.
 2024-01-18 16:21:11 +04:00
Spike Curtis 8910ac715c feat: add tailnet v2 support to wsproxy coordinate endpoint (#11637) 
wsproxy also needs to be updated to use tailnet v2 because the `tailnet.Conn` stores peers by ID, and the peerID was not being carried by the JSON protocol.  This adds a query param to the endpoint to conditionally switch to the new protocol.
 2024-01-18 10:10:36 +04:00
Spike Curtis 07427e06f7 chore: add setBlockEndpoints to nodeUpdater (#11636) 
nodeUpdater also needs block endpoints, so that it can stop sending nodes with endpoints.
 2024-01-18 10:02:15 +04:00
Spike Curtis 5b4de667d6 chore: add setCallback to nodeUpdater (#11635) 
we need to be able to (re-)set the node callback when we lose and regain connection to a coordinator over the network.
 2024-01-18 09:51:09 +04:00
Spike Curtis e725f9d7d4 chore: stop passing addresses on configMaps constructor (#11634) 
moving this out of the constructor so that setting this when creating a new `tailnet.Conn` triggers configuring the engine.
 2024-01-18 09:43:28 +04:00
Spike Curtis a514df71ed chore: add setDERPMap to configMaps (#11590) 
Add setDERPMap
 2024-01-18 09:34:30 +04:00
Spike Curtis 25e289e1f6 chore: add setAddresses to nodeUpdater (#11571) 
Adds setAddresses to nodeUpdater
 2024-01-18 09:24:16 +04:00
Spike Curtis 387723a596 fix: close pg PubSub listener to avoid race (#11640) 
Fixes flake as seen here: https://github.com/coder/coder/runs/20528529187
 2024-01-18 09:18:59 +04:00
Spike Curtis b173195e0d Revert "fix: detect JetBrains running on local ipv6 (#11653)" (#11664) 
This reverts commit 2d61d5332a.
 2024-01-17 15:38:39 +04:00
Spike Curtis 2aa3cbbd03 chore: add logging to nodeUpdater (#11569) 
Add debug logging for nodeUpdater and configMaps
 2024-01-17 14:15:45 +04:00
Spike Curtis bad2ce562e fix: stop asserting fuzz bytes written in test 
Fixes a flake seen here: https://github.com/coder/coder/actions/runs/7541558190/job/20528545916

```
=== FAIL: enterprise/provisionerd TestRemoteConnector_Fuzz (0.06s)
    t.go:84: 2024-01-16 12:32:27.024 [info]  connector: failed provisioner authentication  remote_addr=[::1]:45138 ...
        error= failed to receive jobID:
                   github.com/coder/coder/v2/enterprise/provisionerd.(*remoteConnector).authenticate
                       /home/runner/actions-runner/_work/coder/coder/enterprise/provisionerd/remoteprovisioners.go:438
                 - bufio.Scanner: token too long
    t.go:84: 2024-01-16 12:32:27.024 [debu]  connector: closed connection  remote_addr=[::1]:45138  error=<nil>
    remoteprovisioners_test.go:209: 
            Error Trace:    /home/runner/actions-runner/_work/coder/coder/enterprise/provisionerd/remoteprovisioners_test.go:209
            Error:          "2992256" is not less than "2097152"
            Test:           TestRemoteConnector_Fuzz
            Messages:       should not allow more than 1 MiB
```

This was an attempt to test that malicious actors can't abuse our authentication protocol to make us allocate a bunch of memory.
However, the test asserted on the number of bytes sent by the fuzzer, not the number of bytes read (& allocated) by the service.  The former is affected by network queue sizes and is thus flaky without actively managing the socket queues, which I don't think we want to do.

In actual practise, the thing that matters is how much memory the bufio Scanner allocates. By inspection, the scanner will allocate up to 64k, and testing this is true devolves into testing the go standard library, which I don't think is worth doing.

So... let's just drop the assertion because 

a) its flaky, 

b) it doesn't test what we actually want to test, 

c) the behavior we actually care about is part of the standard library.
 2024-01-17 12:59:45 +04:00
Spike Curtis 38d9ce5267 chore: add setStatus support to nodeUpdater (#11568) 
Add support for the wgengine Status callback to nodeUpdater
 2024-01-17 09:06:34 +04:00
Spike Curtis f6dc707511 chore: add DERPForcedWebsocket to nodeUpdater (#11567) 
Add support for DERPForcedWebsocket to nodeUpdater
 2024-01-17 08:55:45 +04:00
Spike Curtis 8701dbc874 chore: add nodeUpdater to tailnet (#11539) 
Adds a nodeUpdater component, which serves a similar role to configMaps, but tracks information from tailscale going out to the coordinator as node updates.  This first PR just handles netInfo, subsequent PRs will
handle DERP forced websockets, endpoints, and addresses.
 2024-01-11 09:29:42 +04:00
Spike Curtis 7005fb1b2f chore: add support for blockEndpoints to configMaps (#11512) 
Adds support for setting blockEndpoints on the configMaps
 2024-01-11 09:18:31 +04:00
Spike Curtis 617ecbfb1f chore: add support for peer updates to tailnet.configMaps (#11487) 
Adds support to configMaps to handle peer updates including lost and disconnected peers
 2024-01-11 09:11:43 +04:00
Spike Curtis cae095fdb6 fix: stop logging errors on canceled cleanup queries (#11547) 
Fixes flake seen here: https://github.com/coder/coder/actions/runs/7474259128/job/20340051975
 2024-01-10 16:20:29 +04:00
Spike Curtis dfe8efc186 fix: use background context for inmem provisionerd (#11545) 
This test case fails with an error log, showing "context canceled" when trying to send an acquired job to an in-mem provisionerd.

https://github.com/coder/coder/runs/20331469006

In this case, we don't want to supress this error, since it could mean that we acquired a job, locked it in the database, then failed to send it to a provisioner.
(We also don't want to mark the job as failed because we don't know whether the job made it to the provisionerd or not --- in the failed test you can see that the job is actually processed just fine).

The reason we got context canceled is because the API was shutting down --- we don't want provisionerdserver to abruptly stop processing job stuff as the API shuts down as this will leave jobs in a bad state.  This PR fixes up the use of contexts with provisionerdserver and the associated drpc service calls.
 2024-01-10 15:29:57 +04:00
Spike Curtis 89e3bbe0f5 chore: add configMaps component to tailnet (#11400) 
Work in progress on a subcomponent of the Conn which will handle configuring the wireguard engine on changes.  I've implemented setAddresses as the simplest case and added unit tests of the reconfiguration loop.

Besides making the code easier to test and understand, the goal is for this component to handle disconnect and loss updates about peers, and thereby, implement the v2 Tailnet API.

Further PRs will handle peer updates, status updates, and net info updates.

Then, after the subcomponent is implemented and tested, I will refactor Conn to use it instead of the current monolithic architecture.
 2024-01-10 10:58:53 +04:00
Spike Curtis fdd60d316e fix: fix MetricsAggregator check for metric sameness (#11508) 
Fixes #11451

A refactor of the Agent API passes metrics as protobufs, which include pointers to label name/value pairs.  The aggregator tested for sameness by doing a shallow compare of label values, which for different stats reports would compare unequal because the pointers would be different.

This fix does a deep compare.

While testing I also noted that we neglect to compare template names. This is unlikely to have caused any issue in practice, since the combination of username/workspace is unique, but in the context of comparing metric labels we should do the comparison.

If a user creates a workspace, deletes it, then recreates from a different template, we could in principle have reported incorrect stats for the old template.
 2024-01-09 15:21:30 +04:00
Spike Curtis 21093c00f0 fix: stop logging error on canceled query (#11506) 
Fixes flake seen here: https://github.com/coder/coder/actions/runs/7447779208/job/20260756050
 2024-01-09 14:38:56 +04:00
Spike Curtis 58873fa7e2 chore: remove unused context/cancel in tailnet Conn (#11399) 
Spotted during code read; unused fields
 2024-01-05 08:15:42 +04:00
Spike Curtis 64638b381d feat: promote PG Coordinator out of experimental (#11398) 
Promotes PG Coordinator out of experimental to GA
 2024-01-05 08:03:36 +04:00
Spike Curtis f9ebe8c719 fix: send end of logs when dbfake completes job (#11402) 2024-01-04 15:18:00 +04:00
Spike Curtis 48cd4c3a10 feat: promote single-tailnet out of experimental (#11366) 2024-01-04 09:27:36 +04:00