Skip to content

fix: correctly reject quota-violating builds #9233

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.

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

Merged
merged 1 commit into from
Aug 22, 2023
Merged

fix: correctly reject quota-violating builds #9233

merged 1 commit into from
Aug 22, 2023

Conversation

ammario
Copy link
Member

@ammario ammario commented Aug 21, 2023

fix: correctly reject quota-violating builds

Due to a logical error in CommitQuota, all workspace Stop->Start operations
were being accepted, regardless of the Quota limit. This issue only
appeared after #9201, so this was a minor regression in main for about
3 days. This PR adds a test to make sure this kind of bug doesn't recur.

To make the new test possible, we give the echo provisioner the ability
to simulate responses to specific transitions.

Side note, I changed workspaceproxy.go to return a nil value in a case
to pass staticcheck.

@ammario
Copy link
Member Author

ammario commented Aug 21, 2023

Need to figure out how to test this and how this broke in the first place.

@ammario ammario force-pushed the pr9233 branch 2 times, most recently from 7c2ad81 to 9de1e3a Compare August 21, 2023 22:31
@ammario ammario requested a review from kylecarbs August 21, 2023 22:32
@ammario
Copy link
Member Author

ammario commented Aug 21, 2023

a2f49c0:

Made the comment more obvious as to jank happening.

@ammario ammario enabled auto-merge (squash) August 21, 2023 22:59
@ammario ammario force-pushed the pr9233 branch 3 times, most recently from 726f4f4 to 760c718 Compare August 22, 2023 02:39
Due to a logical error in CommitQuota, all workspace Stop->Start operations
were being accepted, regardless of the Quota limit. This issue only
appeared after #9201, so this was a minor regression in main for about
3 days. This PR adds a test to make sure this kind of bug doesn't recur.

To make the new test possible, we give the echo provisioner the ability
to simulate responses to specific transitions.

Side note, I changed workspaceproxy.go to return a nil value in a case
to pass staticcheck.
@ammario ammario merged commit 545a256 into main Aug 22, 2023
@ammario ammario deleted the pr9233 branch August 22, 2023 02:55
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants