Skip to content

Commit

Permalink
pebble: Sleep before starting KFP API server (#221)
Browse files Browse the repository at this point in the history
Sleep for 1.1 seconds before attempting to start the KFP API server, in
order to avoid issues with Pebble when the corresponding service fails
fast. This is a temporary measure until a fix for canonical/pebble#240
is provided.

Refs #220

Signed-off-by: Phoevos Kalemkeris <[email protected]>
  • Loading branch information
phoevos authored and DnPlas committed Aug 4, 2023
1 parent 54d5799 commit a309a70
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
7 changes: 5 additions & 2 deletions charms/kfp-api/src/charm.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env python3
# Copyright 2021 Canonical Ltd.
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

"""Charm the Kubeflow Pipelines API.
Expand Down Expand Up @@ -67,6 +67,9 @@ def __init__(self, *args):
self._grcp_port = self.model.config["grpc-port"]
self._http_port = self.model.config["http-port"]
self._exec_command = (
# TODO: Remove 'sleep' as soon as a fix for
# https://github.com/canonical/pebble/issues/240 is provided
"sleep 1.1 && "
"/bin/apiserver "
f"--config={CONFIG_DIR} "
f"--sampleconfig={SAMPLE_CONFIG} "
Expand Down Expand Up @@ -184,7 +187,7 @@ def _kfp_api_layer(self) -> Layer:
self._container_name: {
"override": "replace",
"summary": "ML Pipeline API Server",
"command": self._exec_command,
"command": f"bash -c '{self._exec_command}'",
"startup": "enabled",
"environment": self.service_environment,
"on-check-failure": {"kfp-api-up": "restart"},
Expand Down
9 changes: 7 additions & 2 deletions charms/kfp-api/tests/unit/test_operator.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2021 Canonical Ltd.
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.

from contextlib import nullcontext as does_not_raise
Expand Down Expand Up @@ -418,12 +418,17 @@ def test_install_with_all_inputs_and_pebble(
assert pebble_plan
assert pebble_plan.services
pebble_plan_info = pebble_plan.to_dict()
assert pebble_plan_info["services"]["ml-pipeline-api-server"]["command"] == (
pebble_exec_command = pebble_plan_info["services"]["ml-pipeline-api-server"]["command"]
exec_command = (
# TODO: Remove 'sleep' as soon as a fix for
# https://github.com/canonical/pebble/issues/240 is provided
"sleep 1.1 && "
"/bin/apiserver "
"--config=/config "
"--sampleconfig=/config/sample_config.json "
"-logtostderr=true "
)
assert pebble_exec_command == f"bash -c '{exec_command}'"
test_env = pebble_plan_info["services"]["ml-pipeline-api-server"]["environment"]
# there should be 1 environment variable
assert 1 == len(test_env)
Expand Down

0 comments on commit a309a70

Please sign in to comment.