Repository navigation
[DPE-11482] refactor(refresh): consume the pg_cron-gate lib fix on 16/edge - #2036
Draft
marceloneppel wants to merge 21 commits into
Draft
marceloneppel wants to merge 21 commits into
marceloneppel wants to merge 21 commits into
Conversation
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Adapt the charm to the pinned single-kernel lib tip (abe9a57): - CharmState now requires s3_requirer: construct one lib-vendored S3Requirer(self, s3-parameters) shared by CharmState and PostgreSQLBackups (ops permits only one S3Requirer per charm); patch targets in test_backups.py follow the vendored module path. - Declare the status-peers peer relation: CharmState owns StatusesState on it at this pin. - The lib RefreshManager types its charm as AbstractPostgreSQLCharm: pass cast(self) with the dispatch surface the refresh manager drives already in place. ruff format rides on the pick test_tls patch line.
RefreshManager.reconcile() calls both charm.update_pebble_layers() and charm.ensure_pgdata_dirs_and_symlinks() on every substrate; the VM charm mirrors the library VM charm no-op implementations.
… construction RefreshManager calls the charm-injected set_default_status callable during its construction (reconcile_refresh_status); _set_primary_status_message reaches this seam before the refresh_manager attribute is assigned, raising AttributeError on every charm start. CI surfaced it as hook failures on refresh-v-three-relation-changed across all upgrade-family integration suites. Fall back to a direct ops status set until construction completes, mirroring the direct sets the library itself performs while reconciling.
…resh-manager early publish The integration branch now carries the /6 stack fix publishing charm.refresh_manager before charm_refresh construction, so charm_refresh resume callbacks can dispatch through the manager while its constructor is still running (unblocks post-snap-refresh resumes on machines).
The refresh-resume path installs the snap through the library's VMWorkload.install_snap_package, which has no vitality-hint logic, so the snap name was never appended to resilience.vitality-hint on resume (test_snap_refresh's post-refresh hint assertion failed deterministically). Route all install paths through an OOMProtectedVMWorkload subclass that runs ensure_snap_oom_protection before the snap ensure, delete the duplicated _install_snap_package helper, and retarget the install unit tests to the unified seam.
The library's VMWorkload now configures the snap resilience vitality-hint on every snap-install path, so the charm-side OOMProtectedVMWorkload subclass, the oom module, its constants, and their tests move to the library (single-kernel PR #273).
Consume the capability-gated shared_preload_libraries fix from the lib's draft PR head (canonical/postgresql-single-kernel-library#354, ce1a35ca) via the commit archive URL instead of the released 16.3.14. Resolves to 16.3.15 (unreleased). Reverts to a released pin once the lib PR lands and publishes.
On CI runners force-refresh-start can take ~18 minutes end-to-end (~7.5 min snap fetch from the store on a cold runner plus ~10 min post-refresh replication health settle). The charm flow itself completes; the 600s/900s action budgets just expire mid-settle and fail the test. Bump force-refresh-start and resume-refresh waits to 30 minutes in the upgrade test files.
…f racing it The post-refresh wait accepted is_active OR is_blocked, so it exited ~2s after the origin flip - while the charm was still active and before charm_refresh's compatibility check ran. The test then read is_blocked == False and skipped force-refresh-start entirely; the "Refresh incompatible" block landed seconds later and the final active+idle wait could never succeed, burning its full 1200s budget (observed on every local-charm run of this branch: zero force-refresh-start calls in the captured CLI log, app blocked for the entire window). Every CI-packed charm is unreleased (...postN.dev0+gSHA), so the block always arrives - wait for it specifically, then force with check-compatibility False. Give the forced flow a 30-minute budget (snap fetch ~7.5 min + replication settle ~10 min on cold runners; the previous 1200s expired mid-settle). Assert the snap revision actually advanced, so the test can no longer pass vacuously while the refresh never progressed (as it did on the base branch).
units is dict[str, UnitStatus]: iterating it yields the unit-name keys,
so the pre-revision capture crashed with AttributeError ('str' object
has no attribute 'workload_status') at test start - before any refresh
step - on both arches in CI. Take the UnitStatus from units.values()
(matching the post-refresh capture) and keep the name key for the
juju.run unit argument.
The terraform module deploys from 16/stable by default, and the 16/stable charm's workload message does not advertise the snap revision (e.g. just "Primary"), so the strict pre-capture regex missed and raised AttributeError at test start. Parse both revisions optionally: compare the progression when the deployed charm reports one, and rely on the forced refresh completing (which only returns once the pinned snap is installed and replication is healthy) plus the refreshed unit's own revision report otherwise.
The workload message content varies by charm version (16/stable reports just "Primary", the refreshed charm reported an empty message in the VM run), so parsing it is fragile: the strict post-capture raised AttributeError on an empty message after the forced refresh had already succeeded. The flow's progression is guaranteed without it: the block-wait only succeeds once charm_refresh blocked as incompatible, force-refresh-start only returns after the pinned snap is installed and replication is healthy, and a refresh that never progressed stays blocked and fails the waits.
The pg_cron-gate fix (PSKL #354) is merged and published on PyPI as 16.3.15, so the plain released pin replaces the ce1a35ca archive URL (superset convention: 16.3.15 > 16.3.12, contains the refresh module, early-publish fix, OOM protection and the capability-gated shared_preload_libraries fix).
Lib 16.3.15's CharmConfig declares plugin_pg_cron_enable as required; the option rides the generic plugin-option path and ships disabled by default. (§4 ruling of the handoff: minimal #2028 subset, exposure only, no handler code or tests.)
marceloneppel
added
the
not bug or enhancement
github-actions
Bot
added
the
Libraries: Out of sync
Without a snap-revision delta the local-charm refresh is charm-code-only, so charm_refresh skips the compatibility check and never raises the Refresh incompatible block the terraform refresh test waits for (the deployed 16/stable charm pins the same snap revision this charm would).
This branch has not been deployed
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
1 participant
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
Part of the refresh module migration into the single kernel library (canonical/postgresql-single-kernel-library).
The upgrade-family integration tests were deadlocking in a charm crash loop on refresh, and the terraform local-charm refresh tests raced the incompatible-refresh block. Both are fixed on the stacked branch (#1942 + #2031), but that stack sits on
add-pg-cron(#2028). This PR brings the same content to16/edgedirectly, so it can merge independently of #2028 — #2028's pg_cron work is intentionally not included (the single exception, see Solution, is one config option the library itself requires).Solution
Combines VM PRs #1942 (refresh-module migration) and #2031 (terraform local-charm race fix), replayed commit-for-commit onto
16/edge(18/18 kept, all re-signed):RefreshManagerinstead of owning its own refresh plumbing: charm-specific refresh helpers deleted (now in the library),RefreshManager.on_init()replaces the post-init resume block,set_unit_statusdelegates to the manager, and not-yet-migrated side effects move behind duck-typed bridge methods.shared_preload_librarieson what the installed snap ships and unblocks the crash loop.poetry.lockregenerated with the repo's Poetry 2.5.1; the diff is scoped to the library entry.force-refresh-start/resume-refreshwait budgets raised to 30 minutes (the refresh action can take ~18 min end-to-end on CI runners).tests/integration/terraform/test_terraform_deploy.py: the post-refresh wait requires theRefresh incompatibleblock specifically (unconditionalforce-refresh-startwith a 30-minute budget); the workload-message snap-revision assertion is dropped (the message format varies by charm version).config.yaml: exposesplugin-pg-cron-enable(boolean, defaultfalse) — the library'sCharmConfigcontract requires it at 16.3.15. This is the minimal subset of [DPE-11290] Add pg_cron configuration and tests #2028 the library needs (exposure only, no handler code, no tests); every other option already exists on16/edge.refresh_versions.toml: snap pins bumped 416/415 → 428/427 (the bump rides in [DPE-11290] Add pg_cron configuration and tests #2028's single commit). Without a snap-revision delta the local-charm refresh is charm-code-only andcharm_refreshskips the compatibility check, so the terraform local-charm refresh test would never see theRefresh incompatibleblock it waits for. The 16/edge snap channel already serves 428.Unit suite: 150/150 green with the pin; lint (
ruff check,ruff format --check) clean.Checklist