Migrations: Fix package migrations not running after fresh install with packages (closes #22202)#22204
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression where pending package migrations could leave a fresh install stuck in RuntimeLevel.Upgrading after the installer-triggered runtime restart, by ensuring CoreRuntime.StartAsync only defers to UnattendedUpgradeBackgroundService on the initial cold boot. The PR also refactors how upgrade migrations resolve/roundtrip property-data culture/variation, adjusts OpenIddict backoffice host handling, tweaks SQL Server distributed lock hints, and bumps RC versions.
Changes:
- Prevent
CoreRuntime.StartAsyncfrom returning early inUpgradingstate during in-process restarts so synchronous migration notifications can run. - Introduce
PropertyDataCultureResolverand apply it across multiple upgrade migrations to handle orphaned languages + invariant rows on culture-varying property types. - Update BackOffice OpenIddict redirect-host merging behavior and add
ROWLOCKto SQL Server distributed lock queries; bump versions to17.3.0-rc2.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version.json | Bumps product version to 17.3.0-rc2. |
| src/Umbraco.Web.UI.Client/package.json | Bumps backoffice npm package version to 17.3.0-rc2. |
| src/Umbraco.Infrastructure/Runtime/CoreRuntime.cs | Fixes unattended-upgrade deferral logic to not block migrations on runtime restart. |
| src/Umbraco.Infrastructure/Migrations/PropertyDataCultureResolver.cs | New helper to resolve culture + create migration-safe Property instances for roundtripping. |
| src/Umbraco.Infrastructure/Migrations/Upgrade/V_18_0_0/MigrateSingleBlockList.cs | Uses the new resolver/helper for culture resolution and migration property creation. |
| src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_1_0/FixConvertLocalLinks.cs | Uses the new resolver/helper for culture resolution and migration property creation. |
| src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertLocalLinks.cs | Uses the new resolver/helper for culture resolution and migration property creation. |
| src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertBlockEditorPropertiesBase.cs | Uses the new resolver/helper for culture resolution and migration property creation. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PropertyDataCultureResolverTests.cs | Adds unit tests covering culture resolution and migration property creation edge cases. |
| src/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManager.cs | Changes BackOfficeHost handling to append (if missing) rather than replace detected hosts. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Security/BackOfficeApplicationManagerTests.cs | Adds tests to validate the updated backoffice host preservation/deduplication behavior. |
| src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs | Adds ROWLOCK to locking queries. |
| src/Umbraco.Cms.Persistence.EFCore/Locking/SqlServerEFCoreDistributedLockingMechanism.cs | Mirrors the ROWLOCK hint change for the EF Core SQL Server locking mechanism. |
|
I just tested this, and it works on my end with Clean Starter Kit and The Starter Kit. 😄 |
|
I've run through a whole bunch of configuration combinations. Overall it looks great, and definitively an improvement 👍 Here are my results. InstallingThese are tests covering installation from scratch - with and without
UpgradingThese tests cover upgrading from Umbraco 16. In the test cases with a package,
Wait... failing tests?The failing combinations here are "soft failures", and I don't think they're caused by this PR. The package migrations actually run. However, the backoffice does not display the expected content structure afterwards. After a site restart, the content is present, so I am guessing this is a caching issue - either in core or in the package itself. We need to investigate this, but it's certainly not going to prevent this PR from being merged. |
Description
#22202 identifies a problem related to recent changes related to package migrations, found in 17.3.0-rc.
The result was that registered package migrations (e.g. from the Starter Kit) never executed after completing the install screen, leaving the site permanently returning HTTP 503 "The application is currently being upgraded."
To resolve I've change the behaviour such that we defer to
UnattendedUpgradeBackgroundServiceonly on initial boot, when there could be unattended upgrades to run, not on runtime restart after a new install.Root cause
This change create this regression: #22020 moved unattended upgrades into
UnattendedUpgradeBackgroundServiceand changed the runtime level for pending package migrations fromRuntimeLevel.RuntoRuntimeLevel.UpgradingwhenPackageMigrationsUnattendedis enabled.On a fresh install with a package that has a
PackageMigrationPlan(e.g. the Umbraco Starter Kit):BootUmbracoAsync()→CoreRuntime.StartAsync(false)→DetermineRuntimeLevel()setsLevel = Install(no database).UnattendedUpgradeBackgroundService.ExecuteAsync()seesLevel != Upgrading→ exits immediately. It is a one-shotBackgroundServiceand never runs again.RestartRuntimeStep→CoreRuntime.RestartAsync()→StartAsync(isRestarting: true).DetermineRuntimeLevel()detects the pending package migration → setsLevel = Upgrading.if (State.Level == RuntimeLevel.Upgrading) { return; }— exits unconditionally, expecting the background service to handle the migration.Level = Upgrading, every request returns 503.Before #22020, pending package migrations set
Level = Run, soStartAsyncdid not return early and the migrations ran synchronously via theRuntimeUnattendedUpgradeNotificationhandler.Fix
Add
&& isRestarting is falseto the early return condition. During a restart (which only happens after the install screen or an attended upgrade), the background service has already exited, so we fall through to the existing synchronous notification path which handles the migration correctly.This is safe because the concern that motivated the background service — ensuring the HTTP server binds its port before unattended migrations run, so hosting platforms don't kill the process — is only important for unattended upgrades, not for brand new installs (or attended upgrades, when by definition there won't be any unattended ones to run.
Testing
To test, do a fresh install with the Starter Kit package.