-
-
Notifications
You must be signed in to change notification settings - Fork 607
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
Refactor hosts files #313
Refactor hosts files #313
Conversation
Going to try this out tonight |
Think I'm onboard with this 👍 |
01b236a
to
628026b
Compare
7b66220
to
0a3a673
Compare
@fullyint can you do a PR for doc updates on https://github.com/roots/docs? We should merge at the same time. |
bef2ec3
to
4e25c45
Compare
docs in roots/docs#4 After this PR, commands will use |
fail: | ||
msg: "Environment missing. Use `-e` to define `env`:\nansible-playbook server.yml -e env=<environment>" | ||
when: env is not defined | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be separated into its own playbook and included so it's DRY?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea.
I added a commit to include the new variable-check.yml
playbook. It's DRY but also a bit more complex to look at. The messages needed to differ slightly per playbook.
You'll see that I went with yaml:
- include: variable-check.yml
vars:
playbook: server.yml
even though it could have been more succinct:
- include: variable-check.yml playbook=server.yml
@fullyint this will need an update to |
@swalkinshaw Re: updating Inventory. This PR removes the
So, I don't think I don't notice anything else that might need adjustment in |
+1 |
e7bdcd7
to
8c36f54
Compare
@fullyint squash + changelog entry? Ready to merge after that 👍 |
Added changelog entry and squashed |
Please feel free to tell me that I should open a new issue if this doesn't belong here: With this change we now have to enter the domain name instead of the server's IP.
(Our staging stages are named
After replacing the IP with the domain name it works again. |
Setting the stage By specifying the The For example, the default command The error you saw If you ran the command (sounds like you've already solved things, but...)
There are probably other possible approaches too. Check out hosts and groups at Ansible docs. Using IP vs domain As for IP vs domain, I think you're saying that you used to be able to use IPs in I think you can still use IPs in your hosts file. Given the example hosts file above, if you were to issue the command You might also be able to use those same commands with this simplified hosts file (untested):
I hope that addresses your point. If you feel there is a bug or fix that is needed, feel free to respond here. If you have questions or would like assistance troubleshooting, check out Roots discourse. |
hosts
directory[web]
its own group instead of a child group of<environment>
.Commands
-i hosts/<environment>
with-e env=<environment>
ansible-playbook dev.yml
. You'd need vagrant vm in ssh config till Add ansible-ssh role to generate ssh config #314 is merged../deploy.sh <environment> <site>
I kept hosts files separate by environment. Alternatively, a single file could look like this: