Module from hostmaster-6.x-2.0-rc4/profiles/hostmaster/modules/hosting/subdirs/hosting_subdirs.module is not enabled.

On site delete we get:

Calling hook drush_subdirs_pre_provision_delete
Undefined offset: 1 subdirs.drush.inc:249
unlink(/var/aegir/config/server_master/apache/subdirs.d/drupal8-1.dev.voorbeeld-site.nl/.conf): No such file or directory subdirs.drush.inc:251
Error encountered attempting to delete site location config file for subdirectory drupal8-1.dev.voorbeeld-site.nl
opendir(/var/aegir/config/server_master/apache/subdirs.d/drupal8-1.dev.voorbeeld-site.nl): failed to open dir: No such file or directory subdirs.drush.inc:124
Returned from hook drush_subdirs_pre_provision_delete
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helmo’s picture

Assigned: Unassigned » anarcat
Priority: Normal » Major

$alias in the _subdirs_delete_site_location() function is expected to be a string with a slash in it.

None of the provision functions seem to validate this.
We could quickly add

if (count($alias_parts) < 2) { return FALSE; }

But why are these being called? Do we have any other provision extensions that depend on a hosting module being enabled?

Related: #2046167: [META] Remaining issues with subsite support (Part II)

anarcat’s picture

Priority: Major » Critical

that's a long time problem: there is no standard mechanism to enable/disable backend module from the frontend.

i am bumping this to critical since the subdir code has been merged in 2.x and this has the potential of breaking everything.

anarcat’s picture

helmo’s picture

From: function drush_commandfile_list() {

* Commands implementing hook_drush_load() in MODULE.drush.load.inc with
* a return value FALSE will not be loaded.

Should we implement a hook_drush_load() which checks the frontend module status in the hostmaster context ?

ergonlogic’s picture

Maybe we could add a 'subdir' flag in the site context, and then switch based on that?

anarcat’s picture

ideally, this would be an aegir-wide parameter that any task sends a list of aegir-related modules enabled, and then we have *one* _load hook that we kick in the backend?

Maybe that will still required a hook_drush_load() in every .load.inc for every module, but at least we can simplify the message passing to be more generic from the frontend's perspective?

Just throwing ideas out there...

helmo’s picture

I think every drush/provision contrib would have to implement hook_drush_load(), at least if there is something that should be prevented from running if the hosting counterpart is not enabled.
It might even be a slight performance gain if less code needs to be processed.

Some pseudo code:

// In subdirs/subdirs.drush.load.inc
function subdirs_drush_load() {
  $this_module = 'subdirs' ;
  return provision_is_module_enabled_in_hosting_frontend($this_module); 
}


// Somewhere in provision.... a part that's loaded before all the drush_load()'s get called.
function provision_is_module_enabled_in_hosting_frontend($module_name) {
  $context = drush_get_context('HOSTING_MODULES_ENABLED');
  // should we log a debug message here? 
  return (!empty($context[$module_name]));
}

// Somewhere in ... the verification task of the master server???
$modules = ...
drush_set_context('HOSTING_MODULES_ENABLED', $modules);

ergonlogic’s picture

Normally, there shouldn't be any code in Provision that runs before drush_load()'s. In _drush_add_commandfiles(), it'll include all *.drush.load.inc's before including any commandfiles.

However, Drush provides an '--early' option that allows pre-bootstrap code to be run. It is invoked at the top of drush_main(). Unfortunately, it appears to only be documented in the option itself:

    $options['early']               = array('description' => "Include a file (with relative or full path) and call the drush_early_hook() function (where 'hook' is the filename). The function is called pre-bootstrap and offers an opportunity to alter the drush bootstrap environment or process (returning FALSE from the function will continue the bootstrap), or return output very rapidly (e.g. from caches). See includes/complete.inc for an example.");

I'm finding it hard to trace Drush's bootstrapping, but I think drushrc.php's are parsed just before looking for command files. If so, maybe we should consider writing a /var/aegir/.drushrc.php (or /var/aegir/.drush/drushrc.php), as part of verifying the hostmaster site. It might be triggered by adding a 'verify' task somewhere in hosting features.

ergonlogic’s picture

So this seems to work. In provision/subdirs/subdirs.drush.load.inc:

function subdirs_drush_load() {
  return array_search('subdirs', drush_get_option('hosting_features'));
}

... and in /var/aegir/.drush/drushrc.php:

$options['hosting_features'] = array (
  'subdirs' => 'subdirs',
);

Changing the value in this array to anything other that 'subdirs' blocks the code in subdirs.drush.inc from running.

ergonlogic’s picture

Since git.drupal.org is still broken, here are a couple patches that implement what I suggested in #8 and #9. This is a general purpose solution allows us to block drush hooks by implementing something like:

function subdirs_drush_load() {
  return array_search('subdirs', drush_get_option('hosting_features', array()));
}
helmo’s picture

+++ b/subdirs/subdirs.drush.load.inc
@@ -0,0 +1,10 @@
+  return array_search('subdirs', drush_get_option('hosting_features', array()));

Why array_search()?
I think looking up the array key is faster. And the array value could just be a boolean. This could later be extended to be an array containing more info about the module ... possibly version info.
It also reduces ambiguity for a contrib developer.

This updated provision patch also adds the hook to the dns module. Which is an easy thing to use for testing... after this patch the provision-zone command no longer appears in the help listing when calling 'drush'


// Option 1
return array_search('subdirs', drush_get_option('hosting_features', array()));

// Option 2
return array_key_exists('subdirs', drush_get_option('hosting_features', array()));

// Option 3
$hosting_features = $drush_get_option('hosting_features', array())
return !empty($hosting_features['subdirs']);
anarcat’s picture

Just for the sake of it, let me add:

// Option 4
return in_array('subdirs', drush_get_option('hosting_features', array()));

ie. that we just have the modules as the values in the array - why bother with anything more complicated?

Also, at this point it is a little unclear for me when this code will run and the features array actually be written into that drushrc file... Can you be a little more specific about this?

helmo’s picture

After applying the patch I verified the hostmaster site to fill the /var/aegir/.drush/drushrc.php

Saving the admin/hosting/features page triggers a verify of the hostmaster site.

I'm ok with option 4... I'll update the patches.

helmo’s picture

Updated patch for provision ... the hosting one from #10 is still good.

ergonlogic’s picture

Assuming we're otherwise okay with this, I'll make the changes in #14, and commit. I have some nice explanatory commit messages that I'd hate to waste :)

ergonlogic’s picture

Assigned: anarcat » Unassigned
Priority: Critical » Major
Status: Needs review » Active

I pushed these in provision and hosting. I'm re-setting to 'active', since there's also a suggestion in #1 to validate better.

danquah’s picture

Retested after the patch update in #14 btw, it's all good :)

helmo’s picture

Status: Active » Fixed

I've created #2099883: Validate an alias has parts (subdirs) to checkout the validation.

helmo’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 5d6120d on 6.x-2.x, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by ergonlogic:
    Issue #2098389: Record enabled Hosting Features in /var/aegir/.drush/...