Skip to content

Commit

Permalink
Merge branch 'main' into kf-6542-feat-add-logs-env-vars-to-kfp-ui
Browse files Browse the repository at this point in the history
  • Loading branch information
NohaIhab authored Nov 25, 2024
2 parents e8912b5 + 9b522ff commit a6398f3
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/integrate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ jobs:
run: |
# Requires the model to be called kubeflow due to kfp-viewer
juju add-model kubeflow
sg snap_microk8s -c "tox -e bundle-integration-${{ matrix.sdk }} -- --model kubeflow --bundle=./tests/integration/bundles/kfp_latest_edge.yaml.j2"
sg snap_microk8s -c "tox -e bundle-integration-${{ matrix.sdk }} -- --model kubeflow --bundle=./tests/integration/bundles/kfp_latest_edge.yaml.j2" --charmcraft-clean
- name: Get all
run: kubectl get all -A
Expand Down
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ KFP charms use Jinja2 templates in order to store manifests that are applied dur
6. Build the manifest with `kustomize` (see step 4) and save the file
7. Compare both files to spot the differences (e.g. using diff `diff kfp-manifests-vX.yaml kfp-manifests-vY.yaml > kfp-vX-vY.diff`)

### Spot the updated images used by kfp-api

Kfp-api uses also two images `launcher` and `driver` apart from its workload container one. Those are updated on every release but this change is not visible when comparing manifests. In order to update those, grab their sources from the corresponding comments in the [config.yaml](./charms/kfp-api/config.yaml) file and switch to the target version of that file. Then, use the new images to update the config options' default value.

### Introduce changes

Once the comparison is done, add any changes to the relevant aggregated ClusterRoles to the
Expand Down
1 change: 1 addition & 0 deletions requirements-integration-v1.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ pytest
pytest-operator
pyyaml
tenacity
sh
2 changes: 2 additions & 0 deletions requirements-integration-v1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ rich==13.7.1
# via typer
rsa==4.9
# via google-auth
sh==2.1.0
# via -r requirements-integration-v1.in
shellingham==1.5.4
# via typer
six==1.16.0
Expand Down
4 changes: 3 additions & 1 deletion tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ This directory has the following structure:
├── conftest.py
├── helpers
│   ├── bundle_mgmt.py
│   ├── charmcraft.py
│   ├── k8s_resources.py
│   └── localize_bundle.py
├── kfp_globals.py
Expand Down Expand Up @@ -57,10 +58,11 @@ Communication with the KFP API happens using the KFP Python SDK. A `kfp.client`
2. Run integration tests against the preferred bundle definition in `integration/bundles`

```
tox -e bundle-integration -- --model kubeflow --bundle=./tests/integration/bundles/<bundle_template> <--no-build>
tox -e bundle-integration -- --model kubeflow --bundle=./tests/integration/bundles/<bundle_template> <--no-build> <--charmcraft-clean>
```

Where,
* `--model` tells the testing framework which model to deploy charms to
* `--bundle` is the path to a bundle template that's going to be used during the test execution
* `--no-build` tells the test suite whether to build charms and run tests against them, or use charms in Charmhub
* `--charmcraft-clean` tells the test suite whether to run `charmcraft clean` and delete the lxc instances after building the charms. If `--no-build` is passed, this has no effect.
6 changes: 6 additions & 0 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,9 @@ def pytest_addoption(parser: Parser):
"to render the bundle definition template."
"If set to False, the integration tests will be run against charms in Charmhub.",
)
parser.addoption(
"--charmcraft-clean",
action="store_true",
help="Whether to run charmcraft clean and delete lxc instances created by charmcraft."
"It defaults to False."
)
23 changes: 23 additions & 0 deletions tests/integration/helpers/charmcraft.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import logging
from os import chdir
from pathlib import Path
from typing import Dict

from kfp_globals import basedir

import sh

log = logging.getLogger(__name__)


def charmcraft_clean(charms_paths: Dict[str, Path]) -> None:
"""
Run `charmcraft clean` for passed paths to charms.
"""
pwd = sh.pwd()
for charm,charm_path in charms_paths.items():
log.info(f"Charmcraft clean {charm}")
chdir(charm_path)
sh.charmcraft.clean()
# Return to original directory so it doesn't affect tests execution
chdir(pwd)
21 changes: 11 additions & 10 deletions tests/integration/test_kfp_functional_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from helpers.bundle_mgmt import render_bundle, deploy_bundle
from helpers.k8s_resources import apply_manifests, fetch_response
from helpers.localize_bundle import get_resources_from_charm_file
from helpers.charmcraft import charmcraft_clean
from kfp_globals import (
CHARM_PATH_TEMPLATE,
KFP_CHARMS,
Expand All @@ -18,6 +19,7 @@
SAMPLE_VIEWER,
)

import sh
import kfp
import lightkube
import pytest
Expand Down Expand Up @@ -68,6 +70,7 @@ def create_and_clean_experiment_v1(kfp_client: kfp.Client):
async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
"""Build and deploy kfp-operators charms."""
no_build = request.config.getoption("no_build")
charmcraft_clean_flag = True if request.config.getoption("--charmcraft-clean") else False

# Immediately raise an error if the model name is not kubeflow
if ops_test.model_name != "kubeflow":
Expand All @@ -93,6 +96,9 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)])
context.update([(f"{charm.replace('-', '_')}", charm_file)])

if charmcraft_clean_flag == True:
charmcraft_clean(charms_to_build)

# Render kfp-operators bundle file with locally built charms and their resources
rendered_bundle = render_bundle(
ops_test, bundle_path=bundlefile_path, context=context, no_build=no_build
Expand All @@ -101,16 +107,11 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
# Deploy the kfp-operators bundle from the rendered bundle file
await deploy_bundle(ops_test, bundle_path=rendered_bundle, trust=True)

# Wait for everything to be up. Note, at time of writing these charms would naturally go
# into blocked during deploy while waiting for each other to satisfy relations, so we don't
# raise_on_blocked.
await ops_test.model.wait_for_idle(
status="active",
raise_on_blocked=False, # These apps block while waiting for each other to deploy/relate
raise_on_error=True,
timeout=3600,
idle_period=30,
)
# Use `juju wait-for` instead of `wait_for_idle()`
# due to https://github.com/canonical/kfp-operators/issues/601
# and https://github.com/juju/python-libjuju/issues/1204
log.info("Waiting on model applications to be active")
sh.juju("wait-for","model","kubeflow", query="forEach(applications, app => app.status == 'active')", timeout="30m")


# ---- KFP API Server focused test cases
Expand Down
21 changes: 10 additions & 11 deletions tests/integration/test_kfp_functional_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from helpers.bundle_mgmt import render_bundle, deploy_bundle
from helpers.k8s_resources import apply_manifests, fetch_response
from helpers.localize_bundle import get_resources_from_charm_file
from helpers.charmcraft import charmcraft_clean
from kfp_globals import (
CHARM_PATH_TEMPLATE,
KFP_CHARMS,
Expand Down Expand Up @@ -71,6 +72,7 @@ def create_and_clean_experiment_v2(kfp_client: kfp.Client):
async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
"""Build and deploy kfp-operators charms."""
no_build = request.config.getoption("no_build")
charmcraft_clean_flag = True if request.config.getoption("--charmcraft-clean") else False

# Immediately raise an error if the model name is not kubeflow
if ops_test.model_name != "kubeflow":
Expand All @@ -96,6 +98,9 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)])
context.update([(f"{charm.replace('-', '_')}", charm_file)])

if charmcraft_clean_flag == True:
charmcraft_clean(charms_to_build)

# Render kfp-operators bundle file with locally built charms and their resources
rendered_bundle = render_bundle(
ops_test, bundle_path=bundlefile_path, context=context, no_build=no_build
Expand All @@ -104,17 +109,11 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client):
# Deploy the kfp-operators bundle from the rendered bundle file
await deploy_bundle(ops_test, bundle_path=rendered_bundle, trust=True)

# Wait for everything to be up. Note, at time of writing these charms would naturally go
# into blocked during deploy while waiting for each other to satisfy relations, so we don't
# raise_on_blocked.
await ops_test.model.wait_for_idle(
status="active",
raise_on_blocked=False, # These apps block while waiting for each other to deploy/relate
raise_on_error=True,
timeout=3600,
idle_period=30,
)

# Use `juju wait-for` instead of `wait_for_idle()`
# due to https://github.com/canonical/kfp-operators/issues/601
# and https://github.com/juju/python-libjuju/issues/1204
log.info("Waiting on model applications to be active")
sh.juju("wait-for","model","kubeflow", query="forEach(applications, app => app.status == 'active')", timeout="30m")

# ---- KFP API Server focused test cases
async def test_upload_pipeline(kfp_client):
Expand Down

0 comments on commit a6398f3

Please sign in to comment.