[DPE-9685] Release storage on scale-down#1848
Draft
marceloneppel wants to merge 14 commits into
Draft
Conversation
The charm implemented no removal hooks, so on unit teardown the charmed-postgresql snap services kept the Juju storage mounts busy. Juju's unmount then failed with "target is busy", leaving storage stuck detaching and blocking machine and model removal (only destroy-model --force could clear it). Stop the workload in the storage-detaching hook, which Juju runs before stop, so the mounts are free by the time Juju unmounts them. This stops every charmed-postgresql snap service plus the charm's topology-observer and log-rotation processes, and is idempotent across the per-storage detaching events. Fixes #1550. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…torage-on-teardown Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The storage-detaching regression only manifests on Juju 4.0: 3.6 masks it
with a cleanup_storage shortcut that removes still-Dying storage, and on
4.0 only rootfs (machine-scoped) storage reproduces the stuck unmount, so
running the teardown check on 3.6 proves nothing. Move it out of
test_storage.py into its own spread task pinned to a juju40 variant with
rootfs storage (force-deployed because the charm still declares
assumes: juju < 4).
Also make the list_storage adapter tolerate Juju 4.0's empty list-storage
output, which it prints instead of "{}" for a model with no storage.
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…torage-on-teardown # Conflicts: # src/cluster.py # tests/unit/test_cluster.py Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Stopping the workload in storage-detaching also fired on scale-down
(remove-unit), where the surviving leader still needs the departing
unit's Patroni reachable to remove it from the raft cluster. Stopping it
early broke that reconfiguration, leaving the cluster unable to elect a
primary ("Primary unit not found") and failing the HA/scaling
integration tests.
The storage unmount only actually hangs on full teardown
(remove-application/destroy-model), so guard the handler on
planned_units() == 0. On scale-down it now does nothing, restoring the
pre-fix cluster behaviour, while destroy-model/remove-application still
release the storage.
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Match the one-liner docstring density of the surrounding handlers instead of an 8-line block (the why lives in the commit that added the guard), and date the new test file 2026 (it was copied as 2025). Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
github-actions
Bot
added
the
Libraries: Out of sync
2 tasks
stop() leaves the snap services enabled, so a mid-teardown restart of the unit could re-enable them and re-grab the storage mounts before Juju finishes unmounting. stop(disable=True) prevents that. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The task ran only on juju40 (where #1550 reproduces). Add juju36 so the teardown change (stopping/disabling the snap in storage-detaching) is also exercised on 3.6, where the bug is masked by force-removal but a new breakage would still surface. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
On Juju 4.0 storage-detaching runs before the relation-departed and stop hooks and nothing else stops the snap, so a scaled-down unit's storage stayed mounted and Juju could not unmount it. The teardown fix only stops the snap when the whole app is going away, leaving single-unit removals leaked. Stopping the snap frees the storage, but once stopped the unit can no longer be removed from the pysyncobj raft cluster (the leader can no longer resolve its member IP), leaking a ghost member; a node also cannot remove itself from its own endpoint. So the departing unit drops itself from raft via a surviving peer before stopping. When several units are removed at once the first peer tried may itself be departing, so it iterates peers until a reachable node accepts the removal — on a minority scale-down a survivor always remains and quorum holds. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Only full-app teardown was exercised for #1550; the scale-down (remove-unit) path — where the departing unit must drop itself from raft via a peer before stopping — had no integration coverage. Add a remove-two-at-once scenario (minority removal, so quorum holds) that asserts the storage is released and the survivors stay active, reusing its deployment for the teardown test to avoid a second cluster. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
destroy_unit returns once the removal is queued; the storage detach/destroy finishes asynchronously on a separate provisioner, so a one-shot check right after wait_for_idle could catch a transient detaching state. Poll until no storage is stuck detaching (the #1550 symptom) — the timeout doubles as the failure signal for a snap-held mount that never unmounts. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…ent removal
Removing two units at once on Juju 4.0 can transiently error a departing unit on a canceled hook ('context canceled' as the uniter tears it down) and stall its removal, so the test's wait-for-2-survivors never settled on CI runners (it passed on a bare-metal box, so it is a timing race, not a logic bug). Resolve the departing unit's transient error and keep waiting; a survivor erroring or storage staying stuck still fails the test. The resolve is best-effort and only catches jubilant's CLIError (the unit changed state between the status check and the call).
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
The storage-detaching hook runs before relation-departed/stop on all Juju versions, not just 4.0; what is 4.0-specific is that 3.6 force-removes still-Dying storage (masking the stuck mount) while 4.0 leaves it to unmount. The comment conflated the two. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
marceloneppel
force-pushed
the
fix/release-storage-on-scale-down
branch
from
e490450 to
e4f68bc
Compare
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.
Stacked on #1827 (release storage on teardown); review/merge that first. This branch targets
fix/1550-release-storage-on-teardown, so its diff is the commits on top of that base.Issue
#1827 stops the snap only on full application teardown, so a scaled-down unit (
juju remove-unit) still left its storage mounted and Juju could not unmount it (target is busy). This covers the scale-down path for #1550.Solution
Stop the snap on scale-down too. A stopped unit can no longer be removed from the pysyncobj raft cluster (the leader can no longer resolve its member IP, leaking a ghost member), and a node cannot remove itself from its own endpoint — so the departing unit first drops itself from raft via a surviving peer's endpoint (
remote_address=), then stops. When several units are removed at once the first peer tried may itself be departing, so it iterates peers until a reachable node accepts the removal; on a minority scale-down a survivor always remains, so quorum holds. The teardown path is unchanged. Unit and integration tests cover the raft-removal fall-through and the concurrent 2-of-4 removal on thejuju40spread variant.Checklist