Skip to content
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

fix(docker-compose): fix yml to use mapping instead of list #2558

Merged
merged 1 commit into from
Dec 12, 2017

Conversation

juliangieseke
Copy link
Contributor

@juliangieseke juliangieseke commented Dec 9, 2017

further reading:
docker/compose#2854
https://docs.docker.com/compose/compose-file/#args


this PR unblocks #2547 - rebase it on this one and it should run successfully when run through docker-compose exec toolchain dcos-system-test-driver /dcos-ui/system-tests/driver-config/jenkins.sh we decided to merge #2547 before this one because the tests are running on ci, only our local docker-compose setup is broken.


HINT: you can prevent the cluster (and tests) from being started with adding exit 1 in the first line of system-tests/_scripts/launch-cluster.sh - it will fail with Process '../_scripts/launch-cluster.sh' exited with 1 which is okay and means the config is correctly parsed.

if you see the error @nLight mentioned in #2547 it didnt work.


Caveat: it looks like docker doesnt recognize when you change the passed variables after docker-compose up -d 🤔 had to export the changed var…

@d2iq-mergebot
Copy link

This repo has @mesosphere-mergebot integration. You can interact with the following commands.

@mesosphere-mergebot ship-it  
@mesosphere-mergebot test  
@mesosphere-mergebot integrate  

@juliangieseke
Copy link
Contributor Author

Alfred test this please

1 similar comment
@juliangieseke
Copy link
Contributor Author

Alfred test this please

@ahoskins
Copy link
Contributor

@juliangieseke I'm a bit confused: in the description it says this PR unblocks #2547, but 2547 has merged without this merging first. Is this PR still needed?

@juliangieseke
Copy link
Contributor Author

Yes, we decided that this one isnt blocking #2547, because it only prevents us from running the tests through docker-compose - but it is still a fix we should apply.

I simply forgot to update this PR after merging the other one

Copy link
Contributor

@ahoskins ahoskins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for the clarification @juliangieseke

@ahoskins ahoskins merged commit 629fbe7 into master Dec 12, 2017
@ahoskins ahoskins deleted the jgieseke/fix-docker-compose branch December 12, 2017 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants