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

Apply configurations to all possible conf directories on Debian #89

Merged
merged 3 commits into from
Mar 5, 2016

Conversation

oxyc
Copy link
Contributor

@oxyc oxyc commented Mar 4, 2016

owner: root
group: root
mode: 0644
with_items:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's just one item, let's just use the shorthand:

  with_items: "{{ php_conf_paths }}"


__php_apc_conf_filename: 20-apcu.ini
__php_opcache_conf_filename: 05-opcache.ini
__php_fpm_daemon: php5-fpm
__php_fpm_pool_conf_path: "/etc/php5/fpm/pool.d/www.conf"
__php_fpm_conf_path: "/etc/php5/fpm"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this variable because of Debain madness.

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

Hmmm:

Currently RedHat has:

__php_conf_path: /etc
__php_fpm_pool_conf_path: "/etc/php-fpm.d/www.conf"

But then this is in php-fpm.conf:

include={{ php_conf_path }}/fpm/pool.d/*.conf

Edit: Also where should --with-config-file-path be pointed when installing from source? It's a mute task as it's done later in configure.yml right?

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

Tests confirm that --with-config-file-path is mute. It's overridden anyways. The RedHat pool.d I still need to verify

oxyc added a commit to oxyc/ansible-role-php that referenced this pull request Mar 4, 2016
@geerlingguy
Copy link
Owner

Looks good to me?

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

I'm not sure about the source install on RedHat. I'll run some tests to make sure.

Are there two conf paths on RedHat? From the previous config it seems there is both /etc/fpm and /etc/php-fpm.d

@geerlingguy
Copy link
Owner

@oxyc - K, pending those results, I'll merge it in tonight or tomorrow morning. Working on a few other roles tonight.

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

Not sure I'll get it done tonight. Found some other bugs as well. Really need to upgrade my internet connection so I can start building these containers locally..

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

Seems that the /etc/php5/fpm/pool.d/www.conf does not exist when installing from source with --with-fpm. Not on RedHat either. Did this use to work? I have to look into it more later

@geerlingguy
Copy link
Owner

Don't have time to dig in now, but did we create that file using template?

@oxyc
Copy link
Contributor Author

oxyc commented Mar 4, 2016

We do not. It's available after installed from a package manager but not when installed from source.

Also the file has never been included on RedHat (the path was incorrect in php-fpm.conf).

@geerlingguy
Copy link
Owner

I'm also looking into this now (a little at least) as I'm working on some drupal-pi improvements, as I'm seeing the same:

Destination /etc/php5/fpm/pool.d/www.conf does not exist !

@oxyc
Copy link
Contributor Author

oxyc commented Mar 5, 2016

I had to give up. Think I need to install php 7.0 and play around properly. I think this was broken before this patch though. Currently I need a break from these crazy configs (I also read the paths have changed AGAIN for php7 on ubuntu when installed from the ppa...)

Here's a few notes from things I attempted (still couldn't solve it though). Maybe you can solve it.

oxyc@32d91a5
Running systemctl status php-fpm.service reported this path was the one included. Still same error though so maybe it's not needed.

oxyc@f248498 Made sure the pool conf existed before modifying. Guess this is incorrect and we should add the file somehow.

oxyc@5e116f1
Fixed the path to the conf paths in php-fpm.conf. RedHat had it incorrect.

oxyc@72873e8
I think this was broken because the variable was used in install-from-source.yml already.

@geerlingguy
Copy link
Owner

It was definitely broken before this PR; I'm using that for testing on Debian Jessie, and it's breaking at the same spot. So I'll see if I can at least get that fixed.

I think we're safe to merge this, then fix the install-from-source issues in a separate PR, what do you think?

@oxyc
Copy link
Contributor Author

oxyc commented Mar 5, 2016

I think it's safe. There's one questionable commit that might affect php7 with fpm though 9eadfc1, this change was what prompted me to test php7.

geerlingguy added a commit that referenced this pull request Mar 5, 2016
Apply configurations to all possible conf directories on Debian
@geerlingguy geerlingguy merged commit f195d7e into geerlingguy:master Mar 5, 2016
ivangrynenko pushed a commit to ivangrynenko/ansible-role-php that referenced this pull request Jun 14, 2016
… Add note for root password issues and document sudo requirement.
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