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

Add checks config fields and get_checks API #668

Merged
merged 33 commits into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
1ef5861
Add checks config fields and get_checks API
benhoyt Dec 1, 2021
e76ec56
Fix existing tests
benhoyt Dec 1, 2021
08ec4ff
Add tests for checks additions; fix a couple of issues
benhoyt Dec 2, 2021
38b365c
Make default for restart (int) 0 instead of None
benhoyt Dec 2, 2021
30722eb
Merge branch 'main' into auto-restart-and-checks
benhoyt Dec 6, 2021
f77f445
Merge branch 'main' into auto-restart-and-checks
benhoyt Dec 12, 2021
ef170f0
Type annotation tweaks from sed-i
benhoyt Dec 14, 2021
6c22f7c
Merge branch 'main' into auto-restart-and-checks
pengale Dec 17, 2021
9ea0764
Merge branch 'main' into auto-restart-and-checks
benhoyt Jan 5, 2022
879718f
Merge remote-tracking branch 'origin/auto-restart-and-checks' into au…
benhoyt Jan 5, 2022
ec60a5b
Update per changes to https://github.com/canonical/pebble/pull/86
benhoyt Feb 15, 2022
752dcee
Merge branch 'main' into auto-restart-and-checks
benhoyt Feb 15, 2022
cbfe6a0
Add real Pebble test for /v1/checks and /v1/health
benhoyt Feb 15, 2022
90792db
Fix lint issues
benhoyt Feb 15, 2022
b355ee0
Comment out real test_checks till that's merged in Pebble
benhoyt Feb 15, 2022
30c12ac
Oops, quiet linter again
benhoyt Feb 15, 2022
c1e9988
Re-enable commented-out "real Pebble" test
benhoyt Feb 21, 2022
a0e40ae
Merge branch 'main' into auto-restart-and-checks
benhoyt Feb 21, 2022
01287e0
Oops, add --http flag back to pebble run in "real Pebble" tests
benhoyt Feb 21, 2022
b454cce
Fix test_checks_and_health on Python 3.5 (json.load can't take bytes)
benhoyt Feb 21, 2022
15d96a8
Temp fix for canonical/grafana-k8s-operator build issue
benhoyt Feb 22, 2022
59ef681
Revert "Temp fix for canonical/grafana-k8s-operator build issue"
benhoyt Feb 23, 2022
6c8c557
Merge branch 'main' into auto-restart-and-checks
benhoyt Feb 23, 2022
d7900ea
Merge branch 'main' into auto-restart-and-checks
jnsgruk Mar 5, 2022
48fa314
expose the departing unit on RelationDeparted events (#712)
rwcarlsen Mar 5, 2022
8fc08db
Add better docstring to CheckInfo
benhoyt Mar 8, 2022
926e728
Comment test_checks_and_health
benhoyt Mar 8, 2022
42e92ce
Add link to layer spec to Plan and Layer docstrings
benhoyt Mar 8, 2022
2f75e04
Merge branch 'main' into auto-restart-and-checks
benhoyt Mar 8, 2022
68903f4
Comment tweaks/fixes
benhoyt Mar 8, 2022
8e689d4
Add missing model.Container.get_checks and .get_check methods
benhoyt Mar 8, 2022
d7dbd9f
Merge branch 'main' into auto-restart-and-checks
benhoyt Mar 10, 2022
c12ff4a
Merge branch 'main' into auto-restart-and-checks
jnsgruk Mar 25, 2022
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
188 changes: 177 additions & 11 deletions ops/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import binascii
import cgi
import copy
import datetime
import email.parser
import enum
Expand Down Expand Up @@ -518,6 +519,8 @@ def __init__(self, raw: str):
self._raw = raw
self._services = {name: Service(name, service)
for name, service in d.get('services', {}).items()}
self._checks = {name: Check(name, check)
for name, check in d.get('checks', {}).items()}

@property
def services(self):
Expand All @@ -527,14 +530,21 @@ def services(self):
"""
return self._services

@property
def checks(self):
"""This plan's checks mapping (maps check name to Check).

This property is currently read-only.
"""
return self._checks

def to_dict(self) -> typing.Dict[str, typing.Any]:
"""Convert this plan to its dict representation."""
as_dicts = {name: service.to_dict() for name, service in self._services.items()}
if not as_dicts:
return {}
return {
'services': as_dicts,
}
fields = [
('services', {name: service.to_dict() for name, service in self._services.items()}),
('checks', {name: check.to_dict() for name, check in self._checks.items()}),
]
return {name: value for name, value in fields if value}

def to_yaml(self) -> str:
"""Return this plan's YAML representation."""
Expand All @@ -552,7 +562,8 @@ class Layer:
Attributes:
summary: A summary of the purpose of this layer
description: A long form description of this layer
services: A mapping of name: :class:`Service` defined by this layer
services: A mapping of name to :class:`Service` defined by this layer
checks: A mapping of check to :class:`Check` defined by this layer
"""

# This is how you do type annotations, but it is not supported by Python 3.5
Expand All @@ -569,6 +580,8 @@ def __init__(self, raw: typing.Union[str, typing.Dict] = None):
self.description = d.get('description', '')
self.services = {name: Service(name, service)
for name, service in d.get('services', {}).items()}
self.checks = {name: Check(name, check)
for name, check in d.get('checks', {}).items()}

def to_yaml(self) -> str:
"""Convert this layer to its YAML representation."""
Expand All @@ -579,7 +592,8 @@ def to_dict(self) -> typing.Dict[str, typing.Any]:
fields = [
('summary', self.summary),
('description', self.description),
('services', {name: service.to_dict() for name, service in self.services.items()})
('services', {name: service.to_dict() for name, service in self.services.items()}),
('checks', {name: check.to_dict() for name, check in self.checks.items()}),
]
return {name: value for name, value in fields if value}

Expand Down Expand Up @@ -608,6 +622,12 @@ def __init__(self, name: str, raw: typing.Dict = None):
self.user_id = raw.get('user-id')
self.group = raw.get('group', '')
self.group_id = raw.get('group-id')
self.on_success = raw.get('on-success', '')
self.on_failure = raw.get('on-failure', '')
self.on_check_failure = dict(raw.get('on-check-failure', {}))
self.backoff_delay = raw.get('backoff-delay', '')
self.backoff_factor = raw.get('backoff-factor')
self.backoff_limit = raw.get('backoff-limit', '')

def to_dict(self) -> typing.Dict:
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
"""Convert this service object to its dict representation."""
Expand All @@ -625,6 +645,12 @@ def to_dict(self) -> typing.Dict:
('user-id', self.user_id),
('group', self.group),
('group-id', self.group_id),
('on-success', self.on_success),
('on-failure', self.on_failure),
('on-check-failure', self.on_check_failure),
('backoff-delay', self.backoff_delay),
('backoff-factor', self.backoff_factor),
('backoff-limit', self.backoff_limit),
]
return {name: value for name, value in fields if value}

Expand Down Expand Up @@ -666,10 +692,12 @@ def __init__(
name: str,
startup: typing.Union[ServiceStartup, str],
current: typing.Union[ServiceStatus, str],
restarts: int = 0,
):
self.name = name
self.startup = startup
self.current = current
self.restarts = restarts

def is_running(self) -> bool:
"""Return True if this service is running (in the active state)."""
Expand All @@ -690,16 +718,83 @@ def from_dict(cls, d: typing.Dict) -> 'ServiceInfo':
name=d['name'],
startup=startup,
current=current,
restarts=d.get('restarts', 0),
)

def __repr__(self):
return ('ServiceInfo('
'name={self.name!r}, '
'startup={self.startup}, '
'current={self.current})'
'current={self.current}, '
'restarts={self.restarts})'
).format(self=self)


class Check:
"""Represents a check configuration in a Pebble configuration layer."""

def __init__(self, name: str, raw: typing.Dict = None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the choice to make raw an untyped dict instead of unpacking it?
I would rather have Check('name', override:str='', level:str='', period:str='', ...), makes for a way easier to read initializer, and easier usage of the class.
If you really dislike doing check=Check('name', **my_raw_dict), then we should expose an alternative classmethod
Check.from_dict(name:str, dict_:dict).

If we were on more modern python I could also go for a dataclass or a TypedDict, but until then, this feels like better code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually the whole Check class could very well be a dataclass, and save some eq, repr and to_dict boilerplate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this should arguably be done with a from_dict. However, I've made this consistent with the other layer-configuration classes, which all do it the same way. If doing it again maybe we could have considered dataclasses or some such (then again, that's 3.7+ and ops still needs to support 3.5 for now). So I'm going to leave as is to be consistent.

self.name = name
raw = raw or {}
self.override = raw.get('override', '')
try:
self.level = CheckLevel(raw.get('level', ''))
except ValueError:
self.level = raw.get('level')
self.period = raw.get('period', '')
self.timeout = raw.get('timeout', '')
self.failures = raw.get('failures')

http = raw.get('http')
if http is not None:
http = copy.deepcopy(http)
self.http = http

tcp = raw.get('tcp')
if tcp is not None:
tcp = copy.deepcopy(tcp)
self.tcp = tcp

exec_ = raw.get('exec')
if exec_ is not None:
exec_ = copy.deepcopy(exec_)
self.exec = exec_

def to_dict(self) -> typing.Dict:
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
"""Convert this check object to its dict representation."""
fields = [
('override', self.override),
('level', self.level.value),
('period', self.period),
('timeout', self.timeout),
('failures', self.failures),
('http', self.http),
('tcp', self.tcp),
('exec', self.exec),
]
return {name: value for name, value in fields if value}

def __repr__(self) -> str:
return 'Check({!r})'.format(self.to_dict())

def __eq__(self, other: typing.Union[typing.Dict, 'Check']) -> bool:
"""Compare this check configuration to another."""
if isinstance(other, dict):
return self.to_dict() == other
elif isinstance(other, Check):
return self.to_dict() == other.to_dict()
else:
raise ValueError("Cannot compare pebble.Check to {}".format(type(other)))


class CheckLevel(enum.Enum):
"""Enum of check levels."""

UNSET = ''
ALIVE = 'alive'
READY = 'ready'


class FileType(enum.Enum):
"""Enum of file types."""

Expand Down Expand Up @@ -774,6 +869,52 @@ def __repr__(self):
).format(self=self)


class CheckInfo:
"""Check status information."""
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to get some more detail in this docstring since this is one of the main classes users will interact with for this - or at least a link to a discourse doc that discusses it. Particularly - what is status (i.e. the main thing users should be checking). I looked in the docs you made and couldn't find any mention there of status, UP, DOWN, etc. either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The main doc is the Pebble SDK doc with a section on health checks: https://juju.is/docs/sdk/pebble#heading--health-checks. I have added a new model.Container.get_checks and model.Container.get_check method (to match get_services and get_service) and added a brief doc section on that here -- I'll add that to the Discourse doc as soon as this is merged.

I've also added much better docstrings on CheckInfo, which will appear in the API documentation (and is linked to from the above).


def __init__(
self,
name: str,
level: str,
healthy: bool,
failures: int = 0,
last_error: str = None,
error_details: str = None,
):
self.name = name
self.level = level
self.healthy = healthy
self.failures = failures
self.last_error = last_error
self.error_details = error_details

@classmethod
def from_dict(cls, d: typing.Dict) -> 'CheckInfo':
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
"""Create new CheckInfo object from dict parsed from JSON."""
try:
level = CheckLevel(d.get('level', ''))
except ValueError:
level = d.get('level')
return cls(
name=d['name'],
level=level,
healthy=d['healthy'],
failures=d.get('failures', 0),
last_error=d.get('last-error', ''),
error_details=d.get('error-details', ''),
)

def __repr__(self):
return ('CheckInfo('
'name={self.name!r}, '
'level={self.level!r}, '
'healthy={self.healthy}, '
'failures={self.failures}, '
'last_error={self.last_error!r}, '
'error_details={self.error_details!r})'
).format(self=self)


class ExecProcess:
"""Represents a process started by :meth:`Client.exec`.

Expand Down Expand Up @@ -1132,7 +1273,7 @@ def _request_raw(
"""Make a request to the Pebble server; return the raw HTTPResponse object."""
url = self.base_url + path
if query:
url = url + '?' + urllib.parse.urlencode(query)
url = url + '?' + urllib.parse.urlencode(query, doseq=True)

if headers is None:
headers = {}
Expand Down Expand Up @@ -1422,7 +1563,7 @@ def add_layer(
self._request('POST', '/v1/layers', body=body)

def get_plan(self) -> Plan:
"""Get the Pebble plan (currently contains only combined services)."""
"""Get the Pebble plan (contains combined layer configuration)."""
resp = self._request('GET', '/v1/plan', {'format': 'yaml'})
return Plan(resp['result'])

Expand Down Expand Up @@ -1953,3 +2094,28 @@ def send_signal(self, sig: typing.Union[int, str], services: typing.List[str]):
'services': services,
}
self._request('POST', '/v1/signals', body=body)

def get_checks(
self,
level: CheckLevel = None,
names: typing.List[str] = None
) -> typing.List[CheckInfo]:
"""Get the check status for the configured checks.

Args:
level: Optional check level to query for. Because "ready" implies
"alive", if level is AliveLevel, checks with level "ready" are
included too.
names: Optional list of check names to query for (default is to
fetch all).

Returns:
List of CheckInfo objects.
"""
query = {}
if level is not None:
query['level'] = level.value
if names:
query['names'] = names
resp = self._request('GET', '/v1/checks', query)
return [CheckInfo.from_dict(info) for info in resp['result']]
3 changes: 3 additions & 0 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,9 @@ def exec(self, command, **kwargs):
def send_signal(self, sig: typing.Union[int, str], services: typing.List[str]):
raise NotImplementedError(self.send_signal)

def get_checks(self, level=None, names=None):
raise NotImplementedError(self.get_checks)


class NonAbsolutePathError(Exception):
"""Error raised by _MockFilesystem.
Expand Down
7 changes: 7 additions & 0 deletions test/pebble_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ def main():
choices=[s.value for s in pebble.ChangeState], default='all')
p.add_argument('--service', help='optional service name to filter on')

p = subparsers.add_parser('checks', help='show (filtered) checks')
p.add_argument('--level', help='check level to filter on, default all levels',
choices=[c.value for c in pebble.CheckLevel], default='')
p.add_argument('name', help='check name(s) to filter on', nargs='*')

p = subparsers.add_parser('exec', help='execute a command')
p.add_argument('--env', help='environment variables to set', action='append',
metavar='KEY=VALUE')
Expand Down Expand Up @@ -157,6 +162,8 @@ def main():
elif args.command == 'changes':
result = client.get_changes(select=pebble.ChangeState(args.select),
service=args.service)
elif args.command == 'checks':
result = client.get_checks(level=pebble.CheckLevel(args.level), names=args.name)
elif args.command == 'exec':
environment = {}
for env in args.env or []:
Expand Down
Loading