pure-docker: move pure-docker to ./pure-docker #454
Conversation
|
question: why should these be in the same repository at all? I think this was a mistake generally, so now would be time to fix it. I think we'd be better off moving docker compose or pure docker to a separate repo. Or, even better - I would love if docker-compose did not rely on external configuration files at all. We could bake all configs into the images (controlled by env vars) and then literally just have docker-compose be a single file hosted on docs.sourcegraph.com |
|
thanks for taking a look @slimsag !
I wanted to do this initially, but the dependency on external config files made that complicated :( Also I think we're very dependent on the git fork workflow, and splitting repos might be a very unpleasant experience for customers (I'm already a bit concerned about this pure-docker move potentially being quite difficult for git, though I haven't gotten that far yet)
This would be great idea! But because we're stuck in this one repo (without great support effort at least, I think) I suppose there isn't much incentive for it yet
This would make configuration complicated, also because of git workflow (doesn't seem to be a paradigm that we're aiming to move away from in the near future) |
…cker into pure-docker-move
|
Makes sense!
FYI, the customers relying on pure docker view the diff manually (using
eyes) and copy over the changes to their files. They don’t fork this repo
or apply diffs automatically, so as long as the visual diff makes sense we
should be fine as far as moving pure-docker goes.
On the docker compose side.. I wouldn’t be surprised if most customers
using it are.. not forking this repository. We suggest it - but I wouldn’t
be shocked if most are just running a direct `git clone` of it. Take that
for what you will :)
|
| @@ -31,6 +31,6 @@ docker run --detach \ | |||
| -p 0.0.0.0:80:80 \ | |||
| -p 0.0.0.0:443:443 \ | |||
| -v $VOLUME:/caddy-storage \ | |||
| --mount type=bind,source="$(pwd)"/caddy/builtins/http.Caddyfile,target=/etc/caddy/Caddyfile \ | |||
| --mount type=bind,source="$(pwd)"/../caddy/builtins/http.Caddyfile,target=/etc/caddy/Caddyfile \ | |||
There was a problem hiding this comment.
I think that these path changes might become problematic. They'll likely copy these over and not notice that ../ is now the wrong path. Maybe instead we could duplicate the config files into the subdirectory as well?
There was a problem hiding this comment.
Do you think moving this to a separate variable would be more clear? i.e.
REFERENCE_REPO_ROOT="$(pwd)/.."
...
--mount type=bind,source="$REFERENCE_REPO_ROOT"/caddy/foobarThere was a problem hiding this comment.
Hmm, I think that would be less clear potentially - because they don't have these scripts in their repo root. Maybe ../ is not so bad on second thought :)
There was a problem hiding this comment.
Oh yikes haha
I'll make sure to include a release note in the pure-docker upgrade notes to call out that there are path changes to be expected due to this move! (and leave this ../ as is)
|
I have added a note to https://github.com/sourcegraph/sourcegraph/issues/23112 (cc @daxmc99 ) to ping me for potential help when preparing this upgrade :) Going to merge this now! |
closes https://github.com/sourcegraph/sourcegraph/issues/22427 - feature Docker Compose more prominently in this reference repository.
make docker-compose front and center, push pure-docker to a subdirectory.
Created the following patches: