fix: don't create autostart workspace builds with no available provisioners #19067
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
provisioner is not available Signed-off-by: Callum Styan <callumstyan@gmail.com>
transition so that we don't accumulate autostart jobs that will eventually have to be cleaned up by the reaper routine Signed-off-by: Callum Styan <callumstyan@gmail.com>
DB/provisioner to be available, so add a way to skip the check Signed-off-by: Callum Styan <callumstyan@gmail.com>
| } | ||
|
|
||
| // Check if any provisioners are active (not stale) | ||
| now := dbtime.Now() |
There was a problem hiding this comment.
This should be using the passed in time from runOnce rather than getting it's own time
Comment on lines
+148
to
+150
| if testing.Testing() { | ||
| staleInterval = TestingStaleInterval | ||
| } |
There was a problem hiding this comment.
If you use the passed in time from runOnce I don't think changing the interval would be necessary, you could write the test in a way that the default interval works fine
| @@ -52,6 +56,9 @@ type Executor struct { | |||
| experiments codersdk.Experiments | |||
|
|
|||
| metrics executorMetrics | |||
|
|
|||
| // Skip provisioner availability check (should only be true for *some* tests) | |||
There was a problem hiding this comment.
It says it should only be true for some tests, but it seems it's true for almost every test using coderdtest. How many tests are actually broken by this change? If it's only a few, I'd rather we just fix those tests (e.g. by adding provisioners) rather than add a skip flag.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix #17941
With just the test addition the test will fail since the workspace build is triggered, we attempt to move the workspace into the
startstate:require.Equal(t, codersdk.WorkspaceTransitionStop, workspace.LatestBuild.Transition)With the 2nd commit we skip the calls to
wsbuilder.Newandbuilder.Buildand so the transition is never scheduled.However, many tests don't fully setup provisioner or wait for the provisioner + DB to be fully available, so the third commit adds a somewhat smelly way to skip the provisioner check in tests that don't need to test that functionality.