-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(executor): add option for additional mounts in docker runner #56434
base: main
Are you sure you want to change the base?
Conversation
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
0e07128
to
c1d4102
Compare
Hi @rdeusser! Sorry about the delay on the review. Ownership of executors has shuffled around a bit, and this one got lost in the weeds. I've got this on my list for today 👍 |
Adding a volume mount to Executors seems like a good way to solve persisting files, but because executors run in so many different environments, one of the main goals for executors is to decouple them from infrastructure. Adding volume mounts actually works against that. Instead of modifying executors, there are a few options to solve this pain point. I'll list them in order of my preference:
I'm sure there are other options as well; these are what the team here came up with. I hate to rain on your parade, @rdeusser - I like the engineering work you've done in this PR - but I recommend this PR be closed and an approach that works with the existing capabilities be taken instead. |
@peterguy In general decoupling from the infrastructure seems like a great way to support different environments more easily, but the reality is that we still have to deal with that infrastructure. Besides the fact that virtually all of the supported environments have a concept of a volume, adding volume mounts does not work against that.
None of the solutions provided solve the issue for us. Even moving to the Kubernetes-based executor doesn't solve the issue because the PVC's are deleted after each job is run. Additionally, the Go module cache isn't the only problem this solves for us. In order to clone repos/go mod download them, our certificates need to be present inside the container otherwise the executor will fail when downloading dependencies. The other option is to maintain a custom scip-go image with our certificates in it. The solution for enterprise customers can't be for all of us to maintain custom scip-go/lsif-go images. There are no approaches that I'm aware of that work with the existing capabilities, hence the purpose of the PR. The gist of what's needed here is that we need to provide a place to persistently store cached Go modules to the indexing environment prepared by the executor. |
Thanks for adding more detail @rdeusser! I appreciate your patience; this PR, and the concept of persistent volumes in executors, has sparked quite a lengthy discussion around here. 😄 We're adding this PR to our backlog, but given our workload now, we can't give an estimate of when we'll be able to work on it. To give you some insight into the discussions this PR has generated, here are some comments from various engineers involved in the discussion:
Notice that much of the discussion is around supporting volume mounts in all of the deployment scenarios. Testing volume mounts in all of the installation options will take a non-trivial amount of effort; if you get a chance to test out volume mounts in Firecracker and K8s, update this PR with the result! |
Description
We use the code intel feature on a lot of Go code and each time a repo was indexed it had to repopulate the Go module cache all over again, wasting time and unnecessarily putting pressure on our Artifactory instance. To solve this, I added the ability to specify mounts, semicolon-separated, through the environment variable
EXECUTOR_DOCKER_ADDITIONAL_MOUNTS
(e.g.export EXECUTOR_DOCKER_ADDITIONAL_MOUNTS=type=volume,source=gocache,target=/gocache;type=volume,source=gomodcache,target=/gomodcache
). To fully solve the issue, it was necessary to add the environment variablesGOCACHE
andGOMODCACHE
to the executor secrets under theCode Graph
tab in the Sourcegraph UI as well as add those environment variables to therequested_envvars
array in the Code Graph inference section.Test plan
Tested in production environment.
Screenshot showing the volumes are referenced in the docker runner started by the executor:

Screenshot showing that the

/gomodcache
directory in thegomodcache
volume is populated:On one indexing job we saw an improvement of 92.4%.