campaigns: fall back to bind workspaces when running steps with mixed users #422
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
It won't cost much to download the utility image once, even if it's not used, so let's just do it and simplify. This also means we don't end up with a circular dependency once workspace creator selection depends on campaign spec steps.
mrnugget
approved these changes
Jan 12, 2021
| images.add(image, nil) | ||
| } | ||
| // We also need to ensure we have our own utility images available. | ||
| images.add(dockerVolumeWorkspaceImage, nil) |
There was a problem hiding this comment.
👍 on reducing the interface and hardcoding the image here.
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
scjohns
pushed a commit
that referenced
this pull request
Apr 24, 2023
… users (#422) Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
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 fixes sourcegraph/sourcegraph#17172. I'm inclined to get this in before I try releasing 3.23.2 again.
The only real drawback of this PR (besides the extra lines of code) is that
srcwill now always ensure thesourcegraph/src-campaign-volume-workspacecontainer is available, since we don't know at the point of downloading Docker images which workspace we'll use. (To figure that out, we'd need the images, and then the snake eats itself.) Since Docker is good at caching, I'm not too concerned: this is only one extradocker image inspectcall in any case except for the very first cold run with a new version. (Also, we can definitely optimise those more later.)Speaking of versioning, there's one other change to make this work as well: when I implemented #412, I missed that newer versions of the workspace image wouldn't be pulled when new
srcversions were released. I've fixed this now (otherwise all my careful Docker image tagging would be for naught!), but note there's a touch of extra complexity in here around moving thebuildTagto another package so we can access it. I think it's fine, but it's worth an extra set of eyes.