Skip to content

Commit be4487a

Browse files
committed
refactor(template): move pool template rendering into K8sTemplateLoader
- Extend K8sTemplateLoader with optional pool_template and build_pool_manifest - BatchSandboxProvider delegates _build_pool_manifest_from_template to loader - Remove provider-level pool template / jinja env / render imports - Migrate pool manifest tests from test_template_api.py to test_k8s_template_loader.py
1 parent c627c04 commit be4487a

4 files changed

Lines changed: 203 additions & 143 deletions

File tree

rock/sandbox/operator/k8s/provider.py

Lines changed: 5 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
"""K8s provider implementations for managing sandbox resources."""
22

33
import base64
4-
import copy
54
import fnmatch
65
import hashlib
76
import json
@@ -11,7 +10,6 @@
1110
from dataclasses import dataclass
1211
from typing import Any, Protocol
1312

14-
import jinja2
1513
import yaml
1614
from cryptography.hazmat.primitives.ciphers.aead import AESGCM
1715
from kubernetes import client
@@ -28,7 +26,6 @@
2826
from rock.sandbox.operator.k8s.constants import K8sConstants
2927
from rock.sandbox.operator.k8s.template_loader import K8sTemplateLoader
3028
from rock.sandbox.remote_sandbox import RemoteSandboxRuntime
31-
from rock.utils.jinja_render import render_node
3229

3330
logger = init_logger(__name__)
3431

@@ -312,14 +309,11 @@ def __init__(self, k8s_config: K8sConfig):
312309
self._nacos_provider = None
313310
self._image_auth_key = self._load_image_auth_key(k8s_config)
314311

315-
# Pool template for Template API (Warm path)
316-
self._pool_template = k8s_config.pool_template
317-
self._jinja_env = jinja2.Environment()
318-
319-
# Initialize template loader with config templates
312+
# Initialize template loader with config templates and pool template
320313
self._template_loader = K8sTemplateLoader(
321314
templates=k8s_config.templates,
322315
default_namespace=k8s_config.namespace,
316+
pool_template=k8s_config.pool_template,
323317
)
324318
logger.info(f"Available K8S templates: {', '.join(self._template_loader.available_templates)}")
325319

@@ -1057,54 +1051,10 @@ async def _delete_pool(self, pool_name: str) -> None:
10571051
def _build_pool_manifest_from_template(self, pool_name: str, spec: TemplateSpec) -> dict[str, Any]:
10581052
"""Build Pool CRD manifest from config template and spec.
10591053
1060-
Renders Jinja2 variables in the pool_template config:
1061-
from_image, cpu_count, memory_mb, disk_gb, num_gpus, accelerator_type,
1062-
buffer_min, buffer_max, pool_min, pool_max
1054+
Delegates to the shared K8sTemplateLoader so pool and sandbox template
1055+
rendering live in one place.
10631056
"""
1064-
template = copy.deepcopy(self._pool_template)
1065-
1066-
ctx: dict[str, Any] = {
1067-
"from_image": spec.from_image,
1068-
"cpu_count": spec.cpu_count,
1069-
"memory_mb": spec.memory_mb,
1070-
}
1071-
if spec.disk_gb is not None:
1072-
ctx["disk_gb"] = spec.disk_gb
1073-
if spec.num_gpus is not None:
1074-
ctx["num_gpus"] = spec.num_gpus
1075-
if spec.accelerator_type is not None:
1076-
ctx["accelerator_type"] = spec.accelerator_type
1077-
if spec.buffer_min is not None:
1078-
ctx["buffer_min"] = spec.buffer_min
1079-
if spec.buffer_max is not None:
1080-
ctx["buffer_max"] = spec.buffer_max
1081-
if spec.pool_min is not None:
1082-
ctx["pool_min"] = spec.pool_min
1083-
if spec.pool_max is not None:
1084-
ctx["pool_max"] = spec.pool_max
1085-
1086-
rendered = render_node(template, self._jinja_env, ctx)
1087-
1088-
# Ensure capacitySpec values are integers (Jinja2 renders them as strings)
1089-
cap = rendered.get("capacitySpec", {})
1090-
if cap:
1091-
for k in ("bufferMin", "bufferMax", "poolMin", "poolMax"):
1092-
if k in cap:
1093-
cap[k] = int(cap[k])
1094-
1095-
manifest = {
1096-
"apiVersion": K8sConstants.CRD_API_VERSION,
1097-
"kind": K8sConstants.CRD_KIND_POOL,
1098-
"metadata": {
1099-
"name": pool_name,
1100-
"namespace": self.namespace,
1101-
"labels": {
1102-
K8sConstants.LABEL_MANAGED_BY: K8sConstants.LABEL_MANAGED_BY_TEMPLATE_API,
1103-
},
1104-
},
1105-
"spec": rendered,
1106-
}
1107-
return manifest
1057+
return self._template_loader.build_pool_manifest(pool_name, spec)
11081058

11091059
def _map_pool_to_template_status(self, pool: dict) -> dict:
11101060
"""Map Pool CRD to template status dict (design doc format)."""

rock/sandbox/operator/k8s/template_loader.py

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""K8S template loader for BatchSandbox manifests."""
1+
"""K8S template loader for BatchSandbox and Pool manifests."""
22

33
import copy
44
import json
@@ -14,17 +14,24 @@
1414

1515

1616
class K8sTemplateLoader:
17-
"""Loader for K8S BatchSandbox templates."""
17+
"""Loader for K8S BatchSandbox and Pool CRD manifests."""
1818

19-
def __init__(self, templates: dict[str, dict[str, Any]], default_namespace: str = "rock"):
19+
def __init__(
20+
self,
21+
templates: dict[str, dict[str, Any]],
22+
default_namespace: str = "rock",
23+
pool_template: dict[str, Any] | None = None,
24+
):
2025
"""Initialize template loader.
2126
2227
Args:
23-
templates: Dictionary of template configurations from K8sConfig
28+
templates: Dictionary of BatchSandbox template configurations from K8sConfig
2429
default_namespace: Default namespace if template doesn't specify one
30+
pool_template: Optional Pool CRD template for Template API (Warm path)
2531
"""
2632
self._templates: dict[str, dict[str, Any]] = templates
2733
self._default_namespace = default_namespace
34+
self._pool_template = pool_template
2835

2936
if not self._templates:
3037
raise ValueError("No templates provided. At least one template must be defined in K8sConfig.templates.")
@@ -177,6 +184,74 @@ def build_manifest(
177184

178185
return manifest
179186

187+
def build_pool_manifest(self, pool_name: str, spec: Any) -> dict[str, Any]:
188+
"""Build a complete Pool CRD manifest from the pool template and spec.
189+
190+
The pool template is rendered with Jinja2 against a context built from
191+
``spec``: from_image, cpu_count, memory_mb, disk_gb, num_gpus,
192+
accelerator_type, buffer_min, buffer_max, pool_min, pool_max. Optional
193+
fields are only added to the context when they are not None.
194+
195+
After rendering, ``capacitySpec`` integer fields are converted from
196+
strings back to ints (Jinja2 renders numbers as strings).
197+
198+
Args:
199+
pool_name: Name for the Pool CRD (also the template ID).
200+
spec: Template creation spec with the attributes listed above.
201+
202+
Returns:
203+
Complete Pool CRD manifest.
204+
205+
Raises:
206+
ValueError: If no pool template was configured.
207+
"""
208+
if not self._pool_template:
209+
raise ValueError("No pool template configured. Set k8s.pool_template in config.")
210+
211+
template = copy.deepcopy(self._pool_template)
212+
213+
ctx: dict[str, Any] = {
214+
"from_image": spec.from_image,
215+
"cpu_count": spec.cpu_count,
216+
"memory_mb": spec.memory_mb,
217+
}
218+
if spec.disk_gb is not None:
219+
ctx["disk_gb"] = spec.disk_gb
220+
if spec.num_gpus is not None:
221+
ctx["num_gpus"] = spec.num_gpus
222+
if spec.accelerator_type is not None:
223+
ctx["accelerator_type"] = spec.accelerator_type
224+
if spec.buffer_min is not None:
225+
ctx["buffer_min"] = spec.buffer_min
226+
if spec.buffer_max is not None:
227+
ctx["buffer_max"] = spec.buffer_max
228+
if spec.pool_min is not None:
229+
ctx["pool_min"] = spec.pool_min
230+
if spec.pool_max is not None:
231+
ctx["pool_max"] = spec.pool_max
232+
233+
rendered = render_node(template, self._jinja_env, ctx)
234+
235+
# Ensure capacitySpec values are integers (Jinja2 renders them as strings)
236+
cap = rendered.get("capacitySpec", {})
237+
if cap:
238+
for k in ("bufferMin", "bufferMax", "poolMin", "poolMax"):
239+
if k in cap:
240+
cap[k] = int(cap[k])
241+
242+
return {
243+
"apiVersion": K8sConstants.CRD_API_VERSION,
244+
"kind": K8sConstants.CRD_KIND_POOL,
245+
"metadata": {
246+
"name": pool_name,
247+
"namespace": self._default_namespace,
248+
"labels": {
249+
K8sConstants.LABEL_MANAGED_BY: K8sConstants.LABEL_MANAGED_BY_TEMPLATE_API,
250+
},
251+
},
252+
"spec": rendered,
253+
}
254+
180255
@property
181256
def available_templates(self) -> list[str]:
182257
"""Get list of available template names."""

tests/unit/sandbox/operator/test_k8s_template_loader.py

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import pytest
44

55
from rock.sandbox.operator.k8s.constants import K8sConstants
6+
from rock.sandbox.operator.k8s.provider import TemplateSpec
67
from rock.sandbox.operator.k8s.template_loader import K8sTemplateLoader
78

89

@@ -377,3 +378,121 @@ def test_build_manifest_passes_encrypted_image_auth(self):
377378

378379
annotations = manifest["spec"]["template"]["metadata"]["annotations"]
379380
assert annotations["example.com/image-auth"] == "dGVzdC1lbmNyeXB0ZWQ="
381+
382+
383+
class TestBuildPoolManifest:
384+
"""Tests for Pool CRD manifest building via K8sTemplateLoader."""
385+
386+
POOL_TEMPLATE = {
387+
"capacitySpec": {
388+
"bufferMin": "{{ buffer_min | default(1) }}",
389+
"bufferMax": "{{ buffer_max | default(3) }}",
390+
"poolMin": "{{ pool_min | default(1) }}",
391+
"poolMax": "{{ pool_max | default(10) }}",
392+
},
393+
"template": {
394+
"metadata": {"labels": {"app": "rock-pool"}},
395+
"spec": {
396+
"tolerations": [{"operator": "Exists"}],
397+
"containers": [{
398+
"name": "main",
399+
"image": "{{ from_image }}",
400+
"resources": {
401+
"limits": {
402+
"cpu": "{{ cpu_count }}",
403+
"memory": "{{ memory_mb }}Mi",
404+
},
405+
"requests": {
406+
"cpu": "{{ cpu_count }}",
407+
"memory": "{{ memory_mb }}Mi",
408+
},
409+
},
410+
}],
411+
},
412+
},
413+
}
414+
415+
@pytest.fixture
416+
def pool_loader(self):
417+
"""Create a loader with a pool template."""
418+
return K8sTemplateLoader(
419+
templates={"default": {"ports": {"proxy": 8000}, "template": {"spec": {}}}},
420+
default_namespace="rock-test",
421+
pool_template=self.POOL_TEMPLATE,
422+
)
423+
424+
def test_build_pool_manifest_basic(self, pool_loader):
425+
"""Pool CRD wrapper is assembled correctly."""
426+
spec = TemplateSpec(from_image="python:3.11", cpu_count=2, memory_mb=2048)
427+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
428+
429+
assert manifest["apiVersion"] == K8sConstants.CRD_API_VERSION
430+
assert manifest["kind"] == K8sConstants.CRD_KIND_POOL
431+
assert manifest["metadata"]["name"] == "tpl-abc123"
432+
assert manifest["metadata"]["namespace"] == "rock-test"
433+
assert manifest["metadata"]["labels"][K8sConstants.LABEL_MANAGED_BY] == K8sConstants.LABEL_MANAGED_BY_TEMPLATE_API
434+
435+
def test_build_pool_manifest_renders_image(self, pool_loader):
436+
"""Jinja2 renders from_image variable."""
437+
spec = TemplateSpec(from_image="python:3.11", cpu_count=2, memory_mb=2048)
438+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
439+
440+
container = manifest["spec"]["template"]["spec"]["containers"][0]
441+
assert container["image"] == "python:3.11"
442+
443+
def test_build_pool_manifest_renders_cpu(self, pool_loader):
444+
"""Jinja2 renders cpu_count variable."""
445+
spec = TemplateSpec(from_image="python:3.11", cpu_count=4, memory_mb=2048)
446+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
447+
448+
container = manifest["spec"]["template"]["spec"]["containers"][0]
449+
assert container["resources"]["limits"]["cpu"] == "4"
450+
451+
def test_build_pool_manifest_renders_memory(self, pool_loader):
452+
"""Jinja2 renders memory_mb variable."""
453+
spec = TemplateSpec(from_image="python:3.11", cpu_count=2, memory_mb=4096)
454+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
455+
456+
container = manifest["spec"]["template"]["spec"]["containers"][0]
457+
assert container["resources"]["limits"]["memory"] == "4096Mi"
458+
459+
def test_build_pool_manifest_capacity_defaults(self, pool_loader):
460+
"""Capacity uses defaults and is converted to integers."""
461+
spec = TemplateSpec(from_image="python:3.11", cpu_count=2, memory_mb=2048)
462+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
463+
464+
cap = manifest["spec"]["capacitySpec"]
465+
assert cap["bufferMin"] == 1
466+
assert cap["bufferMax"] == 3
467+
assert cap["poolMin"] == 1
468+
assert cap["poolMax"] == 10
469+
470+
def test_build_pool_manifest_custom_capacity(self, pool_loader):
471+
"""Custom capacity values are rendered and converted to integers."""
472+
spec = TemplateSpec(
473+
from_image="python:3.11",
474+
cpu_count=2,
475+
memory_mb=2048,
476+
buffer_min=2,
477+
buffer_max=5,
478+
pool_min=3,
479+
pool_max=20,
480+
)
481+
manifest = pool_loader.build_pool_manifest("tpl-abc123", spec)
482+
483+
cap = manifest["spec"]["capacitySpec"]
484+
assert cap["bufferMin"] == 2
485+
assert cap["bufferMax"] == 5
486+
assert cap["poolMin"] == 3
487+
assert cap["poolMax"] == 20
488+
489+
def test_build_pool_manifest_without_pool_template(self):
490+
"""Calling build_pool_manifest without a pool template raises ValueError."""
491+
loader = K8sTemplateLoader(
492+
templates={"default": {"ports": {"proxy": 8000}, "template": {"spec": {}}}},
493+
default_namespace="rock-test",
494+
)
495+
spec = TemplateSpec(from_image="python:3.11", cpu_count=2, memory_mb=2048)
496+
497+
with pytest.raises(ValueError, match="No pool template configured"):
498+
loader.build_pool_manifest("tpl-abc123", spec)

0 commit comments

Comments
 (0)