Closed NohaIhab closed 5 months ago
Thank you for reporting us your feedback!
The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5376.
This message was autogenerated
the admission-webhook
charm has a pebble health check in place:
def _admission_webhook_layer(self) -> Layer:
"""Create and return Pebble framework layer."""
layer_config = {
"summary": "admission-webhook layer",
"description": "Pebble config layer for admission-webhook-operator",
"services": {
self._container_name: {
.....
# Service details
.....
"on-check-failure": {"admission-webhook-up": "restart"},
},
},
"checks": {
"admission-webhook-up": {
"override": "replace",
"period": "30s",
"timeout": "20s",
"threshold": 4,
"tcp": {"port": self._port},
}
},
}
If functioning correctly, the pebble service should be restarted when the service port is not listening on self._port
From what we see in the logs, the check does fail, but it is not restarting the service.
This is unexpected behavior from Pebble.
Check "admission-webhook-up" failure threshold 4 hit, triggering action
when the threshold is hit for the first time, and yet the service is not restarted.Check "admission-webhook-up" failure threshold 4 hit, triggering action
only the first time that the threshold is hit, then all the other reported failures do not log this.to prevent Case 1 scenario discussed above from happening, a check that the cert files exist in the container can be added before going into update_layer
.
If the cert files don't exist, we can just log that and do nothing, knowing that pebble_ready
handler will be triggered later and the certs will be uploaded then.
fix now in PR #125
closing since this was fixed in #125 and backported in #126
This issue is to document the investigation of the bug reported in https://github.com/canonical/admission-webhook-operator/issues/126
Description
the
admission-webhook
pebble service does not start, with the error:The webhook service needs to find the certificate files (
cert.pem
andkey.pem
) in the workload container directory/etc/webhook/certs
. When these cert files are not found, this puts the workload into an usable state, where the webhook service is not up. This is problematic because users cannot create Notebooks, as reported in the issue.Case where the flow works correctly
In the charm, we upload the certificates to the container on
pebble-ready
event, so the event sequence for this to be successful is:admission_webhook_pebble_ready
handler is: https://github.com/canonical/admission-webhook-operator/blob/0707be37a97d2f52b785caf670ec98aa66d97ce9/src/charm.py#L305-L311 whereon_event
method is: https://github.com/canonical/admission-webhook-operator/blob/0707be37a97d2f52b785caf670ec98aa66d97ce9/src/charm.py#L325-L346and the
config_changed
handler is set toon_event
the key thing to observe here is when the first call of
update_layer
is, because that is when the service is attempted to start. chisme'supdate_layer
is where the Pebble service is getting started if the pebble service definition was updated, the method in chisme:the code flow would be:
pebble_ready
handler executes -> certificates are pushed to container ->on_event
is called ->update_layer
first call so the service is started correctlyconfig_changed
handler executeson_event
->update_layer
is called for the second time -> services equivalence check does not pass, so the service is not restartedCases where it can go wrong
Case 1
If
update_layer
gets called for the first time when the cert files are not yet uploaded, this can happen when the following conditions are true:update_layer
gets fired beforepebble_ready
container.can_connect()
returnsTrue
, so theupdate_layer
function does not raise an Error and proceeds to executecontainer.replan()
which starts the service1.
is possible for charm execution, for exampleconfig_changed
can happen beforepebble_ready
2.
to apply, this means thatpebble_ready
event was fired as well, BUT there is no guarantee thatpebble_ready
handler has executed, it is a possible race condition here.Race condition
the error-prone scenario is like this:
config_changed
is firedconfig_changed
handler execution startsconfig_changed
handler execution reaches thecontainer.can_connect()
check inupdate_layer
, the container becomes readypebble_ready
event is queued, but theconfig_changed
handler is in the middle of execution so it continuesconfig_changed
handlercontainer.can_connect()
check inupdate_layer
returnsTrue
config_changed
handler adds the layer and callscontainer.replan()
pebble_ready
handler executesif current_layer.services != new_layer.services:
returnsFalse
because the service is the samepebble_ready
handler skips adding the layer andcontainer.replan()
i.e. it does not restart the serviceAt this state, the charm is unable to recover from the error, because every handler will skip restarting the service.
Case 2
If the
pebble_ready
handler executes, but somehow the cert files are not uploaded. Initially, we thought this is possible if the pebblepush
API was non-blocking, meaning that the charm continues to the next handler before the files have completed copying. To eliminate this possibility, I tested with pushing files of different sizes to the container while calculating the time it took to push them. I observed that bigger files take more time to execute. Also, I added a check that pebblepull
API does not raise an error right after thepush
call. so concluding that this case is unlikely.