Skip to content

flake: TestProxyRegisterDeregister #14961

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.

Sign up for GitHub

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

Jump to bottom
Closed
spikecurtis opened this issue Oct 3, 2024 · 2 comments · Fixed by #14974
Closed

flake: TestProxyRegisterDeregister #14961

spikecurtis opened this issue Oct 3, 2024 · 2 comments · Fixed by #14974
Assignees
@spikecurtis

Comments

@spikecurtis
Copy link
Contributor

seen here: https://github.com/coder/coder/actions/runs/11161236686/job/31023381497

 t.go:99: 2024-10-03 11:41:47.893 [debu]  coderd: synced licensed entitlements
    t.go:99: 2024-10-03 11:41:47.893 [debu]  coderd: got pubsub update
    t.go:99: 2024-10-03 11:41:47.893 [debu]  coderd: syncing licensed entitlements
    t.go:99: 2024-10-03 11:41:47.893 [debu]  coderd: POST  host=localhost:41331  path=/api/v2/workspaceproxies/me/register  proto=HTTP/1.1  remote_addr=127.0.0.1  start="2024-10-03T11:41:47.893940325Z"  took=28.75µs  status_code=403  latency_ms=0  request_id=ce9ead79-f400-4846-9d6d-9a2388dc7ca9
    workspaceproxy_test.go:365: 
        	Error Trace:	/home/runner/work/coder/coder/enterprise/coderd/workspaceproxy_test.go:365
        	Error:      	Received unexpected error:
        	            	POST http://localhost:41331/api/v2/workspaceproxies/me/register: unexpected status code 403: Workspace Proxy is a Premium feature. Contact sales!
        	Test:       	TestProxyRegisterDeregister/OK

looks like registration is racing license entitlements
@spikecurtis
Copy link
Contributor Author

I've just seen another flake related to not being licensed

    t.go:99: 2024-10-04 06:22:53.566 [debu]  coderd: synced licensed entitlements
    t.go:99: 2024-10-04 06:22:53.567 [debu]  coderd: GET  host=localhost:43129  path=/api/v2/entitlements  proto=HTTP/1.1  remote_addr=127.0.0.1  start="2024-10-04T06:22:53.56694674Z"  took=60.131µs  status_code=200  latency_ms=0  request_id=b221d18f-10ab-4fa3-add4-78adead02107
    coderdenttest.go:302: 
        	Error Trace:	/home/runner/work/coder/coder/enterprise/coderd/coderdenttest/coderdenttest.go:326
        	            				/home/runner/work/coder/coder/enterprise/coderd/coderdenttest/coderdenttest.go:302
        	            				/home/runner/work/coder/coder/enterprise/cli/create_test.go:53
        	            				/home/runner/work/coder/coder/enterprise/cli/create_test.go:152
        	Error:      	Received unexpected error:
        	            	external provisioner daemons require an entitled license:
        	            	    github.com/coder/coder/v2/enterprise/coderd/coderdenttest.NewExternalProvisionerDaemon
        	            	        /home/runner/work/coder/coder/enterprise/coderd/coderdenttest/coderdenttest.go:326
        	Test:       	TestEnterpriseCreate/WorkingAmbiguousTemplateName

at https://github.com/coder/coder/actions/runs/11174989245/job/31065628852

Perhaps this isn't related to TestProxyRegisterDeregister and relates to some other change we recently made to license entitlements.

@spikecurtis
Copy link
Contributor Author

It looks like we kick off api.updateEntitlements(ctx) multiple times at the start of the test. We do it once when generating the first user (this was added because we sometimes generate a trial license at that time), then send a message on the pubsub, which triggers yet another call to updateEntitlements() via the refresh loop. Then we call it again after adding the license, and again send on the pubsub, which triggers another call.

So, at that start of a coderdent test, we call update like 4 times, twice before the license is actually added. If these race with the 2 additional times after the license is added then we can end up without entitlements at the end.

@spikecurtis spikecurtis mentioned this issue Oct 4, 2024
Merged
spikecurtis added a commit that referenced this issue Oct 5, 2024
@spikecurtis
fix: serialize updateEntitlements() (#14974)
288df75 
fixes #14961

Adding the license and updating entitlements is flaky, especially at the start of our `coderdent` testing because, while the actual modifications to the `entitlements.Set` were threadsafe, we could have multiple goroutines reading from the database and writing to the set, so we could end up writing stale data.

This enforces serialization on updates, so that if you modify the database and kick off an update, you know the state of the `Set` is at least as fresh as your database update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@spikecurtis spikecurtis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: serialize updateEntitlements() coder/coder
2 participants
@spikecurtis @deansheather