[DPE-10839] feat(config): complete update_config with API apply, restart engine, and bridges (4/6) - #180
Merged
Merged
Conversation
marceloneppel
changed the title
marceloneppel
changed the base branch from
tls-4-tests
to
skl-01-update-config-3-template
July 11, 2026 01:42
marceloneppel
force-pushed
the
skl-01-update-config-3-template
branch
from
July 11, 2026 16:12
cba92af to
fdf2a44
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 11, 2026 16:12
08f5993 to
40752dd
Compare
marceloneppel
force-pushed
the
skl-01-update-config-3-template
branch
from
July 20, 2026 13:22
fdf2a44 to
ef76d9a
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 20, 2026 13:22
40752dd to
912b1a4
Compare
marceloneppel
changed the title
Merged
2 tasks
marceloneppel
changed the title
marceloneppel
force-pushed
the
skl-01-update-config-3-template
branch
from
July 20, 2026 19:04
ef76d9a to
d994db8
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
2 times, most recently
from
July 20, 2026 20:25
9efcc0a to
e80e84f
Compare
marceloneppel
force-pushed
the
skl-01-update-config-3-template
branch
from
July 21, 2026 17:46
d994db8 to
f5d55f2
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-3-template
branch
from
July 23, 2026 17:34
f5d55f2 to
79f2a4f
Compare
marceloneppel
force-pushed
the
feature/migrate-update-config
branch
from
July 23, 2026 17:38
113b21b to
814ec30
Compare
marceloneppel
changed the title
This was referenced Aug 14, 2026
…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
marceloneppel
marked this pull request as ready for review
August 17, 2026 19:23
marceloneppel
requested review from
carlcsaposs-canonical,
dragomirp,
juju-charm-bot and
taurus-forever
and removed request for
a team
August 17, 2026 19:23
dragomirp
reviewed
Aug 18, 2026
dragomirp
approved these changes
Aug 18, 2026
…y log update_config runs on nearly every hook, and the ported line had merged the K8s charm's info level with the VM charm's parameter payload, so the full PostgreSQL parameter dict landed in every operator's log at default verbosity. Each substrate's original line goes back as it was: the info announcement operators grep for, and the parameter dump at debug. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
taurus-forever
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 capstone of the migration: the
update_configorchestration itself — apply the Patroni REST API config patch, decide reload-vs-restart, and persist the config hash — plus the charm-owned bridges it calls back into.Solution
managers/config.py:update_configorchestration wiringapply_api_config,handle_restart_need,is_tls_enabled,is_restart_pending,generate_config_hash, and config-hash persistence.request_restart,refresh_endpoints,restart_services), matching the TLS migration's collaborator-injection pattern; declared@abstractmethodon the abstract charm and stubbed per substrate.apply_api_configdirectly.is_tls_enabledadditionally requires the certs to be on disk (TLSManager.client_tls_files_on_disk). This is a deliberate divergence from the charms, which check the relation databag alone: issued certs reach the databag before the push writes them to the workload, and the push can defer — on K8s whenever the container is not ready — so any of the otherupdate_configcall sites firing in that window renderedssl: onagainst files that were not there yet.self.workloadwith a type-checking-onlycast, soworkload/vm.py— and with it the snap charm lib, which K8s does not install — stays out of the K8s import graph.generate_config_hashis acached_propertyand the config-render line logs at info level, both restoring what the charms do.Final (code-only) PR in the update_config stack; the
ConfigManagerconstructor grows to seven args, so the minimal fixture update to keep the suite green rides here, and the new unit tests land in the stack's test PR (#188). Charm adoption — deleting the charm-side copies, catchingDeployedWithoutTrustError, wiring the bridges — follows in per-charm PRs once the lib is released.Checklist