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

[TASK] Add support for generic configuration snippets #33

Closed
wants to merge 3 commits into from
Closed

[TASK] Add support for generic configuration snippets #33

wants to merge 3 commits into from

Conversation

Tuurlijk
Copy link

This patch adds support for generic reusable configuration snippets like php request handling through a php-fpm socket or hhvm.

@jdauphant
Copy link
Owner

Hi @Tuurlijk ,
Thanks for the pull request.

Why use snippets and not use the powerful template system of Ansible ?
By using the template system of Ansible, we can keep the role more simple and still allow complex configuration.

Thanks in advance for you answer

@Tuurlijk
Copy link
Author

Tuurlijk commented Feb 4, 2015

To avoid code duplication in nginx configuration.

here is an example configuration:

nginx_snippets:
  php-fpm:
    - add_header X-TYPO3-Homestead-backend php-fpm
    - try_files $uri $uri/ /index.php?$args
    - location ~ \.php$ {
        try_files     $uri =404;
        fastcgi_pass  unix:/var/run/php5-fpm.sock;
        fastcgi_index index.php;
        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        include       fastcgi_params;
      }
  php-fpm-xhprof:
    - add_header X-TYPO3-Homestead-backend php-fpm-xhprof
    - try_files $uri $uri/ /index.php?$args
    - location ~ \.php$ {
        try_files     $uri =404;
        fastcgi_pass  unix:/var/run/php5-fpm-xhprof.sock;
        fastcgi_index index.php;
        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        include       fastcgi_params;
      }
  hhvm:
    - add_header X-TYPO3-Homestead-backend hhvm
    - try_files $uri $uri/ /index.php?$args
    - location ~ \.(hh|php)$ {
        try_files     $uri =404;
        fastcgi_pass  unix:/var/run/hhvm/sock;
        fastcgi_index index.php;
        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        include       fastcgi_params;
      }

nginx_sites:
  default:
    - listen 80 default_server
    - server_name _
    - root "{{ typo3_source_path }}/homestead/"
    - include snippets/hhvm.conf
  default-ssl:
    - listen 443 default_server
    - server_name _
    - root "{{ typo3_source_path }}/homestead/"
    - include snippets/php-fpm.conf
    - ssl on
    - ssl_certificate {{nginx_conf_dir}}/ssl/typo3.homestead.crt
    - ssl_certificate_key {{nginx_conf_dir}}/ssl/typo3.homestead.key

How would you do this with ansible templates?

@jdauphant
Copy link
Owner

It could be implemented like that :

nginx_hhvm: |
      add_header X-TYPO3-Homestead-backend hhvm;
      try_files $uri $uri/ /index.php?$args;
      location ~ \.(hh|php)$ {
        try_files     $uri =404;
        fastcgi_pass  unix:/var/run/hhvm/sock;
        fastcgi_index index.php;
        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        include       fastcgi_params;
      }

nginx_sites:
 hhvm_test:
     - |
       listen 80;
       server_name test_hhvm;
       root "/homestead/";
       {{nginx_hhvm}}

The result is :

#Ansible managed: /home/julien/project/ansible-roles/nginx/templates/site.conf.j2 modified on 2015-02-05 20:14:10 by julien on pc
server {
   listen 80;
   server_name test_hhvm;
   root "/homestead/";
   add_header X-TYPO3-Homestead-backend hhvm;
   try_files $uri $uri/ /index.php?$args;
   location ~ \.(hh|php)$ {
     try_files     $uri =404;
     fastcgi_pass  unix:/var/run/hhvm/sock;
     fastcgi_index index.php;
     fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
     include       fastcgi_params;
   }

}

This work with the actual role but made ugly config file.

I have made a patch that made good visual config file with this configuration :
362b995

What do you think about that ?

@Tuurlijk
Copy link
Author

Tuurlijk commented Feb 5, 2015

Wow, ansible is powerful. I like it. It will do the job.

But this will require a re-provision if I want to modify 'nginx_hhvm' and see the change reflected in all sites. Currently this is not so nice to do since the tags attached to the actions are too generic (configuration,nginx) and can not be 'AND'-ed when calling ansible provision (as far as I know).

Can you please add an explicit tag to either all the actions in configuration.yml, OR to the configuration.yml include statement?

- include: configuration.yml
  tags: nginx-configuration

I would prefer the last option.

@jdauphant
Copy link
Owner

The tags selection is not so good with ansible but I prefer not introduce too much tags.
Actually you can do use Ansible options like that : --tags nginx --skip-tags packages

@jdauphant
Copy link
Owner

My role is also made that you can use it multiple time on the same server.
You can separate your sites in groups and use your tags on playbooks level.

@Tuurlijk
Copy link
Author

Tuurlijk commented Feb 6, 2015

I know you can add and remove tags. But the 'nginx' tag is used in more tasks than just the configuration task. It will also trigger the remove-* tasks and go over the installation task again. So I still think it makes sense to be able to trigger just the 'site configuration' alone.

Tuurlijk pushed a commit to Tuurlijk/TYPO3.Homestead that referenced this pull request Feb 9, 2015
[TASK] Split nginx configuration into nginx and sites
@jdauphant
Copy link
Owner

Hello @Tuurlijk ,
For the snippets issue, it will be perfect if you can test the commit 362b995 before I integrate it to the main branch.

For the tags issue :
You can temporary set the variable nginx_installation_type to "None", this will remove the packages step which is the longer part. (it will do the same as "--tags nginx --skip-tags packages")

I will think about integrate some undocumented tags like nginx-configuration, nginx-packages, nginx-service (In an ideal world, I would prefer this integrated directly in ansible-playbook)

@Tuurlijk
Copy link
Author

Tested your solution, works as expected.

Things I dislike about it are:

  • Pipe notation is less readable
  • The Semicolons are back
  • The variables are in the global scope
  • Duplication of code in generated configurations

@jdauphant
Copy link
Owner

  • You can mixte notation (but I recommends not to do)
nginx_sites:
 hhvm_test:
     - listen 80
     - server_name test_hhvm
     - root "/homestead/"
     - |
       {{nginx_hhvm}}
  • If you use the new notation, you don't have to use the - to describe a list neither, you use the same number of caracters as before and you have more proximity with the nginx configuration. If I had to redo the role today, I will only add notation with "|" like that :
nginx_sites:
 hhvm_test: |
       listen 80;
       server_name test_hhvm;
       root "/homestead/";
       {{nginx_hhvm}}
  • Ansible give you lot of possiblity to manage the scope of your variables at your playbook level (you can take a look here :
    https://github.com/jdauphant/ansible-variables-precedence )
  • Also for me, name the variable "nginx_hhvm" is terrible in my example
  • You manage automatically the code, duplication in generated code is such a problem for you ?

@Tuurlijk
Copy link
Author

No, code duplication is not a real problem in this case. It's just in my mind.

Can you please merge in your solution?

Thx for the tip on being able to mix the notations. but I agree mixing them does not make it more readable.

@jdauphant
Copy link
Owner

It's already merged and I just release it at 1.2 : https://github.com/jdauphant/ansible-role-nginx/releases/tag/v1.2

Thanks for your help @Tuurlijk

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