-
Notifications
You must be signed in to change notification settings - Fork 39
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
Remove installation script #36
Conversation
After finishing it, I can start work towards #33. |
@mongrelion I think this is ready. |
After merging this I think I could provide a publicly accessible "live" demo usage of this role, by using it here instead of current one. This site is deployed daily and have nice interface for analysis. |
It's a big change but it's looking good. One thing I noticed, though, is that we don't make sure after installation that Docker is properly running. I'm creating a separate issue for that. As per the demo, it sounds good to me. Create a separate issue for that and make sure to add a link to the demo on the README :) |
Do we really need to check if it is running? There is this handler: https://github.com/mongrelion/ansible-role-docker/blob/master/handlers/main.yml#L2 also we are having unit tests for this here: https://github.com/mongrelion/ansible-role-docker/blob/master/tests/test_default.py#L37 I think that enabling this: https://github.com/mongrelion/ansible-role-docker/blob/master/tests/test_default.py#L33 would solve most cases |
Replacing installation script with ansible tasks along with some more cleanup.
Installation method based on previous script and official docs (https://docs.docker.com/install)
Added:
latest
asdocker_version
)latest
versiontasks/configure.yml
file for cleaner lookRemoved:
docker_upgrade
andupgrade_docker
variables. Backwards compatibility is maintained by settingdocker_version: latest
when any one of those two vars is detected (not checking value)Resolves #31