craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

[4.x]: Clearing caches in CP causes project config to show differences in CP preview #11971

Closed turnstylerj closed 2 years ago

turnstylerj commented 2 years ago

What happened?

Description

I’ve been having a strange issue with project config related to caches and am trying to figure out whether it’s an issue with the host we use (Fortrabbit) or a Craft issue, and was hoping describing what’s going on to the team more familiar with how project config is working under the hood could at least help point me in the right direction.

With Fortrabbit, we use their plugin craft-copy to deploy code, which is essentially a git push that runs project-config/apply afterwards to apply any checked in changes to the deployed database. The issue we’re noticing is that, specifically after clearing caches in our deployed environment under Utilities > Caches with “All” checked, we’re getting the CP banner saying we have pending project config changes.

On clicking the banner, or going to Utilities > Project Config, the CP is displaying the diff comparing the current DB to older modifications of the project config files. The database is correct, and if I ssh into the server and pull down the project config files, they are also correct. It’s just as if clearing the caches somehow causes the CP diff to compare to an older set of project config files. Rebuilding project config with php craft project-config/rebuild on the server clears it up temporarily, but the actual project config files on the server don’t reflect the difference shown in the CP diff before or after rebuilding.

Any thoughts/ideas on where I might look to try narrowing this down? Fortrabbit seems to be a bit unsure as well.

Steps to reproduce

  1. Deploy to a Craft site using an automatic project-config/apply deploy hook.
  2. Clear all caches in the CP via Utilities panel

Expected behavior

Clearing caches does not affect project config.

Actual behavior

Clearing caches causes the CP to compare the current database to previous project config modifications.

Craft CMS version

4.2.4

PHP version

8.1.3

Operating system and version

Linux 4.15.0-163-generic

Database type and version

MySQL 8.0.23

Image driver and version

Imagick 3.7.0 (ImageMagick 7.1.0-26)

Installed plugins and versions

"craftcms/aws-s3": "2.0.1", "craftcms/feed-me": "5.0.4", "craftcms/redactor": "3.0.2", "doublesecretagency/craft-cpcss": "2.5.0", "fortrabbit/craft-copy": "^2.1", "internetztube/craft-element-relations": "2.0.1", "mmikkel/retcon": "2.5.0", "nystudio107/craft-imageoptimize": "4.0.2", "nystudio107/craft-minify": "4.0.0-beta.2", "nystudio107/craft-retour": "4.1.3", "nystudio107/craft-seomatic": "4.0.7", "ostark/craft-async-queue": "^3.0", "percipioglobal/craft-colour-swatches": "4.2.0.1", "spicyweb/craft-neo": "3.3.9", "verbb/cloner": "2.0.1", "verbb/field-manager": "3.0.2", "vlucas/phpdotenv": "^5.4.0", "weareferal/matrix-field-preview": "4.0.4"

brandonkelly commented 2 years ago

We recently became aware that Fortrabbit isn’t ever deleting old files during deployments – only updtating/adding files. I can’t help but wonder if that’s the culprit here. (cc @ostark)

turnstylerj commented 2 years ago

That is definitely also happening but doesn't seem to be a huge problem since the older non-deleted files are no longer referenced in the main config file anyway. The part that's perplexing to me is the diff showing actual modifications like it's comparing to an older version of an existing file, only after clearing caches in the CP.

And it's only the diff in the CP, if I pull down the actual project config files and check them in, there are no modifications to existing files reflected in git, only the addition of the files that weren't deleted on the server.

ostark commented 2 years ago

@brandonkelly Yes, but it's not full true. There are different deployment strategies depending on the stack:

Universal Apps: overwrite but not delete (to keep user uploads)

Pro App: fully atomic (switch code+vendor in one go)

ostark commented 2 years ago

Applies to both stacks: During deployment files removed from git do not exist, which means: applying project config post composer install should work without any issues. In production we also recommend to disable allowAdminChanges.

Automatically removing config/project/* after running apply should mitigate the issue, but I'm not sure what happens when deleting the cache.

turnstylerj commented 2 years ago

Yeah, we are on the Universal Apps stack (with allowAdminChanges disabled so project-config/apply is only happening on deploy) so we are experiencing the files not getting deleted, but it seems like a separate issue from this. Here's an example screenshot I took of modifications to an existing file that were showing in the CP diff on the deployed site after clearing caches:

Screen Shot 2022-09-07 at 4 07 59 PM

These were older modifications to the file and both the database and the project config files in ~/config/project were correct. If I used SCP to pull down the project config files from the server and replace my development project config directory with them, I see a handful of file additions (to be expected with the older files not being removed on the server) but there are zero modifications to existing files, which is inconsistent with what the CP diff was displaying.

There must be some way the CP diff is comparing the database to a file other than what I'm able to pull down from ~/config/project, which is the puzzling part to me. And it only happens after we clear caches in the CP on the server, which had me wondering if it was potentially a Craft issue.

brandonkelly commented 2 years ago

The incoming YAML files are read from disk without any internal caching. Maybe the old files are being cached by Opcache or some such though?

turnstylerj commented 2 years ago

Hmm that's interesting, @ostark possibly an issue with how opcache is cleared out/versioned for ~/config/project when caches are cleared? It never happens outside of actually clearing the caches in the CMS under Utilities > Caches, that seems to be the trigger 100% of the time.

ostark commented 2 years ago

AFAIK some state of Project Config is actively cached by Craft. It's definitely not related to OpCache or disk cache.

brandonkelly commented 2 years ago

@ostark The incoming YAML files aren’t ever cached. Only the applied config.

turnstylerj commented 2 years ago

Could the cached applied config potentially be the culprit, like maybe somehow clearing caches in the CP isn't correctly refreshing the cached applied config? Seems like that would fit all the data points we're talking about, since the database fields and the files in ~/config/project both represent the correct configuration.

turnstylerj commented 2 years ago

Fortrabbit is now telling us this is not a deployment or hosting issue, so I'm kind of at a loss on this one. It happens exclusively in the hosted environment and never locally (though local is in dev mode). Can you think of any way I can help diagnose/provide more information?

It just happened again after clearing caches in the hosted environment and followed exactly the steps expected:

  1. Verified the database had the correct fields/settings
  2. With the diff still showing those old changes in the CP, pulled down the files from the server and they were exactly as tracked in git outside of the additions from the old files that don’t get deleted (seems unrelated?), no modifications.
  3. Ran php craft project-config/rebuild on the server to delete the current ~/config/project directory and recreate it from current db state (which cleared out the diff/banner in the CP).
  4. Pulled the newly generated files down and checked in to repo, and there are no modifications outside of minor differences from generating the config on the server like '' values changed to null and a bump of the dateModified timestamp.
brandonkelly commented 2 years ago

Sorry for the delay @turnstylerj – been slammed the past couple weeks preparing for and hosting our developer conference.

Going back to this comment, you said:

If I used SCP to pull down the project config files from the server and replace my development project config directory with them, I see a handful of file additions (to be expected with the older files not being removed on the server) but there are zero modifications to existing files, which is inconsistent with what the CP diff was displaying.

That seems like it could be consistent with the screenshot? The built-in diff tool doesn’t split the diff based on YAML files; it stitches the entire incoming YAML files together into one big config array, and then compares that with the loaded config. The green portion of the screenshot you posted looks like it could be the start of one of the additional files that never got removed.

turnstylerj commented 2 years ago

Ohh interesting, thanks for the clarification @brandonkelly! I will do a closer comparison next time this happens to verify if that's the whole issue.

turnstylerj commented 2 years ago

Closing this out; after further testing on multiple apps with this in mind, it's definitely this issue with Fortrabbit's deployment strategy. Thanks for the clarification @brandonkelly!

brandonkelly commented 2 years ago

Thanks for the update @turnstylerj!