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
58 changes: 22 additions & 36 deletions tests/catalog/test_compute_instance_catalog_item_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,10 @@ def test_compute_instance_catalog_item_field_definitions(
) -> None:
field_defs = [
{
"path": "spec.cpu_cores",
"display_name": "CPU Cores",
"path": "spec.instance_type",
"display_name": "Instance Type",
"editable": True,
"default": {"numberValue": 4},
},
{
"path": "spec.memory_gb",
"display_name": "Memory GB",
"editable": False,
"default": {"numberValue": 16},
"default": {"stringValue": "standard-2x4"},
},
]
name = unique_name("e2e-ci-fd")
Expand All @@ -105,52 +99,44 @@ def test_compute_instance_catalog_item_field_definitions(
try:
item = grpc.get_compute_instance_catalog_item(catalog_item_id=catalog_item_id)
returned_fds = item["object"].get("fieldDefinitions", [])
assert len(returned_fds) == 2

cpu_fd = next(fd for fd in returned_fds if fd["path"] == "spec.cpu_cores")
assert cpu_fd["displayName"] == "CPU Cores"
assert cpu_fd["editable"] is True
assert len(returned_fds) == 1

# editable=false is omitted by protobuf (default value), so we only check displayName
mem_fd = next(fd for fd in returned_fds if fd["path"] == "spec.memory_gb")
assert mem_fd["displayName"] == "Memory GB"
it_fd = next(fd for fd in returned_fds if fd["path"] == "spec.instance_type")
assert it_fd["displayName"] == "Instance Type"
assert it_fd["editable"] is True

updated_fds = [
{
"path": "spec.cpu_cores",
"display_name": "Number of CPU Cores",
"path": "spec.instance_type",
"display_name": "VM Size",
"editable": True,
"default": {"numberValue": 4},
},
{
"path": "spec.memory_gb",
"display_name": "Memory GB",
"editable": False,
"default": {"numberValue": 16},
"default": {"stringValue": "standard-2x4"},
},
]
grpc.update_compute_instance_catalog_item(catalog_item_id=catalog_item_id, field_definitions=updated_fds)

item = grpc.get_compute_instance_catalog_item(catalog_item_id=catalog_item_id)
returned_fds = item["object"].get("fieldDefinitions", [])
assert len(returned_fds) == 2
cpu_fd = next(fd for fd in returned_fds if fd["path"] == "spec.cpu_cores")
assert cpu_fd["displayName"] == "Number of CPU Cores"
assert len(returned_fds) == 1
it_fd = next(fd for fd in returned_fds if fd["path"] == "spec.instance_type")
assert it_fd["displayName"] == "VM Size"

reduced_fds = [
updated_fds_v2 = [
{
"path": "spec.cpu_cores",
"display_name": "Number of CPU Cores",
"editable": True,
"default": {"numberValue": 4},
"path": "spec.instance_type",
"display_name": "VM Size",
"editable": False,
"default": {"stringValue": "standard-4x8"},
},
]
grpc.update_compute_instance_catalog_item(catalog_item_id=catalog_item_id, field_definitions=reduced_fds)
grpc.update_compute_instance_catalog_item(catalog_item_id=catalog_item_id, field_definitions=updated_fds_v2)

item = grpc.get_compute_instance_catalog_item(catalog_item_id=catalog_item_id)
returned_fds = item["object"].get("fieldDefinitions", [])
assert len(returned_fds) == 1
assert returned_fds[0]["path"] == "spec.cpu_cores"
it_fd = next(fd for fd in returned_fds if fd["path"] == "spec.instance_type")
assert it_fd["displayName"] == "VM Size"
assert it_fd.get("editable", False) is False
finally:
grpc.delete_compute_instance_catalog_item(catalog_item_id=catalog_item_id)

Expand Down
22 changes: 14 additions & 8 deletions tests/core/osac_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,20 @@


class OsacCLI:
def __init__(self, *, binary: str, address: str, token_script: str, namespace: str) -> None:
def __init__(
self,
*,
binary: str,
address: str,
token_script: str,
namespace: str,
default_instance_type: str | None = None,
) -> None:
self.binary: str = binary
self.namespace: str = namespace
self._address: str = address
self._token_script: str = token_script
self.default_instance_type: str | None = default_instance_type
# Each OsacCLI instance gets its own config directory so that parallel
# xdist workers (or multiple CLI fixtures) don't overwrite each other's
# login credentials via the shared ~/.config/osac/config.json.
Expand Down Expand Up @@ -50,8 +59,6 @@ def create_compute_instance(
*,
template: str,
network_attachments: list[dict[str, Any]] | None = None,
cores: int | None = None,
memory_gib: int | None = None,
boot_disk_size: int = 20,
image: str = "quay.io/containerdisks/fedora:latest",
image_source_type: str = "registry",
Expand All @@ -74,12 +81,11 @@ def create_compute_instance(
run_strategy,
]

if instance_type is not None:
if cores is not None or memory_gib is not None:
raise ValueError("Cannot specify cores/memory_gib together with instance_type")
args.extend(["--instance-type", instance_type])
effective_instance_type = instance_type if instance_type is not None else self.default_instance_type
if effective_instance_type is not None:
args.extend(["--instance-type", effective_instance_type])
else:
args.extend(["--cores", str(2 if cores is None else cores), "--memory-gib", str(4 if memory_gib is None else memory_gib)])
raise ValueError("instance_type or default_instance_type must be set")

# Add network attachments
if network_attachments is not None:
Expand Down
31 changes: 31 additions & 0 deletions tests/vmaas/conftest.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from __future__ import annotations

import os
import subprocess
import time
import uuid
from collections.abc import Iterator

import pytest

Expand All @@ -16,8 +18,12 @@
wait_for_virtual_network_ready,
)
from tests.core.k8s_client import K8sClient
from tests.core.osac_cli import OsacCLI
from tests.core.runner import env

DEFAULT_IT_CORES: int = 2
DEFAULT_IT_MEMORY_GIB: int = 4


@pytest.fixture(scope="session")
def k8s_virt_client(namespace: str) -> K8sClient:
Expand Down Expand Up @@ -145,3 +151,28 @@ def default_subnet(default_networking: dict[str, str]) -> str:
def default_subnet_ref(default_networking: dict[str, str]) -> str:
"""Convenience fixture that returns the subnet CR name (for K8s API usage)."""
return default_networking["subnet_cr_name"]


@pytest.fixture(scope="session")
def default_instance_type(private_grpc: GRPCClient, test_run_id: str) -> Iterator[str]:
"""Create a default ACTIVE instance type for VM tests; clean up after."""
it_name = f"e2e-default-it-{test_run_id}"
private_grpc.create_instance_type(
name=it_name,
cores=DEFAULT_IT_CORES,
memory_gib=DEFAULT_IT_MEMORY_GIB,
description="Default E2E instance type",
)
yield it_name
try:
private_grpc.delete_instance_type(name=it_name)
except subprocess.CalledProcessError as e:
output = ((e.stdout or "") + (e.stderr or "")).lower()
if "not found" not in output:
raise


@pytest.fixture(scope="session", autouse=True)
def _set_cli_default_instance_type(cli: OsacCLI, default_instance_type: str) -> None:
"""Wire the session-scoped default instance type into the shared CLI fixture."""
cli.default_instance_type = default_instance_type
10 changes: 7 additions & 3 deletions tests/vmaas/test_compute_instance_api_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
from tests.core.runner import poll_until

TEST_IMAGE_REF: str = "quay.io/containerdisks/fedora:latest"
TEST_BOOT_DISK_SIZE: int = 20
TEST_CORES: int = 2
TEST_MEMORY_GIB: int = 4
TEST_BOOT_DISK_SIZE: int = 20


def test_compute_instance_api_fields(
Expand Down Expand Up @@ -89,12 +89,16 @@ def test_compute_instance_api_fields(
assert vm_status == "Running", f"VM should be Running, got {vm_status}"

# Immutability: cores
output, rc = k8s_hub_client.patch(resource="computeinstance", name=instance_name, patch='{"spec":{"cores":8}}')
output, rc = k8s_hub_client.patch(
resource="computeinstance", name=instance_name, patch='{"spec":{"cores":8}}'
)
assert rc != 0, "cores field should be immutable"
assert "cores is immutable" in output, f"Expected immutability error, got: {output}"

# Immutability: memoryGiB
output, rc = k8s_hub_client.patch(resource="computeinstance", name=instance_name, patch='{"spec":{"memoryGiB":16}}')
output, rc = k8s_hub_client.patch(
resource="computeinstance", name=instance_name, patch='{"spec":{"memoryGiB":16}}'
)
assert rc != 0, "memoryGiB field should be immutable"
assert "memoryGiB is immutable" in output, f"Expected immutability error, got: {output}"

Expand Down
20 changes: 14 additions & 6 deletions tests/vmaas/test_compute_instance_cli_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
from tests.core.helpers import wait_for_cr, wait_for_deletion, wait_for_provision, wait_for_running
from tests.core.k8s_client import K8sClient
from tests.core.osac_cli import OsacCLI
from tests.vmaas.conftest import DEFAULT_IT_CORES, DEFAULT_IT_MEMORY_GIB

TEST_CORES: int = 2
TEST_MEMORY_GIB: int = 4
TEST_BOOT_DISK_SIZE: int = 20
TEST_IMAGE: str = "quay.io/containerdisks/fedora:latest"
TEST_IMAGE_SOURCE_TYPE: str = "registry"
Expand All @@ -26,8 +25,6 @@ def test_compute_instance_cli_explicit_fields(
uuid: str = cli.create_compute_instance(
template="osac.templates.ocp_virt_vm",
network_attachments=[{"subnet": default_subnet}],
cores=TEST_CORES,
memory_gib=TEST_MEMORY_GIB,
boot_disk_size=TEST_BOOT_DISK_SIZE,
image=TEST_IMAGE,
image_source_type=TEST_IMAGE_SOURCE_TYPE,
Expand All @@ -41,8 +38,19 @@ def test_compute_instance_cli_explicit_fields(

ci_spec: dict[str, Any] = k8s_hub_client.get_json(resource="computeinstance", name=ci_name)
spec: dict[str, Any] = ci_spec["spec"]
assert spec["cores"] == TEST_CORES, f"cores mismatch: {spec['cores']} != {TEST_CORES}"
assert spec["memoryGiB"] == TEST_MEMORY_GIB, f"memoryGiB mismatch: {spec['memoryGiB']} != {TEST_MEMORY_GIB}"
assert spec["cores"] == DEFAULT_IT_CORES, f"cores mismatch: {spec['cores']} != {DEFAULT_IT_CORES}"
assert spec["memoryGiB"] == DEFAULT_IT_MEMORY_GIB, f"memoryGiB mismatch: {spec['memoryGiB']} != {DEFAULT_IT_MEMORY_GIB}"

# Verify osac.openshift.io/instance-type-name label is set by reconciler
labels: dict[str, str] = ci_spec["metadata"].get("labels", {})
it_label: str | None = labels.get("osac.openshift.io/instance-type-name")
assert it_label is not None, (
f"osac.openshift.io/instance-type-name label should be set, got: {it_label!r}"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
assert len(it_label) > 0, (
f"osac.openshift.io/instance-type-name label should be non-empty, got: {it_label!r}"
)

assert spec["bootDisk"]["sizeGiB"] == TEST_BOOT_DISK_SIZE, (
f"bootDisk.sizeGiB mismatch: {spec['bootDisk']['sizeGiB']} != {TEST_BOOT_DISK_SIZE}"
)
Expand Down
Loading