draft: fix machine refresh - #447
Draft
paulomach wants to merge 3 commits into
Draft
Conversation
`test_upgrade.py` fails consistently on 8.4/edge: the first unit to refresh ends up in `hook failed: "upgrade-charm"` and retries forever, so the snap is never refreshed and the test times out before `resume-refresh` can run. `charm_refresh.Machines()` calls `refresh_snap()` from its own `__init__`, which runs on the right-hand side of `self._refresh = charm_refresh.Machines(...)` in `MySQLOperatorCharm.__init__`. `refresh_snap()` therefore executes before `self._refresh` exists, and both of its statements raise `AttributeError`: - `set_unit_status()` reads `self._refresh` -> "'MySQLOperatorCharm' object has no attribute '_refresh'" - `self._charm.install_and_configure_mysql_dependencies(...)` -> that method is a `MySQL` staticmethod in `mysql_vm_helpers`, not a charm attribute Take the refresh instance as an explicit keyword argument in `set_unit_status()` (mirroring the PostgreSQL charm) so callers running during construction can pass it, and fall back to `getattr` for any other early call. Call the installer on `MySQL`. Also reconcile the unit after the snap is installed. `_on_update_status()` and `_on_config_changed()` both return early while a refresh is in progress, so nothing else starts mysqld, rejoins the cluster, or clears the maintenance status for the refreshing unit — and the refresh only completes once the unit reports healthy. `_post_snap_refresh()` closes that loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_refresh_from_edge` never ran `force-refresh-start` or `resume-refresh`, so every refresh stalled until the final 20-minute `all_active` wait failed. `jubilant.all_blocked` requires the app *and every unit* to be blocked, but charm_refresh only sets the app status while a refresh is in progress or incompatible — the units stay active. The wait therefore always timed out, the `except TimeoutError` branch logged the misleading "Refresh completed without snap refresh (Python code only)", and both the incompatibility workaround and `resume-refresh` were skipped. Since `pause-after-unit-refresh` defaults to `first`, `resume-refresh` is mandatory, so the refresh could never complete. Wait on the app status alone and drop the `try`/`except`, so a refresh that does not start is reported as a failure of the wait it actually failed. This is what the last run tripped over: a locally built charm is not tagged, so its charm version is a dev version (`8.4/1.80.0.post2.dev0+a18ec2d0c`) and `charm_refresh._is_charm_version_compatible` rejects any unreleased version. `force-refresh-start` with `check-compatibility=false` is the intended escape hatch and is now reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`CharmSpecificMachines.refresh_snap()` installed the new snap revision but
never called `charm_refresh.Machines.update_snap_revision()`. The library
requires that call before `next_unit_allowed_to_refresh` may be set, so the
unconditional assignment in `MySQLOperatorCharm.__init__` raised:
Exception: Must call `update_snap_revision()` before setting
`next_unit_allowed_to_refresh = True`
The refresh itself succeeded (unit rejoined the cluster as SECONDARY), but the
hook crashed at the end of it, failing the `force-refresh-start` action and
therefore the upgrade integration test.
While here, gate `next_unit_allowed_to_refresh` on the unit actually being
healthy again, matching the PostgreSQL charm: post-snap-refresh work (start
mysqld, rejoin the cluster, publish member state) moves out of `refresh_snap`
into `MySQLOperatorCharm._post_snap_refresh`, driven from `__init__` because
both `_on_update_status` and `_on_config_changed` bail out while a refresh is
in progress. The next unit is only unblocked on the success path, so a broken
refresh no longer rolls forward across the whole application.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
test_upgrade.pyfails consistently on 8.4/edge: the first unit to refreshends up in
hook failed: "upgrade-charm"and retries forever, so the snap isnever refreshed and the test times out before
resume-refreshcan run.charm_refresh.Machines()callsrefresh_snap()from its own__init__,which runs on the right-hand side of
self._refresh = charm_refresh.Machines(...)in
MySQLOperatorCharm.__init__.refresh_snap()therefore executes beforeself._refreshexists, and both of its statements raiseAttributeError:set_unit_status()readsself._refresh-> "'MySQLOperatorCharm' object hasno attribute '_refresh'"
self._charm.install_and_configure_mysql_dependencies(...)-> that method isa
MySQLstaticmethod inmysql_vm_helpers, not a charm attributeTake the refresh instance as an explicit keyword argument in
set_unit_status()(mirroring the PostgreSQL charm) so callers running duringconstruction can pass it, and fall back to
getattrfor any other early call.Call the installer on
MySQL.Also reconcile the unit after the snap is installed.
_on_update_status()and_on_config_changed()both return early while a refresh is in progress, sonothing else starts mysqld, rejoins the cluster, or clears the maintenance
status for the refreshing unit — and the refresh only completes once the unit
reports healthy.
_post_snap_refresh()closes that loop.Co-Authored-By: Claude Opus 5 noreply@anthropic.com