hashbangcode / vlad

Vlad - Vagrant LAMP Ansible Drupal
173 stars 53 forks source link

Adding conditional imports and apache/mysql defaults #197

Closed mbarcia closed 9 years ago

mbarcia commented 9 years ago

Ansible's conditional imports can load varsfiles based upon ansible facts (ansible*). This allows us to set values according to OS family and distribution/release.

This PR loads vars_files according OS family and distribution, and adds defaults for apache and mysql.

This useful technique can be used in other places where repetition of code between Debian and RedHat can be mitigated. It's a pity that, at the moment, it cannot be used in roles but only in playbooks.

mbarcia commented 9 years ago

Tracking issue is #196

philipnorton42 commented 9 years ago

I can see what you are doing here, but I think a slight alteration might be in order.

Essentially, we want to avoid adding role based variables into the main system itself. So variables like apache2_docroot and mysql_service_name would be defined in the role itself.

I see your note about conditional imports not being available in roles, which is a shame, but as we only officially support Redhat or Debian then I think using ansible_os_family to include one file or the other from the default/main.yml file (like the following) is probably enough:

What do you think? I think your use of variables in the roles themselves is great, but some tweaking needed :).

mbarcia commented 9 years ago

Hello Philip,

Thanks for the feedback. It seems I was being misleaded by a very old comment somewhere. But effectively, the confusion comes because vars_files is not up for the task, but _includevars is.

include_vars is the solution (introduced in Ansible 1.4).

So, I've completed the switch to include_vars in 70187fc (after you placed the comment).

vars_files would still be needed for storing a few global variables that may not be part of any particular role. Sounds good? Cheers,

philipnorton42 commented 9 years ago

Ok, I'm going to close this pull request, but only because of my points above. The variables for a role should be contained within the role, rather than in the main Vlad settings files.