Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,14 @@ Before writing ANY new code:

### Utility Module Placement

When adding functions to `utilities/`, place them in the correct module:
When adding functions to `utilities/`, place them in the correct module. See
[Code Organization](docs/CODE_ORGANIZATION.md) for constants import rules and the fixtures package layout.

- **`utilities/cluster.py`** — cluster-wide operations (oc commands, node operations, cluster state)
- **`utilities/infra.py`** — infrastructure helpers (SSH, networking infrastructure, pod operations)
- **`utilities/virt.py`** — VM lifecycle, VMI operations, migration helpers
- **`utilities/storage.py`** — storage operations (PVC, DataVolume, StorageClass)
- **`utilities/constants/<submodule>.py`** — shared constants; import from the submodule, not the package root (except `Images`)

**NEVER** add functions to the wrong utility module — match the domain.

Expand Down Expand Up @@ -214,7 +216,7 @@ When reviewing quarantine PRs, verify the **quarantine mechanism matches the fai
- **Tests belong under the feature they test** - do NOT create standalone directories for cross-cutting concerns. If a test measures VM downtime during migration over a specific network type, it belongs under that network type's directory (e.g., `tests/network/l2_bridge/`), not a separate top-level directory.
- **Test file naming REQUIRED** - ALWAYS use `test_<functionality>.py` format
- **Local helpers location** - place helper utils in `<feature_dir>/utils.py`
- **Local fixtures location** - place in `<feature_dir>/conftest.py`
- **Local fixtures location** - place in `<feature_dir>/conftest.py` for feature-local fixtures, or `tests/fixtures/<team>/` for shared fixtures (see [Code Organization](docs/CODE_ORGANIZATION.md#fixtures-testsfixtures-and-conftestpy))
- **Move to shared location** - move to `utilities/` or `tests/conftest.py` ONLY when used by different team directories

### Internal API Stability
Expand Down Expand Up @@ -261,6 +263,7 @@ uv run tox -e utilities-unittests

## Related Documentation

- [`docs/CODE_ORGANIZATION.md`](docs/CODE_ORGANIZATION.md) — Constants, utilities, and fixtures layout and import rules
- [`docs/QUARANTINE_GUIDELINES.md`](QUARANTINE_GUIDELINES.md) — Test quarantine and de-quarantine procedures
- [`docs/SOFTWARE_TEST_DESCRIPTION.md`](SOFTWARE_TEST_DESCRIPTION.md) — STD docstring format and requirements
- [`docs/CODING_AND_STYLE_GUIDE.md`](docs/CODING_AND_STYLE_GUIDE.md) — Detailed coding and style conventions
Expand Down
8 changes: 4 additions & 4 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@
import utilities.infra # noqa
from libs.storage.config import StorageClassConfig
from utilities.bitwarden import get_cnv_tests_secret_by_name
from utilities.constants import (
AMD_64,
from utilities.constants.architecture import AMD_64
from utilities.constants.namespaces import NamespacesNames
from utilities.constants.pytest import (
QUARANTINED,
SETUP_ERROR,
TIMEOUT_5MIN,
NamespacesNames,
)
from utilities.constants.timeouts import TIMEOUT_5MIN
from utilities.data_collector import (
collect_default_cnv_must_gather_with_vm_gather,
get_data_collector_dir,
Expand Down
15 changes: 9 additions & 6 deletions docs/ARCHITECTURE_SUPPORT.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ pytest -m s390x ...
Note: to run on the default architecture `amd64`, there's no need to set any architecture-specific markers.

## Adding new images or new architecture support
Images for different architectures are managed under [constants.py](../utilities/constants.py) - `ArchImages`
The data structures are defined under [images.py](../libs/infra/images.py)

See [Code Organization — Constants](CODE_ORGANIZATION.md#constants-utilitiesconstants) for import rules and the constants module map.

Images for different architectures are managed under [images.py](../utilities/constants/images.py) — `ArchImages`.
The data structures are defined under [images.py](../libs/infra/images.py).

### Adding new images
To add a new image:
- Add the image name under the relevant dataclass under [images.py](../libs/infra/images.py)
- Add the image name to the `ArchImages` under the relevant architecture and OS under [constants.py](../utilities/constants.py)
- Add the image name to the `ArchImages` under the relevant architecture and OS under [images.py](../utilities/constants/images.py)
- Add the image to the image mapping under [os_utils.py](../utilities/os_utils.py); refer to existing images for the format

### Adding new architecture support
Expand All @@ -43,6 +46,6 @@ To add a new architecture:
- Add a new pytest marker for the architecture
- Add a new pytest global config file for the architecture under [tests/global_config_<architecture>.py](../tests/global_config_<architecture>.py)
- The file should contain the relevant OS matrix(es); see [global_config_amd64.py](../tests/global_config_amd64.py) for an example
- Add the architecture name as a constant under [constants.py](../utilities/constants.py)
- Add the architecture name to the list of supported architectures under [get_test_images_arch_class](../utilities/constants.py)
- Add the architecture name to the `ArchImages` under the relevant architecture and OS under [constants.py](../utilities/constants.py)
- Add the architecture name as a constant under [architecture.py](../utilities/constants/architecture.py)
- Add the architecture name to `SUPPORTED_CPU_ARCHITECTURES` under [architecture.py](../utilities/constants/architecture.py)
- Add the architecture name to the `ArchImages` under the relevant architecture and OS under [images.py](../utilities/constants/images.py)
162 changes: 162 additions & 0 deletions docs/CODE_ORGANIZATION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# Code Organization

Where shared constants, utility functions, and pytest fixtures belong, and how to import them.

For general coding style see [CODING_AND_STYLE_GUIDE.md](CODING_AND_STYLE_GUIDE.md). For enforceable
AI/review rules see [AGENTS.md](../AGENTS.md).

## Shared principles

- **Thematic modules** — one concern per file; no catch-all modules.
- **Search first** — before adding anything, check existing `utilities/`, `libs/`, and `tests/` code.
- **Absolute imports** — always `from utilities.… import …`, never relative imports.
- **Submodule imports** — import from the specific module that owns the symbol; do not rely on
package-level re-exports except where documented below.
- **No helpers in conftest** — `conftest.py` and `test_*.py` contain fixtures and tests only;
helpers belong in `utilities/`, `libs/`, or feature `utils.py` files.

---

## Constants (`utilities/constants/`)

Test and utility code shares string literals, timeouts, resource names, and configuration values
through the `utilities/constants/` package. Each file is a thematic submodule.

### Import policy

```python
# Preferred — import from the owning submodule
from utilities.constants.timeouts import TIMEOUT_5MIN
from utilities.constants.cluster import KUBERNETES_ARCH_LABEL

# Exception — Images only (computed at package import time; see below)
from utilities.constants import Images
```

- Do **not** use `from utilities.constants import X` for any name other than `Images`.
- Do **not** add re-exports to `utilities/constants/__init__.py` (except `Images`).

### Adding a new constant

1. Read submodule docstrings (`Covers` / `Not here` sections) to pick the right file.
2. Add the constant there; update the module docstring if the scope changes.
3. Submodule files must **not** import from other `utilities/constants/` submodules — only from
`libs/`, `ocp_resources`, or the standard library.
4. Import the constant at call sites from the submodule directly.
5. Keep single-use values local to the test or feature module instead of adding them to the package.

### `Images` exception

`Images` is resolved in `utilities/constants/__init__.py` based on cluster architecture.
This avoids a circular import between `utilities/architecture.py` and the constants package.
Use `from utilities.constants import Images` only for this alias. For `ArchImages`, OS flavor
strings, and other image constants, import from `utilities/constants/images.py`.

Architecture-specific image setup is described in [ARCHITECTURE_SUPPORT.md](ARCHITECTURE_SUPPORT.md).

### Module map

| Submodule | Use for |
| --- | --- |
| `aaq.py` | Application-Aware Quota resource names, quota field keys, namespace labels, quota spec dicts |
| `architecture.py` | CPU architecture strings (`AMD_64`, `ARM_64`, …), vendor identifiers, supported architecture sets |
| `cluster.py` | Kubernetes node labels, `NODE_STR`, API verb strings, env vars, pod security labels, audit-log commands |
| `components.py` | CNV operator/pod/deployment/service **name strings** and Kubernetes kind strings (`kubectl get <kind>/<name>`) |
| `cpu_models.py` | CPU model exclusion lists for guest compatibility |
| `hco.py` | HCO status conditions, upgrade streams, TLS profiles, feature gate keys, CNV CRD list |
| `images.py` | `ArchImages`, OS flavor strings, image disk names, `DEFAULT_FEDORA_REGISTRY_URL` |
| `instance_types.py` | Instance type and VM preference name strings (`U1_*`, `RHEL*_PREFERENCE`, `WINDOWS_*_PREFERENCE`) |
| `monitoring.py` | Alert severities, operator health metrics, KubeVirt VMI metric names, Prometheus service name |
| `namespaces.py` | `NamespacesNames` — well-known OpenShift and CNV namespace strings |
| `networking.py` | SR-IOV, bridge types, ports, KubeMacPool config, bonding, network test pod specs |
| `oadp.py` | OADP test file names, backup storage location names |
| `os_matrix.py` | Common-templates test matrix **parameter keys** (`IMAGE_NAME_STR`, `DV_SIZE_STR`, …) |
| `pytest.py` | Pytest exit codes, quarantine strings, fixture scope strings, unprivileged test credentials |
| `storage.py` | `StorageClassNames`, CDI/HPP labels, hotplug constants, DataVolume source types, DataImportCron values |
| `tekton.py` | Tekton pipeline ref and task name strings for Windows VM automation |
| `timeouts.py` | `TIMEOUT_*` integers and `TCP_TIMEOUT_30SEC` |
| `virt.py` | Virtctl commands, migration/eviction values, Windows version tags, CPU/memory topology, VM hardware constants |

Each submodule docstring lists what belongs there and what belongs elsewhere.

---

## Utility functions (`utilities/`)

Shared non-fixture logic lives under `utilities/`. Monolithic modules are being split into
thematic subpackages over time (same model as `utilities/constants/`).

### Placement

| Location | Use for |
| --- | --- |
| `utilities/cluster.py` | Cluster-wide operations (oc commands, node operations, cluster state) |
| `utilities/infra.py` | Infrastructure helpers (SSH, networking infrastructure, pod operations) |
| `utilities/virt.py` | VM lifecycle, VMI operations, migration helpers |
| `utilities/storage.py` | Storage operations (PVC, DataVolume, StorageClass) |
| Other `utilities/*.py` | Domain-specific helpers (HCO, monitoring, artifactory, …) |

When a module grows into a package (e.g. `utilities/virt/`), add functions to the submodule
that matches the concern and import from there:

```python
from utilities.virt.migration import migrate_vm_and_verify
```

Small single-purpose modules (`exceptions.py`, `logger.py`, …) may remain as single files
until splitting improves clarity.

**Never** add functions to the wrong domain module — match the table above.

---

## Fixtures (`tests/fixtures/` and `conftest.py`)

Pytest fixtures provide test setup and teardown. Fixture **definitions** for shared use are
moving into `tests/fixtures/`; `conftest.py` files register plugins and hold feature-local
fixtures only.

### Hierarchy

| File / directory | Role |
| --- | --- |
| [`conftest.py`](../conftest.py) | Pytest hooks, session configuration, `pytest_plugins` registration |
| [`tests/conftest.py`](../tests/conftest.py) | Cross-team fixtures still being consolidated |
| `tests/<team>/<feature>/conftest.py` | Feature-local fixtures when needed |
| `tests/fixtures/<team>/<topic>.py` | Shared fixture implementations |

### `tests/fixtures/` package

Shared fixtures are defined in Python modules under `tests/fixtures/`, grouped by team and topic:

```
tests/fixtures/
network/
l2_bridge.py
cluster.py
```

Register new fixture modules in the root `conftest.py` `pytest_plugins` list:

```python
pytest_plugins = [
"tests.fixtures.network.l2_bridge",
"tests.fixtures.network.cluster",
]
```

Add a new entry when introducing a fixture module used across multiple test directories.

### Rules

- **`conftest.py` is for fixtures only** — no helper functions, utility functions, or classes.
- **Fixture names are nouns** — describe what the fixture provides (`vm_with_disk`), not an action
(`create_vm_with_disk`).
- **One action per fixture** — split combined setup into separate fixtures composed by tests or
`@pytest.mark.usefixtures`.
- **Return or yield the resource** — even setup-only fixtures should yield the created object.
- **Use `@pytest.mark.usefixtures`** when the test does not use the fixture return value.
- **Feature-local helpers** — place in `<feature_dir>/utils.py`, not in `conftest.py`.

Fixture scope, ordering, and logging rules are in [AGENTS.md](../AGENTS.md) and
[CODING_AND_STYLE_GUIDE.md](CODING_AND_STYLE_GUIDE.md).
10 changes: 7 additions & 3 deletions docs/CODING_AND_STYLE_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,16 @@ Only add comments when necessary. For example, when using complex regex.
- Tests are to be placed in `test_<functionality>.py` file; this is where the actual tests are written.
`<functionality_name>` describes the functionality tested in this test file.
- If helper utils are needed, they should be placed in the test's subdirectory.
- If specific fixtures are needed, they should be placed in a `conftest.py` file under the test's subdirectory.
- If specific fixtures are needed, they should be placed in a `conftest.py` file under the test's subdirectory, or in [`tests/fixtures/`](../tests/fixtures/) when shared across teams — see [Code Organization](CODE_ORGANIZATION.md#fixtures-testsfixtures-and-conftestpy).

## conftest
- Top level [conftest.py](../conftest.py) contains pytest native fixtures.

See [Code Organization](CODE_ORGANIZATION.md#fixtures-testsfixtures-and-conftestpy) for the full fixture layout.

- Top level [conftest.py](../conftest.py) contains pytest hooks and `pytest_plugins` registration.
- General tests [conftest.py](../tests/conftest.py) contains fixtures that are used in multiple tests by multiple teams.
- If needed, create new `conftest.py` files in the relevant directories.
- Shared fixture implementations belong in [`tests/fixtures/<team>/<topic>.py`](../tests/fixtures/) and are registered via `pytest_plugins` in the root `conftest.py`.
- If needed, create new `conftest.py` files in the relevant directories for feature-local fixtures only.


## Fixtures
Expand Down
5 changes: 3 additions & 2 deletions docs/DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ The project is structured as follows:
- [utilities](../utilities): Base directory for utility functions
- Each module contains a set of utility functions related to a specific topic, for example:
- [infra](../utilities/infra.py): Infrastructure-related (cluster resources) utility functions
- [constants](../utilities/constants.py): Constants used in the project
- [docs](../docs): Documentation
- [constants](../utilities/constants/): Shared constants package — see [Code Organization](CODE_ORGANIZATION.md#constants-utilitiesconstants)
- [tests/fixtures](../tests/fixtures/): Shared pytest fixture implementations — see [Code Organization](CODE_ORGANIZATION.md#fixtures-testsfixtures-and-conftestpy)
- [docs](../docs): Documentation — including [Code Organization](CODE_ORGANIZATION.md) for constants, utilities, and fixtures
- [py_config](../tests/global_config.py) contains tests-specific configuration which can be controlled from the command line.
Please refer to [pytest-testconfig](https://github.com/wojole/pytest-testconfig) for more information.

Expand Down
2 changes: 1 addition & 1 deletion docs/QUARANTINE_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Apply pytest's `xfail` marker with `run=False`, for example:

```python
import pytest
from utilities.constants import QUARANTINED
from utilities.constants.pytest import QUARANTINED


@pytest.mark.xfail(
Expand Down
5 changes: 4 additions & 1 deletion libs/storage/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

from ocp_resources.datavolume import DataVolume

from utilities.constants import HPP_CAPABILITIES, StorageClassNames
from utilities.constants.storage import (
HPP_CAPABILITIES,
StorageClassNames,
)
from utilities.storage import HppCsiStorageClass

LOGGER = logging.getLogger(__name__)
Expand Down
10 changes: 4 additions & 6 deletions libs/vm/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@

from libs.vm.spec import CPU, Devices, Domain, Memory, Metadata, Template, VMISpec, VMSpec
from libs.vm.vm import BaseVirtualMachine, container_image, containerdisk_storage
from utilities import constants
from utilities.constants import MULTIARCH, OS_FLAVOR_FEDORA
from utilities import constants as constants_module
from utilities.constants.architecture import MULTIARCH
from utilities.constants.images import OS_FLAVOR_FEDORA, ArchImages


def fedora_vm(
Expand All @@ -31,10 +32,7 @@ def fedora_vm(


def fedora_image(arch: str | None = None) -> str:
if arch:
images = getattr(constants.ArchImages, arch.upper())
else:
images = constants.Images
images = getattr(ArchImages, arch.upper()) if arch else constants_module.Images

return container_image(base_image=images.Fedora.FEDORA_CONTAINER_IMAGE, arch=arch)

Expand Down
2 changes: 1 addition & 1 deletion libs/vm/vm.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
)
from tests.network.libs import cloudinit
from utilities import infra
from utilities.constants import CLOUD_INIT_DISK_NAME
from utilities.constants.virt import CLOUD_INIT_DISK_NAME
from utilities.virt import get_oc_image_info, vm_console_run_commands

if TYPE_CHECKING:
Expand Down
4 changes: 2 additions & 2 deletions scripts/tests_analyzer/tests/test_pytest_marker_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2874,7 +2874,7 @@ class TestImportContinuationSafety:
def test_import_continuation_line_is_safe(self, tmp_path: Path) -> None:
"""Import continuation lines do not trigger conservative fallback."""
source = (
"from utilities.constants import (\n"
"from utilities.constants.timeouts import (\n"
" TIMEOUT_1MIN,\n"
" TIMEOUT_2MIN,\n"
")\n"
Expand All @@ -2886,7 +2886,7 @@ def test_import_continuation_line_is_safe(self, tmp_path: Path) -> None:
"--- a/utilities/example.py\n"
"+++ b/utilities/example.py\n"
"@@ -1,3 +1,4 @@\n"
" from utilities.constants import (\n"
" from utilities.constants.timeouts import (\n"
" TIMEOUT_1MIN,\n"
"+ TIMEOUT_2MIN,\n"
" )\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@
from ocp_utilities.infra import assert_nodes_in_healthy_condition, assert_nodes_schedulable
from timeout_sampler import TimeoutSampler

from utilities.constants import KUBELET_READY_CONDITION, TIMEOUT_1MIN, TIMEOUT_5MIN, TIMEOUT_5SEC, TIMEOUT_10MIN
from utilities.constants.monitoring import KUBELET_READY_CONDITION
from utilities.constants.timeouts import (
TIMEOUT_1MIN,
TIMEOUT_5MIN,
TIMEOUT_5SEC,
TIMEOUT_10MIN,
)
from utilities.hco import get_installed_hco_csv, wait_for_hco_conditions
from utilities.infra import wait_for_pods_running
from utilities.operator import wait_for_cluster_operator_stabilize
Expand Down
Loading
Loading