This breaks all clone and migrate task, but in the not obvious way: only internally invoked provision-verify tasks are silently skipped due to the fact we have them enclosed in the if (!drush_get_error()) {} test. This leads to not properly rewritten vhost files, but also, due to not updated properly drush aliases, the provision-deploy used to rewrite paths in the files table no longer works. It is also possible some other things are silently broken due to this behavior. This can be related to #1041386: Drush 4.0 & 4.1 rsync (via Aegir) syncs but reports error, but works in Drush 3.3.

Not sure yet if that is a Drush bug or just Provision incompatibility with some changes in Drush 4.x so I'm opening this in the Provision queue.

BTW: tested with the same results also with recent Drush 4.2. Not sure what is the real reason in this chain of issues (what is first), but Drush 3.3 doesn't have any of them (so no mess in vhosts aliases and paths are properly rewritten).

Comments

Anonymous’s picture

Thanks

I think I reverted HEAD to use 3.3. At this rate I dont expect we'll depend on Drush 4 for a while!

Sounds like something is just not right with how many of their functions return, now..

Anonymous’s picture

Priority: Critical » Major
omega8cc’s picture

I know we reverted to Drush 3.3 but what is the plan? Should we move this issue to the Drush queue or try to work on this here (trying to guess what and how they changed in 4.x)?

Anonymous’s picture

The plan is:

1) get out of bed
2) look at drush_get_error() and see if we're *meant* to do it a different way in drush 4
3) fix it if 2), else move it to Drush queue
4) remain on 3.3 until these problems are fixed

I'll attempt 1) very soon :)

omega8cc’s picture

LOL! re: 1)
OK :))

Anonymous’s picture

I accomplished 1)

I can see that drush_get_error() is still the same and even bits of Drush itself still use if (!drush_get_error()) so I'll be surprised if this is the issue here and doubly so if it's Aegir doing something wrong if Drush does it the same way as us too.

Will have to fire up some test machines and debug.

omega8cc’s picture

Title: Drush 4.x breaks Aegir in many ways due to changed result of the if (!drush_get_error()) {} test (probably) » Drush 4.x breaks Aegir in some (hidden) ways - paths in files not rewritten, aliases not moved cleanly between vhosts etc.

Yeah, that must be something different, as the provision-deploy doesn't work as expected on site clone/migrate/rename (the paths are not rewritten in the files table), while it is not inside the if (!drush_get_error()) test. Changing the issue title.

omega8cc’s picture

Just to confirm: this weird behavior is only with Drush 4.x, while Drush 3.3 works as expected, so we are downgrading right now all servers back to 3.3.

greg.1.anderson’s picture

drush_get_error merely reports whether or not anyone called drush_set_error. If there are some drush commands that are calling drush_set_error in instances where they should not be (that is, the condition being reported is not an error), please post an issue to the drush queue, and we can change those calls to be drush_log(..., 'warning') instead.

If you do that, it would be very helpful to be specific about which calls to drush_set_error are erroneous, and why.

omega8cc’s picture

Thanks Greg! I posted this in the Provision queue as I was not sure if this is something we need to change/update in Aegir to be compatible with Drush 4.x or this is somehow related to #1041386: Drush 4.0 & 4.1 rsync (via Aegir) syncs but reports error, but works in Drush 3.3.

Now it looks like we hit here more than one issue, as not only our previously working (with Drush 3.3) logic stopped to work for internally called verify task, but also provision-deploy, which is *not* enclosed in that error-test-if also fails to do something previously working.

I will still test Drush 4.x with Aegir to help with debugging it, but so far we have too many issues with Drush 4 in production and we have to revert to 3.3 (with some backports from 4) for now.

I will update this issue or maybe move it to the Drush queue if I will find anything new/helpful.

greg.1.anderson’s picture

Yes, of course you should continue to use drush-3.3 in production until Aegir is validated to work with drush-4. When testing against drush-4, run with --debug mode, and you will easily be able to find calls to drush_set_error that are being reported in your calls to drush_get_error. Other issues will have to be tackled on a case-by-case basis.

greg.1.anderson’s picture

Anonymous’s picture

Sorry omega8cc, I just did a fresh install of HEAD, and using Drush 4 straight from CVS, the files table is being correctly rewritten for me on Clone.

So I can't reproduce this so far :s

I haven't tested aliases yet. What exactly are you saying happens there: aliases from site A are appearing in site B's vhost after clone?

omega8cc’s picture

@mig5

Interesting, maybe the commit mentioned in #12 fixed the issue?
You should use Drush 4.1 or 4.2 to reproduce, btw.
I will test Drush HEAD asap.

Re: aliases - duplicate aliases were found in both original and cloned vhost files.
Also, if you enabled automatic www. aliases, they were displayed as added
in the frontend, but weren't added to vhost file on site import.

greg.1.anderson’s picture

I fixed some stuff in HEAD besides #12; if you can't reproduce now, I think that's a good sign. I didn't change anything related to drush_set_error, though.

Regarding duplicate aliases, if you use alias group files (groupname.aliases.drushrc.php), drush will now create two aliases: @alias, and @groupname.alias. Look for the marker property '#hidden' to ignore the duplicates.

Anonymous’s picture

Re: duplicate aliases, I believe omega8cc is referring to URL aliases (I.e ServerAlias) as opposed to Drush alias.

Omega8cc, the latter stuff you mention sounds like a duplicate of http://drupal.org/node/1004526#comment-4042364

I may even have fixed what you're describing in my branch per #1004526: Automatic aliases are not persisted across rename and clone . Handling the aliases of a site when cloning is totally ballsed up and has been since the Rename feature got ripped out and merged into Migrate.

I'm still yet to test the alias issue here.

omega8cc’s picture

Title: Drush 4.x breaks Aegir in some (hidden) ways - paths in files not rewritten, aliases not moved cleanly between vhosts etc. » Drush 4.x and 5.0-dev/HEAD breaks Aegir in some (hidden) ways - paths in files not rewritten on clone/migrate/rename

I tested it with Drush HEAD from CVS over 10 times in the last 2 hours and I can still reproduce it every time: paths in the files table are not rewritten on site clone or migrate/rename. After switching back to Drush 3.3 it just works. No idea what can be wrong. I'm using Aegir HEAD/master.

Domain aliases seems to be separate issue: for example the www. alias is not added to the vhost after clone/rename/import, only invoking another Verify task on the site adds all domain aliases properly, like it is on site install - it works the same with Drush 3.3 and 4 and 5, so it is no related to Drush - so I'm changing the issue subject.

Anonymous’s picture

I might screencast my steps so you can tell me what i am doing since it Just Works here! :)

omega8cc’s picture

How many times did you try to reproduce it?
I tried it enough already to be sure it doesn't work with Aegir master and Drush 4, 5 and HEAD.
I tested all of them and the path in the files table is never rewritten.
:(

greg.1.anderson’s picture

@omega8cc: Have you tried to use drush-HEAD to do some basic drush commands, without using Aegir? For example, does drush status work without an error? If you cd to your sites directory and run drush status, does it show that the database is connected?

drush-4 added additional environment checks at bootstrap and will fail-fast in some instances. If this is what is causing your problems, you might gain more information calling drush directly.

Anonymous’s picture

Wow, now I am reproducing the bug.

Unsure how I avoided it the first time!

Anonymous’s picture

And I can confirm that downgrading to Drush 3.3 fixes it.

Still investigating.

Greg, has there been some change in the way engines are included?

The part that is being skipped is in provision/platform/deploy.provision.inc, and the code is simply:

 140 function drush_provision_drupal_post_provision_deploy() {
 141   // call the drush updatedb command.
 142   provision_backend_invoke(d()->name, 'updatedb');
 143   // We should be able to fully load Drupal now.
 144   if (drush_bootstrap(DRUSH_BOOTSTRAP_DRUPAL_FULL)) {
 145     drush_include_engine('drupal', 'deploy');
 146     drush_set_option('packages', _scrub_object(provision_drupal_system_map()), 'site');
 147     _provision_drupal_rebuild_caches();
 148   }

The only part I see that is being skipped is the drush_include_engine('drupal', 'deploy')

The updatedb, and the cache rebuilding, are both called.

Anonymous’s picture

Ah.

I have found the issue: we are fetching the old_uri from the 'options' context which was renamed to 'cli' in Drush 4.

Fix coming up.

Anonymous’s picture

Status: Active » Fixed

I don't know if this is the best way to fix this by adding compatibility, but it works.

So omega8cc, if you could pull from master and confirm that'd be great.

I grepped around and the only other case I could find where we still explicitly called from the 'options' context was when fetching the http service type when migrating (upgrading) aegir itself. It would have defaulted always to 'apache' in Drush 4, possibly causing issues with upgrading Aegir under nginx. So I 'fixed' that too. I'd love if someone could test a hostmaster-migrate

greg.1.anderson’s picture

I didn't search, but I thought there was already an issue fixed in Aegir where the 'cli' and 'options' contexts are merged together to get the cli options for drush-3 or drush-4. The code in #24 looks fine too.

omega8cc’s picture

I just tested it again with Aegir HEAD and Drush-5.0/HEAD and now it works!

I didn't notice any other issues now, so it looks like Aegir HEAD
is again compatible with Drush HEAD.

Thanks!

Status: Fixed » Closed (fixed)

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

  • Commit 28b8103 on prod-koumbit, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by mig5:
    #1047922 introduce more compatibility with drush 4, to avoid breaking...
  • Commit e98d144 on prod-koumbit, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by anarcat:
    add @deprecated notices for the hack introduced for #1047922