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

Remove installation script #36

Merged
merged 11 commits into from
Apr 9, 2018
Merged

Conversation

paulfantom
Copy link
Collaborator

@paulfantom paulfantom commented Apr 4, 2018

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:

  • easier upgrade method (just specify latest as docker_version)
  • testing on latest version
  • tasks/configure.yml file for cleaner look

Removed:

  • installation script and everything connected with it
  • unused files
  • some simple templates
  • docker_upgrade and upgrade_docker variables. Backwards compatibility is maintained by setting docker_version: latest when any one of those two vars is detected (not checking value)

Resolves #31

@paulfantom paulfantom changed the title WIP: Remove installation script Remove installation script Apr 4, 2018
@paulfantom
Copy link
Collaborator Author

After finishing it, I can start work towards #33.

@paulfantom
Copy link
Collaborator Author

paulfantom commented Apr 8, 2018

@mongrelion I think this is ready.

@paulfantom
Copy link
Collaborator Author

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.

@mongrelion
Copy link
Owner

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 :)

@mongrelion mongrelion merged commit f7bbaf5 into mongrelion:master Apr 9, 2018
@paulfantom paulfantom deleted the remove_script branch April 9, 2018 09:24
@paulfantom
Copy link
Collaborator Author

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

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.

None yet

2 participants