Skip to content

Conversation

dragomirp
Copy link
Contributor

@dragomirp dragomirp commented Jul 10, 2025

  • Set active status only when the charm is related to a provider charm
  • Re-enable LXD restart test

@dragomirp dragomirp marked this pull request as ready for review July 25, 2025 11:20
@dragomirp dragomirp requested review from a team, marceloneppel and taurus-forever and removed request for a team July 25, 2025 11:21
src/charm.py Outdated
"""Only sets an Active status."""
self.unit.status = ActiveStatus()
"""Sets initial Waiting status and checks if writes should be restarted."""
self.unit.status = WaitingStatus()
Copy link
Member

Choose a reason for hiding this comment

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

We should most probably replace it with Blocked status, as it needs a relation to be formed (based on the status's meaning from the figure from https://documentation.ubuntu.com/juju/3.6/reference/status/#application-status).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blocked status may require tweaks in downstream CIs, IIRC libjuju's wait for idle errors out by default on blocked status.

@dragomirp dragomirp requested a review from marceloneppel July 28, 2025 10:28
Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

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

LGTM, AS IS, but please explain the test skip + improve blocked status if possible. Tnx!

client = Client(namespace=ops_test.model.info.name)
client.delete(Pod, name=f"{TEST_APP_NAME}-0")
else:
pytest.skip("Unstable LXC restart test")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please mention this problem in PR description at least.
BTW. what is unstable in LXD restart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This removes the unstable marker for LXD added in #368 (comment). The test passes now, so it's no longer to be skipped.

src/charm.py Outdated
"""Only sets an Active status."""
self.unit.status = ActiveStatus()
"""Sets initial Waiting status and checks if writes should be restarted."""
self.unit.status = BlockedStatus("Waiting for integration")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for reacting on Marcelo comment about Waiting=>Blocked switch, but the Juju status message is still confusingly saying waiting, which may confuse end-users.

Let's hint them to act:
Integrate with the database provider (e.g. data-integrator)
or
No database relation available (e.g. sysbench charm)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed message to No database integration available.

@dragomirp dragomirp changed the title [MISC] Waiting status when database relation is not initialised [MISC] Blocked status when database relation is not initialised Jul 28, 2025
@dragomirp dragomirp merged commit 97fea08 into main Jul 28, 2025
30 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants