Skip to content

Commit

Permalink
fix: handle incorrect custom images config (#497)
Browse files Browse the repository at this point in the history
* fix: handle incorrect custom images config
  • Loading branch information
NohaIhab authored Jun 5, 2024
1 parent 871e230 commit d192c53
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 12 deletions.
39 changes: 27 additions & 12 deletions charms/kfp-profile-controller/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
from charmed_kubeflow_chisme.components.charm_reconciler import CharmReconciler
from charmed_kubeflow_chisme.components.kubernetes_component import KubernetesComponent
from charmed_kubeflow_chisme.components.leadership_gate_component import LeadershipGateComponent
from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels
from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch
from lightkube.generic_resource import create_global_resource
from lightkube.models.core_v1 import ServicePort
from lightkube.resources.core_v1 import Secret
from ops.charm import CharmBase
from ops.main import main
from ops.model import BlockedStatus

from components.pebble_components import (
KfpProfileControllerInputs,
Expand Down Expand Up @@ -75,19 +77,20 @@ def parse_images_config(config: str) -> Dict:
Returns:
Dict: A list of images.
"""
error_message = (
f"Cannot parse a config-defined images list from config '{config}' - this"
"config input will be ignored."
)
if not config:
return []
try:
images = yaml.safe_load(config)
except yaml.YAMLError as err:
logger.warning(
f"{error_message} Got error: {err}, while parsing the custom_image config."
logger.error(
f"Charm Blocked due to error parsing the `custom_images` config. "
f"Caught error: {str(err)}"
)
raise ErrorWithStatus(
"Error parsing the `custom_images` config - fix `custom_images` to unblock. "
"See logs for more details",
BlockedStatus,
)
raise err
return images


Expand All @@ -99,10 +102,14 @@ class KfpProfileControllerOperator(CharmBase):

def __init__(self, *args):
super().__init__(*args)
self.images = self.get_images(
DEFAULT_IMAGES,
parse_images_config(self.model.config["custom_images"]),
)
try:
self.images = self.get_images(
DEFAULT_IMAGES,
parse_images_config(self.model.config["custom_images"]),
)
except ErrorWithStatus as e:
self.unit.status = e.status
return

# expose controller's port
http_port = ServicePort(CONTROLLER_PORT, name="http")
Expand Down Expand Up @@ -237,7 +244,15 @@ def get_images(
if image_name in images:
images[image_name] = custom_image
else:
self.log.warning(f"image_name {image_name} not in image list, ignoring.")
logger.error(
f"Image name `{image_name}` set in `custom_images` config not found in "
f"images list: {', '.join(images.keys())}."
)
raise ErrorWithStatus(
"Incorrect image name in `custom_images` config - fix `custom_images` to "
"unblock. See logs for more details",
BlockedStatus,
)

# This are special cases comfigmap where they need to be split into image and version
for image_name in [
Expand Down
23 changes: 23 additions & 0 deletions charms/kfp-profile-controller/tests/unit/test_operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from unittest.mock import MagicMock

import pytest
import yaml
from charmed_kubeflow_chisme.testing import add_sdi_relation_to_harness
from ops.model import ActiveStatus, BlockedStatus
from ops.testing import Harness
Expand Down Expand Up @@ -192,3 +193,25 @@ def test_pebble_services_running(
# Assert the environment variables that are set from inputs are correctly applied
environment = container.get_plan().services["kfp-profile-controller"].environment
assert environment == EXPECTED_ENVIRONMENT


def test_custom_images_config_with_incorrect_config(
harness, mocked_lightkube_client, mocked_kubernetes_service_patch
):
"""Asserts that the unit goes to blocked on corrupted config input."""
harness.update_config({"custom_images": "{"})
harness.begin()

assert isinstance(harness.model.unit.status, BlockedStatus)
assert harness.charm.model.unit.status.message.startswith("Error parsing")


def test_custom_images_config_with_incorrect_images_names(
harness, mocked_lightkube_client, mocked_kubernetes_service_patch
):
"""Asserts that the unit goes to blocked on incorrect images names in the config input."""
harness.update_config({"custom_images": yaml.dump({"name1": "image1", "name2": "image2"})})
harness.begin()

assert isinstance(harness.model.unit.status, BlockedStatus)
assert harness.charm.model.unit.status.message.startswith("Incorrect image name")

0 comments on commit d192c53

Please sign in to comment.