Code to use drush to enqueue Aegir tasks for enabled sites, just as if it had been done via the ui

Hi Ken,

Your patch didn't upload, can you upload it again as a comment to this issue?

I think this would be a fantastic addition to Provision, so I'm moving it to the other queue.

Review coming. Thanks!

So this drush command works as expected. Thanks very much for the patch! As previously said, I think this would be great as included in Provision, as it'd make the 'headless Aegir' or 'backend only' installation much more powerful.

1) For this reason I suggest we rename it to 'provision-task' which is simpler. Adrian or anarcat can have the yay or nay here, that's just my opinion :)

This would mean renaming all the functions to 'provision_task_' prefixes instead of 'aegir_task_enqueue_'

I think we could also make aegir_task_enqueue_add_task simply 'provision_add_task', it matches hosting_add_task then in the frontend :)

Most of what I have below are just cosmetic suggestions / typos, which is a good thing :)

2)

function aegir_task_enqueue_drush_command() {
  $items = array();
  $items['task-enqueue'] = array(
    'description' => 'Enqueue Aegir site tasks as if requested via Aegir UI.',
    'arguments' => array(
      'task' => "Required. This is the first parameter. The name of a site task that is selectable in the Aegir UI, that does not require parameters (in the Aegir UI) and that may be done on an 'enabled' site",
        'site' => "Required. This is the second parameter. The word 'all' or the name of a site (site folder). a value of 'all' will enqueue the task on all the client sites. ",
        'additiona_sites' => 'Optional. The names of sites (site folders). ',
      ),

Rather than specify 'additional_sites' as a third argument here, it would seem clearer to me if just in the 'site' argument we reworded it like this:

'site' => "Required. This is the second parameter. The word 'all' or the site name. A value of 'all' will enqueue the task on all sites. Multiple sites can be specified in the one command.",

3)

      'options' => array(
        '--uri' => 'Required. Identifies the URI of the Aegir drupal site to use',
        '--root' => 'Optional. Identifies the Drupal root directory to use. When this is not set drush defaults to the current folder.',
      ),

Technically --uri is optional here too. If inside a valid --root, one can cd into the sites/$aegir_site and run Drush commands without it, and it will be bootstrapped.

I'd consider rewording to something like this:

      'options' => array(
        '--root' => 'Optional. Identifies the Drupal root directory to use. When this is not set drush defaults to the current folder.',
        '--uri' => 'Optional. Identifies the URI of the Aegir Drupal site to use. When this is not set, Drush defaults to the current working directory if inside a bootstrappable site folder, or otherwise sites/default, if inside a valid --root.',
      ),

But these are just opinions, not errors, feel free to challenge them :)

4) aegir_task_enqueue_get_sites() is a good function that i think should be in the frontend as 'hosting_get_sites'. Currently our only similar function is hosting_site_list() which is very focused on a rowed layout for display in the frontend.

I think we'd accept a 'hosting_get_sites()' function in a separate patch to hosting_site.module that is the same as your one here, and then you can reuse that function in the backend same as you do for hosting_add_task, hosting_available_tasks etc.

5) You are hiding/preventing certain tasks which is a good idea, and you are correct re: import and install tasks.

However, 'enable' and 'delete' *should* be allowed tasks, but only in certain circumstances: a site should be able to be 'enabled' if it has been 'disabled' (which is an allowed task in your patch). A site should also be able to be 'deleted' after it has been 'disabled'. This is how it works in the frontend - if you can conditionally allow these two tasks, then it'll match the functionality of the frontend. Take a look at the various 'safe_tasks' logic of hosting_task.module in the frontend to see how we do it there.

Since you can access the site node objects, you could check for its HOSTING_SITE_$STATUS in {hosting_site}. Since you are already skipping sites that are deleted via hosting_get_site_by_url(), you could check if a site is in HOSTING_SITE_DISABLED state, and if so, 'enable' and 'delete' should both be allowed.

6) A variation on 5) - certain (in fact most) tasks should not be allowed to be performed on the main 'aegir' site such as disable. We currently check for this in the same place as I mentioned in 5) - that is, in hosting_task_menu_access(). In this case, we check if the site's profile is 'hostmaster' and if so, we return after defining a certain array of safe_tasks

Perhaps we should move some of that functionality out of a GUI-specific function in Hosting, so that it can be reused by the backend here.

Note: I imagine this would also affect the 'all' option, as additional checking would be needed to ensure an unsafe task is sent to the main aegir site.

7) Not a show stopper, but I would love to see this command extend beyond just sites. For instance, platforms can be verified, locked, unlocked or deleted (These are all tasks in the frontend that don't require user-intervention in forms). You could control this similar to how we already do in Provision, via the 'context'. Look at provision/platform/delete.provision.inc to see the interchanging between site and platform. I *think* the same could work here, but not sure.

Once again, thanks for the excellent work here!