canonical / bundle-kubeflow

Charmed Kubeflow
Apache License 2.0
104 stars 50 forks source link

Use a non-fixed image name in ROCKs tests and CI #785

Open orfeas-k opened 10 months ago

orfeas-k commented 10 months ago

Why it needs to get done

Context

At the moment, sanity test_rock.py files expect the image to have a specific name (name:version). This means that whenever we want to run those tests, the local docker cache should have an image with this name.

For our CI, this means that WFs must always follow this name pattern which results in a more complicated API e.g. we would not need to pass rock-name and rock-version here, if we didn't need to use that exact name for exporting the image to the local docker cache.

The same goes for integration environment in ROCKs repos' tox.ini files which also expect the image to have a specific name. This results in the same complications described above, but for integration-test-rock.yaml.

What needs to get done

  1. Update test_rock.py to read the LOCAL_ROCK_IMAGE value either from args or from a local env variable. If this is not available, then fall back to the current implementation
  2. Update integration environment in tox.ini files to do the same as well.
  3. Simplify the API in our ROCKs CI by removing redundant inputs and using dummy names for images where possible. This could be done in a more uniform way if we tackle first https://github.com/canonical/charmed-kubeflow-workflows/issues/29 and then implement the solution only in the action.

When is the task considered done

  1. test_rock.py files are updated.
  2. tox.ini integration environments are updated.
  3. CI is updated

Not sure if we should do that (1 and 2) for one repo and tackle the rest as soon as we get to work on them or have a massive effort of updating tests in all of our ROCKs repositories

syncronize-issues-to-jira[bot] commented 10 months ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5130.

This message was autogenerated

ca-scribner commented 7 months ago

This should probably be tackled at the same time as #783, which proposes refactoring our sanity tests to a more reusable script