-
Notifications
You must be signed in to change notification settings - Fork 153
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
Allow the quickstart compose file's envvars to be overridden #112
Conversation
The quickstart `docker-compose.yml` file hardcodes some parameters. Docker Compose supports an interpolation mechanism which allows the same defaulting to happen, but in a way which permits the user to modify these parameters when running docker-compose. Making this change will allow this Compose file to be used by more consumers, whilst still enabling the same low-friction quickstart workflow. Alternative mechanisms, such as adding a `docker-compose.env` file and wget'ing it as part of the quickstart, are possible - and don't increase the quickstart friction very much. This commit implements the mechanism which doesn't change the quickstart experience *at all*.
This commit changes the prehashed password for the `test` user in the quickstart `docker-compose.yml` file to the same password, but in plaintext. This makes it clearer for consumers of this file as to how they should change the default user and password. Suggesting in the status quo that a hash must be specified, especially one which will contain dollar signs, makes it less likely a user will override this setting using the functionality introduced in `7f2e0a2`.
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
Hmm I'm a bit lukewarm on this. Thanks for submitting it, obviously, and sorry for the delay - we're in flux right now with concourse/concourse#2534 and that's slowing down our PR influx. The added flexibility is nice but I'm not sure we want the quick start being used for much more than the initial tire-kicking, and I'm afraid that by opening it up to more configuration we'll make it look more like a real-world Concourse deployment template. We don't intend to maintain this file for use by downstream deployments - it's just there as an example. More real-world use cases should probably use a full-fledged Docker Compose file (and forego Thanks again and sorry for the wait! |
Closing for now based off @vito 's feedback. Thanks for making hte PR though! |
The quickstart
docker-compose.yml
file hardcodes some parameters. Docker Compose supports an interpolation mechanism which allows the same defaulting to happen, but in a way which permits the user to modify these parameters when running docker-compose.Making this change will allow this Compose file to be used by more consumers, whilst still enabling the same low-friction quickstart workflow.
Alternative mechanisms, such as adding a
docker-compose.env
file and wget'ing it as part of the quickstart, are possible - and don't increase the quickstart friction very much. This commit implements the mechanism which doesn't change the quickstart experience at all.This PR also makes it clearer to the prospective overriding user how to set the user password they want.
Tests
I'm not including any tests. Here's my rationale:
if there already exist any CI/etc tests of the
docker-compose.yml
file, then this PR will either break them, or it won't. If it doesn't, and the status quo experience (i.e. no externally set envvars) doesn't change, then by definition the status quo has been maintained. Even if the additional functionality intended to be implemented here doesn't work, nothing of value was lost.if there aren't any tests of this file, then implementing a test framework to run up machines on which to run this set of containers seems like something that needs project-level organisation, and isn't in scope for a drive-by PR'ing. IMHO :-)