department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
97 stars 69 forks source link

Investigate adequacy of `drush deploy` for deployment tasks. #10525

Open ndouglas opened 2 years ago

ndouglas commented 2 years ago

Description

In the course of my work on this PR, I encountered an issue that I believe indicates a problem with drush deploy. This problem can lead to PRs being merged that are not internally consistent and might cause problems with subsequent deploys to production.

drush deploy performs the following steps:

And note:

Screen Shot 2022-08-30 at 12 08 54 PM

One problem comes with modules that 1) are installed by config and 2) include post_update hooks.

In practice, post_update hooks will get triggered, then the module will be installed, but its post_update hook will not be executed. It will be executed on the next run. However, the module itself may be coded based on the assumption that this post_update hook has been executed.

An example of this is the above PR.

When the Tugboat preview is built, we see the following:

2022-08-30T12:27:43.743Z - 62b4bc8824e7d20539713818# /bin/sh -c drush deploy
 [notice] Database updates start.
>  [success] No pending updates.
 [success] Cache rebuild start.
>  [success] Cache rebuild complete.
 [success] Config import start.
+------------+------------------------------------------------------+-----------+
| Collection | Config                                               | Operation |
+------------+------------------------------------------------------+-----------+
|            | linky.settings                                       | Create    |
|            | views.view.managed_links                             | Create    |
|            | entity_track.settings                                | Create    |
|            | entity_usage.settings                                | Create    |
|            | linkychecker.settings                                | Create    |
|            | linkyreplacer.settings                               | Create    |
|            | core.extension                                       | Update    |
|            | filter.format.rich_text                              | Update    |
|            | filter.format.rich_text_limited                      | Update    |
|            | core.entity_form_display.paragraph.q_a_group.default | Update    |
|            | editor.editor.rich_text                              | Update    |
|            | simplesamlphp_auth.settings                          | Update    |
+------------+------------------------------------------------------+-----------+

 Import the listed configuration changes? (yes/no) [yes]:
 > >  [notice] Synchronized extensions: install entity_route_context.
>  [notice] Synchronized extensions: install entity_track.
>  [notice] Synchronized extensions: install entity_usage.
>  [notice] Synchronized extensions: install linky.
>  [notice] Synchronized extensions: install linkychecker.
>  [notice] Synchronized extensions: install linkyreplacer.
>  [notice] Synchronized configuration: create views.view.managed_links.
>  [notice] Synchronized configuration: update filter.format.rich_text_limited.
>  [notice] Synchronized configuration: update filter.format.rich_text.
>  [notice] Synchronized configuration: update editor.editor.rich_text.
>  [notice] Synchronized configuration: update core.entity_form_display.paragraph.q_a_group.default.
>  [notice] Synchronized configuration: update simplesamlphp_auth.settings.
>  [notice] Finalizing configuration synchronization.
>  [success] The configuration was imported successfully.
 [success] Cache rebuild start.
>  [success] Cache rebuild complete.
 [success] Deploy hook start.
>  [success] No pending deploy hooks.

The configuration is imported, but the post_update hooks have not been run.

Why is this a problem?

If you manually run drush deploy, or drush updb, you might encounter an error:

$ tugboat shell 62b4bc8724e7d254987137da
root@php:/var/lib/tugboat# drush deploy
 [notice] Database updates start.
 -------------- --------------- ------------- -------------------------- 
  Module         Update ID       Type          Description               
 -------------- --------------- ------------- -------------------------- 
  entity_usage   regenerate_2x   post-update   Re-generate entity_usage  
                                               statistics.               
 -------------- --------------- ------------- -------------------------- 

 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > > >  [warning] Post update function entity_usage_post_update_regenerate_2x not found in file entity_usage.post_update.php
> >  [error]  Update failed: entity_usage_post_update_regenerate_2x 
>  [error]  Update aborted by: entity_usage_post_update_regenerate_2x 
>  [error]  Finished performing updates. 

In SiteProcess.php line 214:

  The command "/var/lib/tugboat/bin/drush updatedb --no-cache-clear --uri=default --root=/var/lib/tugboat/docroot" f  
  ailed.                                                                                                              

  Exit Code: 1(General error)                                                                                         

The root cause of this particular problem is not particularly important. What's important is that there is a problem. drush deploy will be run on production on the next deploy, and it can trigger code that was not invoked as part of the PR's deploy. The code's not tested by the deployment mechanism, not generally visible in the PR interface (since it doesn't exist within files kept in version control), etc.

This can probably be fixed by something as simple as running drush updb a second time, after drush deploy, but this issue may have further implications given that drush deploy is supposed to standardize the deployment process.

Acceptance Criteria

ndouglas commented 2 years ago

This also causes PR errors like this, since the database is not up-to-date:

+--------------------------------+----------+----------------------------------+

| Title                          | Severity | Summary                          |

+--------------------------------+----------+----------------------------------+

| Database updates               | Error    | Out of date                      |

|                                |          |                                  |

| Module and theme update status | Error    | Unsupported release [1]          |

|                                |          |                                  |

|                                |          | [1]                              |

|                                |          | http://default/admin/reports/upd |

|                                |          | ates                             |

|                                |          |                                  |

+--------------------------------+----------+----------------------------------+

I've only seen this message since the Drupal 9.4 update, though, so I don't know whether dirty updates have been making their way to prod (and fixed transparently) or if something else changed.