docker / app

Make your Docker Compose applications reusable, and share them on Docker Hub

Apache License 2.0

1.57k stars 177 forks source link

fix port '80' is already in use (voting-app production params) #446

Closed tullo closed 5 years ago

tullo commented 5 years ago

If the voting app is deployed using production params, the vote service fails to deploy as port 80 already is taken by vote service.

Steps to reproduce: voting-app$ docker-app render --parameters-files voting-app.dockerapp/parameters/production.yml | docker stack deploy --compose-file - voting

codecov[bot] commented 5 years ago

Codecov Report

Merging #446 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #446   +/-   ##
=======================================
  Coverage   62.65%   62.65%           
=======================================
  Files          49       49           
  Lines        2362     2362           
=======================================
  Hits         1480     1480           
  Misses        679      679           
  Partials      203      203

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cdcf401...5e2847c. Read the comment docs.

ijc commented 5 years ago

Thanks, this looks like a reasonable change. However you've said "vote service" twice in the commit message, one of them should be "result service". I think it's worth fixing that in the history so future historians aren't confused.

Would you mind also updating the README.md (right near the end) to match what is now going to be written in the file?

ijc commented 5 years ago

Thanks @tullo the commit LGTM https://github.com/docker/app/pull/446/commits/5e2847cee8a8cacbbc7d4486517991f671271d6e however this PR appears to include two other commits duplicated previously merged PRs of yours. Please could you rebase onto master to make those go away.

Something like git pull --rebase should work if your local branch has a good upstream configured or if not o=«your upstream remote»; git fetch $o; git rebase $o/master.

tullo commented 5 years ago

The code coverage check fails for these simple changes, not sure how to 'make' them green!