Skip to content
This repository was archived by the owner on Jul 1, 2022. It is now read-only.

options to add Nginx and PHP-FPM to deployotron #37

Closed
wants to merge 1 commit into from
Closed

options to add Nginx and PHP-FPM to deployotron #37

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 12, 2015

At first, thank you for the wonderful project.

I am changing the config for my specific needs and thought it might be useful for someone to use the tool with Nginx and PHP-FPM.

@xendk
Copy link
Member

xendk commented Apr 13, 2015

Ah... As I was saying to @richardbporter in #34, I think I've messed up by implementing sh/drush commands as separate actions.

Even using Apache, we've had to override the command, as some older servers didn't have the service command.

Add to that that there's always been a plan to add a "run a generic (drush-)command", and it seems obvious to use the same for restarting Apache/Nginx/FPM/Varnish/whatever you need.

So I was thinking of having post-(deploy|cc|online) keys in the config where one just added the (drush-)commands needed, rather than have actions for five different environments and their permutations. Like:

$aliases['staging'] = array(
  'remote-host' => 'example.com',
  'remote-user' => 'deploy_user',
  'uri' => 'default',
  'root' => '/path',
  'deployotron' => array(
    'branch' => 'develop',
    'dump-dir' => '/backups', // Defaults to /tmp.
    'num-dumps' => 3, // Defaults to 5. 0 for unlimited.
    'post-deploy' => array(
      'sudo service apache2 restart',
    ),
    'post-online' => array(
      'sudo service varnish restart',
    ),
  ),
);

It does add a bit of configuration, but as there's really no sane default, it doesn't make sense to pretend there is one as --restart-apache2 and --restart-varnish makes you think.

It cuts down on code and what's left is more generic, so I think that's the way forward.

@richardbporter
Copy link
Contributor

That would be great. Any idea of a starting point for this?

One issue would be ordering the actions, ex. running 'fra' before updb/cc all.

@ghost
Copy link
Author

ghost commented Apr 14, 2015

Just an idea from @richardbporter use case:

What about defining commands in aliases as an array and process it according how it appears.

something like:

$aliases['staging'] = array(
  'remote-host' => 'example.com',
  'remote-user' => 'deploy_user',
  'uri' => 'default',
  'root' => '/path',
  'deployotron' => array(
    'branch' => 'develop',
    'dump-dir' => '/backups', // Defaults to /tmp.
    'num-dumps' => 3, // Defaults to 5. 0 for unlimited.
    'drush_commands' => array(
        'fra -y', 
        'updb -y', 
        'cc all'
    ), // All other drush commands could be piped in the array likewise
    'restart_services' => array(
        'php5-fpm', 
        'nginx', 
        'apache2', 
        'varnish', 
        'redis'
    ), // All other services commands could be piped in the array likewise
    'post-online' => array(
      'drush vset maintenance_mode 0',
    ),
  ),
);

The RestartServices class could be something like reading the array and looping over it.

@xendk
Copy link
Member

xendk commented Apr 14, 2015

@rajibmp yeah, but half the issue is that restarting apache2 might be any of a number of different commands.

I've worked a bit on it on https://github.com/reload/deployotron/compare/generic-commands?expand=1 it basically adds pre and post hooks for all actions.

Still needs a bit of work. The idea is that commands that start with "drush " gets handled as drush commands (rather than generic sh commands), which makes the difference transparent. Determining options also needs some love, "drush help deploy" has become a bit confusing and "--pre-flowdock-token" isn't a particularly good switch name.

We also need to think #36 into it somehow, seems that drush fra would be a good smoke test for drush command handling.

@xendk
Copy link
Member

xendk commented Apr 16, 2015

@rajibmp @richardbporter Mind taking a look at #38 and seeing if it works for you?

@xendk
Copy link
Member

xendk commented Apr 27, 2015

Now that #38 is in, there's really no need for this.

@xendk xendk closed this Apr 27, 2015
@ghost
Copy link
Author

ghost commented Apr 28, 2015

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants