Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ops to v2.14 #273

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 12 additions & 51 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ authors = []

[tool.poetry.dependencies]
python = "^3.10"
ops = "^2.6.0,<2.10.0"
ops = "^2.14.0"
lightkube = "^0.14.0"
tenacity = "^8.2.3"
jinja2 = "^3.1.2"
Expand Down Expand Up @@ -56,8 +56,8 @@ pytest = "^7.4.0"
pytest-xdist = "^3.3.1"
pytest-forked = "^1.6.0"
pytest-cov = "^4.1.0"
ops-scenario = "^5.6.2"
ops = "<2.10.0"
ops-scenario = "^6.1.1"
ops = "^2.14.0"
pytest-mock = "^3.11.1"

[tool.poetry.group.integration.dependencies]
Expand All @@ -69,7 +69,7 @@ juju = "^3.2.2"
mysql-connector-python = "~8.0.33"
pyyaml = "^6.0.1"
tenacity = "^8.2.2"
ops = "<2.10.0"
ops = "^2.14.0"
allure-pytest = "^2.13.2"
allure-pytest-collection-report = {git = "https://github.com/canonical/data-platform-workflows", tag = "v16.2.0", subdirectory = "python/pytest_plugins/allure_pytest_collection_report"}

Expand Down
2 changes: 1 addition & 1 deletion src/abstract_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ def reconcile(self, event=None) -> None: # noqa: C901
logger.warning(
"Modifying relations during an upgrade is not supported. The charm may be in a broken, unrecoverable state. Re-deploy the charm"
)
self._database_provides.delete_all_databags()
self._database_provides.delete_all_databags(event)
elif (
not self._upgrade.in_progress
and isinstance(workload_, workload.AuthenticatedWorkload)
Expand Down
35 changes: 24 additions & 11 deletions src/relations/database_provides.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,24 @@ def __init__(self, charm_: "abstract_charm.MySQLRouterCharm") -> None:
charm_.framework.observe(self._interface.on.database_requested, charm_.reconcile)
charm_.framework.observe(charm_.on[self._NAME].relation_broken, charm_.reconcile)

# TODO python3.10 min version: Use `list` instead of `typing.List`
def _relations(self, event) -> typing.List[ops.Relation]:
"""All relations, including relations that are breaking

Needed because of breaking change in ops 2.10
https://github.com/canonical/operator/pull/1091#issuecomment-1888644075

May break during deferred ops events or collect status ops events
(https://github.com/canonical/operator/pull/1091#issuecomment-2191460188)
"""
if isinstance(event, ops.RelationBrokenEvent) and event.relation.name == self._NAME:
return [*self._interface.relations, event.relation]
return self._interface.relations

def external_connectivity(self, event) -> bool:
"""Whether any of the relations are marked as external."""
requested_users = []
for relation in self._interface.relations:
for relation in self._relations(event):
try:
requested_users.append(
_RelationThatRequestedUser(
Expand All @@ -200,11 +214,10 @@ def external_connectivity(self, event) -> bool:
pass
return any(relation.external_connectivity for relation in requested_users)

@property
# TODO python3.10 min version: Use `list` instead of `typing.List`
def _shared_users(self) -> typing.List[_RelationWithSharedUser]:
def _shared_users(self, event) -> typing.List[_RelationWithSharedUser]:
shared_users = []
for relation in self._interface.relations:
for relation in self._relations(event):
try:
shared_users.append(
_RelationWithSharedUser(relation=relation, interface=self._interface)
Expand Down Expand Up @@ -234,7 +247,7 @@ def reconcile_users(
f"{exposed_read_write_endpoint=}, {exposed_read_only_endpoint=}"
)
requested_users = []
for relation in self._interface.relations:
for relation in self._relations(event):
try:
requested_users.append(
_RelationThatRequestedUser(
Expand All @@ -247,25 +260,25 @@ def reconcile_users(
_UnsupportedExtraUserRole,
):
pass
logger.debug(f"State of reconcile users {requested_users=}, {self._shared_users=}")
logger.debug(f"State of reconcile users {requested_users=}, {self._shared_users(event)=}")
for relation in requested_users:
if relation not in self._shared_users:
if relation not in self._shared_users(event):
relation.create_database_and_user(
router_read_write_endpoint=router_read_write_endpoint,
router_read_only_endpoint=router_read_only_endpoint,
exposed_read_write_endpoint=exposed_read_write_endpoint,
exposed_read_only_endpoint=exposed_read_only_endpoint,
shell=shell,
)
for relation in self._shared_users:
for relation in self._shared_users(event):
if relation not in requested_users:
relation.delete_user(shell=shell)
logger.debug(
f"Reconciled users {event=}, {router_read_write_endpoint=}, {router_read_only_endpoint=}, "
f"{exposed_read_write_endpoint=}, {exposed_read_only_endpoint=}"
)

def delete_all_databags(self) -> None:
def delete_all_databags(self, event) -> None:
"""Remove connection information from all databags.

Called when relation with MySQL is breaking
Expand All @@ -274,7 +287,7 @@ def delete_all_databags(self) -> None:
will need to be created.
"""
logger.debug("Deleting all application databags")
for relation in self._shared_users:
for relation in self._shared_users(event):
# MySQL charm will delete user; just delete databag
relation.delete_databag()
logger.debug("Deleted all application databags")
Expand All @@ -288,7 +301,7 @@ def get_status(self, event) -> typing.Optional[ops.StatusBase]:
)
# TODO python3.10 min version: Use `list` instead of `typing.List`
exceptions: typing.List[status_exception.StatusException] = []
for relation in self._interface.relations:
for relation in self._relations(event):
try:
requested_users.append(
_RelationThatRequestedUser(
Expand Down
13 changes: 12 additions & 1 deletion src/relations/database_requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,18 @@ class CompleteConnectionInformation(ConnectionInformation):
"""

def __init__(self, *, interface: data_interfaces.DatabaseRequires, event) -> None:
relations = interface.relations
# Needed because of breaking change in ops 2.10
# https://github.com/canonical/operator/pull/1091#issuecomment-1888644075
# May break during deferred ops events or collect status ops events
# (https://github.com/canonical/operator/pull/1091#issuecomment-2191460188)
if (
isinstance(event, ops.RelationBrokenEvent)
and event.relation.name == interface.relation_name
):
relations = [*interface.relations, event.relation]
else:
relations = interface.relations

endpoint_name = interface.relation_name
if not relations:
raise _MissingRelation(endpoint_name=endpoint_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_breaking_requires_and_complete_provides(
secret = scenario.Secret(
id="foo",
contents={0: {"username": "foouser", "password": "foobar"}},
owner="application",
owner="app",
)
relation = relation.replace(
local_app_data={
Expand Down Expand Up @@ -91,7 +91,7 @@ def test_complete_requires_and_breaking_provides(
secret = scenario.Secret(
id=f"foo-{relation.relation_id}",
contents={0: {"username": "foouser", "password": "foobar"}},
owner="application",
owner="app",
)
relation = relation.replace(
local_app_data={
Expand Down