Updated: Comment #14
Problem/Motivation
When there are updates to Drupal core, contributed modules dealing with configuration overrides need to react to schema changes. It's currently not reliable to use the update helper functions, because there is no guarantee your update function is actually run before Drupal adds a new version of the helper function (update dependencies don't help because as per our instructions, you first need to update core and only after that update contrib modules, at least for major updates)
Conversions to the config system have been using config() during the upgrade path. We have a convention that API functions (apart from raw database queries) can't be used during the upgrade path, because the API might change the the upgrade path has to be completely static as much as possible.
Proposed resolutions
- Add the old configuration as an update-only service and have future update functions change the service appropriately by establishing an UpdateConfigFactory that extends ConfigFactory (much like UpdateModuleHandler extends ModuleHandler)
- add a config_update_0() API that is locked to the existing configuration API and that we'll only update until the upgrade path is supported in some way (usually around beta). After there's an expectation that 7.x sites can upgrade to Drupal 8, any change to CMI file storage would result in config_update_x() being added on top
- wait until this becomes a problem then add this retrospectively, the problem with this is that contrib modules will also need to use config_update_x() and right now they'll already be using config(), it's not going to be acceptable to change things for existing updates once the upgrade path is supported
- never under any circumstances change the configuration file format once we reach beta/rc, historically we never make that kind of guarantee just in case but it might be OK to make it here
- possibly work around it with update dependencies, but again that's hard once you have contrib modules relying on the API
Remaining tasks
- Agree on an approach to solving the problem.
- Create the patch.
User interface changes
None
API changes
TBD
Related Issues
#2001310: Disallow firing hooks during update
#1871700: Provide an upgrade path to the new Block architecture from Drupal 7
#1977498: Add version tracking for configuration schema and data
Original report by catch
Conversions to the config system have been using config() during the upgrade path. We have a convention that API functions (apart from raw database queries) can't be used during the upgrade path, because the API might change the the upgrade path has to be completely static as much as possible.
config(), as a raw storage API the configuration is a bit different so I've been ignoring this issue to some extent, but the more I think about it I don't think it's an exception to the rule.
Here's where it can go wrong:
- system_update_xx() (and various hundreds of others) use config() during the upgrade path.
- at some point Drupal 8.0 is released. Some sites upgrade to 8.0.
- Drupal 8.6 is released and changes the way that configuration files are written changes in some way that requires an update to existing configuration.
- Drupal 8.6 adds an update function to update all existing configuration files to the new format.
- sites upgrading from 7.x direct to Drupal 8.6 now have all of their files written out into the new format by the pre-8.x update function. Then an upgrade path added in 8.6 running on those new configuration files, which attempts to upgrade them from the old format to the new one.
- Drupal 8.15 changes the structure again.
Ways we can deal with this:
- add a config_update_0() API that is locked to the existing configuration API and that we'll only update until the upgrade path is supported in some way (usually around beta). After there's an expectation that 7.x sites can upgrade to Drupal 8, any change to CMI file storage would result in config_update_x() being added on top.
- wait until this becomes a problem then add this retrospectively, the problem with this is that contrib modules will also need to use config_update_x() and right now they'll already be using config(), it's not going to be acceptable to change things for existing updates once the upgrade path is supported.
- never under any circumstances change the configuration file format once we reach beta/rc, historically we never make that kind of guarantee just in case but it might be OK to make it here.
- possibly work around it with update dependencies, but again that's hard once you have contrib modules relying on the API.
If the example above isn't any good and this whole issue confuses you, please read #1134088: Properly document update-api functions and #1239370: Module updates of new modules in core are not executed to see some more background on how broken our current major upgrade system is and why all this weird crap is required to stop it from completely collapsing in on itself.
Opening this as a critical task because we absolutely have to resolve this issue before people start trying to upgrade sites with any hope of keeping them around.
Comments
Comment #1
BerdirThis pattern doesn't work for contrib anyway. It's currently not reliable to use the update helper functions, because there is no guarantee your update function is actually run before Drupal adds a new version of the helper function (update dependencies don't help because as per our instructions, you first need to update core and only after that update contrib modules, at least for major updates). The only thing that works for contrib is using the normal function and require to run the update with the latest available core version...
Comment #2
sunAlmost identical / closely related: #1848998: Problems with update_module_enable()
Comment #3
xjmLet me see if I understand this:
system_update_8090()
for 8.6 and addsconfig_update_8090()
(or whatever) based on that API.config_update_8090()
to upgrade its data inmymodule_update_8001()
.system_update_8092()
in 8.15 and addsconfig_update_8092()
(or whatever).config_update_8092()
inmymodule_update_8002()
, but sites that are not already on the8090
schema from 8.6 choke on the previous update inmymodule_update_8001()
, because core was updated to8092
beforemymodule_update_8001()
was run.Comment #4
xjmDo config files need their own schema version, like stored in the file?
Comment #5
BerdirExactly. Unlike the core modules, where we have 100% control over the order in which updates are run, contrib modules aren't so lucky. Their only option really is just using the normal API function (or maybe an update_something_latest()) and update it if necessary when core changes something.
Comment #6
catchupdate_something_latest() would work if contrib updates are being run after core has been upgraded from 7.x to 8.x already, but for that major version upgrade, they'd probably still need to use a specific version and hook_update_dependencies() since otherwise the _latest() function could be operating against any schema version, not necessarily with the latest one?
Comment #7
BerdirAccording to our upgrade instructions, for major updates, contrib *always* needs to run after the main 7.x -> 8.x upgrade path was executed. But there is no way of saying of that core upgrade means 7.x => 8.0 or 7.x => 8.15 which have have changed the update helper function.
Comment #8
gddI think I am OK with this answer to be honest. By the time we reach beta we will have worked through some of the most complicated possible implementations of the config system, namely views and fields/content types. The format is already very open and malleable for specific use cases. I'd be fine making this statement and essentially punting on the problem.
Comment #9
sunComment #10
andypostAfter #1751348: Convert locale settings to configuration system we have very fragile and not testable errors caused by #1856972: config() isn't safe to use during minor updates if there's a change to configuration storage
Comment #11
xjmRelated: #1871700: Provide an upgrade path to the new Block architecture from Drupal 7
Comment #12
Gábor HojtsyRelated: #1977498: Add version tracking for configuration schema and data for post-upgrade path updates of config overrides when the base config structure changes and overrides are not anymore overlaying the config as needed.
Comment #13
Gábor Hojtsy@heyrocker:
So views and field config are just containers for in many cases contrib data structures (displays, filters, field types, etc). So I'd question how deep do we want the config structure freeze to take hold (and whether this is enforceable on contrib). The problem from the point of view of config overrides is that the override will be in the structure of the config. If *any* substructure changes that was overriden (by language, domain module, etc), then the override will not apply anymore, and may even be harmful given that it "stamps over" the base config. So in case any contribs update config structures, be it a view plugin or field configuration, we'd need to find a way to trickle that down to the overrides. That is where #1977498: Add version tracking for configuration schema and data comes in.
If we have a ban on changing config structure altogether, this is not an issue obviously. Is that realistic? If not, and we can apply the structure change ban on core only, we'll need to find ways for how these contrib modules update their structure, how that affects core wrapping structures and then how overrides can / should react. Eg. hooks/events in place to react with config before/after and schema before/after as arguments.
Comment #14
chx CreditAttribution: chx commentedIf Drupal 8.6 changes config then we can add the old config as an update-only service and have that 8.6 update function change the service appropriately. I think.
Comment #14.0
mtiftUpdated issue summary.
Comment #14.1
mtiftUpdated issue summary.
Comment #15
xjmSo there are two separate problems here
For both issues, I think the next step is a failing test that demonstrates that these are actually still bugs.
Comment #16
mtifttagging
Comment #17
chx CreditAttribution: chx commented> The invocation of hooks during configuration entity creation
Except UpdateModuleHandler will boink you over the head if you write an update that saves an entity cos saving an entity fires hooks and you can not fire hooks during update. Just save the raw config.
Comment #18
webchickTagging all critical D8 upgrade path issues as "beta blocker."
Comment #19
catchWe need to discuss this again now there's no upgrade path. This could still be an issue if we make a change to the configuration storage during a minor release but seems like that could be handled with update dependencies - it'd only affect a module updating its configuration where the update runs at the same time as the core update for the storage.
Since that's considerably more of an edge case than when trying to support both minor and major version updates with the same API and having minor updates run immediately after major updates, downgrading to major.
Comment #20
effulgentsia CreditAttribution: effulgentsia commentedOne thing we discussed in Prague is that we should introduce separate hooks for schema changes than for data changes (both for "config" data and "content" data). Then we could run all the schema changes (across all modules) first (with very limited APIs allowed, and no hooks firing), followed by the data change functions (which could go through normal config() and entity CRUD API calls). If we did that, would that fully solve this issue?
Comment #21
catchYes I think that'd be fine.
Comment #21.0
catchUpdated issue summary.
Comment #22
xjmComment #23
chx CreditAttribution: chx commentedHow often will config storage change? Can we tell people to re-migrate?
Comment #24
catch#2250119: Run updates in a full environment.