I'm creating this issues since I unintentionally hijacked the thread regarding automatic platform creation here:

#373062: 0.4 - automate platform creation

This ticket here deals specifically with the idea of implement Platform Management (that is, more tasks that one can do against Platforms). This idea was spawned from Anarcat's comment #8 in that ticket.

Please find attached the two current patches I've made that implement the following:

1) A Delete task against platforms. The Delete task will remove the entire platform on the filesystem and its apache config, *if* no sites are found in the sites/ folder. If sites are found, it rolls back. If none are found, the platform is removed from the filesystem, and the status of the platform changes to HOSTING_PLATFORM_DELETED, basically the same as how we deal with sites.

You can create a platform on the same path if the previous version was deleted.

2) A 'Lock' task, suggested by univate, that again modifies the status of the platform (the status field in the hosting_platform table was introduced for these patches) to make it similar to a site in its 'Disabled' state. When a platform is Locked, no site can be created on it. The platform cannot be viewed in the site node form. It is also not shown in the Summary block under 'Platforms' if the user does not have the new permission created 'view locked platforms'.

Adrian has mentioned he'd like to see this Lock idea implemented in an access control/client permissions scenario rather than here in platforms. However I still think it might be a useful tool to apply a 'global' lock like this against a platform, rather than have to go to each Client and disable their ability to see/use it. Let's say a copy of core gets bastardised or hacked - you may want to immediately apply a global lock on the platform, preventing anyone from creating sites on it, while you migrate your sites away to a new platform. There may be also other use cases for this.

3) The antithesis of 2), an 'Unlock' task which simply changes the status of the platform back again.

If all platforms are locked, the site node form tries to degrade gracefully with a drupal_set_message stating something like 'You have no enabled platforms. Please unlock or create a platform in order to create a site on it'. The resulting fields depending on a platform, such as the Profile and Language, are also hidden in this instance, preventing any php warnings/exceptions in this case.

Much testing is welcome - univate's confirmed it all works as expected, but I wanted to get some discussion going around the Lock idea before I'm comfortable committing it, as I don't want to be taking this in a direction Adrian feels is wrong. Nonetheless I do feel we *definitely* need a Delete task on platforms, and I think the Lock task is a useful tool that isn't exclusive of the idea of a finer grained platform access control for clients *as well*. See #558450: platform access control / permissions for that ticket.

The latest patches are attached.
If you don't want to apply patches because you're tired of stinking CVS, you can checkout my git branches

http://git.mig5.net/?p=modules/provision/.git;a=shortlog;h=refs/heads/37...
http://git.mig5.net/?p=modules/hosting/.git;a=shortlog;h=refs/heads/3730...

Remember to run update.php, as there's a schema update to add the 'status' column to the hosting_platform table.

CommentFileSizeAuthor
#3 373062_hosting.patch16.24 KBmig5
#3 373062_provision.patch9.06 KBmig5
#2 373062_hosting.patch14.93 KBmig5
#2 373062_provision.patch9.06 KBmig5
373062_provision_platform_management_2.patch9.02 KBmig5
373062_hosting_platform_management_4.patch14.52 KBmig5

Comments

Anonymous’s picture

Anonymous (not verified) commented

edit (why did I write all this in the wrong ticket? )

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 373062_provision.patch9.06 KB
new 373062_hosting.patch14.93 KB

I am so keen to get some more testing on this, I'll even supply a patch for those of you working on HEAD but not using git remotes/branches :)

You can still use the git branches, they've been freshly merged in from many changes in HEAD.

Test a Delete, Lock and Unlock of a platform. Test a Delete of a platform that has a site still on it and it should fail.

It'll now also abort and rollback if permissions are messed up and it can't *open* the sites dir of the platform.

Remember to run update.php . Schema change.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 373062_provision.patch9.06 KB
new 373062_hosting.patch16.24 KB

Now does not show deleted platforms in the migrate form.

anarcat’s picture

anarcat commented
Status: Needs review » Needs work

I finally had some time to review the code here. I would have like to test it, but not enough time. :) In general, the code is excellent quality and should be committed, albeit with some slight modifications (so I'm marking this as needs work, but it's not because it's totally wrong, i'm really just nitpicking here... assuming it works of course).

+++ b/platform/hosting_platform.install
@@ -76,3 +82,18 @@ function hosting_platform_update_6000() {
+
+// Add a status field to platforms so we can Delete tasks on them
+function hosting_platform_update_6001() {
+  $ret = array();
+  $ret[] = update_sql("ALTER TABLE {hosting_platform} ADD COLUMN status int(11) NOT NULL default '1'");
+  return $ret;
+}
+
+// Add a status field to platforms so we can Delete tasks on them
+function hosting_platform_update_6002() {
+  $ret = array();
+  $ret[] = update_sql("ALTER TABLE {hosting_platform} ADD COLUMN status int(11) NOT NULL default '1'");
+  return $ret;
+}
+

This seems like a dupe, for sure and will fail with a duplicate column warning.

+++ b/platform/hosting_platform.module
@@ -290,12 +366,27 @@ function hosting_platform_view($node, $teaser = FALSE, $page = FALSE) {
+function _hosting_platform_status($node) {
+  static $labels = array(
+    HOSTING_PLATFORM_QUEUED => "Queued",
+    HOSTING_PLATFORM_ENABLED => "Enabled",
+    HOSTING_PLATFORM_DELETED => "Deleted",
+    HOSTING_PLATFORM_LOCKED => "Locked",
+  );
+  return $labels[$node->platform_status];
+}

this is quite elegant, i like that.

+++ b/task/hosting_task.module
@@ -88,6 +88,21 @@ function hosting_task_menu_access($node, $task) {
+      // If the platform's been locked, we can unlock it, delete, batch migrate existing sites or verify
+      if ($node->platform_status == HOSTING_PLATFORM_LOCKED) {
+        $platform_tasks = array('verify', 'unlock', 'delete', 'migrate');
+      }
+      else {
+      // If the platform's unlocked, we can lock it, delete it or batch migrate sites
+        $platform_tasks = array('verify', 'lock', 'delete', 'migrate');
+      }
+      return (in_array($task, $platform_tasks));

shouldn't this be subject to permission checks (if the user has the right to lock/unlock platforms)?

Now for provision:

+++ b/platform/provision_drupal.drush.inc
@@ -333,6 +335,30 @@ function provision_drupal_find_sites() {
+// Simple function to return true if *any* sites are found
+function provision_drupal_find_any_sites() {

not sure we need that function... don't we already have a find_sites() function? if not, this should be it and not simply return true/false but a list (potentially empty) of sites.

+++ b/platform/delete.provision.inc
@@ -2,27 +2,39 @@
+function drush_provision_drupal_pre_provision_delete($url = NULL, $backup_file = NULL) {

.. also, this function should be where the rollback is triggered: if we're in the platform and sites are found, flag an error with drush_set_error() (which doesn't belong in find_sites() anyways) and then everything will rollback.

I'm on crack. Are you, too?

Anonymous’s picture

Anonymous (not verified) commented

Thanks, somehow didn't see you gave feedback on this the other day.

Yeah I fixed the dupe in the branch I have on git.aegirproject.org - don't know if you saw those

http://git.aegirproject.org/?p=provision.git;a=shortlog;h=refs/heads/dev...
http://git.aegirproject.org/?p=hostmaster.git;a=shortlog;h=refs/heads/de...

The reason I made my own find_any_sites() function was because I needed to return an error if sites *were* found (the opposite of the find_sites() function. Compare the two functions, the first of which you are correct was already there

function provision_drupal_find_sites() {
  if ($dir = opendir("./sites")) {
    while (FALSE !== ($subdir = readdir($dir))) {
      // skip internal directory pointers
      if ($subdir != '.' && $subdir != '..') {
        $file = "./sites/$subdir/settings.php";
        if (file_exists("$file") && ($subdir != 'default') && !is_link("./sites/$subdir")) {
          $sites[$subdir] = $file;
        }
      }
    }
    closedir($dir);
  } else {
    drush_log(dt("Cannot find sites directory"), 'error');
    $sites = FALSE;
  }
  return $sites;
}

// Simple function to return true if *any* sites are found
function provision_drupal_find_any_sites() {
  if ($dir = opendir("./sites")) {
    while (FALSE !== ($subdir = readdir($dir))) {
      // skip internal directory pointers
      if ($subdir != '.' && $subdir != '..') {
        $file = "./sites/$subdir/settings.php";
        if (file_exists("$file") && ($subdir != 'default') && !is_link("./sites/$subdir")) {
          $sites[$subdir] = $file;
          drush_set_error(dt("Existing site !site was found on this platform. Platform cannot be deleted until !site is deleted or migrated off this platform.", array('!site' => $subdir)));   
          $sites_exist = TRUE;
        }
        else {
          $sites_exist = FALSE;
        }
      }
    }
    closedir($dir);
  } else {
    drush_set_error(dt("Could not open sites directory to check for sites. Aborting."));
  }
  return $sites_exist;
}

But I agree this could be done better.

Thanks for the other feedback, I'll get cracking on it.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

This is ready for review again, and I have also rolled in Platform access control per client, for good measure. See #558450: platform access control / permissions

dev-platform_management branch in git.aegirproject.org (both repos)

adrian’s picture

adrian commented
Status: Needs review » Fixed

this stuff is really good. i am merging it into master.

new issues will need to be created if there are issues.

Status: Fixed » Closed (fixed)

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

  • Commit d56de4d on 6.x-2.x, 7.x-3.x, dev-ssl-ip-allocation-refactor, dev-sni, dev-helmo-3.x by mig5: 
    move platform tasks out of platform hook_access and into hosting_task....

  • Commit d56de4d on 6.x-2.x, 7.x-3.x, dev-ssl-ip-allocation-refactor, dev-sni, dev-helmo-3.x by mig5: 
    move platform tasks out of platform hook_access and into hosting_task....