Skip to content

Commit

Permalink
Fix stuck reconciliation event (#231)
Browse files Browse the repository at this point in the history
* Move timeout call to front

* Remove list-timers

* Use status instead of is-active

* Lint
  • Loading branch information
cbartz authored Feb 29, 2024
1 parent 1110f2f commit 9105573
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 33 deletions.
8 changes: 4 additions & 4 deletions src-docs/event_timer.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Manages the timer to emit juju events at regular intervals.

- <b>`unit_name`</b> (str): Name of the juju unit to emit events to.

<a href="../src/event_timer.py#L63"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/event_timer.py#L62"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `__init__`

Expand All @@ -61,7 +61,7 @@ Construct the timer manager.

---

<a href="../src/event_timer.py#L154"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/event_timer.py#L147"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `disable_event_timer`

Expand All @@ -85,7 +85,7 @@ Disable the systemd timer for the given event.

---

<a href="../src/event_timer.py#L115"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/event_timer.py#L108"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `ensure_event_timer`

Expand Down Expand Up @@ -119,7 +119,7 @@ The timeout is the number of seconds before an event is timed out. If not set or

---

<a href="../src/event_timer.py#L86"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/event_timer.py#L85"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `is_active`

Expand Down
15 changes: 4 additions & 11 deletions src/event_timer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import jinja2

from utilities import execute_command
from utilities import logger as utilities_logger

BIN_SYSTEMCTL = "/usr/bin/systemctl"

Expand Down Expand Up @@ -96,21 +95,15 @@ def is_active(self, event_name: str) -> bool:
TimerStatusError: Timer status cannot be determined.
"""
try:
# We choose status over is-active here to provide debug logs that show the output of
# the timer.
_, ret_code = execute_command(
[BIN_SYSTEMCTL, "is-active", f"ghro.{event_name}.timer"], check_exit=False
[BIN_SYSTEMCTL, "status", f"ghro.{event_name}.timer"], check_exit=False
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as ex:
raise TimerStatusError from ex

if ret_code == 0:
return True

if utilities_logger.isEnabledFor(logging.DEBUG):
try:
execute_command([BIN_SYSTEMCTL, "list-timers", "--all"], check_exit=False)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as ex:
logger.exception("Unable to list systemd timers: %s", ex)
return False
return ret_code == 0

def ensure_event_timer(self, event_name: str, interval: int, timeout: Optional[int] = None):
"""Ensure that a systemd service and timer are registered to dispatch the given event.
Expand Down
2 changes: 1 addition & 1 deletion templates/dispatch-event.service.j2
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Description=Dispatch the {{event}} event on {{unit}}
[Service]
Type=oneshot
# For juju 3 and juju 2 compatibility. The juju-run binary was renamed to juju-exec for juju 3.
ExecStart=/usr/bin/bash -c '/usr/bin/juju-exec "{{unit}}" "JUJU_DISPATCH_PATH={{event}} timeout {{timeout}} ./dispatch" || /usr/bin/juju-run "{{unit}}" "JUJU_DISPATCH_PATH={{event}} timeout {{timeout}} ./dispatch"'
ExecStart=/usr/bin/timeout {{timeout}} /usr/bin/bash -c '/usr/bin/juju-exec "{{unit}}" "JUJU_DISPATCH_PATH={{event}} ./dispatch" || /usr/bin/juju-run "{{unit}}" "JUJU_DISPATCH_PATH={{event}} ./dispatch"'

[Install]
WantedBy=multi-user.target
17 changes: 0 additions & 17 deletions tests/unit/test_event_timer.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.
import logging
import secrets

from event_timer import EventTimer
Expand All @@ -26,19 +25,3 @@ def test_is_active_false(exec_command):

event = EventTimer(unit_name=secrets.token_hex(16))
assert not event.is_active(event_name=secrets.token_hex(16))


def test_is_active_false_list_timers(exec_command, caplog):
"""
arrange: create an EventTimer object and mock exec command to return non-zero exit code
and set log level to debug
act: call is_active
assert: list-timers is called
"""
exec_command.return_value = ("", 1)
caplog.set_level(logging.DEBUG)

event = EventTimer(unit_name=secrets.token_hex(16))
assert not event.is_active(event_name=secrets.token_hex(16))
assert exec_command.call_count == 2
assert "list-timers" in exec_command.call_args_list[1][0][0]

0 comments on commit 9105573

Please sign in to comment.