test: Add integration tests for Kubernetes docker-registry deployment #328
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has checked 157 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 81 | 1 | 75 | 0 |
Click to see the invalid file list
- tests/integration/data/test_registries/pod.yaml
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
eaudetcobello
force-pushed
the
KU-2526/registry-testr
branch
2 times, most recently
from
32a8634 to
1edb193
Compare
|
@eaudetcobello can you add
add your module ( |
eaudetcobello
force-pushed
the
KU-2526/registry-testr
branch
from
68f1289 to
23e890a
Compare
tests/integration/test_registry.py
Outdated
| # Check that the units are ready | ||
| k8s_app = kubernetes_cluster.applications["k8s"] | ||
|
|
||
| assert k8s_app |
There was a problem hiding this comment.
I don't think this is a sufficient check. See
There was a problem hiding this comment.
Ok, i agree with @bschimke95. you should have the same test as in test_k8s or just drop this test all together. I don't really think we need test_ready_nodes. The model is going to deploy and be active/idle when they nodes are ready. I say just drop this test altogether.
tests/integration/test_registry.py
Outdated
|
|
||
| k8s_unit = kubernetes_cluster.applications["k8s"].units[0] | ||
| try: | ||
| created = create_from_yaml(api_client, yaml_objects=test_pod_manifest) |
There was a problem hiding this comment.
create_from_yaml is an async function, right?
Should this be created = await create_from_yaml(...) then?
There was a problem hiding this comment.
No, not async
There was a problem hiding this comment.
ok, create_from_yaml is ruthless.
Fail 1) it gives no type-hinting.
Fail 2) the comments on the return types are ambiguous
Returns:
The created kubernetes API objects.
Fail 3) What it actually returns:
a List of Lists of API objects....
Give this a shot
| created = create_from_yaml(api_client, yaml_objects=test_pod_manifest) | |
| created.extend(*create_from_yaml(api_client, yaml_objects=test_pod_manifest)) |
tests/integration/test_registry.py
Outdated
| # Check that the units are ready | ||
| k8s_app = kubernetes_cluster.applications["k8s"] | ||
|
|
||
| assert k8s_app |
There was a problem hiding this comment.
Ok, i agree with @bschimke95. you should have the same test as in test_k8s or just drop this test all together. I don't really think we need test_ready_nodes. The model is going to deploy and be active/idle when they nodes are ready. I say just drop this test altogether.
tests/integration/test_registry.py
Outdated
|
|
||
| k8s_unit = kubernetes_cluster.applications["k8s"].units[0] | ||
| try: | ||
| created = create_from_yaml(api_client, yaml_objects=test_pod_manifest) |
There was a problem hiding this comment.
ok, create_from_yaml is ruthless.
Fail 1) it gives no type-hinting.
Fail 2) the comments on the return types are ambiguous
Returns:
The created kubernetes API objects.
Fail 3) What it actually returns:
a List of Lists of API objects....
Give this a shot
| created = create_from_yaml(api_client, yaml_objects=test_pod_manifest) | |
| created.extend(*create_from_yaml(api_client, yaml_objects=test_pod_manifest)) |
eaudetcobello
force-pushed
the
KU-2526/registry-testr
branch
from
433a4fd to
ed102c5
Compare
| @@ -44,7 +43,7 @@ jobs: | |||
| builder-label: ubuntu-22.04 | |||
| tester-arch: AMD64 | |||
| tester-size: xlarge | |||
| modules: '["test_k8s", "test_etcd", "test_ceph", "test_upgrade", "test_external_certs"]' | |||
| modules: '["test_k8s", "test_etcd", "test_ceph", "test_upgrade", "test_external_certs", "test_registry"]' | |||
There was a problem hiding this comment.
@addyess why do we need to explicitly list each test? That feels like it just waits to be forgotten.
There was a problem hiding this comment.
The operator workflows run a unique gh runner for every item in this list. We run all the tests on a unique runner for these 5 test modules b/c they deploy 5 different SKUs. If we pushed the registry tests into another module, they wouldn't have to be added to this list. you COULD push this into the test_k8s module. Or leave it as is and add this modules here.
we could detect this list i suppose. Not good for this PR though
| from . import helpers | ||
|
|
||
| # This pytest mark configures the test environment to use the Canonical Kubernetes | ||
| # bundle with ceph, for all the test within this module. |
|
|
||
| # Create a pod that uses the busybox image from the custom registry | ||
| # Image: {docker_registry_ip}:5000/busybox:latest | ||
| test_pod_manifest = list(yaml.safe_load_all(_get_data_file_path("pod.yaml").open())) |
There was a problem hiding this comment.
let's inline this path. It is only used once.
There was a problem hiding this comment.
| test_pod_manifest = list(yaml.safe_load_all(_get_data_file_path("pod.yaml").open())) | |
| test_pod_manifest = list(yaml.safe_load_all(TEST_DATA_PATH.open())) |
| def _get_data_file_path(name) -> Path: | ||
| """Retrieve the full path of the specified test data file.""" | ||
| return Path(__file__).parent / "data" / "test_registries" / name |
There was a problem hiding this comment.
Yep, this can just be a CONSTANT
| def _get_data_file_path(name) -> Path: | |
| """Retrieve the full path of the specified test data file.""" | |
| return Path(__file__).parent / "data" / "test_registries" / name | |
| TEST_DATA_PATH = Path(__file__).parent / "data/test_registries/pod.yaml" |
Adds a test that deploys the docker-registry charm, pulls an image to the registry, tags it with a unique name, and finally tries to pull the image from the registry.
This validates that the charm sets the custom_containerd_config from charm configuration in the right place on the filesystem.