Allow spokes to change their themes and modules [#1561102]

I have the following setup:

remote (spoke) drupal site
SFTP access to themes on the spoke

As reported on http://drupal.org/node/1079274 and http://drupal.org/node/1083366, there are some issues with the sync process between hub and spokes. Therefore, as a workaround, I've setup user contrib themes as symlinks, avoiding the hub to send any mess/delete info there

However, when verify is run, the theme gets disabled - is no longer existing not even in the HTML source - and the page is broken. The following error notices appear:

    Notice: Undefined index: highlighted in include() (line 120 of /var/aegir/platforms/drupal-7.12/modules/system/page.tpl.php).
    Notice: Undefined index: sidebar_first in include() (line 132 of /var/aegir/platforms/drupal-7.12/modules/system/page.tpl.php).
    Notice: Undefined index: sidebar_second in include() (line 138 of /var/aegir/platforms/drupal-7.12/modules/system/page.tpl.php).
    Notice: Undefined index: rt in drupal_theme_initialize() (line 100 of /var/aegir/platforms/drupal-7.12/includes/theme.inc).
    Notice: Trying to get property of non-object in _drupal_theme_initialize() (line 145 of /var/aegir/platforms/drupal-7.12/includes/theme.inc).
    Notice: Trying to get property of non-object in _theme_load_registry() (line 321 of /var/aegir/platforms/drupal-7.12/includes/theme.inc).
    Notice: Undefined index: rt in theme_get_setting() (line 1316 of /var/aegir/platforms/drupal-7.12/includes/theme.inc).

They go away when:

After some time bartik theme is selected again (automatically), or
One goes to the themes admin page (the theme is also available there, but disabled, can be reactivated

Comment	File	Size	Author
#13	provision-sync_files-1561102-12.patch	1.53 KB	wamilton
#7	provision-sync_files-1561102-7.patch	1.43 KB	wamilton
#4	provision-sync_files_themes-1561102-4.patch	3.23 KB	gandhiano
#3	provision-sync_files_themes-1561102-3.patch	2.96 KB	gandhiano

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

gandhiano CreditAttribution: gandhiano commented 8 May 2012 at 14:44

Component:

Miscellaneous

» Code

Comment #2

anarcat CreditAttribution: anarcat commented 10 May 2012 at 22:26

Status:

Active

» Postponed

I don't think that spoke-specific themes are supported: since the hub (@hostmaster) is considered to be authoritative for those files since #1079274: Sites data get lost on migrate or clone / dont "spoke" the site. This means that the files from the spoke are not synced back to the master, which means that themes installed on the spoke will not be available to provision-verify, which may in turn break the them when it flushes the cache.

I am therefore marking this as postponed (tempted to mark "won't fix") - until we have more intelligence on the spokes, ie. that they run their own provision-verify commands, this is unavoidable.

Comment #3

gandhiano CreditAttribution: gandhiano commented 11 May 2012 at 19:38

Version:	6.x-1.7	» 6.x-1.8
Status:	Postponed	» Needs review
Issue tags:		+rsync, +spokes, +hub, +provision-verify

File	Size
provision-sync_files_themes-1561102-3.patch	2.96 KB

This is also related to the behavior described in #1083366: Make the spokes authoritative for files/ and private/ directories for files/ folder.

I have extended the patch on #33 and updated for 1.8.

Basically it does the following:
* Adds and calls provision_drupal_sync_files()
* Adds and calls provision_drupal_sync_themes()
* Changes _provision_drupal_create_directories() to not change ownership and modes for themes/ folder (because, in the case we allow filesystem user access, it needs to be owned by the user, not aegir)

This can eventually be extended to site-specific modules folder (to have modules installed by users with filesystem access)

Comment #4

gandhiano CreditAttribution: gandhiano commented 12 May 2012 at 15:36

File	Size
provision-sync_files_themes-1561102-4.patch	3.23 KB

The patch above is breaking the creation of new sites:

/var/aegir/platforms/drupal-7.14/sites/seed-sovereignty.org/files/ could not be fetched from remote server webpal.ecobytes.net. Changes might not be available until this has been done. (error: rsync: change_dir "//var/aegir/platforms/drupal-7.14/sites/seed-sovereignty.org/files" failed: No such file or directory (2) rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1526) [Receiver=3.0.7])

Adding the update option to rsync solves the issue. New patch attached.

Comment #5

chrisschaub CreditAttribution: chrisschaub commented 14 May 2012 at 11:42

I think adding a sync modules and sync libraries would be appropriate then as well? One piece missing is the ability to purchase a feature and have it dropped into the modules folder of a client site. This patch will make it possible to do this without the module getting overwritten on the next verify. It really opens up the possibilities on Aegir. Maybe changing the title of this issue would help it get better noticed as well. Maybe something like "Fix Aegir verify to honor remote files, themes and more" ? This whole area is a broken spot in the Aegir design and this patch really helps, greate work gandhiano.

Comment #6

gandhiano CreditAttribution: gandhiano commented 15 June 2012 at 13:32

Title:

User enabled themes on remote (spoke) get disabled on verify

» Improving sync process between master (hub) and remote (spoke)

The patch on #4 had a problem which still has problems in the creation of new websites - sync_files and sync_themes are being called before sync_site, but this one must be before (or else install fails saying that he doesn't find the files/ folder on the spoke)

cschaub, I have changed the issue title, as you suggested.

Comment #7

wamilton CreditAttribution: wamilton commented 20 June 2012 at 19:12

File	Size
provision-sync_files-1561102-7.patch	1.43 KB

Changed to only care about the files directory since that seems to be the primary issue most people are dealing with. The behavior of the specific sites/$url/{modules,themes,libraries} directories is as yet undefined as far as I know. Bringing in a patch that treats them as space for the remote is not yet fair game as far as I can see. The files directory is much more clear cut, and I wanted to get this down to something that could be easier to accept.

I changed the name of the files sync function to provision_drupal_fetch_files() since that's what it's actually doing.

Also, I wrapped it in the same existential check that provisionContext_server::fetch() uses to set an error, and just doesn't do anything if the fetch target, the files dir in this case, doesn't exist. Also, just trying this safe fetch whenever provision_drupal_sync_site() is called, instead of sticking in fetch calls wherever someone tries to sync.

Not setting the update flag either, as fetching beforehand should obviate the need to do that.

Comment #8

wamilton CreditAttribution: wamilton commented 20 June 2012 at 21:56

Version:

6.x-1.8

» 6.x-1.x-dev

Switching tags since I rolled against 6.x-1.x HEAD and therefore the issue obviously persists, and the patch also applies cleanly, so it can fold right in.

Comment #9

theMusician CreditAttribution: theMusician commented 20 June 2012 at 23:10

Status:

Needs review

» Reviewed & tested by the community

I applied the patch to 6.x-1.9 and it applied without issue. However, it didn't seem to really fix the issue for files not being overwritten.

1. Create a file within a site on the hub and verify the site.
2. File appears on remote server after verify.
3. Modify the file on the remote server and verify the site.
4. File on remote server is replaced by the one on the hub.

Permissions in the file directory for all files are aegir:apache.

Comment #10

mxmilkiib CreditAttribution: mxmilkiib commented 21 June 2012 at 01:34

Status:

Reviewed & tested by the community

» Needs work

Comment #11

gandhiano CreditAttribution: gandhiano commented 21 June 2012 at 09:40

Probably putting back the rsync update flag again will deal with #9. Or will it break something else?

-    d()->service('http')->sync(d()->site_path, array('no-delete' => TRUE));
+    d()->service('http')->sync(d()->site_path, array('no-delete' => TRUE, 'update' => TRUE));

Comment #12

theMusician CreditAttribution: theMusician commented 21 June 2012 at 15:37

Confirmed. Adding 'update' => TRUE allows files on the remote to not be altered which is the desired behavior.

However, the hub does not get updated on verify with this fix. That is ok as I believe another function does a complete sync of files when a site gets cloned or migrated.

Comment #13

wamilton CreditAttribution: wamilton commented 21 June 2012 at 19:07

Status:

Needs work

» Needs review

File	Size
provision-sync_files-1561102-12.patch	1.53 KB

Unfortunately, provision_drupal_sync_site() isn't a single point of entry like one might hope. Instead, tons of places in the source call d()->service('http')->sync($path), which is exactly provisionContext_server::sync(), unless your http service has anything to say about it. As such, the strategy of injecting the fetch into provision_drupal_sync_site() doesn't do anything in most cases where a sync would happen.

Modifying provisionContext_server::sync() to invoke provision_drupal_fetch_files() is what we will have to do instead.

Setting the update flag is still not sufficient because sometimes the end user may actually want to replace a file with an older version, which we would then undo if we were to sync, since the hub version would be newer.

Here's a patch that does away with the function provision_drupal_fetch_files() because I don't know where the heck to put it, but just injects the files fetch logic into provisionContext_server::sync(). Additionally, this time I decided I really should make sure that the sync is trying to push changes to the files dir, and actually ensure that all my variables are defined. Sorry about that oversight.

This patch should work, but at the same time needs guidance as far as a proper mechanism for asserting the paths for which the remote server is authoritative. The easy way out of a code refactor at this point is to decide that the business logic of Ægir is in fact that for a hardcoded set of paths, the remote is authoritative. The good way to do it would be to have the server ask the platform and the site for what paths it considers the remote authoritative, perhaps by provisionContext_site and provisionContext_platform extending to declare this.

In any case, plugging in either of those solutions at the same place as this proof of concept ought to work if this patch works.

Comment #14

theMusician CreditAttribution: theMusician commented 26 June 2012 at 15:54

Status:

Needs review

» Needs work

I got around to testing this and the patch applies cleanly, but does not solve the issue of allowing the remote files directory to be authoritative when a verify task is executed.

I do believe this gets messy when directories such as themes and modules become involved as generally users are not changing those via the GUI. Perhaps files should not be altered in any way either as users generally upload new files. Drupal by default changes the file names if the same file is uploaded and Aegir merrily will copy those new files.

I began down this rabbit hole because of the js and css injector modules. Perhaps I should instead alter those files to have timestamps in the file names so that they are always unique and let Aegir proceed along its path.

Perhaps as cschaub mentions in post #5 when the logic is extended to each remote this entire approach will be much more plausible.

Comment #15

anarcat CreditAttribution: anarcat commented 16 July 2012 at 20:57

Title:	Improving sync process between master (hub) and remote (spoke)	» Allow spokes to change their themes and modules
Version:	6.x-1.x-dev	» 6.x-2.x-dev
Category:	bug	» feature

I recommend that discussions about the files/ directory continue in #1083366: Make the spokes authoritative for files/ and private/ directories. I understand the issue has grown rather huge, but we need to resolve it, and not expand into more directions.

Here, there is a valid point that is made: "what if i want to allow my users to modify the themes on the spoke?" It is a valid feature request, and it should be considered. I had originally marked it as postponed but I am now ready to let it live here within the context of 2.x development, where we want to review the hub and spoke model.

There are many problems with this change.

First, it breaks the API and current expectations significantly. This means we will not do this in 1.x. We will fix the files/ problems that was reported in #1083366: Make the spokes authoritative for files/ and private/ directories, but this issue here is different: we are switching much more authority from the hub to the spoke, namely executable code.

Currently, executable lives on the hub, and it's the authority. It will overwrite the spoke's files. This is the case with the platform code, so it's only natural that the same exists with the site-specific code.

If we are to change this, we will need to change the way tasks are ran. Cronjobs, for example, will need to be ran on spokes (this is what #1080758: run cron on remote servers instead of locally is about). Other tasks, like verify, will also need to run on the spoke, which in the end will require drush and provision to be installed on the spoke, unless files are constantly synced back from the spoke to the hub every time a command is ran. The problem with syncing all the time is that it works only for Aegir commands: other drush commands ran on the hostmaster will fail to run in the proper environment.

I personnally believe that we should move more logic to the spoke. We shouldn't make a platform on the hostmaster and rsync, we should just create it on the spoke remote and run tasks there. This is a significant overhaul, and I do not think that attacking just this symptom is a good idea, as it will add only more confusion.

This, therefore, fits in our wider strategy, but will require more changes than just syncing files or not, here and there...