[DPE-4799] Fix upgrade bugs when switching charms #268

dragomirp · 2024-06-27T20:57:13Z

Fixing new snap install failure due to writing to SNAP_DATA prematurely
Fixing upgrade not triggering on a single unit

Lib changes should be upstreamed

Fixes #267

codecov · 2024-06-27T20:58:47Z

Codecov Report

Attention: Patch coverage is 56.25000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.96%. Comparing base (6c83f6e) to head (a95f137).

Files	Patch %	Lines
src/charm.py	50.00%	5 Missing ⚠️
src/upgrade.py	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #268      +/-   ##
==========================================
- Coverage   71.06%   70.96%   -0.11%     
==========================================
  Files           7        7              
  Lines        1182     1195      +13     
  Branches      209      210       +1     
==========================================
+ Hits          840      848       +8     
- Misses        264      269       +5     
  Partials       78       78

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dragomirp · 2024-06-28T01:45:12Z

lib/charms/data_platform_libs/v0/upgrade.py

@@ -921,7 +921,7 @@ def _on_upgrade_charm(self, event: UpgradeCharmEvent) -> None:
            self.charm.unit.status = WaitingStatus("other units upgrading first...")
            self.peer_relation.data[self.charm.unit].update({"state": "ready"})

-            if self.charm.app.planned_units() == 1:
+            if len(self.app_units) == 1:


Planned units don't work on subordinate charms

I do not see PR for LIBPATCH = 18: https://github.com/canonical/data-platform-libs/pulls
You can shoot your leg.

We need subordinate test on dpe-lib... as LIBPATCH=17 should notice this regression.

CC: @marceloneppel : canonical/data-platform-libs#176

Proposal: copy tests from data-integrator to dpe-libs? Yeah, yeah... shared tests...

I do not see PR for LIBPATCH = 18: https://github.com/canonical/data-platform-libs/pulls You can shoot your leg.

Let's make sure the changes work here first. Rollback doesn't seem to work anymore in this PR.

Started a draft for the lib changes: canonical/data-platform-libs#179

However, there are issues with CI there and it will take me time to fix them.

I executed some tests. All passed, as I commented on canonical/data-platform-libs#179 (review).

dragomirp · 2024-06-28T01:46:45Z

src/charm.py

Defer early exit events that may trigger cfg reload and dir creation during upgrade

dragomirp · 2024-06-28T01:48:30Z

src/charm.py

+        if not self.upgrade.idle:
+            logger.debug("Early exit on_update_status: Cluster is upgrading")
+            return
+


Early exit update status, since it will retrigger eventually. Not strictly necessary, but if upgrade is stuck or failed it will overwrite the status.

dragomirp · 2024-06-28T01:49:58Z

tests/integration/test_upgrade_data_integrator.py

+    logger.info("Wait for upgrade to start")
+    await ops_test.model.block_until(
+        lambda: ("waiting" if "pre-upgrade-check" in actions else "maintenance")
+        in {unit.workload_status for unit in application.units},
+        timeout=TIMEOUT,
+    )


Lambda didn't work for me locally, when upgrading a single unit.

taurus-forever · 2024-06-28T10:30:02Z

lib/charms/data_platform_libs/v0/upgrade.py

@@ -921,7 +921,7 @@ def _on_upgrade_charm(self, event: UpgradeCharmEvent) -> None:
            self.charm.unit.status = WaitingStatus("other units upgrading first...")
            self.peer_relation.data[self.charm.unit].update({"state": "ready"})

-            if self.charm.app.planned_units() == 1:
+            if len(self.app_units) == 1:


I do not see PR for LIBPATCH = 18: https://github.com/canonical/data-platform-libs/pulls
You can shoot your leg.

We need subordinate test on dpe-lib... as LIBPATCH=17 should notice this regression.

CC: @marceloneppel : canonical/data-platform-libs#176

Proposal: copy tests from data-integrator to dpe-libs? Yeah, yeah... shared tests...

lucasgameiroborges

I see all 3 test_upgrade.py integration tests failing in (apparently) the same way. Is it expected / known issue?

dragomirp · 2024-06-28T14:12:11Z

I see all 3 test_upgrade.py integration tests failing in (apparently) the same way. Is it expected / known issue?

Rollback doesn't work. Investigating ATM.

dragomirp · 2024-06-28T16:08:38Z

Rollback test was broken. Should be fixed now.

dragomirp · 2024-06-28T16:09:32Z

tests/integration/test_upgrade.py

@@ -152,7 +152,7 @@ async def test_fail_and_rollback(ops_test, continuous_writes, pgb_charm_jammy) -
    application = ops_test.model.applications[PGB]

    logger.info("Refresh the charm")
-    await application.refresh(path=pgb_charm_jammy)
+    await application.refresh(path=fault_charm)


We weren't using the correct charm and hitting other blocked statuses.

marceloneppel

LGTM!

marceloneppel · 2024-06-28T18:59:56Z

lib/charms/data_platform_libs/v0/upgrade.py

@@ -921,7 +921,7 @@ def _on_upgrade_charm(self, event: UpgradeCharmEvent) -> None:
            self.charm.unit.status = WaitingStatus("other units upgrading first...")
            self.peer_relation.data[self.charm.unit].update({"state": "ready"})

-            if self.charm.app.planned_units() == 1:
+            if len(self.app_units) == 1:


I executed some tests. All passed, as I commented on canonical/data-platform-libs#179 (review).

dragomirp · 2024-06-28T19:27:50Z

Merging this for the sake of fixing upgrades, I will sync up lib later if it diverges.

Suppress rendering hooks when upgrading

b1e241b

github-actions bot added the Libraries: OK label Jun 27, 2024

dragomirp added 2 commits June 28, 2024 00:10

Render auth file during upgrade

b7e628e

Add data integrator upgrade test

5589420

dragomirp changed the title ~~[DPE-4799] Suppress rendering hooks when upgrading~~ [DPE-4799] Fix upgrade bugs when switching charms Jun 27, 2024

dragomirp added 2 commits June 28, 2024 01:19

Fix data integrator upgrade test

4c80b2c

Jimmyrig the upgrade lib

1c914cf

github-actions bot added Libraries: Out of sync and removed Libraries: OK labels Jun 28, 2024

Upgrade two data-integrator subordinates

dfdf44a

dragomirp commented Jun 28, 2024

View reviewed changes

src/charm.py

Copy link

Contributor Author

dragomirp Jun 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defer early exit events that may trigger cfg reload and dir creation during upgrade

dragomirp commented Jun 28, 2024

View reviewed changes

dragomirp marked this pull request as ready for review June 28, 2024 02:04

dragomirp requested review from taurus-forever, marceloneppel and lucasgameiroborges June 28, 2024 02:04

taurus-forever approved these changes Jun 28, 2024

View reviewed changes

dragomirp added 2 commits June 28, 2024 14:19

Missig wait in rollback test

d2b8b91

Merge branch 'main' into dpe-4799-snap-switch-upgrade

2d2cf85

lucasgameiroborges reviewed Jun 28, 2024

View reviewed changes

Actually use the fault injection charm

a95f137

dragomirp requested a review from lucasgameiroborges June 28, 2024 16:08

dragomirp commented Jun 28, 2024

View reviewed changes

marceloneppel approved these changes Jun 28, 2024

View reviewed changes

lucasgameiroborges approved these changes Jun 28, 2024

View reviewed changes

dragomirp merged commit c379b0d into main Jun 28, 2024
52 checks passed

dragomirp deleted the dpe-4799-snap-switch-upgrade branch June 28, 2024 19:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DPE-4799] Fix upgrade bugs when switching charms #268

[DPE-4799] Fix upgrade bugs when switching charms #268

dragomirp commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading

dragomirp Jun 28, 2024

taurus-forever Jun 28, 2024

dragomirp Jun 28, 2024

dragomirp Jun 28, 2024

marceloneppel Jun 28, 2024

dragomirp Jun 28, 2024

dragomirp Jun 28, 2024

dragomirp Jun 28, 2024

taurus-forever Jun 28, 2024

lucasgameiroborges left a comment

dragomirp commented Jun 28, 2024

dragomirp commented Jun 28, 2024

dragomirp Jun 28, 2024

marceloneppel left a comment

marceloneppel Jun 28, 2024

dragomirp commented Jun 28, 2024

[DPE-4799] Fix upgrade bugs when switching charms #268

[DPE-4799] Fix upgrade bugs when switching charms #268

Conversation

dragomirp commented Jun 27, 2024 • edited Loading

codecov bot commented Jun 27, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasgameiroborges left a comment

Choose a reason for hiding this comment

dragomirp commented Jun 28, 2024

dragomirp commented Jun 28, 2024

Choose a reason for hiding this comment

marceloneppel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dragomirp commented Jun 28, 2024

dragomirp commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading