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

HHVM provisioning and toggling between php-fpm #45

Closed
wants to merge 2 commits into from

Conversation

nathanielks
Copy link
Contributor

  • Renamed php-55 to php-fpm as it made sense ( to me ) to remove the
    version number from the role as it may change in the future, and
    php-fpm is a more accurate representation of what we’re doing (
    especially considering we don’t have version numbers in any of the
    other roles ).
  • Added base php role. This will load proper php implementation in
    meta/main.yml based upon the variable php_implementation located in
    vars/common.yml. The tasks in the php role are executed after said role.
  • Added nginx-php-fpm.conf and nginx-hhvm.conf to allow decoupling of
    how nginx passes the request back to php. I believe the hhvm
    implementation can be used for both, though we’d have to change from a
    socket connection to a tcp connection. Should we choose, hhvm can be
    used over a unix socket and we can standardize the file name structure
    as well so as to not have to keep changing the nginx php location block.

@nathanielks nathanielks mentioned this pull request Aug 31, 2014
@austinpray
Copy link
Contributor

( ͡° ͜ʖ ͡°)

@austinpray
Copy link
Contributor

(◕‿◕✿)

@@ -0,0 +1,6 @@
---
- name: restart nginx
service: name=nginx state=restarted
Copy link
Member

Choose a reason for hiding this comment

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

This handle should be the same as https://github.com/roots/bedrock-ansible/blob/b859a967a0c08ce5eb3b2b899e35a189dc4bf197/roles/wordpress-sites/handlers/main.yml

Generally reload is preferred over restart for Nginx.

Reloading nginx is safer than restarting because before old process will be terminated, new configuration file is parsed and whole process is aborted if there are any problems with it.

@swalkinshaw
Copy link
Member

Big 👍 for renaming the php role. I'll go over this again later but it looks mostly good.

@nathanielks
Copy link
Contributor Author

Okie doke, made all the requested changes. Also was able to figure out, we can throw all of our handlers in one file and have subsequent tasks utilize them, so I threw all the handlers thus far into roles/common/handers. Using ansible 1.7.1.

Side note: am encountering a peculiar issue and curious your thoughts. On vanilla provision, at the end of the provision, restart php-fpm and reload nginx are both notified and is output in the terminal. This doesn't occur in subsequent provisions. Any idea why this may be happening?

Another note: On subsequent provisions where switching between implementations, a 502 is thrown until nginx has been reloaded or the box restarted.

@nathanielks
Copy link
Contributor Author

To address the 502, should we reload nginx after wordpress-sites has done it's thing, just to be sure?

@austinpray
Copy link
Contributor

Be sure to update the readme ansible version if you are using newer Ansible features. (◕‿◕✿) —
Sent from my iPhone

On Tue, Sep 2, 2014 at 12:00 AM, Nathaniel notifications@github.com
wrote:

Okie doke, made all the requested changes. Also was able to figure out, we can throw all of our handlers in one file and have subsequent tasks utilize them, so I threw all the handlers thus far into roles/common/handers. Using ansible 1.7.1.

Side note: am encountering a peculiar issue and curious your thoughts. On vanilla provision, at the end of the provision, restart php-fpm and reload nginx are both notified and is output in the terminal. This doesn't occur in subsequent provisions. Any idea why this may be happening? On subsequent provisions where switching between implementations, a 502 is thrown until nginx has been reloaded or the box restarted.

Reply to this email directly or view it on GitHub:
#45 (comment)

@nathanielks
Copy link
Contributor Author

Ah, good catch! Done.

@nathanielks
Copy link
Contributor Author

Though, after re-reading the docs, I'm not sure this is a super new feature... did it just not work before? The docs say once loaded, handlers apply across all playbooks. They also mention having a handlers/handlers.yml file, though just including it in roles/common does the same trick without extra configuration.

@nathanielks
Copy link
Contributor Author

I rolled back my version of ansible to 1.5.4 and confirmed: this isn't a new feature. Turns out we've been duplicating our handlers for nothing =p

@austinpray
Copy link
Contributor

I blame @swalkinshaw

@nathanielks
Copy link
Contributor Author

Silly @swalkinshaw 😉

This was referenced Sep 2, 2014
@swalkinshaw
Copy link
Member

Never trust Stack Overflow answers.

@nathanielks
Copy link
Contributor Author

If this is looking good, I'll squash this one as well.

fastcgi_pass 127.0.0.1:9000;
fastcgi_index index.php;
fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
include fastcgi_params;
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be 2 space indentation here. Other conf below is 2 and our others are as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@swalkinshaw
Copy link
Member

I should probably give this one a test first

@swalkinshaw
Copy link
Member

Finally tried this out and I had one problem. Any nginx conf files like nginx-hhvm.conf and nginx-php-fpm.conf need to have their user/group set to root. Right now I just got vagrant:vagrant and only rw for owner so it wasn't working.

@swalkinshaw
Copy link
Member

One other thing, what's the reasoning behind setting the implementation in the new vars/common.yml vs the normal group vars? I kind of forget if we discussed that somewhere.

@nathanielks
Copy link
Contributor Author

I'll get to the file permissions asap. As far as the vars/common.yml logic... to be honest, I'm don't really remember! group_vars/all would accomplish the same thing. Shall I make that switch?

@swalkinshaw
Copy link
Member

Yeah, I think so. Just seems weird to have that one setting there and everything else in another place.

@swalkinshaw
Copy link
Member

@nathanielks can you rebase + squash? Think this is good to go.

@nathanielks
Copy link
Contributor Author

Hey, I rebased and merged and started testing on a vanilla install and am running into some 502's. Will work on this some more and get back with you when it's ready.

@nathanielks
Copy link
Contributor Author

Okie doke. Did some more testing and reporting some findings:

Provisioning a php implementation in a way locks the server into that one implementation. Otherwise, reprovisioning with a different one results in 502 errors. Restarting the machine, however, resolves the 502.

After some further testing, I was able to find something curious: initial provision of a php implementation works out fine. Reprovision with another results in a 502. I ssh'd into the machine, ran ps -waux | grep nginx and got this output:

vagrant@example:~$ ps -waux | grep nginx
root      1475  0.0  0.2  90400  2532 ?        Ss   23:27   0:00 nginx: master process /usr/sbin/nginx
www-data  4715  0.0  0.5  93720  5156 ?        S    23:29   0:00 nginx: worker process
www-data  4716  0.0  0.5  94040  5872 ?        S    23:29   0:00 nginx: worker process
vagrant   7352  0.0  0.0  10468   920 pts/0    S+   23:32   0:00 grep --color=auto nginx

Now here's what's curious: I ran sudo service nginx status, and it gives me this:

vagrant@example:~$ sudo service nginx status
 * nginx is not running

After some more testing, I've found that 502's aren't consistent, but the above two conditions are. Going to keep testing and see what I can come up with.

@swalkinshaw
Copy link
Member

I'm wondering if nginx restart is needed in this case instead of just reload.

@nathanielks
Copy link
Contributor Author

I was wondering that too, but it doesn't do anything. The thing I find curious is nginx is running, but service doesn't have access to it. Found this, so I'm wondering if bindfs may be mucking with things. Still testing a few other things.

@nathanielks
Copy link
Contributor Author

Ah, gained some more insight. From example.dev.error.log:

5877#0: *19 connect() failed (111: Connection refused) while connecting to upstream, client: 127.0.0.1, server: example.dev, request: "GET /wp/wp-cron.php HTTP/1.1", upstream: "fastcgi://127.0.0.1:9000", host: "example.dev"

@nathanielks
Copy link
Contributor Author

This is while fpm has been provisioned. It should be connecting over a socket.

@nathanielks
Copy link
Contributor Author

You know what, I'm wondering if the initial provision's nginx process is still running and somehow get's disconnected from service. I've been testing provisioning one implementation before another on a fresh box and it seems whichever one is provisioned first is the one nginx binds to.

@nathanielks
Copy link
Contributor Author

Opa! I think I got it. There is no pid file set in /etc/nginx/nginx.conf, so the original process can't be stopped and a new process can't be reloaded. Testing now.

@nathanielks
Copy link
Contributor Author

Shoot. I was wrong.

@nathanielks
Copy link
Contributor Author

Okie doke. This can be safely merged now!

@@ -69,5 +69,6 @@ Vagrant.configure('2') do |config|
# Fix for slow external network connections
vb.customize ['modifyvm', :id, '--natdnshostresolver1', 'on']
vb.customize ['modifyvm', :id, '--natdnsproxy1', 'on']
vb.memory = 1024
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this? It's already implemented.

@nathanielks
Copy link
Contributor Author

yerp, on it

@nathanielks
Copy link
Contributor Author

boom.

@nathanielks
Copy link
Contributor Author

Okie doke, ready for review.

@nathanielks
Copy link
Contributor Author

just found out hhvm won't be supporting multiple worker pools, so we could only use one socket for one domain. The workaround is to have separate hhvm instances running on different ports. I know thus far bedrock-ansible has been developed with the ability to run multiple domains/sites, so how would we like to proceed on this one?

@nathanielks
Copy link
Contributor Author

@swalkinshaw I forget where we landed on this...

- name: Install HHVM
apt: pkg="{{ item }}" state=latest
with_items:
- hhvm
Copy link
Member

Choose a reason for hiding this comment

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

No need for with_items here. Just hardcore hhvm in there.

@swalkinshaw
Copy link
Member

@nathanielks me too. I'm thinking we should just ditch separate php-fpm confs/sockets per WP site. Then HHVM and PHP-FPM can both be treated in the same way with 1 socket.

@swalkinshaw swalkinshaw mentioned this pull request Mar 3, 2015
1 task
@swalkinshaw
Copy link
Member

Closing in favour of #136

@swalkinshaw swalkinshaw closed this Mar 3, 2015
@nathanielks
Copy link
Contributor Author

Whoop whoop!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants