Skip to content

Commit aeda63d

Browse files
committed
test(k8s): refuse-and-raise semantics + post-rebase import/registry fixes
Align k8s tests with the refuse-and-raise release semantics and fix stale import paths and registry names introduced by the earlier rebase.
1 parent 749ed51 commit aeda63d

11 files changed

Lines changed: 122 additions & 131 deletions

File tree

src/orb/providers/aws/configuration/template_extension.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class AWSTemplateExtensionConfig(BaseModel):
3131
fleet_role: Optional[str] = Field(None, description="IAM role for Spot Fleet")
3232
key_name: Optional[str] = Field(None, description="Default key name")
3333
user_data_script: Optional[str] = Field(None, description="User data script")
34-
instance_profile: Optional[str] = Field(None, description="Instance profile")
34+
machine_role: Optional[str] = Field(None, description="IAM instance profile / machine role")
3535

3636
# AWS pricing and allocation defaults
3737
max_spot_price: Optional[float] = Field(None, description="Maximum price for Spot instances")

src/orb/providers/aws/infrastructure/adapters/template_adapter.py

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -133,22 +133,6 @@ def extend_template_fields(self, template: Template) -> Template:
133133
if not template.provider_api:
134134
template.provider_api = self._determine_provider_api(template)
135135

136-
# Extend with AWS-specific provider data
137-
if not template.provider_data:
138-
template.provider_data = {}
139-
140-
if "aws" not in template.provider_data:
141-
template.provider_data["aws"] = {}
142-
143-
# Add AWS-specific provider data
144-
template.provider_data["aws"].update(
145-
{
146-
"fleet_type": getattr(template, "fleet_type", None),
147-
"supported_fields": self._AWS_SUPPORTED_FIELDS,
148-
"validation_enabled": True,
149-
}
150-
)
151-
152136
return template
153137

154138
def resolve_template_references(self, template: Template) -> Template:

src/orb/providers/k8s/domain/template/k8s_template_aggregate.py

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,17 @@
33
Mirrors :class:`orb.providers.aws.domain.template.aws_template_aggregate.AWSTemplate`
44
for the kubernetes provider. ``K8sTemplate`` is a strongly-typed
55
``Template`` subclass: kubernetes-specific operator-supplied fields live as
6-
flat first-class attributes rather than as opaque entries under
7-
``Template.provider_data['k8s']``.
6+
flat first-class attributes on the subclass.
87
98
Generic provisioning concepts continue to be expressed on the parent
109
``Template``:
1110
12-
* ``image_id`` — container image string consumed by the spec builders.
13-
* ``tags`` — operator tags; merged into the pod ``metadata.labels``
11+
* ``image_id`` — container image string consumed by the spec builders.
12+
* ``tags`` — operator tags; merged into the pod ``metadata.labels``
1413
surface at spec-build time.
15-
* ``max_instances`` — quota cap; the per-request replica count comes from
14+
* ``max_instances`` — quota cap; the per-request replica count comes from
1615
``request.requested_count``.
17-
* ``instance_profile`` — falls back as the ``serviceAccountName`` when
16+
* ``machine_role`` — falls back as the ``serviceAccountName`` when
1817
:attr:`K8sTemplate.service_account` is not set.
1918
2019
Upcasting an arbitrary ``Template`` to a ``K8sTemplate`` is safe via
@@ -561,11 +560,10 @@ def _promote_extensions_and_defaults(self) -> K8sTemplate:
561560
backwards compatibility with operators who set kubernetes fields
562561
via the DTO surface rather than via the typed template
563562
directly.
564-
2. Fall back to :attr:`Template.instance_profile` for
563+
2. Fall back to :attr:`Template.machine_role` for
565564
:attr:`service_account` when the latter is unset. This honours
566-
the documented mapping of the generic ``instance_profile``
567-
field (see :class:`Template` line 57) onto the kubernetes
568-
``serviceAccountName`` concept.
565+
the documented mapping of the generic ``machine_role``
566+
field onto the kubernetes ``serviceAccountName`` concept.
569567
"""
570568
# Promote provider_config dict entries onto typed fields.
571569
pc = getattr(self, "provider_config", None)
@@ -632,9 +630,9 @@ def _promote_extensions_and_defaults(self) -> K8sTemplate:
632630
if self.volumes is None and pc.get("volumes") is not None:
633631
object.__setattr__(self, "volumes", _coerce_volumes(pc.get("volumes")))
634632

635-
# Service-account fallback to the generic instance_profile.
636-
if self.service_account is None and self.instance_profile:
637-
object.__setattr__(self, "service_account", self.instance_profile)
633+
# Service-account fallback to the generic machine_role.
634+
if self.service_account is None and self.machine_role:
635+
object.__setattr__(self, "service_account", self.machine_role)
638636

639637
return self
640638

src/orb/providers/k8s/infrastructure/adapters/template_adapter.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,11 @@ def validate_template(self, template: Template) -> list[str]: # type: ignore[ov
124124
return errors
125125

126126
def extend_template_fields(self, template: Template) -> Template:
127-
"""Attach kubernetes-specific provider data to *template* in place.
127+
"""Set kubernetes-specific defaults on *template*.
128128
129-
With the typed :class:`K8sTemplate` aggregate in place, the
130-
kubernetes provider does not need to mirror operator-supplied
131-
fields into ``template.provider_data["k8s"]`` — handlers read
132-
the typed fields directly. We still default ``provider_api``
133-
to ``"Pod"`` when the template arrives without one.
129+
With the typed :class:`K8sTemplate` aggregate in place, handlers read
130+
typed fields directly. We default ``provider_api`` to ``"Pod"``
131+
when the template arrives without one.
134132
"""
135133
if not template.provider_api:
136134
template.provider_api = self.get_provider_api()

tests/providers/aws/live/conftest.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,17 @@ def pytest_collection_modifyitems(config, items):
2828
``pytestmark`` only works at module-level — conftest-level constants
2929
are not picked up by pytest's marker discovery. The collection hook
3030
is the canonical place to bulk-apply markers across a directory.
31+
32+
``items`` is the FULL collected list across the pytest session, not
33+
just this subtree — filter to items whose path lives under this
34+
conftest's directory so we do NOT accidentally mark tests outside
35+
this live subtree as serial.
3136
"""
37+
subtree = str(Path(__file__).resolve().parent)
3238
marker = pytest.mark.serial
3339
for item in items:
34-
item.add_marker(marker)
40+
if str(item.path).startswith(subtree):
41+
item.add_marker(marker)
3542

3643

3744
def _find_repo_root(start: Path) -> Path:

tests/providers/k8s/live/conftest.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import json
1616
import logging
1717
import uuid
18+
from pathlib import Path
1819
from typing import Generator
1920

2021
import pytest
@@ -40,13 +41,17 @@ def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item
4041
``pytestmark`` at module level is not picked up by conftest-level
4142
discovery; the collection hook is the canonical place to bulk-apply
4243
markers across a directory subtree.
43-
"""
44-
import pathlib
4544
46-
live_dir = str(pathlib.Path(__file__).parent)
45+
``items`` is the FULL collected list across the pytest session, not
46+
just this subtree — filter to items whose path lives under this
47+
conftest's directory so we do NOT accidentally mark
48+
unit/mocked/contract tests as serial when the parent
49+
``tests/providers/k8s`` directory is collected as a whole.
50+
"""
51+
subtree = str(Path(__file__).resolve().parent)
4752
marker = pytest.mark.serial
4853
for item in items:
49-
if str(item.fspath).startswith(live_dir):
54+
if str(item.path).startswith(subtree):
5055
item.add_marker(marker)
5156

5257

tests/providers/k8s/unit/configuration/test_template_extension.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,13 @@ class TestExtensionRegistration:
131131
"""Verify the DTO config is wired into ``TemplateExtensionRegistry``."""
132132

133133
def test_extension_registered_after_bootstrap(self) -> None:
134-
"""Running k8s extension registration wires the DTO config into the registry."""
135-
# Registration is an explicit call, not an import side-effect — invoke it
136-
# directly so the test is self-contained and order-independent (it must not
137-
# rely on another test having bootstrapped the global registry first).
134+
"""Calling the explicit lifecycle hook registers the DTO config.
135+
136+
Module-level auto-registration was removed to keep import-time
137+
side effects predictable; extension registration now runs through
138+
the explicit ``initialize_k8s_provider`` path (or the standalone
139+
``register_k8s_extensions`` helper for tests).
140+
"""
138141
from orb.infrastructure.registry.template_extension_registry import (
139142
TemplateExtensionRegistry,
140143
)

tests/providers/k8s/unit/domain/template/test_k8s_template.py

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* the generic ``Template.image_id`` is the single source of truth for
66
the container image; shadow fields are gone.
77
* :attr:`K8sTemplate.service_account` falls back to
8-
:attr:`Template.instance_profile` via the ``after`` model-validator.
8+
:attr:`Template.machine_role` via the ``after`` model-validator.
99
* :meth:`K8sTemplate.resolve_pod_labels` projects ``Template.tags`` into
1010
string-keyed string-valued k8s labels, dropping ``None`` entries.
1111
* :meth:`upcast_to_k8s_template` is a clean round-trip from a generic
@@ -103,30 +103,45 @@ def test_extension_config_does_not_define_container_image() -> None:
103103

104104

105105
# ---------------------------------------------------------------------------
106-
# service_account fallback to instance_profile
106+
# service_account fallback to machine_role
107107
# ---------------------------------------------------------------------------
108108

109109

110-
def test_service_account_falls_back_to_instance_profile() -> None:
110+
def test_service_account_falls_back_to_machine_role() -> None:
111111
t = K8sTemplate(
112112
template_id="tpl",
113113
image_id="busybox:latest",
114-
instance_profile="my-svc-acct",
114+
machine_role="my-svc-acct",
115115
)
116116
assert t.service_account == "my-svc-acct"
117117

118118

119-
def test_explicit_service_account_wins_over_instance_profile() -> None:
119+
def test_service_account_falls_back_to_machine_role_via_deprecated_instance_profile() -> None:
120+
"""Old instance_profile kwarg still works via the DeprecationWarning path."""
121+
import warnings
122+
123+
with warnings.catch_warnings(record=True) as w:
124+
warnings.simplefilter("always")
125+
t = K8sTemplate(
126+
template_id="tpl",
127+
image_id="busybox:latest",
128+
instance_profile="my-svc-acct",
129+
)
130+
assert any(issubclass(warning.category, DeprecationWarning) for warning in w)
131+
assert t.service_account == "my-svc-acct"
132+
133+
134+
def test_explicit_service_account_wins_over_machine_role() -> None:
120135
t = K8sTemplate(
121136
template_id="tpl",
122137
image_id="busybox:latest",
123-
instance_profile="instance-profile-name",
138+
machine_role="machine-role-name",
124139
service_account="explicit-sa",
125140
)
126141
assert t.service_account == "explicit-sa"
127142

128143

129-
def test_no_instance_profile_leaves_service_account_unset() -> None:
144+
def test_no_machine_role_leaves_service_account_unset() -> None:
130145
t = K8sTemplate(template_id="tpl", image_id="busybox:latest")
131146
assert t.service_account is None
132147

@@ -164,7 +179,7 @@ def test_upcast_from_generic_template_round_trips_cleanly() -> None:
164179
image_id="busybox:latest",
165180
max_instances=4,
166181
tags={"team": "ml"},
167-
instance_profile="svc-acct",
182+
machine_role="svc-acct",
168183
)
169184
k8s = upcast_to_k8s_template(base)
170185
assert isinstance(k8s, K8sTemplate)

tests/providers/k8s/unit/handlers/test_statefulset_handler.py

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
from orb.domain.request.value_objects import RequestId, RequestType
3232
from orb.domain.template.template_aggregate import Template
3333
from orb.providers.k8s.configuration.config import K8sProviderConfig
34-
from orb.providers.k8s.exceptions.k8s_exceptions import K8sError
3534
from orb.providers.k8s.infrastructure.handlers.statefulset_handler import K8sStatefulSetHandler
3635

3736
# ---------------------------------------------------------------------------
@@ -247,15 +246,19 @@ async def test_release_hosts_selective_highest_ordinal_no_warning() -> None:
247246

248247

249248
@pytest.mark.asyncio
250-
async def test_release_hosts_selective_non_highest_ordinal_refused() -> None:
251-
"""When the victims are not the top-of-stack ordinals, the handler
252-
raises K8sError and issues no scale patch.
253-
254-
Commit b4a77b1a deliberately changed this behaviour: silently evicting
255-
different pods (the controller always picks the highest-ordinal set)
256-
would cause data loss, so the handler refuses the request and tells
257-
the caller which victims to supply instead.
249+
async def test_release_hosts_selective_non_highest_ordinal_raises_and_does_not_scale() -> None:
250+
"""When the victims include non-highest ordinals, the handler refuses
251+
the request with ``K8sError`` and does NOT patch the StatefulSet.
252+
253+
The controller only supports scale-down from the top of the ordinal
254+
stack; the previous warn-and-scale behaviour silently mutated a
255+
different pod than the caller asked for, so the current
256+
implementation raises instead — see the sibling test module
257+
``test_statefulset_release_reject.py`` for the guard function's own
258+
unit coverage.
258259
"""
260+
from orb.providers.k8s.exceptions.k8s_exceptions import K8sError
261+
259262
core_v1 = MagicMock()
260263
apps_v1 = MagicMock()
261264
apps_v1.read_namespaced_stateful_set.return_value = _make_statefulset_status(spec_replicas=5)
@@ -271,30 +274,22 @@ async def test_release_hosts_selective_non_highest_ordinal_refused() -> None:
271274
)
272275

273276
# current=5, victims=[ordinal-1, ordinal-2] — non-highest ordinals.
274-
# The controller would evict ordinals 3 and 4 instead, so the handler
275-
# refuses rather than silently releasing the wrong pods.
276-
with pytest.raises(K8sError) as exc_info:
277+
with pytest.raises(K8sError, match="selective release refused"):
277278
await handler.release_hosts(["orb-deadbeef-1", "orb-deadbeef-2"], request.provider_data)
278279

279-
# The error message must mention the correct top-of-stack victims.
280-
assert "orb-deadbeef-3" in str(exc_info.value) or "orb-deadbeef-4" in str(exc_info.value)
281-
282-
# Refusal happens before any scale patch — the StatefulSet must be
283-
# left untouched.
280+
# Nothing was mutated — neither a scale patch nor a pod deletion.
284281
apps_v1.patch_namespaced_stateful_set_scale.assert_not_called()
285-
286-
# Pods are NEVER deleted directly.
287282
core_v1.delete_namespaced_pod.assert_not_called()
288283
apps_v1.delete_namespaced_stateful_set.assert_not_called()
289284

290285

291286
@pytest.mark.asyncio
292-
async def test_release_hosts_selective_unparseable_victim_names_refused() -> None:
287+
async def test_release_hosts_selective_unparseable_victim_names_raises() -> None:
293288
"""Victim names that do not parse as ``<statefulset>-<ordinal>`` are
294-
refused with K8sError — an unparseable name cannot be the top-of-
295-
stack ordinal, so honouring the request would silently evict the
296-
wrong pods.
297-
"""
289+
treated as non-top-of-stack and therefore refused, since the
290+
controller cannot guarantee they will be the ones evicted."""
291+
from orb.providers.k8s.exceptions.k8s_exceptions import K8sError
292+
298293
core_v1 = MagicMock()
299294
apps_v1 = MagicMock()
300295
apps_v1.read_namespaced_stateful_set.return_value = _make_statefulset_status(spec_replicas=3)
@@ -309,15 +304,10 @@ async def test_release_hosts_selective_unparseable_victim_names_refused() -> Non
309304
namespace="orb-test",
310305
)
311306

312-
# Victim name does not match the StatefulSet's name prefix — its
313-
# ordinal cannot be parsed, so the handler refuses rather than
314-
# silently releasing the wrong pod.
315-
with pytest.raises(K8sError):
307+
with pytest.raises(K8sError, match="selective release refused"):
316308
await handler.release_hosts(["some-other-pod"], request.provider_data)
317309

318-
# Refusal happens before any scale patch.
319310
apps_v1.patch_namespaced_stateful_set_scale.assert_not_called()
320-
# Pods are NEVER deleted directly.
321311
core_v1.delete_namespaced_pod.assert_not_called()
322312
apps_v1.delete_namespaced_stateful_set.assert_not_called()
323313

0 commit comments

Comments
 (0)