feat: Enable BVT Container tests#17070
Conversation
|
BVT Container log file - |
3593d46 to
c8e218a
Compare
c8e218a to
f5edbe7
Compare
|
|
||
| @pytest.mark.runtime_container_tests | ||
| @pytest.mark.require_capability("container") | ||
| def test_bvt_system_footprint(ssh_exec) -> None: |
There was a problem hiding this comment.
This seems like a test that's not really asserting properties of the image-under-test but is rather performing measurements. I'm not sure that it fits into this framework.
|
|
||
| @pytest.mark.runtime_container_tests | ||
| @pytest.mark.require_capability("container") | ||
| def test_bvt_logging(ssh_exec) -> None: |
| assert ok and len(out) > 0, f"hostname command failed or returned empty: {out!r}" | ||
| print(f"Hostname: {out}") | ||
|
|
||
| # Routing table (informational) |
There was a problem hiding this comment.
It seems like there's a desire for informational image collection. Does some of it really want to be a snapshot-based test?
|
Did you consider allowing test cases to provide custom How would distroless container setup be done? |
There was a problem hiding this comment.
Pull request overview
This PR aims to enable Container Base BVT (Business/Build Verification) testing for AZL4 images by adding a new runtime (live container) pytest suite alongside existing static/offline image checks.
Changes:
- Added a new pytest-driven “container-bvt-tests” suite to the container-base image configuration.
- Introduced podman-based container runtime utilities and pytest fixtures/markers to support executing commands inside a live container.
- Reorganized/expanded container-base test coverage into static rootfs checks and new runtime BVT checks.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| base/images/tests/utils/pytest_plugin.py | Registers an additional pytest marker for package preinstall; plugin marker coverage likely needs expanding for the new runtime suite. |
| base/images/tests/utils/container_runtime.py | New podman orchestration helpers for starting/executing in/destroying test containers. |
| base/images/tests/conftest.py | Adds runtime-container fixtures and a requires_pkg autouse installer hook. |
| base/images/tests/cases/test_os_release.py | Adds an import (currently unused). |
| base/images/tests/cases/container-base/test_container.py | Removes prior container-base test location (moved into static/). |
| base/images/tests/cases/container-base/static/test_container.py | Adds container-base static (offline) checks, including a rootfs size gate. |
| base/images/tests/cases/container-base/runtime/test_container_bvt.py | Adds runtime BVT tests executed via podman exec, including networking/resource/package-management checks. |
| base/images/images.toml | Enables the new container BVT suite for images.container-base and defines the suite configuration. |
…/ tests with runtime_container_tests + runtime-package-management
Currently no tests require container creation using Dockerfile hence skipped the implementation in this PR. Do you want to have base code ready to take Dockerfile as input args in this PR itself? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
base/images/tests/conftest.py:248
running_containercreates a fresh container per test, but it always callscreate_container_with_exec(...)withoutimage_ref, which triggers apodman loadfrom the OCI archive each time. For--image-pathpointing at a tarball, this can add ~10s per test and make the suite much slower than necessary. Consider adding a session-scoped fixture that resolves/loads the image once and passesimage_ref=intocreate_container_with_execfor each per-test container run.
logger.info("Creating fresh container for test %s", request.node.name)
container = create_container_with_exec(
image_path,
workdir,
container_name=None, # auto-generate a unique name per test
image_name=image_name,
)
base/images/tests/cases/container-base/runtime/test_container_bvt.py:454
- This test hard-depends on external connectivity to
http://httpbin.org/getand asserts a ≥90% success rate across 50 fetches. That makes CI runs vulnerable to transient Internet/DNS issues or outbound network policy (flaky failures unrelated to the image). Consider gating this behind an explicit opt-in (env var/marker), using a repo-controlled endpoint, or reducing it to a best-effort informational check (skip/fail only when network is expected to be available).
@pytest.mark.require_capability("container")
@pytest.mark.require_capability("runtime-package-management")
@pytest.mark.requires_pkg("curl")
def test_bvt_sustained_http_fetch(container_exec) -> None:
"""BVT: Sustained external HTTP — 50 sequential fetches, ≥90% success.
Ports the legacy "Core Networking Test" (50-iteration page fetch).
"""
iterations = 50
url = "http://httpbin.org/get"
# One bash loop is far cheaper than 50 podman exec invocations.
cmd = (
f"i=0; ok=0; while [ $i -lt {iterations} ]; do "
f"curl -s -o /dev/null -w '%{{http_code}}\\n' "
f"--connect-timeout 5 --max-time 10 {url} | "
f"grep -q '^2' && ok=$((ok+1)); i=$((i+1)); done; echo $ok"
)
ok, out = _cmd_ok(container_exec, cmd, timeout=iterations * 12)
assert ok, f"Sustained fetch loop failed: {out!r}"
success = int(out.strip().splitlines()[-1])
success_rate = success / iterations
print(f"Sustained HTTP fetch: {success}/{iterations} succeeded ({success_rate:.0%})")
assert success_rate >= 0.9, \
f"HTTP success rate {success_rate:.0%} below 90% threshold ({success}/{iterations})"
base/images/tests/conftest.py:266
- The
container_execfixture docstring says the framework “does not install packages or mutate the container in any way”, but_apply_requires_pkg(autouse) will install packages into the live container for tests markedrequires_pkg. Please update the docstring to reflect the new behavior (or narrow the statement to “unlessrequires_pkgis used”).
"""Execute a shell command in the running container via ``podman exec``.
Returns a callable ``(command: str, timeout: int = 60) ->
subprocess.CompletedProcess[str]``. The framework does **not** install
packages or mutate the container in any way — tests see the image as
shipped. If a test needs a binary that the image does not ship, it
must declare the dependency itself (and either skip or fail loudly).
"""
Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/c4ffcb16-3c13-4f4d-86e1-2e089c5576a1 Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
…twork is restricted Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/5a6f8871-2c4d-409b-86ad-2f2607b38e11 Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
…g, subprocess timeout Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/66b748da-ab1a-49bd-a6de-a5c86adc4061 Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
Applied in commit 9d99f4c:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
base/images/tests/utils/pytest_plugin.py:160
require_capabilityis documented as taking exactly one capability, but the implementation silently accepts 0 or multiple args and only checksmarker.args[0]. This can reintroduce incomplete gating if someone accidentally passes multiple args again. Consider validatinglen(marker.args) == 1and raisingpytest.UsageError(or failing the test) when misused so the mistake is caught immediately.
def pytest_runtest_setup(item: pytest.Item) -> None:
"""Skip tests whose marks are not satisfied."""
# require_capability: skip if image doesn't have the required capability.
# Each marker takes exactly one capability name; stack the decorator
# multiple times to require several capabilities on a single test.
caps = parse_capabilities(item.config.getoption("--capabilities", default=None))
for marker in item.iter_markers("require_capability"):
required = marker.args[0] if marker.args else None
if required and required not in caps:
pytest.skip(f"requires capability '{required}' (not in: {sorted(caps)})")
| from utils.container_runtime import ( | ||
| ContainerExecInstance, | ||
| create_container_with_exec, | ||
| destroy_exec_container, | ||
| _get_image_reference, | ||
| ) |
| image_path: Path, image_type: str, image_name: str | None, workdir: Path, | ||
| _container_image_ref: str | None, | ||
| request: pytest.FixtureRequest | ||
| ) -> ContainerExecInstance | None: |
| markers = list(request.node.iter_markers("requires_pkg")) | ||
| if not markers: | ||
| return | ||
| if "container_exec" not in request.fixturenames: | ||
| pytest.skip("requires_pkg marker is only valid for runtime container tests") | ||
|
|
||
| pkgs: list[str] = [] | ||
| for m in markers: | ||
| pkgs.extend(m.args) | ||
| if not pkgs: | ||
| return | ||
|
|
||
| container_exec = request.getfixturevalue("container_exec") | ||
| cmd = "dnf install -y " + " ".join(pkgs) | ||
| logger.info("requires_pkg: installing %s", pkgs) | ||
| result = container_exec(cmd, timeout=300) | ||
| if result.returncode != 0: | ||
| pytest.skip( | ||
| f"requires_pkg: failed to install {pkgs} (rc={result.returncode}): " | ||
| f"{result.stderr.strip()[:200]}" | ||
| ) |
| def create_container_with_exec( | ||
| image_path: Path, | ||
| workdir: Path, | ||
| container_name: str | None = None, | ||
| image_name: str | None = None, | ||
| image_ref: str | None = None, | ||
| ) -> ContainerExecInstance: | ||
| """Create a running container with podman exec access. | ||
|
|
||
| Args: | ||
| image_path: Path to image file or image reference | ||
| workdir: Working directory (unused but kept for compatibility) | ||
| container_name: Container name (auto-generated if None) | ||
| image_name: Image name for logging (derived if None) | ||
| image_ref: Pre-resolved image reference (skips ``podman load``). | ||
| When provided, ``image_path`` is not touched. Use this to | ||
| amortize the ~10s tarball load across many container creates. | ||
| """ |
| "Container Size Max Check": walks the extracted rootfs and asserts | ||
| the total on-disk size is under AZL_CONTAINER_MAX_SIZE_MB (MiB). | ||
| """ | ||
| max_mb = int(os.environ.get("AZL_CONTAINER_MAX_SIZE_MB", _DEFAULT_MAX_ROOTFS_MB)) |
|
@copilot apply changes based on the comments in this thread |
…ection, unused params, env validation Agent-Logs-Url: https://github.com/microsoft/azurelinux/sessions/310e39f7-537e-488a-9b12-f763068ffefc Co-authored-by: bhagyapathak <13693535+bhagyapathak@users.noreply.github.com>
Applied in commit 743b470:
|
reubeno
left a comment
There was a problem hiding this comment.
Thanks for the updates; I've left comments and questions throughout the code. Happy to meet synchronously to go over it.
I think my biggest questions are around the structure.
In my earlier feedback I was thinking something along these lines:
base/images/tests/
cases/
static/
test_*.py # shared static tests useful across images; majority of static tests
<image_name>/
test_*.py # tests specific to the named image; should be quite minimal
runtime/
test_*.py # shared runtime tests useful across images; majority of runtime tests
<image_name>/
test_*.py # tests specific to the named image; should be quite minimal
This would allow the static test suite definition to just point to static/ and the runtime one to point to runtime/.
Would be good to get your input here -- and what structure you were going for.
| "--image-path", "{image-path}", | ||
| "--image-name", "{image-name}", | ||
| "--capabilities", "{capabilities}", | ||
| "-v", "-s" |
There was a problem hiding this comment.
I don't think it makes sense to hard-code -v or -s in the test case, as that's a generic pytest option that the adapter for running pytest-based tests should be able to handle. azldev already does something like that, where it translates its own verbosity options to --log-cli-level. We could refine that, but it doesn't seem like should be test suite specific.
| [test-suites.container-bvt-tests.pytest] | ||
| working-dir = "tests" | ||
| install = "pyproject" | ||
| test-paths = ["cases/container-base/runtime/test_container_bvt.py"] |
There was a problem hiding this comment.
Why not just specify the whole runtime/ dir? But also, it's odd that the naming of the test suite implies it should be runnable against any container, but it's got a hard-coded reference to container-base in particular.
| @pytest.mark.require_capability("runtime-package-management") | ||
| @pytest.mark.requires_pkg("procps-ng", "gawk") | ||
| def test_bvt_system_footprint(container_exec) -> None: | ||
| """BVT: Memory, disk, CPU, and process footprint metrics.""" |
There was a problem hiding this comment.
I still don't understand what it means to have a test that doesn't really assert anything meaningful (beyond the ability to run the commands below). This seems more like a metrics-gathering piece of code than a validation test case--do we need a different way to capture metrics?
| message = f"BVT log test entry {tag}" | ||
|
|
||
| ok, _ = _cmd_ok(container_exec, f"logger -t bvt_test '{message}'", timeout=10) | ||
| assert ok, "logger command failed — util-linux may be missing" |
There was a problem hiding this comment.
Doesn't seem to make sense to say that util-linux may be missing if we're already requiring it via the decorator?
|
|
||
| # Find the entry: try journalctl, then /var/log/messages, then /var/log/syslog | ||
| found = False | ||
| ok, out = _cmd_ok(container_exec, f"journalctl --no-pager --since '1 minute ago' 2>/dev/null | grep '{tag}' || true", timeout=15) |
There was a problem hiding this comment.
Is journalctl even in the base container? We don't run in systemd in it. Was this meant to be a VM test?
| assert path.exists(), f"File {path} does not exist" | ||
|
|
||
|
|
||
| def test_file_exists_etc_dnf_dnf_conf(rootfs: Path) -> None: |
There was a problem hiding this comment.
This is a good example of a test that could apply to all images that support runtime package management, and not even just containers. I don't think it belongs under container-base.
| assert path.exists(), f"License file {path} does not exist" | ||
|
|
||
|
|
||
| def test_license_coreutils_single_exists(rootfs: Path) -> None: |
There was a problem hiding this comment.
Is there a reason that bash and coreutils-single were called out in particular from a license perspective? Or are they spot checks?
| return result | ||
|
|
||
|
|
||
| def _parse_loaded_images(load_stdout: str) -> list[str]: |
There was a problem hiding this comment.
Please don't text-scrape command output -- there must be a better programmatic way to get the data we need.
| # SPDX-License-Identifier: MIT | ||
| """Container runtime orchestration for dynamic testing. | ||
|
|
||
| Provides fixtures and utilities for creating running containers with exec access |
There was a problem hiding this comment.
Is there a reason we're not using a higher-level Python library for automating podman? At least something like https://pypi.org/project/podman/? Worthwhile to spend a few mins looking to see if there's something even higher-level that's geared toward testing?
| container_exec = request.getfixturevalue("container_exec") | ||
|
|
||
| # Detect which package manager is available in this container. | ||
| # AZL images may ship tdnf, dnf, or both; prefer tdnf when present. |
There was a problem hiding this comment.
No reason to use tdnf if these tests are only intended for 4.0+.
christopherco
left a comment
There was a problem hiding this comment.
Retarget to 4.0 branch
|
Closing this PR in favor of #17372 |
Merge Checklist
All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)
*-staticsubpackages, etc.) have had theirReleasetag incremented../cgmanifest.json,./toolkit/scripts/toolchain/cgmanifest.json,.github/workflows/cgmanifest.json)./LICENSES-AND-NOTICES/SPECS/data/licenses.json,./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md,./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)*.signatures.jsonfilessudo make go-tidy-allandsudo make go-test-coveragepassSummary
This PR ports tests from the legacy CBL-Mariner ContainerBase BVT to the current pytest-based container BVT.
Runtime tests: [base/images/tests/cases/container-base/runtime/test_container_bvt.py]
Static tests: [base/images/tests/cases/container-base/static/test_container.py]
Change Log
Does this affect the toolchain?
NO
Associated issues
Links to CVEs
Test Methodology