Problem/Motivation
We should do as much as we can from detecting errors that will not allow a update to succeed and stopping the update cleanly.
Some of the checks we can run before an update starts and others we can only check after an update has been staged.
Proposed resolution
Create a list and child issues for validation checkers we will need. Some these checkers will prevent and update and others will prevent/abort
an update.
- #3241105: Ensure core package is validated in readiness check and pre-start
- Done:
\Drupal\automatic_updates\Validation\StagedProjectsValidation validates only expect project updated - #3228955: [PP-1] Handle new or deleted files in `sites/default`
- #3231996: Ensure file system is writeable
- #3231997: Ensure there is enough disk space to do the update
- file ownership
- #3232000: Ensure the composer.lock file matches existing Drupal projects
- #3231999: Ensure Composer is version 2.x
- #3230235: Validate that an update is supported before it begins
For each issue
- Create a service tagged with
event_subscriberthat will listen to 1 or more of the update events in `\Drupal\automatic_updates\AutomaticUpdatesEvents`There are few validation services that be used as an example such as
ComposerExecutableValidator,StagedProjectsValidation, andUpdateVersionSubscriber.for a general intro to event subscribers see https://www.drupal.org/docs/creating-custom-modules/subscribe-to-and-dis...
- Each service should add validation error if they found a validation problem. An example of this can be found in
UpdateVersionSubscriber:
$error = ValidationResult::createError([ $this->t('Updating from one major version to another is not supported.'), ]); $event->addValidationResult($error); - Create a test for the new service. A kernel or unit test should be sufficient. We already have general test that validation subscribers can stop an update with an error so we don't need a test for each new service.
Example tests are
ComposerExecutableValidatorTest,StagedProjectsValidationTest - If the validation is similar to validation that was used as ReadinesChecker in the 8.x-1.x version of this module you may consult the existing checkers in the 8.x-1.x branch for internal logic to be adapted
Some validation will be new to the 8.x-2.x version so there will always be an existing ReadinessChecker.
- Depending on the new service it may break other existing tests because it may stop an update unless the test site is prepared to pass the new validation. That is expected and is not a problem. If you don't know how to fix the test just mention that in a comment on the issue and someone else can help.
Remaining tasks
make the child issues
Comments
Comment #2
tedbowComment #3
tedbowComment #4
tedbowComment #5
tedbowWe should try to get as many done as possible before we demo this.
Comment #6
tedbowComment #7
tedbowComment #8
tedbowComment #9
tedbowComment #10
tedbowComment #11
tedbowComment #12
tedbowComment #13
tedbowComment #14
tedbowComment #15
tedbowComment #16
jurgenhaasIn light of the issue with the Drupal Core 9.3.1 release earlier this week, where the `core.services.yml` file was corrupt, we got hit hard by that because we're running automatic updates every night and that killed all the Drupal sites which got updated before the release of 9.3.2 a couple of hours later. When I mention automatic updates here, it's not about the initiative of this channel but some comprehensive GitLab pipelines combines with Ansible, that do a great job each and every night.
Why I'm posting this here? Our test suite currently doesn't cover yml file validation yet. We want to prevent an issue like the one above in the future and I wonder if the automatic update initiative has thought about that too. What I'm thinking about is a simple tool that validates all the yml files in the code tree right before deployment.
If that's something this initiative is about to cover as well, I'd be interested to share ideas and help to bring that to light.
Comment #17
tedbow@jurgenhaas closing because are must have validators are done.
If you want to create an issue for that validator that would be good. I don't think it is core MVP which is all we are trying to for 8.x-2.x. But it would be good to have an issue to come back to
Comment #18
jurgenhaasThanks @tedbow it's ok to have this closed. Having thought about this meanwhile, it feels better suited in the test pipelines for Drupal projects to make sure, all packaged files are syntactically correct. Doing that downstream when deploying, this is simply too late, I guess. In the GitLab migration initiative, the process just got started to build pipeline templates, and I'll raise the idea over there instead.