canonical
Add type checking of fields in Layer specification to avoid list vs str issues
When defining dependent layer (based on Layer specification) I've noticed that I cannot use layer names longer than 1 character. If I change layer definition to p, everything works.
return {
"summary": "opensearch layer",
"description": "pebble config layer for opensearchproject/opensearch",
"services": {
"procps": {
"override": "merge",
"command": "yum install -y procps",
"startup": "enabled",
"summary": "procps",
},
"opensearch": {
"requires": "procps",
"override": "merge",
"environment": environment,
"summary": "opensearch",
"command": cmd,
"startup": "disabled",
}
}
}
unit-opensearch-k8s-0: 16:12:32 ERROR juju.worker.uniter.operation hook "opensearch-pebble-ready" (via hook dispatching script: dispatch) failed: exit status 1
unit-opensearch-k8s-0: 16:12:32 ERROR juju.worker.uniter pebble poll failed for container "opensearch": hook failed
unit-opensearch-k8s-0: 16:12:37 ERROR unit.opensearch-k8s/0.juju-log Uncaught exception while in charm code:
Traceback (most recent call last):
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 729, in _request_raw
response = self.opener.open(request, timeout=self.timeout)
File "/usr/lib/python3.8/urllib/request.py", line 531, in open
response = meth(req, response)
File "/usr/lib/python3.8/urllib/request.py", line 640, in http_response
response = self.parent.error(
File "/usr/lib/python3.8/urllib/request.py", line 569, in error
return self._call_chain(*args)
File "/usr/lib/python3.8/urllib/request.py", line 502, in _call_chain
result = func(*args)
File "/usr/lib/python3.8/urllib/request.py", line 649, in http_error_default
raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 400: Bad Request
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "./src/charm.py", line 142, in <module>
main(CharmOpenSearch)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/main.py", line 406, in main
_emit_charm_event(charm, dispatcher.event_name)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/main.py", line 140, in _emit_charm_event
event_to_emit.emit(*args, **kwargs)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 278, in emit
framework._emit(event)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 722, in _emit
self._reemit(event_path)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 767, in _reemit
custom_handler(event)
File "./src/charm.py", line 102, in _on_opensearch_pebble_ready
container.add_layer("opensearch", layer, combine=True)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/model.py", line 1067, in add_layer
self._pebble.add_layer(label, layer, combine=combine)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 879, in add_layer
self._request('POST', '/v1/layers', body=body)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 700, in _request
response = self._request_raw(method, path, query, headers, data)
File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 740, in _request_raw
raise APIError(body, code, status, message)
ops.pebble.APIError: cannot parse layer YAML: service "p" does not exist
Just to clarify, the issue is with the requires functionality, and if you change the procps layer name to p, and therefore requires: p it works?
I bet requires is meant to be a list and we are iterating over it. Such that when it is a string we just iterate characters.
Try
requires: [procps] or requires:
- procps
On Fri, May 21, 2021, 19:16 Dariusz @.***> wrote:
When defining dependent layer (based on Layer specification https://github.com/canonical/pebble#layer-specification) I've noticed that I cannot use layer names longer than 1 character:
return { "summary": "opensearch layer", "description": "pebble config layer for opensearchproject/opensearch", "services": { "procps": { "override": "merge", "command": "yum install -y procps", "startup": "enabled", "summary": "procps", }, "opensearch": { "requires": "procps", "override": "merge", "environment": environment, "summary": "opensearch", "command": cmd, "startup": "disabled", } } }unit-opensearch-k8s-0: 16:12:32 ERROR juju.worker.uniter.operation hook "opensearch-pebble-ready" (via hook dispatching script: dispatch) failed: exit status 1 unit-opensearch-k8s-0: 16:12:32 ERROR juju.worker.uniter pebble poll failed for container "opensearch": hook failed unit-opensearch-k8s-0: 16:12:37 ERROR unit.opensearch-k8s/0.juju-log Uncaught exception while in charm code: Traceback (most recent call last): File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 729, in _request_raw response = self.opener.open(request, timeout=self.timeout) File "/usr/lib/python3.8/urllib/request.py", line 531, in open response = meth(req, response) File "/usr/lib/python3.8/urllib/request.py", line 640, in http_response response = self.parent.error( File "/usr/lib/python3.8/urllib/request.py", line 569, in error return self._call_chain(*args) File "/usr/lib/python3.8/urllib/request.py", line 502, in _call_chain result = func(*args) File "/usr/lib/python3.8/urllib/request.py", line 649, in http_error_default raise HTTPError(req.full_url, code, msg, hdrs, fp) urllib.error.HTTPError: HTTP Error 400: Bad Request
During handling of the above exception, another exception occurred:
Traceback (most recent call last): File "./src/charm.py", line 142, in
main(CharmOpenSearch) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/main.py", line 406, in main _emit_charm_event(charm, dispatcher.event_name) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/main.py", line 140, in _emit_charm_event event_to_emit.emit(*args, **kwargs) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 278, in emit framework._emit(event) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 722, in _emit self._reemit(event_path) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/framework.py", line 767, in _reemit custom_handler(event) File "./src/charm.py", line 102, in _on_opensearch_pebble_ready container.add_layer("opensearch", layer, combine=True) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/model.py", line 1067, in add_layer self._pebble.add_layer(label, layer, combine=combine) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 879, in add_layer self._request('POST', '/v1/layers', body=body) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 700, in _request response = self._request_raw(method, path, query, headers, data) File "/var/lib/juju/agents/unit-opensearch-k8s-0/charm/venv/ops/pebble.py", line 740, in _request_raw raise APIError(body, code, status, message) ops.pebble.APIError: cannot parse layer YAML: service "p" does not exist — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/539, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7ICNKHRODVW7KVBSJTTO3STRANCNFSM45KAAYVQ .
We should still have a bug that notices if you are using a string instead of a list and error/warn because it is a common pitfall and not what you want. We protect against that sort of misuse elsewhere, and it leads to better information for people writing a charm.
On Sat, May 22, 2021 at 8:39 AM Jon Seager @.***> wrote:
Ah yes, John is right, requires in a list. Nice spot.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/539#issuecomment-846402590, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7LDCXN62LHOIQJEZBDTO6QXHANCNFSM45KAAYVQ .
Sounds good -- I've renamed the issue title to reflect what we intend to do to fix it. Could add some simple checks into the list fields, or potentially start using a involved schema definition tool like voluptuous
I vote for a more comprehensive check of the layer spec to catch more issues. Specifically, I managed to create a layer with an empty string for command and it caused confusing Pebble connection errors rather than indicating that there was an issue with the layer definition.
For the records, Layer has in the mean time been typed (so linters and static type checkers will warn of potential issues). This is not as strong a guarantee as an ORM layer would be, but it's a start.
@dasm Do you think the fact that Layer now has type annotations (LayerDict fields and so on) plus static typing checking is enough here? It seems to me that would catch most bugs of this kind.