db-update.sh factory hook doesn't output an error code when drupal:update has failed

[joachim-n]

The db-update.sh factory hook is currently failing on my ACSF instance.

The log output is:

[staging-2364|26825] - The db-update.sh script has completed: Completed in 1 seconds. Exit code: 0. Standard out: Running BLT deploy tasks on championstennis.dev-endeavorsf1.acsitefactory.com domain in 01dev environment on the endeavorsf1 subscription. > drupal:config:import > drupal:toggle:modules 0.898s total time elapsed.; Standard error: [info] Skipped pre-config-import target hook. No hook is defined. [warning] BLT will NOT import configuration, /mnt/www/html/endeavorsf101dev/docroot/../config/default/core.extension.yml was not found. [info] modules.01dev.enable is not set. [info] modules.01dev.uninstall is not set.

This message clearly shows there is a problem. However, the log level in the ACSF instance is 'INFO'. That's obviously wrong -- something has failed.

I filed a ticket with Acquia support (https://insight.acquia.com/support/tickets/760236), and the response was that this was correct behaviour for the ACSF platform:

The work process runs an external script (db-update.sh), which emits a "warning" message while it runs. However, as this script runs and exits without returning an error message, the Site Factory task log displays the script's output as an "info" level message.

So therefore the problem is that db-update.sh should be exiting with an error code when its command fails.

[mikemadison13]

@joachim-n can you share the version of BLT you are using please?

[joachim-n]

I'm using 9.1.8. I'm not yet able to update to 9.2.x, as the README says that works with Drupal 8.6 and my project is still on 8.5.

[lcatlett]

@joachim-n - try updating to BLT 9.1.9, as @grasmash backported fixes from BLT 9.2.x that other users have confirmed to address this issue for ACSF projects still on 8.5

[joachim-n]

10.0.x:

DRUSH_PATHS_CACHE_DIRECTORY=$cacheDir $blt drupal:update --environment=$env --site=${name[0]} --define drush.uri=$drush_uri --verbose --yes --no-interaction

9.1.8:

$blt drupal:update --environment=$env --site=${name[0]} --define drush.uri=$domain --verbose --yes

The final line of the script doesn't appear to have changed substantially in terms of what the script might return as an exit code, so to my eye it doesn't look like this has been fixed.

[mikemadison13]

@lcatlett you and i discussed upping log levels on ACSF itself to help with issues like this. do you think that could be a solution here?

unclear to me if this is truly BLT's issue to solve.

[alexxed]

On version 9.x, the message "BLT will NOT import configuration" is stopping db update from being executed: https://github.com/acquia/blt/blob/9.x/src/Robo/Commands/Setup/ConfigCommand.php#L47-L63 but the comment says it's not fatal. This was introduced 2 years ago #1696

This has changed only on the 10.0.x branch: https://github.com/acquia/blt/blob/10.0.x/src/Robo/Commands/Setup/ConfigCommand.php#L21-L38 and the change looks like it would run updates in your case.

So "/mnt/www/html/endeavorsf101dev/docroot/../config/default/core.extension.yml was not found." needs to be fixed to get by this in 9.x if an upgrade to 10.0.x is not possible.

[alexxed]

@mikemadison13 should we backport 83a9fce in 9.x as well? I see you have a PR backporting it to 9.2.x.

[mikemadison13]

@alexxed yes, there are a couple of things that need to be back ported into 9.x and this is one. thank you!

[alexxed]

Ok then, the backport would fix it too.

[lcatlett]

This was not necessarily a specific bug to fix but more about the user experience and better integration with other Acquia products. Because there was also a specific internal Acquia PF ticket advocating for this in addition to this one in BLT, I put in #3412 which will resolve the larger issue and the suggestions for how to more logically set individual status codes of BLT tasks are not necessarily required. I'll put in a PR to 9.x and 10.x if others agree.