Skip to content

Conversation

DavidWittman
Copy link
Contributor

Closes #44

@ninthnails
Copy link
Contributor

Very neat stuff! I came across this issue many times wondering why the hell slave service was started even though i was not suppose to install it. Now it all make sense!

One thing I would like to see though is simple test in the build playbook. That won't be trivial to do but it will increase robustness of the role. Unfortunately the tests only install in master-slave. Some refactor needed.

@JasonGiedymin JasonGiedymin added this to the v0.3.4 milestone Feb 18, 2016
@DavidWittman
Copy link
Contributor Author

👍 for better testing around these types of things. Have you guys used Molecule at all? I've been using Test Kitchen for testing most of my roles but this looks much better suited for Ansible.

@JasonGiedymin
Copy link
Member

Almost all our roles "try" to use vagrant. The ci/ playbook simply uses this role. See here and here.

This is somewhat similar to how molecule works. In fact many of the things it does we do here albeit more verbosely.

See this repo's Vagrantfile and Zookeeper Vagrantfile.

The things I like about this are:

  • uses vagrant, easy to get on any platform (along with vbox or parallels if that suites your fancy)
  • uses tests written with ansible
  • no need for additional dependencies
  • weirdly enough you can have the same tests triggered with travis, see here

@JasonGiedymin
Copy link
Member

That being said, if it helps with easing testing than what we've already got here I'm all for it.

@ernestas-poskus
Copy link
Member

@DavidWittman DavidWittman force-pushed the 44-disable-services branch from 12b2f63 to c9dc4ad Compare June 6, 2016 15:47
@DavidWittman
Copy link
Contributor Author

@ernestas-poskus rebased and bumped version

@ernestas-poskus
Copy link
Member

@DavidWittman please squash commits

@DavidWittman
Copy link
Contributor Author

You can squash them on merge now in Github if you so please.
On Jun 6, 2016 12:18 PM, "Ernestas Poskus" [email protected] wrote:

@DavidWittman https://github.com/DavidWittman please squash commits


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#45 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AArmL7vnpyO5BHxXjcPnR_gAUnRcTTZKks5qJFZ6gaJpZM4HcgJk
.

@ernestas-poskus
Copy link
Member

@DavidWittman

I have no permissions to enable Github squash, please do it on your side.

@DavidWittman
Copy link
Contributor Author

@ernestas-poskus You don't have this option?

image

@ernestas-poskus
Copy link
Member

@DavidWittman

Unfortunately no

screenshot from 2016-06-09 19-35-05

so as Settings for repo are absent
screenshot from 2016-06-09 19-34-57

@DavidWittman
Copy link
Contributor Author

@ernestas-poskus You have to click "Merge pull request" first and then you'll get a dropdown on the "Confirm merge" button. It's confusing.

@ernestas-poskus ernestas-poskus merged commit fcea7ed into AnsibleShipyard:master Jun 9, 2016
@ernestas-poskus
Copy link
Member

You got me 😄 this is the first time I am doing this without CLI

@ernestas-poskus
Copy link
Member

Thank you for contribution 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants