Problem/Motivation

We need to decide on what preflight checks we want to run for automatic updates.

Let's categorize these into 2 flavors:
1) runtime
2) module enablement/configuration

Proposed resolution

Checks to include

Potential channels for warning site owners

  • Warning on admin pages if it hasn't been run recently
  • Error on admin pages if it hasn't been run and there's a PSA or available sec update, See above and #3053796: PSA exists and preflight is failing
  • Ditto if it failed (warning and error in the respective categories)
  • Status report entry
  • Incorporate into update status report
  • Email to site owners (which would eventually replace the "update available" nagging)
  • Other stuff?

Remaining tasks

  • Decisions, decisions, decisions
  • Implementation
  • UX review
  • Need a way to disable preflight so it doesn’t annoy on platforms that it wouldn’t work. i.e. pantheon, acquia, platform, etc.
    • Wouldn't these platforms just have auto-updates off and wire up their own update/deployment instead?

Steps to test:

To test readiness checks and see failures

  • Read only - change permissions on core folder to chmod 555 core or vendor: chmod 555 vendor/composer
  • Diskspace - change DiskSpace::MINIMUM_DISK_SPACE = 999999999999999999999999999
  • Then visit admin/config/automatic_updates and manually run readiness checks.
CommentFileSizeAuthor
#69 image (2).png10 KBheddn
#69 image (1).png26.35 KBheddn
#69 image.png36.87 KBheddn
#69 automatic_updates.gif3.58 MBheddn
#54 3043521-54.patch43.66 KBheddn
#53 3043521-53.patch43.64 KBheddn
#53 interdiff_51-53.txt16.02 KBheddn
#51 interdiff_50-51.txt2 KBheddn
#51 3043521-51.patch44.17 KBheddn
#50 3043521-50.patch43.01 KBheddn
#50 3043521-50.patch43.01 KBheddn
#48 interdiff_47-48.txt18.16 KBheddn
#48 3043521-48.patch41.51 KBheddn
#47 3043521-47.patch42.65 KBheddn
#47 interdiff_45-47.txt14.37 KBheddn
#45 interdiff_41-45.txt11.94 KBheddn
#45 3043521-45.patch36.44 KBheddn
#43 3043521-43.patch34.58 KBmbaynton
#43 interdiff-41_43.txt2.55 KBmbaynton
#41 interdiff_34-41.txt9.89 KBheddn
#41 3043521-41.patch33.53 KBheddn
#34 interdiff_32-33.txt16.81 KBheddn
#34 3043521-33.patch30.38 KBheddn
#32 interdiff_31-32.txt4.26 KBheddn
#32 3043521-32.patch25.74 KBheddn
#31 interdiff_30-31.txt4.55 KBheddn
#31 3043521-31.patch22.33 KBheddn
#30 3043521-30.patch21.4 KBheddn
#30 interdiff_28-30.txt33.06 KBheddn
#28 3043521-28.patch17.27 KBheddn
#27 3043521-27.patch0 bytesheddn
#25 interdiff_21-25.txt14.86 KBheddn
#25 3043521-25.patch17.43 KBheddn
#22 interdiff.txt7.54 KBeiriksm
#21 3043521.patch10.39 KBeiriksm
#16 3043521.patch7.29 KBheddn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn created an issue. See original summary.

mbaynton’s picture

Would it be worth arranging for modules to be able to run possibly lengthy/batchable operations and add to the report? Seems like a great idea at first, although I'm a bit curious if there's many valid use cases.

catch’s picture

Patches applied to core

Should probably also have a check that there's no pending updates to be run - i.e. ensure people run update.php before updating the code base again.

heddn’s picture

Issue summary: View changes

#3 make sense. Added to the list.

For a pre-flight check of can we run auto update... what would the batch system do? Your description doesn't make sense to me. Is it for something else?

mbaynton’s picture

My train of thought there was based on the assumption that some of the core checks would be batched, like inspecting each file for patches. The assumption might be wrong; really I was just wondering if we should provide ways for contrib to participate with the same set of tools core has.

What does this refer to?

2) module enablement/configuration
heddn’s picture

#5 The points when we can check are with hook_requirements, cron job, site status page and at run time.

2) is more of the hook_requirements phase.

From what I'm seeing, we want a method for contrib to alter/insert their own checks into this preflight system? Give some examples when this would be necessary? I assume it is needed, but want to see some examples.

catch’s picture

Is PHP version worth adding here as well? #3039287: Implement changes required to remove support for PHP 5.5 and 5.6 would allow us to detect if someone's running lower than the minimum supported PHP for the version they're currently on. Needs composer or similar to check requirements for the release to update to of course.

mbaynton’s picture

@catch The "pre-flight check" as it was conceived of at MWDS would be available to site owners to perform well ahead of the new release, which makes taking into account properties of the new release (like its PHP version requirements) a little tricky. The use case of folks in timezones 12 hours offset from US who want to go to bed was brought up, for example. Maybe just PHP version could be checked as a special case that wouldn't require evaluating the new release's composer.json?

The other thing with warning users that their environment is about to be insufficient though is that, unlike these auto update preflight checks that are thinking about who can write to what files etc, PHP version will apply to audiences not necessarily using the auto update pre-flight check too. Making it a dynamic element of PSAs or something seems better to me.

@heddn let's forget I said anything about allowing contrib to influence the preflight checks. I have been unable to think of very compelling use cases either; best I've got is perhaps one would want to write a module that performed additional checks particular to a given popular hosting platform.

The points when we can check are with hook_requirements, cron job, site status page and at run time

So with cron jobs being able to put stuff on the report, there won't be a "re-check now" button necessarily?

pwolanin’s picture

Issue summary: View changes
Warped’s picture

Would it make sense, as part of security also, to have a module that flags changed core files? Daily do a compare to the hashed value. You would know then of any added/changed/missing files. Could even have attributes that allowed certain files to be modified (i.e. .htacess, etc) that could be patched, rather than replaced? Does the directory structure as it stands allow some directories that could be prepared in advance, and then just swapped? (Move \core to \backup, move \newcore to \core)? That would allow things to be done in stages, minimizing update time. If that module also zipped the directory any time a change was detected, at update time, a simple zip and compare hashes would verify that no changes had been made in the mean time.

xjm’s picture

Re: #7, PHP version should be included, yes.

larowlan’s picture

sufficient memory limit to unzip a tarball - assuming that is tarball are involved

heddn’s picture

Issue summary: View changes
heddn’s picture

Issue summary: View changes
heddn’s picture

Issue summary: View changes
heddn’s picture

Status: Active » Needs review
FileSize
7.29 KB

I'm not suggesting we do all the preflight checks here in this issue. This is more of a decision / policy issue here. But this implements a read only check and shows some chosen code libraries and gives us an idea of how we could adopt the rest of the checks.

What I want feedback on is the file system library and drupal finder. And any other high-level architecture type things. I'm not looking for code nits or feedback so much on the actual code in question here. It is more of PoC at this time.

Schnitzel’s picture

During the DrupalCon Seattle Sprints I discussed Preflight checks for hosting infrastructures that would like to provide a link to some documentation which explains why autoupdate is not supported by the infrastructure.

For this we would like to add to the proposed solution:
- Check for environment Variable DRUPAL_AUTOUPDATE_NOT_SUPPORTED_HELP_URL and if it is existing show it with a message and the link

Schnitzel’s picture

Issue summary: View changes
xjm’s picture

Title: Preflight checks for autoupdate » Update readiness checks for autoupdate (pre-flight check)

Since the "pre-flight check" metaphor has confused a lot of people, retitling to describe what the feature is actually checking: Your site's update readiness.

From our discussion at DrupalCon Seattle: In addition to checks about the site's state, there's a second type of check we need to do, and that's whether the updates themselves are compatible with auto-update. In our initial MVP, these might exclude updates that:

  • Require a node access rebuild.
  • Require a database update. (Particularly an actual DB update, not just a required cache clear. Related followup: Discuss other ways to ensure a cache clear.)
  • Update a dependency (although the discussion includes plans to try to eventually handle this for simple cases)
  • Add a new dependency.
  • Update the bootloader/autoupdater itself.
  • Update .htaccess, robots.txt, settings.php, etc.
  • New minors

This might be a boolean metadata item on the release node that's in the d.o feed of release information, rather than something we try to calculate in core, since there could be a lot of reasons a release is not auto-updateable. I think this is actually a separate issue from the update readiness check, because it would be checked as part of the update process rather than prior to the release. However, we will probably want to use similar reporting channels and UX to the update readiness check and might want to take both into account for the architecture, which is why I'm summarizing here at @heddn's suggestion.

Edit: If we have a security update that is not auto-updatable, even if it's not highly critical, that might also be something we'll want to issue a PSA about. Most of the time the sec team will know in advance if such an update is going to happen. In the rarer situation where we don't, we could issue the PSA at the same time as the advisory.

xjm’s picture

Issue summary: View changes
eiriksm’s picture

Just wanted to throw this in there as well. What if we made the preflight check a plugin? Could make it easier to get to core, we could do the plugin itself in contrib?

eiriksm’s picture

FileSize
7.54 KB

forgot the interdiff. I basically just renamed the service to a plugin, and moved the interface. And added the plugin manager.

heddn’s picture

I like the idea of plugins. More flexibility.

heddn’s picture

From a next steps point of view, we can always easily add to the list of preflight checks. And with all the discussions on this at drupalcon seattle, I think we got pretty decent attention on this issue. I think we're ready to start doing things now.

Let's start doing stuff. I'd like to spin up the rest of the checks into their own issue. And keep the existing file system check. That way we keep things easily reviewable and even open this up for a larger community to contribute. Each of the checks is an easy preflight check. Pratically speaking, let's add a drupal_set_message and entry on the site status page. And flesh out a full file system check. And finish this.

heddn’s picture

So, here's the rest of the things I consider as part of the MVP for a preflight. We'll need to add more features and more checks. But is the first pass for an initial implementation.

heddn’s picture

Hmm, we probably don't want to actually run the readiness checks on every page load. I'm going to move that to cron. And add a button to manually run the process. And cache the results. For now, I'm _not_ going to run it via batch API. If needed, we can add that later.

heddn’s picture

I'll post an interdiff later. Here's the latest.

heddn’s picture

catch’s picture

+++ b/automatic_updates.install
@@ -12,6 +12,29 @@ use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
+  $plugin_manager = \Drupal::service('plugin.manager.preflight');

Is it worth using 'update readiness'/'update readiness check' for the plugin code as well instead of 'preflight'? Don't have a very strong opinion but it'd make the code match the ui (and some code comments) more. I've never seen preflight used outside an air travel situation, the analogy is fine but not sure it's self-explanatory.

Also I'm not sure about using plugins here. We're never expecting people to configure which plugins run, it's more a way of allowing each check to be provided independently - for that tagged services might be a better option (see https://www.drupal.org/docs/8/api/plugin-api/why-plugins).

heddn’s picture

Switches to using tagged services. I'm not sure how helpful the interdiff will be, as a lot of things moved around.

heddn’s picture

Moved around some magic numbers and add a link from the state report page.

heddn’s picture

FileSize
25.74 KB
4.26 KB

Here we add a disk space checker.

eiriksm’s picture

Just a couple of remarks from looking at it:

  1. +++ b/automatic_updates.install
    @@ -5,13 +5,47 @@
    +  $checker_results = \Drupal::service('automatic_updates.readiness_checker')->results();
    

    Should we add a flag for enabling/disabling readiness checks as well? Maybe someone wants to enable PSA but not enable readiness checks? In that case I think it makes sense to move this code out of the actual hook and do the same with the logic for PSAs

  2. +++ b/automatic_updates.module
    @@ -34,5 +36,23 @@ function automatic_updates_page_top(array &$page_top) {
    +  \Drupal::service('automatic_updates.readiness_checker')->run();
    

    Can we assign this to a variable and typehint this for easier navigation? Just a preference.

  3. +++ b/src/Controller/ReadinessCheckerController.php
    @@ -0,0 +1,54 @@
    +  use StringTranslationTrait;
    

    ControllerBase already uses StringTranslationTrait. Also, my personal preference is to pass it to the constructor and assign it to $this->stringTranslation

  4. +++ b/src/Form/AdminForm.php
    @@ -4,12 +4,40 @@ namespace Drupal\automatic_updates\Form;
    +    $instance->checker = $container->get('automatic_updates.readiness_checker');
    

    Interesting pattern. I have not seen it like this before. Are there any advantages to not passing these to the constructor? I feel like the convention is to pass it to the constructor, but I am curious and open to be wrong :)

Also, we probably need a test for the disk space checker :)

heddn’s picture

FileSize
30.38 KB
16.81 KB

Here we add more tests and response to UX feedback from the UX meeting yesterday.

heddn credited AaronMcHale.

heddn credited benjifisher.

heddn credited ckrina.

heddn’s picture

Adding issue credit for ux reviews.

heddn’s picture

FileSize
33.53 KB
9.89 KB

Here we respond to #34.

re 34.4: this is something I read about from alexpott and seems to be something that will probably start to happen more to insulate against upstream change to constructors. But it was implemented wrong, so I've fixed it here.

mbaynton’s picture

I can't be the only one that gets "You have requested a non-existent service "automatic_updates.readiness_checker"." if I try to enable patch 41 on a fresh site, or uninstall early versions of the module and then re-enable after applying 41?

mbaynton’s picture

What about something like this to prevent double warnings when you have drupal root and vendor on the same filesystem?

I'm not having success getting the module as a whole to work still.

heddn’s picture

Issue summary: View changes

re #42: I tested this morning with the module uninstalled, applied the patch then installed the module. No errors/issues.

Also updated IS with steps to test.

heddn’s picture

I like the idea for #43. I've added some test coverage too. Interdiff is against 41.

heddn’s picture

Discussed the current state of things in here with @catch. Since some of this work is core-bound, we have to be careful what composer dependencies we use. With that in mind:

  1. We want to remove PHP league's file API usage and only use core's file API
  2. And have 2 levels of readiness checks. Warnings (example: you have some changed files) and Errors (example: you do not have enough disk space)

Over in #3043235: [Policy, No patch] How to provide automated, in place security updates of Drupal core, we've basically decided to use a "patch" approach for updating.
The main reason for using the external file api (#1 above) was for caching. Since we've basically decided not to hash all the files at run time application of updates (only the files that have changed since the last update will get hashed), the caching argument looses merit.

The reason for 2 levels of readiness checks is that we need to give levels of assurance for automatic updates. We can't say 100%, a priori, that an update will or will not apply if there are some patches against core. But we can warn that the filesystem is patched and therefore, depending on the contents of the update, it might or might not get update. If there is no patching, then we can say 100% the site is update ready. But we also have some things, like disk space or old version of core or any number of things that will 100% make the site not eligible for an upgrade. These are errors. So two levels of readiness.

heddn’s picture

FileSize
14.37 KB
42.65 KB

This does 46.2. Next up is 46.1.

heddn’s picture

FileSize
41.51 KB
18.16 KB

This addresses 46.1. I also checked about webflo/drupal-finder dependency. Per catch, that's probably ok to keep. If we need to for core we could either use that project or move something akin to its logic into core.

heddn’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

This needs a re-roll now that #3046855: Notify site administrators when new PSA is posted has landed.

heddn’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
43.01 KB
43.01 KB

And a re-roll.

heddn’s picture

FileSize
44.17 KB
2 KB

We'll also need to address the issues uncovered with #3050876: PSA UX follow-up: blocking update.php. But this is pretty close now I think. Everything but that is ready for review.

mbaynton’s picture

Well, my mistake was trying to run it on the current stable core, not 8.7.x-dev. Now that I can actually do stuff:

  1. I'm glad to see league/flysystem gone; I was going to ask about the dependency implications tonight.
  2. But about that ReadOnlyFilesystem check, why did you choose to test by performing renaming? Granted the files you selected are innocuous wrt. the site's operation, but I'm thinking it would be better to write out our own file for this purpose instead, try of course to remove it again, but also not perpetually fail all future readiness checks if this reverse operation doesn't happen for some reason. I could probably offer a patch for that if there's not some reason that renames are better that I'm missing.

    Otherwise I do like the principle of changing the contents of the the directory as the test of writability. FWIW the current implementation in the core_update module renames the old file to somewhere else to facilitate rollbacks and then writes out a brand new file so if that or a model like it is used, this is the most accurate test.

  3. Someone should probably at least observe that the ReadOnlyFilesystem check is not comprehensive in that it only tests a very small sampling of the locations where we might need to write. IMHO the actual updater (once we get there) should do what core_update does and initiate rollback if it encounters something it needs to write but cannot, at which point I don't feel as strongly about the readiness check.
  4. Regarding the two levels of checks introduced in #47, I suggest demoting disk space to warning. Primarily because there's been discussion of running the readiness check as the first step of applying the update, and automatically electing not to proceed if the readiness check identifies things that are guaranteed to make the update fail. Disk space is not one of these because the value we require is high and because there's lots of craziness in the wild with filesystem provisioning that might cause the reported value to be higher or lower than what will actually work before you get an ENOSPC. We should warn the user if they should think about clearing up some space, but the actual space these updates are going to require are going to be increments of kilobytes, and we'll end up with a way high false positive rate here and sites not getting updated for no real reason.
  5. Nitpick on the re-implementation of different filesystem detection, mine tested vendor in the event of either of those stat() calls failing; just erring on the side of checking both places in case we figure out for sure the filesystems. New implementation will end up testing NULL == NULL in that case and err on treating them as the same device.
  6. I don't do tons of D8 development myself, but that flashy marketing video the DA ran ahead of the Driesnote wanted me to know that D8 is API-first :). Is there merit to making the readiness check route itself JSON, which the UI ajaxes out to? Besides being maybe more in line with Drupal's current direction, it would also give us a little more headroom to run semi-long stuff in there. Test more places for writability for example.
heddn’s picture

FileSize
16.02 KB
43.64 KB

Just the type of feedback I was hoping for. Very helpful. Thank you for putting time into reviewing the work here.

I think this addresses all the feedback in #52.

  1. :)
  2. Changed to copy/delete.
  3. This only checks read-only permission the disk. Each service should do one thing and do it well. With that in mind, testing one file is sufficient. The audience here is for the sites that don't grant write access to the web user. We can add more checks later as they make sense. In face, there's a stub of one in this patch for checking modified code. I do check vendor separately as in theory (although I've not seen it in practice) one could have that mounted on a different filesystem. And it was was easy to do. I'm sure over time we'll learn more about how to test for read-only disks. Hopefully this is enough to get started.
  4. I've dropped the disk space requirements to 10mb. It is an arbitrary check, but hopefully all patched updates use less than 10mb of space. And not having enough space is a hard blocker for updating and an easy fix (clean-up space). I read up on disk_free_space and didn't see any complaints about its accuracy. I'm in favour of landing this check and changing it if we discover it is troublesome.
  5. fixed
  6. I'm happy to accept follow-up patches. But I see that as a nice-to-have.

I envision we'll do this:

FWIW the current implementation in the core_update module renames the old file to somewhere else to facilitate rollbacks and then writes out a brand new file so if that or a model like it is used, this is the most accurate test.

But for checking read only, copy/delete is sufficient.

heddn’s picture

FileSize
43.66 KB
catch’s picture

The audience here is for the sites that don't grant write access to the web user.

Cron isn't necessarily run with the web-user, so should we be somehow doing these checks on cron only, then showing the results on the site based on the behaviour from cron? Otherwise feels like we could get false negatives here.

mbaynton’s picture

Re: #55, point well made. I didn't catch this because I've been imagining the updates being applied at the end of a page request. This is likely on account of being the guy that wrote the module that applies attended core updates, and that model is necessarily as part of http requests.

If we are going to do it in cron, which kind of makes sense because a) we're already dependent on cron to discover the new version and b) for the few who do wire up cron to run as a different user, we don't want to tell them "you still have to make your files webserver writable", then per @mlhess's suggestion at the Seattle BoF, we should also add a readiness warning that you are succeeding in running cron once an hour or better. I see there's a long list of desired checks in the IS, but maybe this one is easy enough to add now?

mbaynton’s picture

Never mind on the hourly cron check for now; this is premature because checking for new versions currently happens at most once a day. That really needs to be addressed, but probably not here.

heddn’s picture

We can't tell who or how cron is run. It can be run via drush (any system user) or via poorman's cron (web system user). Perhaps we need to add some more documentation to the handbook page that discusses this? But as far as readiness checks, I struggle to think we can really do much about false failures/positives. Unless you we have the checkers run via a drush command outside of drupal cron. Hmm, it wouldn't work in core but it would in contrib.

heddn’s picture

#56 errors on frequency of cron:

+++ b/automatic_updates.install
@@ -16,8 +18,72 @@ function automatic_updates_requirements($phase) {
+  if (\Drupal::time()->getRequestTime() > $last_check_timestamp + ReadinessCheckerManagerInterface::LAST_CHECKED_WARNING) {
+    $requirements['automatic_updates_readiness']['severity'] = REQUIREMENT_ERROR;
+    $requirements['automatic_updates_readiness']['value'] = t('Your site has not recently checked if it is ready to apply <a href="@readiness_checks">automatic updates</a>.', ['@readiness_checks' => 'https://www.drupal.org/docs/8/update/automatic-updates#readiness-checks']);

We already add an error when cron or something isn't running the checks often enough.

mbaynton’s picture

#59, that's fair, but that's not what the message value says :P

If I saw that myself I'd most probably resolve it by manually running the readiness checks, which wouldn't say anything about my site actually getting updated in a timely manner when it got to be time to. I still think there's a point in explicitly and separately checking that *cron* is running in a timely fashion & I'll submit a patch for it, unless I'm missing something.

#58, my read of @catch's suggestion is essentially execute the ReadOnlyFilesystem check inside drupal cron and have the online readiness checker just report on its results. That seems doable in core or contrib? Or is your concern that since the check itself wouldn't occur if you manually re-ran the readiness check, users tinkering with permissions could be frustrated/confused by the delay in clearing the readiness error?

mbaynton’s picture

Actually, perhaps the cron running frequently and writable files check should be combined or work in parallel - drupal cron might get run as the webserver user and as another system user, we want to report filesystem writability problems only if none of the cron runs in the last hour were running in such a way that writing was possible.

heddn’s picture

Can we address the question about cron in a follow-up? It is a tricky issue. Some folks run it via drush, some via an external cron calling the cron url, others use ultimate cron, others don't run it at all.

I've had it where unix cron is /usually/ where cron is run, but to force something it gets run manually via drush. And they are running it as a non-web user. And someone else on the team might login as themselves and run it too. Then unix cron kicks in and runs it as the web user. Lots of variables at play. Let's address in a follow-up and land what we have.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

So we can keep things moving in follow-ups, I'm going to land this. We can 1) roll it back or 2) fix it in a follow-up if there are thing we need to change. Because the size of this patch is making it hard to start working on the smaller bits that depend on the plumbing provided by this patch.

  • heddn committed 74c95e0 on 8.x-1.x
    Issue #3043521 by heddn, mbaynton, eiriksm, AaronMcHale, worldlinemine,...
heddn’s picture

Status: Reviewed & tested by the community » Fixed
heddn’s picture

Title: Update readiness checks for autoupdate (pre-flight check) » [META] Update readiness checks for autoupdate (pre-flight check)
Category: Task » Plan
Issue summary: View changes
Status: Fixed » Active

I've updated this issue with the current linked child pages. And also re-opened it as a META so we can track progress.

There are two items in the IS that don't have follow-ups opened.

  1. Files writeable (maybe not owned) by web user.
    • Is this really feasible in a pre-flight check? We can check a handful of unimportant files, and we are already doing this but the only sure way to check is to touch every. single. file. Not likely going to happen in a preflight check. It would be too invasive and could break the site.
  2. Check for environment Variable DRUPAL_AUTOUPDATE_NOT_SUPPORTED_HELP_URL and if it is existing show it with a message and the link.
    • I'm not sure I understand the benefit from this vs a config alter to disable/block the readiness check service.
heddn’s picture

Issue summary: View changes
heddn’s picture

Issue summary: View changes
FileSize
3.58 MB
36.87 KB
26.35 KB
10 KB

I need reviews. Lots of them. That is what is currently blocking commits on several checkers. Starting next week on Tuesday, I'm going to take the least controversial of the checkers and committing them after doing my own self-reviews. So please help out and give reviews so I don't have to resort to that.

The lone exception to Tues is the modified files checker. I'd really, really like a review on that one. But even it, if I don't get a review by EOW I'll be looking to land too.

I'm also attaching a visual of what these checkers look like below. So you can get a feel for what things look like.

All of the patches and existing checkers applied (except modified files checker):

Modified files checker:

Status report page (with a mix of errors and warnings):

Status report page (with only warnings):

ressa’s picture

Great work so far @heddn.

I was able to trigger the disk space warning, by upping MINIMUM_DISK_SPACE in src/ReadinessChecker/DiskSpace.php:
Logical disk "/app/web" has insufficient space. There must be at least 1.0E+47 megabytes free.

I get the message Drupal PSA endpoint http://localhost/automatic_updates/test-json is unreachable. on the admin/reports/status page.
I also see this in automatic_updates.settings.yml, so perhaps the PSA function is simply not ready yet?

# TODO:  Update to correct end point once it is available. See
# https://www.drupal.org/project/automatic_updates/issues/3045273
psa_endpoint: 'http://localhost/automatic_updates/test-json'

I am using Lando, and it seems like it auto-corrects permissions after I chmod 555 the core folder, so I can't test the writability check.

heddn’s picture

Status: Active » Fixed

Thanks for everyone's help here. All the base checkers are in now for D8. And we have #3063125: [META] backport readiness checkers to Drupal 7 opened for backporting them to D7.

Status: Fixed » Closed (fixed)

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

tedbow’s picture

We are now porting this portion to core #3162655: Create Automatic Updates Readiness Checks in new experimental module if anyone wants to help.