diff --git a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs index 8ef3ff875..9e5428800 100644 --- a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs +++ b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs @@ -1292,14 +1292,21 @@ public void LegacyMaintenanceWorkMemHeal_RunsBeforeTheStoreUpgradeCanStartTheOld } /// - /// #3909 through the product's own bootstrap. A store on PostgreSQL 17 whose conf ends with the old 2048 MB - /// value (the shape a reverted upgrade left on a 40 GB+ host) comes back up through - /// with the capped value in force. The 17 runtime is - /// the runtime here, with no package beside it, so nothing is upgraded: this is the store that stays on 17. - /// Gated on DARLING_TEST_PGRUNTIME_OLD. - /// - [Fact] - public async Task Postgres17Store_WithTheOldCapInItsConf_StartsThroughTheBootstrap_Gated() + /// #3909 through the product's own bootstrap, after #4336 moved the settings into one included file. A + /// store on PostgreSQL 17 whose darling-managed.conf still carries the old, uncapped 2048 MB value + /// (the shape a store upgraded to #4336 without a fresh render since, or a reverted upgrade, could leave + /// on a 40 GB+ host) comes back up through on a + /// value PostgreSQL 17 accepts, because every start re-renders darling-managed.conf + /// () from this host's own sizing, which on a + /// major at or below 17 never exceeds . The + /// value is host-dependent (a small host renders less than the cap), so the fact reads the render's own + /// line back and asserts the server runs on exactly that value, and that it is at or under the cap; the + /// cap arithmetic itself is pinned without a server by ManagedConfFileTests. + /// The 17 runtime is the runtime here, with no package beside it, so nothing is upgraded: this is the + /// store that stays on 17. Gated on DARLING_TEST_PGRUNTIME_OLD. + /// + [Fact] + public async Task Postgres17Store_WithTheOldValueInItsManagedConf_StartsWithTheCap_Gated() { var oldRuntime = Environment.GetEnvironmentVariable("DARLING_TEST_PGRUNTIME_OLD"); Assert.SkipWhen(string.IsNullOrWhiteSpace(oldRuntime), @@ -1320,6 +1327,7 @@ public async Task Postgres17Store_WithTheOldCapInItsConf_StartsThroughTheBootstr var dataDirectory = Path.Combine(root.FullName, "store", "pg"); var confPath = Path.Combine(dataDirectory, "postgresql.conf"); + var managedConfPath = Path.Combine(dataDirectory, ManagedConfFile.FileName); var config = new PostgresConfig { Managed = true, Port = FindFreeTcpPort(), DataDirectory = dataDirectory }; using var timeout = new CancellationTokenSource(TimeSpan.FromMinutes(8)); DarlingManagedPostgres? first = null; @@ -1331,15 +1339,167 @@ public async Task Postgres17Store_WithTheOldCapInItsConf_StartsThroughTheBootstr await first.StopIfStartedByThisProcessAsync(); Assert.Equal(17, DarlingStoreUpgrade.TryReadDataDirectoryMajor(dataDirectory)); - await File.AppendAllTextAsync(confPath, "\n# written by a build before #3909\nmaintenance_work_mem = 2048MB\n", timeout.Token); + /* The shape a store carrying the old, pre-cap value could still have in its managed file: replace + the ONE maintenance_work_mem assignment the first start's own render just wrote (this host's own value, + at or under the cap on <=17) with the old, uncapped 2048MB, keeping + exactly one assignment. Then recompute the header's body hash over the edited body, the same way + WriteManagedConfFile itself would, so the file still reads as the product's own render (this is + simulating a store that carried this value in a file THE PRODUCT rendered -- e.g. rendered on + PostgreSQL 18 and then reverted to 17 -- not an operator hand edit, which would only ever reach + the last-good fallback in EnsureManagedConfReadyAsync, never the re-render this fact pins). */ + var managedTextBefore = await File.ReadAllTextAsync(managedConfPath, timeout.Token); + Assert.Single(ActiveValues(managedTextBefore, "maintenance_work_mem")); + var parsedBefore = ManagedConfFile.ParseExisting(managedTextBefore); + Assert.True(parsedBefore.IsWellFormed); + var bodyWithOldValue = System.Text.RegularExpressions.Regex.Replace( + parsedBefore.Body, + @"maintenance_work_mem = '[^']*'", + "maintenance_work_mem = '2048MB'"); + /* Rewrite only the header's body-sha256 line to match the edited body -- the rest of the header + (formula version, RAM, CPUs, ...) stays exactly what the first start's own render wrote. This is + what makes the edited file read as the product's own render rather than a hand edit: see the + remarks above. */ + var headerBeforeHashLine = managedTextBefore[..managedTextBefore.IndexOf(ManagedConfFile.BodyHashPrefix, StringComparison.Ordinal)]; + var managedTextWithOldValue = headerBeforeHashLine + ManagedConfFile.BodyHashPrefix + ManagedConfFile.ComputeBodyHash(bodyWithOldValue) + "\n" + bodyWithOldValue; + Assert.False(ManagedConfFile.IsHandEdited(managedTextWithOldValue)); + Assert.Equal("2048MB", Assert.Single(ActiveValues(managedTextWithOldValue, "maintenance_work_mem"))); + await File.WriteAllTextAsync(managedConfPath, managedTextWithOldValue, timeout.Token); second = new DarlingManagedPostgres(config, NullLogger.Instance, runtimeRoot); var connectionString = await second.EnsureRunningAsync(timeout.Token); + /* The second start re-rendered the file rather than falling back to a last-good copy: this is what + distinguishes this fact from an operator hand edit (the sibling fact below), which the last-good + fallback path would have to handle instead. */ + Assert.True(second.LastManagedConfWriteResult is { Written: true, HandEdited: false }); + Assert.False(second.LastStartUsedLastGoodManagedConf); + + /* The re-render replaced the old line with this host's own value. That value depends on the host's + RAM (a CI runner renders less than the cap), so read it back from the file rather than assume + the cap: the server must run on exactly the rendered value, and that value must be one + PostgreSQL 17 accepts. */ + var managedTextAfter = await File.ReadAllTextAsync(managedConfPath, timeout.Token); + var renderedValue = Assert.Single(ActiveValues(managedTextAfter, "maintenance_work_mem")); + Assert.NotEqual("2048MB", renderedValue); + var (live, expected) = await ReadSettingAndLiteralBytesAsync( - connectionString, "maintenance_work_mem", $"{DarlingManagedPostgres.MaintenanceWorkMemCapMb}MB", timeout.Token); + connectionString, "maintenance_work_mem", renderedValue, timeout.Token); Assert.Equal(expected, live); - Assert.Equal(1, CountOccurrences(await File.ReadAllTextAsync(confPath, timeout.Token), DarlingManagedPostgres.ConfMarkerV14)); + Assert.True(live <= DarlingManagedPostgres.MaintenanceWorkMemCapMb * 1024L * 1024L, + $"expected the rendered maintenance_work_mem ({renderedValue}) to be at or under the {DarlingManagedPostgres.MaintenanceWorkMemCapMb} MB cap on PostgreSQL 17"); + + /* initdb's postgresql.conf carries a commented sample line (#maintenance_work_mem = 64MB), so these + read ACTIVE assignments the way PostgreSQL does, never a raw substring. */ + var postgresqlConfText = await File.ReadAllTextAsync(confPath, timeout.Token); + Assert.True(ManagedConfFile.HasManagedInclude(postgresqlConfText)); + Assert.Empty(ActiveValues(postgresqlConfText, "maintenance_work_mem")); + } + finally + { + if (first is not null) + { + await first.StopIfStartedByThisProcessAsync(); + } + + if (second is not null) + { + await second.StopIfStartedByThisProcessAsync(); + } + + try + { + root.Delete(recursive: true); + } + catch (IOException) + { + /* A temp directory the OS still holds is not this test's failure. */ + } + } + } + + /// + /// #4405: an operator's own line below the managed include, out of PostgreSQL 17's range, is the + /// operator's line to fix — the store must refuse to start rather than silently absorb it, and its + /// refusal must name the fix. Unlike a value inside darling-managed.conf (the sibling fact above), + /// this line is NEVER something the product wrote: every start re-renders darling-managed.conf + /// itself (), and no code path in this service + /// ever appends a bare maintenance_work_mem line to postgresql.conf below the include — the + /// only appends that ever wrote it (v3, v7, v8, v14) always carry their own marker, and #4336 confined all + /// of them to a Legacy (pre-migration) conf. So a bare, unmarked line after the include is an + /// operator edit, and validates the whole + /// merged conf with postgres -C before ever starting the server: PostgreSQL rejects the value. + /// The first start above already succeeded, so + /// saved a last-good copy of darling-managed.conf after it — this data directory does have one, and + /// restores and re-validates it. That still + /// fails, because the rejected line lives in postgresql.conf below the include, not in + /// darling-managed.conf: restoring the last-good managed file leaves the operator's own line in + /// place untouched. So the fallback is tried and still refused, and the start throws + /// naming the fix + /// () instead of starting on + /// a value the render never sanctioned. Gated on DARLING_TEST_PGRUNTIME_OLD. + /// + [Fact] + public async Task Postgres17Store_OperatorLineBelowIncludeOutOfRange_RefusesToStartNamingTheFix_Gated() + { + var oldRuntime = Environment.GetEnvironmentVariable("DARLING_TEST_PGRUNTIME_OLD"); + Assert.SkipWhen(string.IsNullOrWhiteSpace(oldRuntime), + "Set DARLING_TEST_PGRUNTIME_OLD to an assembled PostgreSQL 17 pg-runtime (new-upgraded-store-fixture.ps1 builds one)."); + Assert.SkipUnless(OperatingSystem.IsWindows(), "The bundled runtime is Windows-only."); + Assert.SkipUnless(File.Exists(Path.Combine(oldRuntime!, "pgsql", "bin", "pg_ctl.exe")), + $"DARLING_TEST_PGRUNTIME_OLD={oldRuntime} does not contain pgsql\\bin\\pg_ctl.exe."); + + var root = Directory.CreateTempSubdirectory("darling-pg17op-"); + var runtimeRoot = Path.Combine(root.FullName, "deploy", "pg-runtime"); + var source = Path.Combine(oldRuntime!, "pgsql"); + foreach (var file in Directory.EnumerateFiles(source, "*", SearchOption.AllDirectories)) + { + var target = Path.Combine(runtimeRoot, "pgsql", Path.GetRelativePath(source, file)); + Directory.CreateDirectory(Path.GetDirectoryName(target)!); + File.Copy(file, target); + } + + var dataDirectory = Path.Combine(root.FullName, "store", "pg"); + var confPath = Path.Combine(dataDirectory, "postgresql.conf"); + var config = new PostgresConfig { Managed = true, Port = FindFreeTcpPort(), DataDirectory = dataDirectory }; + using var timeout = new CancellationTokenSource(TimeSpan.FromMinutes(8)); + DarlingManagedPostgres? first = null; + DarlingManagedPostgres? second = null; + try + { + first = new DarlingManagedPostgres(config, NullLogger.Instance, runtimeRoot); + await first.EnsureRunningAsync(timeout.Token); + await first.StopIfStartedByThisProcessAsync(); + Assert.Equal(17, DarlingStoreUpgrade.TryReadDataDirectoryMajor(dataDirectory)); + Assert.True(ManagedConfFile.HasManagedInclude(await File.ReadAllTextAsync(confPath, timeout.Token))); + + /* The first start above succeeded, so it saved a last-good managed-conf copy (design step 2 + -- see the summary above): this data directory does have one to fall back to. */ + var lastGoodPath = Path.Combine(dataDirectory, ManagedConfFile.LastGoodFileName); + Assert.True(File.Exists(lastGoodPath), $"Expected the first start to have saved {lastGoodPath}."); + + /* An operator's own line below the include -- never something this product wrote (see the + summary above). PostgreSQL 17 rejects it outright. */ + await File.AppendAllTextAsync(confPath, "\n# an operator's own line\nmaintenance_work_mem = 2048MB\n", timeout.Token); + Assert.Equal("2048MB", Assert.Single(ActiveValues(await File.ReadAllTextAsync(confPath, timeout.Token), "maintenance_work_mem"))); + + second = new DarlingManagedPostgres(config, NullLogger.Instance, runtimeRoot); + var thrown = await Assert.ThrowsAsync( + async () => await second.EnsureRunningAsync(timeout.Token)); + + Assert.Contains("maintenance_work_mem", thrown.Message, StringComparison.Ordinal); + Assert.Contains( + "Fix the rejected setting with a line after 'include ''darling-managed.conf'''", + thrown.Message, StringComparison.Ordinal); + + /* Not left running on the rejected conf: stopping is safe (there is nothing to stop), and a + connection to the configured port fails. */ + await second.StopIfStartedByThisProcessAsync(); + await Assert.ThrowsAnyAsync(async () => + { + await using var connection = new NpgsqlConnection( + $"Host=127.0.0.1;Port={config.Port};Username=darling;Database=postgres;Pooling=false;Timeout=5"); + await connection.OpenAsync(timeout.Token); + }); } finally { @@ -3508,6 +3668,48 @@ await reader.ReadAsync(cancellationToken), return (reader.GetInt64(0), reader.GetInt64(1)); } + /// + /// The values of every ACTIVE assignment of in postgresql.conf-format text, read + /// with the product's own parser (), so a commented line + /// such as initdb's #maintenance_work_mem = 64MB sample never counts. + /// + private static List ActiveValues(string confText, string setting) + { + var values = new List(); + foreach (var (_, name, value) in DarlingManagedPostgres.ParseConfText(confText)) + { + if (name.Equals(setting, StringComparison.OrdinalIgnoreCase)) + { + values.Add(value); + } + } + + return values; + } + + /// + /// skips initdb's commented sample lines. The lines below are copied from + /// PostgreSQL 17's own postgresql.conf.sample, which is what initdb writes, and a raw substring check + /// on the key name matches them (the failure the gated cap facts above hit on a real PostgreSQL 17 conf). + /// + [Fact] + public void ActiveValues_SkipsInitdbsCommentedSampleLines_AndCountsAnOperatorLine() + { + const string initdbSample = + "#work_mem = 4MB\t\t\t\t# min 64kB\n" + + "#hash_mem_multiplier = 2.0\t\t# 1-1000.0 multiplier on hash table work_mem\n" + + "#maintenance_work_mem = 64MB\t\t# min 64kB\n" + + "#autovacuum_work_mem = -1\t\t# min 64kB, or -1 to use maintenance_work_mem\n" + + "include 'darling-managed.conf'\n"; + + Assert.Contains("maintenance_work_mem", initdbSample, StringComparison.Ordinal); + Assert.Empty(ActiveValues(initdbSample, "maintenance_work_mem")); + Assert.True(ManagedConfFile.HasManagedInclude(initdbSample)); + + var withOperatorLine = initdbSample + "\n# an operator's own line\nmaintenance_work_mem = 2048MB\n"; + Assert.Equal("2048MB", Assert.Single(ActiveValues(withOperatorLine, "maintenance_work_mem"))); + } + private static int CountOccurrences(string text, string value) { var count = 0;