8000 fix: update autostart context to include querying users by sreya · Pull Request #10929 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

fix: update autostart context to include querying users #10929

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
Merged
merged 4 commits into from
Nov 28, 2023
Merged

fix: update autostart context to include querying users #10929

merged 4 commits into from
Nov 28, 2023

Conversation

sreya
Copy link
Collaborator
@sreya sreya commented Nov 28, 2023
  • Fixes an issue where autobuild becomes effectively disabled due to its context not having all the privileges it requires to make scheduling decisions.
  • Updates the Error field to Errors and records all errors that occur over an evaluation iteration.

This bug surfaces some deviations we've made between our testing harness and our production path:

  • coderdtest.NewOptions does more than just instantiate sensible defaults for coderd.API fields. It's also starting background processes like autobuild and the unhanger.Detector prior to instantiating a coderd.API which is surprising.
  • This deviates from how cli/server.go functions...it first instantiates a coderd.API and then starts some background based on the options
  • coderd.New is not only instantiating a coderd.API it's also overwriting options on fields like Database to wrap it in dbauthz. This means we're "getting lucky" when we start autobuild in production, when instantiating that background process you need to be aware that coderd.API is responsible for taking whatever type you passed for a Database and wrapping it in a dbauthz.Store which seems unnecessarily brittle.

The result of this is that we would never catch auth-related errors in autobuild in our tests.

There's a non-trivial amount of code to refactor to make this less fragile so this only fixes the autobuild issues.

@sreya sreya requested a review from coadler November 28, 2023 23:22
@sreya sreya merged commit 48d69c9 into main Nov 28, 2023
@sreya sreya deleted the jon/fixautobuild branch November 28, 2023 23:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@coadler coadler coadler approved these changes

Assignees

@sreya sreya

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sreya @coadler
0