[DPE-10840] test(config): unit tests and golden fixtures for the update_config migration (5/6) - #188
Merged
Merged
Conversation
This was referenced Jul 20, 2026
Merged
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 20, 2026 19:04
912b1a4 to
9efcc0a
Compare
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 20, 2026 19:04
808b479 to
373a525
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 20, 2026 20:25
9efcc0a to
e80e84f
Compare
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 20, 2026 20:25
373a525 to
558c61f
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 21, 2026 17:46
e80e84f to
113b21b
Compare
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 21, 2026 17:47
558c61f to
ba946a4
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 23, 2026 17:38
113b21b to
814ec30
Compare
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 23, 2026 17:45
ba946a4 to
af641bb
Compare
…and bridges The library now owns the full config-update flow (steps 3-10) instead of the charm. update_config renders, applies Patroni-controlled parameters via the API, runs the TLS/pending-restart decision engine, honors the VM snap gate, and persists the config/user hashes -- all in the lib. The substrate-tangled pieces of the restart trigger, endpoint refresh, and monitoring/LDAP service restarts ride charm-side through three injected bridge callables (request_restart, refresh_endpoints, restart_services) until their own migration phases, so the two HIGH-risk substrate diffs in the restart trigger (VM pops postgresql_restarted; K8s updates the metrics scrape job) stay out of the library. is_tls_enabled and generate_config_hash internalize; user_hash stays injected. The config hash is byte-compatible with the charm's, so charm adoption does not force a spurious restart. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The millicore floor is a behaviour change with no counterpart in the charms this port mirrors, so it does not belong in a parity diff a reviewer checks line by line against them. It now lives in its own fix on 16/edge (#206). Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
… the base BaseWorkload gained a non-abstract get_snap_revision that only the VM workload can answer, purely so the type checker would accept a call the substrate guard already keeps off K8s. Reach it the way the event handlers reach the other VM-only workload capability, by narrowing with isinstance at the call site, so the shared interface stops advertising a snap the K8s workload has no access to. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…know The ported line carried the VM charm's wording, which dropped the K8s charm's "Updating Patroni config file" from the logs entirely. Operators grepping either substrate's history for that phrase would have found nothing after the move. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Both charms compute this hash once per hook via cached_property; the port turned it into a plain property, so update_config recomputes it on each of its two accesses. The value cannot differ within a hook, so this only restores the charms' cost profile. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Narrowing the snap gate with isinstance needed VMWorkload at runtime, and that module imports the snap charm lib, which ships only in the vm extra. The K8s charm installs postgresql and k8s, imports the config manager, and would have died at startup on `cannot import name 'snap' from 'charmlibs'`. The library's own test env installs every extra, so nothing here would have caught it. Narrow with a type-checking-only cast instead; the substrate guard is what keeps the call off K8s at runtime. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The K8s charm logged this at info, so operators saw a config render in debug-log at default verbosity; the port left it at the VM charm's debug level and that visibility disappeared on both substrates. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
These base classes are the per-substrate charm definitions, not test scaffolding, and describing them as unit-test only reopens a settled question about whether they belong in the shipped wheel. What the comment needs to say is which side owns the bridges, which the preceding lines already do. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Issued certs land in the relation databag before the push writes them to the workload, and the push can defer — on K8s whenever the container is not ready. Any of the other update_config call sites firing in that window rendered ssl on against files that were not there, which is the non-atomicity the TLS port left open; TLSManager.client_tls_files_on_disk was added for this and had no caller. This diverges from the charms, which check the databag alone. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
August 17, 2026 19:12
69a977a to
aa29985
Compare
…gration Split the migration's unit tests and patroni-template golden fixtures out of the code changes so each code change lands as a small, self-contained PR, with the tests landing together on top of the stack. Covers the peer-state config/user hash accessors, PostgreSQL parameter building and resource introspection, the substrate-conditional patroni template, and the update_config orchestration. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The merged template now emits one field order for both substrates instead of reproducing each charm's, so a byte comparison fails on layout that carries no meaning to Patroni. Comparing the parsed documents keeps the guarantee that matters - same values, same keys, same rule order - and stays strict where order is semantic, since sequences still compare in order and pg_hba is first-match-wins. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The charm template parameterised bin_dir but hardcoded the major version in the WAL, PostgreSQL-log and Patroni-log paths, so a 17 workload would have kept three directories on 16. The golden matrix cannot catch it: it pins version at 16, where both spellings render identically. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The behaviour it pins left this stack along with the floor itself, so the assertion now covers code that only exists in its own fix on 16/edge (#206), where the test belongs. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Pins the window the gate closes: certs present in the databag but not yet written to the workload must not read as TLS enabled. The existing all-files-present test now states the disk input too, since presence in the databag alone no longer decides. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
August 17, 2026 19:13
f21dea5 to
a1c9431
Compare
…ig-tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> # Conflicts: # single_kernel_postgresql/managers/config.py
marceloneppel
marked this pull request as ready for review
August 18, 2026 14:15
marceloneppel
requested review from
carlcsaposs-canonical,
dragomirp,
juju-charm-bot and
taurus-forever
and removed request for
a team
August 18, 2026 14:15
taurus-forever
approved these changes
Aug 18, 2026
dragomirp
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
The
update_configmigration's unit tests and patroni-template golden fixtures were bundled with the code changes, pushing each code PR in the stack past the review-size target.Solution
Split the tests out of the code PRs so each code change lands small and self-contained, with the tests landing together here on top of the code PRs. Only the minimal edits to existing tests needed to keep each code PR green stayed with the code; every new test and both golden fixtures are here.
Covers the peer-state config/user hash accessors (#181), PostgreSQL parameter building and resource introspection (#182), the substrate-conditional patroni template and its per-substrate golden render matrix (#183), and the
update_configorchestration (#180).The golden matrix renders the lib template and the preserved pre-merge charm template with the same context and compares the parsed documents rather than the bytes, because the lib template emits one field order for both substrates instead of reproducing each charm's, and mapping order is not part of the configuration. Sequences still compare in order, so
pg_hba— where PostgreSQL takes the first matching rule — andpartner_addrsstay pinned.One case sits outside that matrix by design: the K8s data paths are asserted to follow the workload's major version. The matrix pins the version at 16, where the parameterised and hardcoded spellings render identically, so it cannot see the difference the charm template carried —
bin_dirfollowed the version while the WAL, PostgreSQL-log and Patroni-log paths did not.Checklist