Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
6 changes: 6 additions & 0 deletions rollingops/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 1.1.5 - 02 October 2026

Fix:
- Publish the etcd request for an existing relation once the `cluster_id` becomes
available, instead of requiring the relation to be recreated.

# 1.1.4 - 24 September 2026

Bump dependencies: notably `dpcharmlibs-interfaces` to get the cross-model
Expand Down
15 changes: 13 additions & 2 deletions rollingops/src/charmlibs/rollingops/_etcd/_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ def __init__(
peer_relation_name=peer_relation_name,
base_dir=self._base_dir,
)
# Must happen before constructing EtcdRequiresV1 below: it builds its
# request (including the client certificate)
self.shared_certificates.create_and_share_certificate()

self.etcd = EtcdRequiresV1(
charm,
Expand All @@ -111,6 +114,11 @@ def __init__(
shared_certificates=self.shared_certificates,
base_dir=self._base_dir,
)
# If the etcd relation already existed before the cluster_id became available,
# the relation-created event that would normally publish the request has already
# fired and been missed. Ensure the request still gets published for that existing
# relation now, without requiring the relation to be recreated.
self.etcd.ensure_request_published()
self._async_lock = EtcdLock(
lock_key=self.keys.lock_key,
owner=owner,
Expand Down Expand Up @@ -155,7 +163,8 @@ def is_available(self) -> bool:
return False
try:
self.etcdctl.ensure_initialized()
except Exception:
except Exception as e:
logger.debug('etcd backend unavailable: %s', e)
return False
return True

Expand Down Expand Up @@ -301,7 +310,9 @@ def _on_run_with_lock(self) -> _RunWithLockOutcome:
result=OperationResult.RELEASE,
)
logger.info(
'Executing callback_id=%s, attempt=%s', operation.callback_id, operation.attempt
'Executing callback_id=%s, attempt=%s on etcd backend',
operation.callback_id,
operation.attempt,
)

try:
Expand Down
41 changes: 40 additions & 1 deletion rollingops/src/charmlibs/rollingops/_etcd/_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@
LeaderElectedEvent,
RelationBrokenEvent,
RelationChangedEvent,
RelationCreatedEvent,
SecretChangedEvent,
)
from ops.framework import Object
from ops.framework import Handle, Object
from ops.model import ModelError

from charmlibs import pathops
Expand Down Expand Up @@ -235,6 +236,44 @@ def etcd_relation(self) -> Relation | None:
relations = self.etcd_interface.relations
return relations[0] if relations else None

def ensure_request_published(self) -> None:
"""Publish this unit's etcd request for an already existing relation, if needed.

``self.etcd_interface`` (the requirer-side event handler) only publishes the request
to the relation databag on ``relation-created`` hook. If the ``cluster_id`` only becomes
available on a later event, that one-time hook has already run and the request was never
sent, even though the etcd relation already exists, leaving rollingops stuck on the peer
backend.

This calls the handler's own relation-created logic directly.

This is a no-op once a request has already been written for this
relation, so the handler is only ever replayed once.
"""
relation = self.etcd_relation
if relation is None:
return

if not self.charm.unit.is_leader():
return

if relation.data[self.charm.app].get('requests'):
# A request was already published for this relation, either by the
# real relation-created event, or by a previous call to this method.
return

logger.info(
'Replaying missed etcd rollingops relation-created handling for '
'cluster_id=%s on relation %s/%s.',
self.cluster_id,
relation.name,
relation.id,
)
event = RelationCreatedEvent(
Handle(self.etcd_interface, 'relation_created', None), relation, app=relation.app
)
self.etcd_interface._on_relation_created_event(event)

def _on_relation_broken(self, event: RelationBrokenEvent) -> None:
"""Remove the stored information about the etcd server."""
self.etcdctl.cleanup()
Expand Down
2 changes: 1 addition & 1 deletion rollingops/src/charmlibs/rollingops/_etcd/_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def _pid(self) -> int | None:
try:
pid = int(pid)
except (ValueError, TypeError):
logger.info('Missing PID or invalid PID found in etcd worker state.')
logger.debug('Missing PID or invalid PID found in etcd worker state.')
pid = None

return pid
Expand Down
4 changes: 3 additions & 1 deletion rollingops/src/charmlibs/rollingops/_peer/_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,9 @@ def _on_run_with_lock(self) -> None:
operations.finish(OperationResult.RELEASE)
return
logger.info(
'Executing callback_id=%s, attempt=%s', operation.callback_id, operation.attempt
'Executing callback_id=%s, attempt=%s on peer backend',
operation.callback_id,
operation.attempt,
)
try:
result = callback(**operation.kwargs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def __init__(
logger.debug('No etcd relation configured. Using peer backend only.')

elif not cluster_id:
logger.info(
logger.debug(
'Etcd relation configured but no cluster_id yet. '
'Using peer backend until cluster_id provided.'
)
Expand Down Expand Up @@ -168,7 +168,6 @@ def __init__(
callback_targets=callback_targets,
base_dir=base_dir,
)
self._etcd_backend.shared_certificates.create_and_share_certificate()

self.framework.observe(charm.on.rollingops_lock_granted, self._on_rollingops_lock_granted)
self.framework.observe(charm.on.rollingops_etcd_failed, self._on_rollingops_etcd_failed)
Expand Down Expand Up @@ -317,7 +316,6 @@ def _on_rollingops_lock_granted(self, event: _RollingOpsLockGrantedEvent) -> Non
logger.error('Peer relation does not exists. Cannot run lock granted.')
return
if self._backend_state.is_peer_managed():
logger.info('Executing rollingop on peer backend.')
self._peer_backend._on_rollingops_lock_granted(event)
return
self._run_etcd_and_mirror_or_fallback()
Expand All @@ -338,7 +336,6 @@ def _run_etcd_and_mirror_or_fallback(self) -> None:
return

try:
logger.info('Executing rollingop on etcd backend.')
outcome = self._etcd_backend._on_run_with_lock()
except Exception as e:
logger.warning(
Expand Down
2 changes: 1 addition & 1 deletion rollingops/src/charmlibs/rollingops/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
# See the License for the specific language governing permissions and
# limitations under the License.

__version__ = '1.1.4'
__version__ = '1.1.5'
8 changes: 8 additions & 0 deletions rollingops/tests/integration/charms/actions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,11 @@ sync-restart:
description: "Time (seconds) to wait before giving up."
type: integer
default: 60

toggle-cluster-id:
description: >-
Toggles the etcd cluster_id between a fixed value (the default) and None,
for manually testing/reproducing the race where the etcd relation exists
while cluster_id is None, and the cluster_id only becomes available
afterwards. Takes effect starting with the next hook after this action
completes.
36 changes: 35 additions & 1 deletion rollingops/tests/integration/charms/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import time
from typing import Any

import ops
from ops import ActionEvent, CharmBase, Framework
from ops.model import ActiveStatus, MaintenanceStatus, WaitingStatus

Expand All @@ -37,6 +38,12 @@

TRACE_FILE = pathops.LocalPath('/var/lib/charm-rolling-ops/transitions.log')

# Default cluster_id, used except while the toggle-cluster-id action has
# cleared it to None. The actual value doesn't matter for manual testing;
# only whether it is None (etcd setup skipped) or set (etcd setup attempted)
# matters.
CLUSTER_ID = 'cluster-12345'


def _now_timestamp_str() -> str:
"""UTC timestamp as a epoch."""
Expand All @@ -54,8 +61,12 @@ def release(self) -> None:
class Charm(CharmBase):
"""Charm the service."""

_stored = ops.StoredState()

def __init__(self, framework: Framework):
super().__init__(framework)
self._stored.set_default(cluster_id=CLUSTER_ID)

callback_targets = {
'_restart': self._restart,
'_failed_restart': self._failed_restart,
Expand All @@ -67,7 +78,7 @@ def __init__(self, framework: Framework):
charm=self,
peer_relation_name='restart',
etcd_relation_name='etcd',
cluster_id='cluster-12345',
cluster_id=self._stored.cluster_id,
callback_targets=callback_targets,
sync_lock_targets={
'stop': sync_backend,
Expand All @@ -78,6 +89,7 @@ def __init__(self, framework: Framework):
self.framework.observe(self.on.failed_restart_action, self._on_failed_restart_action)
self.framework.observe(self.on.deferred_restart_action, self._on_deferred_restart_action)
self.framework.observe(self.on.sync_restart_action, self._on_sync_restart_action)
self.framework.observe(self.on.toggle_cluster_id_action, self._on_toggle_cluster_id_action)

def _restart(self, delay: int = 0) -> None:
self._record_transition('_restart:start', delay=delay)
Expand Down Expand Up @@ -153,6 +165,28 @@ def _on_sync_restart_action(self, event: ActionEvent):
self._record_transition('_sync_restart:timeout', delay=delay, timeout=timeout)
event.fail('Timed out acquiring sync lock')

def _on_toggle_cluster_id_action(self, event: ActionEvent) -> None:
"""Toggle the etcd cluster_id between a fixed value and None.

The charm starts with cluster_id set. Run this action to clear it
(reproducing the race where the etcd relation exists but cluster_id
is None), then run it again to restore it and trigger the fix.

The charm's __init__ runs once per hook, so the new value only takes
effect for ``self.restart_manager`` starting with the *next* hook
(any hook) after this action completes -- it is not applied within
this same action's execution.
"""
if self._stored.cluster_id:
self._stored.cluster_id = None
else:
self._stored.cluster_id = CLUSTER_ID

new_value = self._stored.cluster_id
self._record_transition('action:toggle-cluster-id', cluster_id=new_value)
logger.info('cluster_id is now %s (takes effect on the next hook).', new_value)
event.set_results({'cluster-id': new_value or ''})

def _record_transition(self, name: str, **data: Any) -> None:
TRACE_FILE.parent.mkdir(parents=True, exist_ok=True)
state = self.restart_manager.state
Expand Down
Loading
Loading