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

Docker #786

Closed
wants to merge 5 commits into from
Closed

Docker #786

wants to merge 5 commits into from

Conversation

@mburns
Copy link

@mburns mburns commented Feb 23, 2017

What

This refactors the Docker and Docker Compose files, moves them to the top level of the repo and tries to simplify the onboarding. The goal is to make using blueflood (as a dev onboarding or as a production operator) through a well-test Docker stack.

Why

Having the files in the top-level conforms to the default paths for docker and docker-compose, which is (mildly) convenient.

Ops would like to manage Blueflood (along with the rest of MMI) in production through Docker, so we're trying to get some consistency in place early. This should also aid in simplifying the branching/environment process we currently use for Blueflood, as PR's can build docker images tied to a feature-change in Blueflood that can be deployed to various environments for testing.

TODO

  • combine docker compose files (with and without graphite-api)
  • Ubuntu 12 base image is used in the demo/ dir
  • pom build step, outputs to build/ which is ignored by git.
  • CIT Jenkins publishes changes to Dockerfile
  • simplify entrypoint.sh logic
    • combine with bin/blueflood-start.sh?
  • consolidate various Dockerfile and docker-compose*.yml files throughout the repo
@goru97
Copy link
Contributor

@goru97 goru97 commented Feb 23, 2017

👎

@goru97 goru97 self-requested a review Feb 23, 2017
@coveralls
Copy link

@coveralls coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 74.593% when pulling 1de04c3 on docker into 9ecaff9 on master.

@mburns
Copy link
Author

@mburns mburns commented Feb 23, 2017

@goru97 that is not thoughtful feedback. try again.

Copy link
Contributor

@goru97 goru97 left a comment

The Readme.md file serves as a description of the Blueflood image on docker-hub. People should know, what the Image is about if they come across it on docker-hub.
Renaming docker-entrypoint.sh to entrypoint.sh is not a best practice.

@mburns mburns force-pushed the docker branch 2 times, most recently from 9a398ef to 2d20d92 Feb 23, 2017
@coveralls
Copy link

@coveralls coveralls commented Feb 23, 2017

Coverage Status

Coverage increased (+0.07%) to 74.668% when pulling 2d20d92 on docker into 9ecaff9 on master.

@coveralls
Copy link

@coveralls coveralls commented Feb 23, 2017

Coverage Status

Coverage decreased (-0.02%) to 74.572% when pulling 2d20d92 on docker into 9ecaff9 on master.

@mburns
Copy link
Author

@mburns mburns commented Feb 23, 2017

I've changed back docker-entrypoint.sh, though I'm not a fan. The purpose of that shell script is broader than just Docker, but I'm fine with maintaining consistency.

As for Docker Hub, since we use manual build/upload, the short description and long description is already being managed manually (not synced with the nested README.md). That said, I'm all for improving our documentation and making it clear how people can onboard themselves.

mburns added 3 commits Feb 23, 2017
move nested artifacts/ to build/
@mburns mburns force-pushed the docker branch from 2d20d92 to 48f1f47 Feb 23, 2017
@coveralls
Copy link

@coveralls coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 74.593% when pulling 48f1f47 on docker into 9ecaff9 on master.

@coveralls
Copy link

@coveralls coveralls commented Feb 23, 2017

Coverage Status

Coverage increased (+0.09%) to 75.165% when pulling 0976586 on docker into a03279e on master.

@goru97
goru97 approved these changes Feb 23, 2017
@goru97
Copy link
Contributor

@goru97 goru97 commented Feb 23, 2017

👍 for the changes so far.

@mburns mburns closed this May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants