geerlingguy / ansible-role-php-versions

Ansible Role - PHP Versions
https://galaxy.ansible.com/geerlingguy/php-versions/
MIT License
98 stars 73 forks source link

Role overwrites gerrlingguy.php's php_packages #13

Closed socketwench closed 6 years ago

socketwench commented 7 years ago

Description

When using this role, the php_packages variable is completely overridden by the role's defaults.

For example, this is in my group_vars:

php_packages:
  - php
  - php-fpm
  - php-cli
  - php7.1-common
  - php-gd
  - php-mbstring
  - php-xml
  - php-memcached
  - php7.1-opcache
  - php-sqlite3
  - php7.1-mysql
  - php7.1-curl

When using this role, however, the task reads:

TASK [geerlingguy.php : Ensure PHP packages are installed.] ******************** ok: [45.56.85.211] => (item=[u'php7.1', u'php7.1-apcu', u'php7.1-cli', u'php7.1-common', u'php7.1-curl', u'php7.1-dev', u'php7.1-fpm', u'php7.1-gd', u'php7.1-imap', u'php7.1-json', u'php7.1-mbstring', u'php7.1-mcrypt', u'php7.1-opcache', u'php7.1-sqlite3', u'php7.1-xml', u'php7.1-yaml'])

This corresponds to the defaults provided by this role for my platform: https://github.com/geerlingguy/ansible-role-php-versions/blob/master/vars/Debian.yml

The problem appears to be in the set_fact that does not check if the variable exists elsewhere first.

This is a bit of a problem for database backed sites, since we can't install mysql using the method familiar when only using the php role, and not this role as well.

Workaround

Instead of overriding the php_packages variable, use php_packages_extra

Proposed solution

Either document that this role overrides php_packages completely, or split off the set_fact to account for a preexisting definition.

yethee commented 6 years ago

What version of Ansible are you using?

This behaviour is valid for Ansible of 2.4.1.0. In 2.4.0.0 or lower local variable is not overridden.

socketwench commented 6 years ago

Originally this was on a 2.2 server, but I've since replicated it on 2.4.

arondeparon commented 6 years ago

Just experienced the same issue on Ansible 2.4.

yethee commented 6 years ago

This role works correctly for me on Ansible of 2.4.0.0, when a variable php_packages defined in group vars.

$ ansible --version
ansible 2.4.0.0
  config file = /vagrant/cm/ansible.cfg
  configured module search path = [u'/home/vagrant/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.6 (default, Oct 26 2016, 20:30:19) [GCC 4.8.4]
vars:
  php_version: '7.1'
  php_packages:
    - libpcre3-dev
    - "php{{ php_version }}"
    - "php{{ php_version }}-bcmath"
    - "php{{ php_version }}-apcu"
    - "php{{ php_version }}-cli"
    - "php{{ php_version }}-common"
    - "php{{ php_version }}-curl"
    - "php{{ php_version }}-dev"
    - "php{{ php_version }}-gd"
    - "php{{ php_version }}-imap"
    - "php{{ php_version }}-intl"
    - "php{{ php_version }}-json"
    - "php{{ php_version }}-mbstring"
    - "php{{ php_version }}-mcrypt"
    - "php{{ php_version }}-opcache"
    - "php{{ php_version }}-soap"
    - "php{{ php_version }}-xml"
TASK [geerlingguy.php-versions : Define PHP variables.] ************************
ok: [default] => (item={'key': u'php_extension_conf_paths', 'value': u'__php_extension_conf_paths'})
ok: [default] => (item={'key': u'php_fpm_daemon', 'value': u'__php_fpm_daemon'})
ok: [default] => (item={'key': u'php_uploadprogress_module_path', 'value': u'__php_uploadprogress_module_path'})
ok: [default] => (item={'key': u'php_xhprof_module_path', 'value': u'__php_xhprof_module_path'})
ok: [default] => (item={'key': u'php_fpm_pool_conf_path', 'value': u'__php_fpm_pool_conf_path'})
ok: [default] => (item={'key': u'php_conf_paths', 'value': u'__php_conf_paths'})
ok: [default] => (item={'key': u'php_xdebug_module_path', 'value': u'__php_xdebug_module_path'})
ok: [default] => (item={'key': u'php_pgsql_package', 'value': u'__php_pgsql_package'})
skipping: [default] => (item={'key': u'php_packages', 'value': u'__php_packages'})
ok: [default] => (item={'key': u'php_mysql_package', 'value': u'__php_mysql_package'})
ok: [default] => (item={'key': u'php_memcached_package', 'value': u'__php_memcached_package'})
ok: [default] => (item={'key': u'php_fpm_conf_path', 'value': u'__php_fpm_conf_path'})
ok: [default] => (item={'key': u'php_tideways_module_path', 'value': u'__php_tideways_module_path'})
ok: [default] => (item={'key': u'php_redis_package', 'value': u'__php_redis_package'})

...

TASK [geerlingguy.php : Ensure PHP packages are installed.] ********************
ok: [default] => (item=[u'libpcre3-dev', u'php7.1', u'php7.1-bcmath', u'php7.1-apcu', u'php7.1-cli', u'php7.1-common', u'php7.1-curl', u'php7.1-dev', u'php7.1-gd', u'php7.1-imap', u'php7.1-intl', u'php7.1-json', u'php7.1-mbstring', u'php7.1-mcrypt', u'php7.1-opcache', u'php7.1-soap', u'php7.1-xml'])
Slessi commented 6 years ago

Same behaviour for me on Ansible 2.4.3.0

geerlingguy commented 6 years ago

I... think this is the way this role is intended to be used. If you need more granularity it's probably best to not use the php-versions role at all, but rather use the php role and define versions and any other config variables manually.

This role is mostly for convenience for those who just want to get 7.1, 7.2, 5.6, etc. on an OS that comes out of the box with only one specific version of PHP, but it does a bit of variable magic to get there. It's not meant to layer on top of the variables in the base PHP role, but to override them for ease of use.

byhoratiss commented 6 years ago

I'm sorry to bother you @geerlingguy, but this issue is getting me confused.

If I use the php-versions role, I can set the version I want, like 7.1, but it overwrites the php_packages variables. But if I don't use it, the php role wants to use the 7.2 version of php, which is not the one that I want to use. How could I specify the version I want to use and use specific php_packages?

socketwench commented 6 years ago

I had this exact problem a week ago. >_<

aft2d commented 5 years ago

I'm facing the same problem now. I created a role for my php application. This role has set the php_packages in the vars/main.yml file and uses geerlingguy.php-version + geerlingguy.php as dependents. The variable gets overridden by the "Define PHP variables" task in the php-versions role. After a bit of searching I maybe found a solution? If you replace:

  when:
    - hostvars[inventory_hostname][item.key] is undefined
    - hostvars[inventory_hostname][item.value] is defined

with

  when:
    - vars[item.key] is undefined
    - vars[item.value] is defined

it would probably fix the problem. It would correctly consider the variables inheritance and at the same time and fulfill its purpose since it does not check whether a variable exists at host level but whether the variable exists in the current task.

TASK [geerlingguy.php-versions : Debug php_packages] ******************************************************************************************************************************************************************************************
ok: [lion] => {
    "php_packages": [
        "php7.3-common", 
        "php7.3-cli", 
        "php7.3-zip", 
        "php7.3-curl", 
        "php7.3-mbstring"
    ]
}

TASK [geerlingguy.php-versions : Define PHP variables.] ***************************************************************************************************************************************************************************************
ok: [lion] => (item={'value': u'__php_extension_conf_paths', 'key': u'php_extension_conf_paths'})
ok: [lion] => (item={'value': u'__php_fpm_daemon', 'key': u'php_fpm_daemon'})
ok: [lion] => (item={'value': u'__php_uploadprogress_module_path', 'key': u'php_uploadprogress_module_path'})
ok: [lion] => (item={'value': u'__php_xhprof_module_path', 'key': u'php_xhprof_module_path'})
ok: [lion] => (item={'value': u'__php_fpm_pool_conf_path', 'key': u'php_fpm_pool_conf_path'})
ok: [lion] => (item={'value': u'__php_conf_paths', 'key': u'php_conf_paths'})
ok: [lion] => (item={'value': u'__php_xdebug_module_path', 'key': u'php_xdebug_module_path'})
ok: [lion] => (item={'value': u'__php_pgsql_package', 'key': u'php_pgsql_package'})
skipping: [lion] => (item={'value': u'__php_packages', 'key': u'php_packages'}) 
ok: [lion] => (item={'value': u'__php_mysql_package', 'key': u'php_mysql_package'})
ok: [lion] => (item={'value': u'__php_memcached_package', 'key': u'php_memcached_package'})
ok: [lion] => (item={'value': u'__php_fpm_conf_path', 'key': u'php_fpm_conf_path'})
ok: [lion] => (item={'value': u'__php_tideways_module_path', 'key': u'php_tideways_module_path'})
ok: [lion] => (item={'value': u'__php_redis_package', 'key': u'php_redis_package'})

TASK [geerlingguy.php-versions : Debug php_packages] ******************************************************************************************************************************************************************************************
ok: [lion] => {
    "php_packages": [
        "php7.3-common", 
        "php7.3-cli", 
        "php7.3-zip", 
        "php7.3-curl", 
        "php7.3-mbstring"
    ]
}

The change works as intended for my use-case, but let me know what you think about it. Maybe I missed something. Tested on Ansible 2.7

geerlingguy commented 5 years ago

The point is (as stated earlier) that this role is only used for convenience, and you can use like php_packages_extra if you want to switch versions and add other additional packages. If you need more fine-grained control than that I recommend not using this role at all, just the geerlingguy.php role by itself, and setting up other bits and pieces to configure the exact PHP packages and versioning as you want them.