From 0f34a16549189a34aa287cde01a2cb7a3bac53c9 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 13:13:39 +0300 Subject: [PATCH 01/10] .gitignore: Ignore VSCode files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1170286..3b542f3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ build/ .tox/ __pycache__ *.charm +.vscode From c438d370d6c1671ef4772cf94ad20a50bc4e6eda Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 13:14:14 +0300 Subject: [PATCH 02/10] charm: Bump for CKF 1.8 - Introduce fileviewer image in config.yaml. - Introduce viewer-spec.yaml configMap. --- config.yaml | 4 +++ metadata.yaml | 2 +- src/charm.py | 52 +++++++++++++++++++++++++++++++ src/templates/viewer-spec.yaml.j2 | 38 ++++++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 src/templates/viewer-spec.yaml.j2 diff --git a/config.yaml b/config.yaml index 87672b7..45976f3 100644 --- a/config.yaml +++ b/config.yaml @@ -14,3 +14,7 @@ options: type: boolean default: false description: Whether cookies should require HTTPS + volume-viewer-image: + type: string + default: filebrowser/filebrowser:latest + description: Volume Viewer OCI Image (PVCViewer) diff --git a/metadata.yaml b/metadata.yaml index 7edb79d..85bc2a4 100755 --- a/metadata.yaml +++ b/metadata.yaml @@ -13,7 +13,7 @@ resources: type: oci-image description: 'Backing OCI image' auto-fetch: true - upstream-source: docker.io/kubeflownotebookswg/volumes-web-app:v1.7.0 + upstream-source: docker.io/kubeflownotebookswg/volumes-web-app:v1.8.0-rc.0 requires: ingress: interface: ingress diff --git a/src/charm.py b/src/charm.py index e74e17d..801de2e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -3,11 +3,14 @@ # See LICENSE file for licensing details. import logging +from pathlib import Path +from typing import Dict from charms.kubeflow_dashboard.v0.kubeflow_dashboard_links import ( DashboardLink, KubeflowDashboardLinksRequirer, ) +from jinja2 import Template from oci_image import OCIImageResource, OCIImageResourceError from ops.charm import CharmBase from ops.main import main @@ -15,6 +18,26 @@ from serialized_data_interface import NoCompatibleVersions, NoVersionsListed, get_interfaces +def render_template(template_path: str, context: Dict) -> str: + """ + Render a Jinja2 template. + + This function takes the file path of a Jinja2 template and a context dictionary + containing the variables for template rendering. It loads the template, + substitutes the variables in the context, and returns the rendered content. + + Args: + template_path (str): The file path of the Jinja2 template. + context (Dict): A dictionary containing the variables for template rendering. + + Returns: + str: The rendered template content. + """ + template = Template(Path(template_path).read_text()) + rendered_template = template.render(**context) + return rendered_template + + class CheckFailed(Exception): """Raise this exception if one of the checks in main fails.""" @@ -120,6 +143,11 @@ def main(self, event): "resources": ["notebooks"], "verbs": ["list"], }, + { + "apiGroups": ["kubeflow.org"], + "resources": ["pvcviewers"], + "verbs": ["get", "list", "create", "delete"], + }, ], } ] @@ -134,11 +162,35 @@ def main(self, event): "APP_SECURE_COOKIES": str(config["secure-cookies"]).lower(), "BACKEND_MODE": config["backend-mode"], "APP_PREFIX": "/volumes", + "VOLUME_VIEWER_IMAGE": config["volume-viewer-image"], }, "ports": [{"name": "http", "containerPort": config["port"]}], + "volumeConfig": [ + { + "name": "viewer-spec", + "mountPath": "/etc/config/", # xwris to .yaml? + "files": [ + { + "path": "viewer-spec.yaml", + "content": render_template( + "src/templates/viewer-spec.yaml.j2", {} + ), + } + ], + }, + ], } ], }, + k8s_resources={ + "configMaps": { + "volumes-web-app-viewer-spec-ck6bhh4bdm": { + "viewer-spec.yaml": render_template( + "src/templates/viewer-spec.yaml.j2", {} + ), + }, + }, + }, ) self.model.unit.status = ActiveStatus() diff --git a/src/templates/viewer-spec.yaml.j2 b/src/templates/viewer-spec.yaml.j2 new file mode 100644 index 0000000..fdd6619 --- /dev/null +++ b/src/templates/viewer-spec.yaml.j2 @@ -0,0 +1,38 @@ +# Source: manifests/apps/volumes-web-app/upstream/base/viewer-spec.yaml +# Note: the volumes-web-app allows expanding strings using ${VAR_NAME} +# You may use any environment variable. This lets us e.g. specify images that can be modified using kustomize's image transformer. +# Additionally, 'PVC_NAME', 'NAME' and 'NAMESPACE' are defined +# Name of the pvc is set by the volumes web app +pvc: $NAME +podTemplate: + containers: + - name: main + image: $VOLUME_VIEWER_IMAGE + env: + - name: FB_ADDRESS + value: "0.0.0.0" + - name: FB_PORT + value: "8080" + - name: FB_DATABASE + value: /tmp/filebrowser.db + - name: FB_NOAUTH + value: "true" + - name: FB_BASEURL + value: /pvcviewers/$NAMESPACE/$NAME/ + readinessProbe: + tcpSocket: + port: 8080 + initialDelaySeconds: 2 + periodSeconds: 10 + # viewer-volume is provided automatically by the volumes web app + volumeMounts: + - name: viewer-volume + mountPath: /data + workingDir: /data + serviceAccountName: default-editor +networking: + targetPort: 8080 + basePrefix: "/pvcviewers" + rewrite: "/" + timeout: 30s +rwoScheduling: true From 88d12b87a47b9f8906071f657102bf2bc0486081 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 15:39:42 +0300 Subject: [PATCH 03/10] requirements: Add jinja2 and run update-requirements --- requirements-unit.txt | 4 ++++ requirements.in | 1 + requirements.txt | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/requirements-unit.txt b/requirements-unit.txt index 342b2a6..fa848db 100644 --- a/requirements-unit.txt +++ b/requirements-unit.txt @@ -20,8 +20,12 @@ importlib-resources==6.0.1 # via jsonschema iniconfig==2.0.0 # via pytest +jinja2==3.1.2 + # via -r requirements.in jsonschema==4.17.3 # via serialized-data-interface +markupsafe==2.1.3 + # via jinja2 oci-image==1.0.0 # via -r requirements.in ops==2.6.0 diff --git a/requirements.in b/requirements.in index 0e54c27..21c27c0 100644 --- a/requirements.in +++ b/requirements.in @@ -4,3 +4,4 @@ ops oci-image serialized-data-interface +jinja2 diff --git a/requirements.txt b/requirements.txt index e4c9141..fb00d67 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,8 +14,12 @@ idna==3.4 # via requests importlib-resources==6.0.1 # via jsonschema +jinja2==3.1.2 + # via -r requirements.in jsonschema==4.17.3 # via serialized-data-interface +markupsafe==2.1.3 + # via jinja2 oci-image==1.0.0 # via -r requirements.in ops==2.6.0 From ad1856f47132dedca35ec8b816f484dcbbe179dc Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 15:21:01 +0300 Subject: [PATCH 04/10] tests(integration): Test configMap gets created --- tests/integration/test_charm.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index ef382b6..43eabcf 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -4,10 +4,10 @@ import logging from pathlib import Path -# from lightkube import Client -# from lightkube.resources.core_v1 import Service import pytest import yaml +from lightkube import Client +from lightkube.resources.core_v1 import ConfigMap from pytest_operator.plugin import OpsTest # from random import choices @@ -24,19 +24,28 @@ log = logging.getLogger(__name__) METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) +KATIB_CONFIG = "volumes-web-app-viewer-spec-ck6bhh4bdm" +CHARM_NAME = METADATA["name"] +EXPECTED_CONFIG_MAP = { + "viewer-spec.yaml": '# Source: manifests/apps/volumes-web-app/upstream/base/viewer-spec.yaml\n# Note: the volumes-web-app allows expanding strings using ${VAR_NAME}\n# You may use any environment variable. This lets us e.g. specify images that can be modified using kustomize\'s image transformer.\n# Additionally, \'PVC_NAME\', \'NAME\' and \'NAMESPACE\' are defined\n# Name of the pvc is set by the volumes web app\npvc: $NAME\npodTemplate:\n containers:\n - name: main\n image: $VOLUME_VIEWER_IMAGE\n env:\n - name: FB_ADDRESS\n value: "0.0.0.0"\n - name: FB_PORT\n value: "8080"\n - name: FB_DATABASE\n value: /tmp/filebrowser.db\n - name: FB_NOAUTH\n value: "true"\n - name: FB_BASEURL\n value: /pvcviewers/$NAMESPACE/$NAME/\n readinessProbe:\n tcpSocket:\n port: 8080\n initialDelaySeconds: 2\n periodSeconds: 10\n # viewer-volume is provided automatically by the volumes web app\n volumeMounts:\n - name: viewer-volume\n mountPath: /data\n workingDir: /data\n serviceAccountName: default-editor\nnetworking:\n targetPort: 8080\n basePrefix: "/pvcviewers"\n rewrite: "/"\n timeout: 30s\nrwoScheduling: true', # noqa: E501 +} + + +@pytest.fixture(scope="session") +def lightkube_client() -> Client: + client = Client(field_manager=CHARM_NAME) + return client @pytest.mark.abort_on_fail async def test_build_and_deploy(ops_test: OpsTest): - charm_name = METADATA["name"] - my_charm = await ops_test.build_charm(".") image_path = METADATA["resources"]["oci-image"]["upstream-source"] await ops_test.model.deploy(my_charm, resources={"oci-image": image_path}) await ops_test.model.wait_for_idle( - [charm_name], + [CHARM_NAME], wait_for_active=True, raise_on_blocked=True, raise_on_error=True, @@ -44,6 +53,14 @@ async def test_build_and_deploy(ops_test: OpsTest): ) +@pytest.mark.abort_on_fail +async def test_configmap_created(lightkube_client: Client, ops_test: OpsTest): + """Test configmaps contents with default config.""" + config_map = lightkube_client.get(ConfigMap, KATIB_CONFIG, namespace=ops_test.model_name) + + assert config_map.data == EXPECTED_CONFIG_MAP + + @pytest.mark.abort_on_fail async def test_relate_dependencies(ops_test: OpsTest): await ops_test.model.deploy( @@ -102,6 +119,7 @@ async def test_relate_dependencies(ops_test: OpsTest): # Disabled until we re-enable the selenium tests below +# When reenabling, we should add Service to "from lightkube.resources.core_v1 import" # @pytest.fixture() # def driver(request, ops_test, profile): # profile_name = profile From fbd50a437228654cec0a386461282593fbde2dad Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 15:22:10 +0300 Subject: [PATCH 05/10] images: Update get-images script --- tools/get-images.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/get-images.sh b/tools/get-images.sh index 08522eb..0477ad1 100755 --- a/tools/get-images.sh +++ b/tools/get-images.sh @@ -5,4 +5,5 @@ # dynamic list IMAGE_LIST=() IMAGE_LIST+=($(find -type f -name metadata.yaml -exec yq '.resources | to_entries | .[] | .value | ."upstream-source"' {} \;)) +IMAGE_LIST+=($(find -type f -name config.yaml -exec yq '.options | ."volume-viewer-image" | .default' {} \;)) printf "%s\n" "${IMAGE_LIST[@]}" From 733426d6d5a183441fefd5320f0e93fe7bd03f46 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 18:08:36 +0300 Subject: [PATCH 06/10] tests(integration): Run always with model kubeflow Modify pytest command to always run tests on model kubeflow. Otherwise, they fail deterministaclly since kubeflow-dashboard requires `kubeflow` namespace. --- .github/workflows/integrate.yaml | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 1a25b95..e5533e4 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -66,7 +66,7 @@ jobs: # Requires the model to be called kubeflow due to this bug: # https://github.com/kubeflow/kubeflow/issues/6136 juju add-model kubeflow - sg snap_microk8s -c "tox -e integration -- --model kubeflow" + sg snap_microk8s -c "tox -e integration" - run: sg snap_microk8s -c "microk8s.kubectl get all -A" if: failure() diff --git a/tox.ini b/tox.ini index bc882ce..e814f82 100644 --- a/tox.ini +++ b/tox.ini @@ -74,7 +74,7 @@ deps = description = Run unit tests [testenv:integration] -commands = pytest -v --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +commands = pytest -v --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} --model kubeflow deps = -r requirements-integration.txt description = Run integration tests From 4ceed980835674ee11bb71bd3273c6b86e4bd107 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Thu, 14 Sep 2023 18:03:04 +0300 Subject: [PATCH 07/10] tests(integration): raise_on_blocked=False to avoid flakiness Set raise_on_blocked=False to avoid flakiness due to kubeflow-dashboard going to Blocked((install) Add required relation to kubeflow-profiles) although it has already been added. --- tests/integration/test_charm.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 43eabcf..169ff0b 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -91,8 +91,10 @@ async def test_relate_dependencies(ops_test: OpsTest): await ops_test.model.add_relation("kubeflow-dashboard", "kubeflow-profiles") await ops_test.model.add_relation("istio-pilot:ingress", "kubeflow-dashboard:ingress") await ops_test.model.add_relation("istio-pilot", "kubeflow-volumes") + # raise_on_blocked=False to avoid flakiness due to kubeflow-dashboard going to + # Blocked((install) Add required relation to kubeflow-profiles) although it has been added await ops_test.model.wait_for_idle( - raise_on_blocked=True, + raise_on_blocked=False, raise_on_error=True, timeout=300, ) From d75213def15085ef6069600be3a71c9e85d875d9 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Mon, 18 Sep 2023 11:37:27 +0300 Subject: [PATCH 08/10] tests(integration): Fix --- tests/integration/config-map.yaml | 39 +++++++++++++++++++++++++++++++ tests/integration/test_charm.py | 9 +++---- 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 tests/integration/config-map.yaml diff --git a/tests/integration/config-map.yaml b/tests/integration/config-map.yaml new file mode 100644 index 0000000..35b00c0 --- /dev/null +++ b/tests/integration/config-map.yaml @@ -0,0 +1,39 @@ +viewer-spec.yaml: |- + # Source: manifests/apps/volumes-web-app/upstream/base/viewer-spec.yaml + # Note: the volumes-web-app allows expanding strings using ${VAR_NAME} + # You may use any environment variable. This lets us e.g. specify images that can be modified using kustomize's image transformer. + # Additionally, 'PVC_NAME', 'NAME' and 'NAMESPACE' are defined + # Name of the pvc is set by the volumes web app + pvc: $NAME + podTemplate: + containers: + - name: main + image: $VOLUME_VIEWER_IMAGE + env: + - name: FB_ADDRESS + value: "0.0.0.0" + - name: FB_PORT + value: "8080" + - name: FB_DATABASE + value: /tmp/filebrowser.db + - name: FB_NOAUTH + value: "true" + - name: FB_BASEURL + value: /pvcviewers/$NAMESPACE/$NAME/ + readinessProbe: + tcpSocket: + port: 8080 + initialDelaySeconds: 2 + periodSeconds: 10 + # viewer-volume is provided automatically by the volumes web app + volumeMounts: + - name: viewer-volume + mountPath: /data + workingDir: /data + serviceAccountName: default-editor + networking: + targetPort: 8080 + basePrefix: "/pvcviewers" + rewrite: "/" + timeout: 30s + rwoScheduling: true diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 169ff0b..977bc2e 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -24,12 +24,9 @@ log = logging.getLogger(__name__) METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) -KATIB_CONFIG = "volumes-web-app-viewer-spec-ck6bhh4bdm" +CONFIG_MAP = "volumes-web-app-viewer-spec-ck6bhh4bdm" CHARM_NAME = METADATA["name"] -EXPECTED_CONFIG_MAP = { - "viewer-spec.yaml": '# Source: manifests/apps/volumes-web-app/upstream/base/viewer-spec.yaml\n# Note: the volumes-web-app allows expanding strings using ${VAR_NAME}\n# You may use any environment variable. This lets us e.g. specify images that can be modified using kustomize\'s image transformer.\n# Additionally, \'PVC_NAME\', \'NAME\' and \'NAMESPACE\' are defined\n# Name of the pvc is set by the volumes web app\npvc: $NAME\npodTemplate:\n containers:\n - name: main\n image: $VOLUME_VIEWER_IMAGE\n env:\n - name: FB_ADDRESS\n value: "0.0.0.0"\n - name: FB_PORT\n value: "8080"\n - name: FB_DATABASE\n value: /tmp/filebrowser.db\n - name: FB_NOAUTH\n value: "true"\n - name: FB_BASEURL\n value: /pvcviewers/$NAMESPACE/$NAME/\n readinessProbe:\n tcpSocket:\n port: 8080\n initialDelaySeconds: 2\n periodSeconds: 10\n # viewer-volume is provided automatically by the volumes web app\n volumeMounts:\n - name: viewer-volume\n mountPath: /data\n workingDir: /data\n serviceAccountName: default-editor\nnetworking:\n targetPort: 8080\n basePrefix: "/pvcviewers"\n rewrite: "/"\n timeout: 30s\nrwoScheduling: true', # noqa: E501 -} - +EXPECTED_CONFIG_MAP = yaml.safe_load(Path("./tests/integration/config-map.yaml").read_text()) @pytest.fixture(scope="session") def lightkube_client() -> Client: @@ -56,7 +53,7 @@ async def test_build_and_deploy(ops_test: OpsTest): @pytest.mark.abort_on_fail async def test_configmap_created(lightkube_client: Client, ops_test: OpsTest): """Test configmaps contents with default config.""" - config_map = lightkube_client.get(ConfigMap, KATIB_CONFIG, namespace=ops_test.model_name) + config_map = lightkube_client.get(ConfigMap, CONFIG_MAP, namespace=ops_test.model_name) assert config_map.data == EXPECTED_CONFIG_MAP From f538b722129dd1568e29d8fa84a251edf350ca9a Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Mon, 18 Sep 2023 11:43:33 +0300 Subject: [PATCH 09/10] tests(integration): Fix linting --- tests/integration/test_charm.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 977bc2e..02cc115 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -28,6 +28,7 @@ CHARM_NAME = METADATA["name"] EXPECTED_CONFIG_MAP = yaml.safe_load(Path("./tests/integration/config-map.yaml").read_text()) + @pytest.fixture(scope="session") def lightkube_client() -> Client: client = Client(field_manager=CHARM_NAME) From 6fbc1c66eb5bd73580e5badd9e7ffea25077cdd2 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 19 Sep 2023 13:07:27 +0300 Subject: [PATCH 10/10] Revert "tests(integration): Run always with model kubeflow" This reverts commit 733426d6d5a183441fefd5320f0e93fe7bd03f46. --- .github/workflows/integrate.yaml | 2 +- tox.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index e5533e4..1a25b95 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -66,7 +66,7 @@ jobs: # Requires the model to be called kubeflow due to this bug: # https://github.com/kubeflow/kubeflow/issues/6136 juju add-model kubeflow - sg snap_microk8s -c "tox -e integration" + sg snap_microk8s -c "tox -e integration -- --model kubeflow" - run: sg snap_microk8s -c "microk8s.kubectl get all -A" if: failure() diff --git a/tox.ini b/tox.ini index e814f82..bc882ce 100644 --- a/tox.ini +++ b/tox.ini @@ -74,7 +74,7 @@ deps = description = Run unit tests [testenv:integration] -commands = pytest -v --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} --model kubeflow +commands = pytest -v --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} deps = -r requirements-integration.txt description = Run integration tests