[DPE-10841] feat(tls): emit tls_files_pushed after a successful cert push (6/6) - #189
Merged
Conversation
marceloneppel
changed the title
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 20, 2026 19:04
808b479 to
373a525
Compare
marceloneppel
force-pushed
the
tls-files-pushed
branch
from
July 20, 2026 19:04
7306394 to
c5861ea
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
tls-files-pushed
branch
from
July 20, 2026 20:25
c5861ea to
d91d183
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
tls-files-pushed
branch
from
July 21, 2026 17:47
d91d183 to
cc243c6
Compare
marceloneppel
force-pushed
the
skl-01-update-config-tests
branch
from
July 23, 2026 17:45
ba946a4 to
af641bb
Compare
marceloneppel
force-pushed
the
tls-files-pushed
branch
from
July 23, 2026 17:46
cc243c6 to
ff8e9d1
Compare
2 tasks
…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>
…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
The TLS handler pushes cert files to the workload on certificate_available, and the charm reloads PostgreSQL in a separate observer of the same event. Because defer is per-observer, a push that defers (e.g. a Pebble write fails mid cert renewal) while the charm's reload observer completes leaves the retried push writing the new cert with nothing reloading. Emit a tls_files_pushed event only after push_tls_files() succeeds; a deferred push returns without emitting. The charm's reload bridge observes this instead of certificate_available, so the reload runs only once the files are on disk and a deferred push retries together with its reload. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The update_config + tls_files_pushed stack branched at 16.3.3; 16/edge has since released 16.3.4. Set the version to the next patch so the library is release-ready as soon as this stack lands on 16/edge. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
marceloneppel
force-pushed
the
tls-files-pushed
branch
from
August 17, 2026 19:15
c467833 to
01154fb
Compare
2 tasks
…ig-tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> # Conflicts: # single_kernel_postgresql/managers/config.py
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
marceloneppel
changed the title
marceloneppel
marked this pull request as ready for review
August 18, 2026 14:24
marceloneppel
requested review from
carlcsaposs-canonical,
dragomirp,
juju-charm-bot and
taurus-forever
and removed request for
a team
August 18, 2026 14:24
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
3 participants
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 TLS handler pushes cert files to the workload on
certificate_available; the charm reloads PostgreSQL in a separate observer of the same event.deferis per-observer, so a push that defers (e.g. a Pebble write fails during a cert renewal) while the charm's reload observer completes leaves the retried push writing the new cert with nothing reloading — the reload ran against the stale on-disk files and won't run again. Raised in review on the K8s TLS migration (canonical/postgresql-k8s-operator#1618).Solution
Add a
tls_files_pushedcustom event on theTLShandler, emitted only afterpush_tls_files()succeeds; either defer guard returns without emitting. The charm's reload bridge observes this instead ofcertificate_available, so the reload fires only once the files are on disk, and a deferred push retries together with its reload. This also lets the charms drop the two guards (internal-capresent,client_tls_files_on_disk()) they duplicate from this handler.Stacked on the update_config lib stack; the two charm rewires follow and pin this commit.
Checklist