From 6c20cc25c1cba67db2de85bfe82a5e6d6199da43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E7=84=B6?= Date: Sat, 7 Mar 2026 11:31:15 +0800 Subject: [PATCH] chore(server): pass app_config only to provider factory and providers --- .../services/k8s/agent_sandbox_provider.py | 48 +++-- .../src/services/k8s/batchsandbox_provider.py | 46 +++-- server/src/services/k8s/kubernetes_service.py | 3 - server/src/services/k8s/provider_factory.py | 61 ++----- server/tests/k8s/fixtures/k8s_fixtures.py | 78 ++++++++- .../tests/k8s/test_agent_sandbox_provider.py | 43 ++++- .../tests/k8s/test_batchsandbox_provider.py | 50 ++++-- server/tests/k8s/test_provider_factory.py | 165 +++++++++--------- server/tests/test_agent_sandbox_service.py | 7 +- 9 files changed, 313 insertions(+), 188 deletions(-) diff --git a/server/src/services/k8s/agent_sandbox_provider.py b/server/src/services/k8s/agent_sandbox_provider.py index bebc5b2d..f7081336 100644 --- a/server/src/services/k8s/agent_sandbox_provider.py +++ b/server/src/services/k8s/agent_sandbox_provider.py @@ -31,7 +31,7 @@ ApiException, ) -from src.config import AppConfig, IngressConfig, ExecdInitResources +from src.config import AppConfig from src.services.helpers import format_ingress_endpoint from src.api.schema import Endpoint, ImageSpec, NetworkPolicy from src.services.k8s.agent_sandbox_template import AgentSandboxTemplateManager @@ -82,17 +82,16 @@ class AgentSandboxProvider(WorkloadProvider): def __init__( self, k8s_client: K8sClient, - template_file_path: Optional[str] = None, - shutdown_policy: str = "Delete", - service_account: Optional[str] = None, - ingress_config: Optional[IngressConfig] = None, - enable_informer: bool = True, - informer_factory: Optional[Callable[[str], WorkloadInformer]] = None, - informer_resync_seconds: int = 300, - informer_watch_timeout_seconds: int = 60, app_config: Optional[AppConfig] = None, - execd_init_resources: Optional[ExecdInitResources] = None, + *, + informer_factory: Optional[Callable[[str], WorkloadInformer]] = None, ): + """ + Initialize AgentSandbox provider. + + Configuration is read from app_config (kubernetes.*, agent_sandbox, ingress). + No separate config objects are passed. + """ self.k8s_client = k8s_client self.custom_api = k8s_client.get_custom_objects_api() self.core_api = k8s_client.get_core_v1_api() @@ -101,12 +100,31 @@ def __init__( self.version = "v1alpha1" self.plural = "sandboxes" - self.shutdown_policy = shutdown_policy - self.service_account = service_account + k8s_config = app_config.kubernetes if app_config else None + agent_config = app_config.agent_sandbox if app_config else None + self.shutdown_policy = ( + agent_config.shutdown_policy if agent_config else "Delete" + ) + self.service_account = ( + k8s_config.service_account if k8s_config else None + ) + template_file_path = ( + agent_config.template_file if agent_config else None + ) self.template_manager = AgentSandboxTemplateManager(template_file_path) - self.ingress_config = ingress_config - self.execd_init_resources = execd_init_resources - self._enable_informer = enable_informer + self.ingress_config = app_config.ingress if app_config else None + self.execd_init_resources = ( + k8s_config.execd_init_resources if k8s_config else None + ) + self._enable_informer = ( + k8s_config.informer_enabled if k8s_config else True + ) + informer_resync_seconds = ( + k8s_config.informer_resync_seconds if k8s_config else 300 + ) + informer_watch_timeout_seconds = ( + k8s_config.informer_watch_timeout_seconds if k8s_config else 60 + ) self._informer_factory = informer_factory or ( lambda ns: WorkloadInformer( custom_api=self.custom_api, diff --git a/server/src/services/k8s/batchsandbox_provider.py b/server/src/services/k8s/batchsandbox_provider.py index 0c877477..91a5be76 100644 --- a/server/src/services/k8s/batchsandbox_provider.py +++ b/server/src/services/k8s/batchsandbox_provider.py @@ -31,7 +31,7 @@ ApiException, ) -from src.config import AppConfig, IngressConfig, INGRESS_MODE_GATEWAY, ExecdInitResources +from src.config import AppConfig, INGRESS_MODE_GATEWAY from src.services.helpers import format_ingress_endpoint from src.api.schema import Endpoint, ImageSpec, NetworkPolicy from src.services.k8s.batchsandbox_template import BatchSandboxTemplateManager @@ -60,26 +60,28 @@ class BatchSandboxProvider(WorkloadProvider): def __init__( self, k8s_client: K8sClient, - template_file_path: Optional[str] = None, - ingress_config: Optional[IngressConfig] = None, - enable_informer: bool = True, - informer_factory: Optional[Callable[[str], WorkloadInformer]] = None, - informer_resync_seconds: int = 300, - informer_watch_timeout_seconds: int = 60, app_config: Optional[AppConfig] = None, - execd_init_resources: Optional[ExecdInitResources] = None, + *, + informer_factory: Optional[Callable[[str], WorkloadInformer]] = None, ): """ Initialize BatchSandbox provider. + Configuration is read from app_config (kubernetes.*, ingress). No separate + config objects are passed. + Args: - k8s_client: Kubernetes client wrapper - template_file_path: Optional path to BatchSandbox CR YAML template file - app_config: Optional application config for secure runtime + k8s_client: Kubernetes client wrapper. + app_config: Application config; kubernetes and ingress settings are read from it. + informer_factory: Optional custom informer factory (for tests). """ self.k8s_client = k8s_client self.custom_api = k8s_client.get_custom_objects_api() - self.ingress_config = ingress_config + k8s_config = app_config.kubernetes if app_config else None + self.ingress_config = app_config.ingress if app_config else None + self.execd_init_resources = ( + k8s_config.execd_init_resources if k8s_config else None + ) # Initialize secure runtime resolver self.resolver = SecureRuntimeResolver(app_config) if app_config else None @@ -91,11 +93,23 @@ def __init__( self.group = "sandbox.opensandbox.io" self.version = "v1alpha1" self.plural = "batchsandboxes" - - # Template manager + + template_file_path = ( + k8s_config.batchsandbox_template_file if k8s_config else None + ) + if template_file_path: + logger.info("Using BatchSandbox template file: %s", template_file_path) self.template_manager = BatchSandboxTemplateManager(template_file_path) - self.execd_init_resources = execd_init_resources - self._enable_informer = enable_informer + + self._enable_informer = ( + k8s_config.informer_enabled if k8s_config else True + ) + informer_resync_seconds = ( + k8s_config.informer_resync_seconds if k8s_config else 300 + ) + informer_watch_timeout_seconds = ( + k8s_config.informer_watch_timeout_seconds if k8s_config else 60 + ) self._informer_factory = informer_factory or ( lambda ns: WorkloadInformer( custom_api=self.custom_api, diff --git a/server/src/services/k8s/kubernetes_service.py b/server/src/services/k8s/kubernetes_service.py index 1d79eacc..e92ffd59 100644 --- a/server/src/services/k8s/kubernetes_service.py +++ b/server/src/services/k8s/kubernetes_service.py @@ -110,9 +110,6 @@ def __init__(self, config: Optional[AppConfig] = None): self.workload_provider = create_workload_provider( provider_type=provider_type, k8s_client=self.k8s_client, - k8s_config=self.app_config.kubernetes, - agent_sandbox_config=self.app_config.agent_sandbox, - ingress_config=self.ingress_config, app_config=self.app_config, ) logger.info( diff --git a/server/src/services/k8s/provider_factory.py b/server/src/services/k8s/provider_factory.py index b8791d91..b3958ec9 100644 --- a/server/src/services/k8s/provider_factory.py +++ b/server/src/services/k8s/provider_factory.py @@ -19,7 +19,7 @@ import logging from typing import Dict, Type, Optional -from src.config import AppConfig, KubernetesRuntimeConfig, AgentSandboxRuntimeConfig, IngressConfig +from src.config import AppConfig from src.services.k8s.workload_provider import WorkloadProvider from src.services.k8s.batchsandbox_provider import BatchSandboxProvider from src.services.k8s.agent_sandbox_provider import AgentSandboxProvider @@ -43,27 +43,26 @@ def create_workload_provider( provider_type: str | None, k8s_client: K8sClient, - k8s_config: Optional[KubernetesRuntimeConfig] = None, - agent_sandbox_config: Optional[AgentSandboxRuntimeConfig] = None, - ingress_config: Optional[IngressConfig] = None, app_config: Optional[AppConfig] = None, ) -> WorkloadProvider: """ Create a WorkloadProvider instance based on the provider type. + All provider-specific configuration is read from app_config; no separate + config objects are passed. + Args: - provider_type: Type of provider (e.g., 'batchsandbox', 'pod', 'job'). + provider_type: Type of provider (e.g., 'batchsandbox', 'agent-sandbox'). If None, uses the first registered provider. - k8s_client: Kubernetes client instance - k8s_config: Optional Kubernetes runtime configuration (for template file paths, etc.) - agent_sandbox_config: Optional agent-sandbox configuration (for template/shutdown policy) - app_config: Optional application config for secure runtime + k8s_client: Kubernetes client instance. + app_config: Application config; used by built-in providers for + kubernetes, agent_sandbox, and ingress settings. Returns: - WorkloadProvider instance + WorkloadProvider instance. Raises: - ValueError: If provider_type is not supported or no providers are registered + ValueError: If provider_type is not supported or no providers are registered. """ # Use first registered provider if not specified if provider_type is None: @@ -87,43 +86,11 @@ def create_workload_provider( provider_class = _PROVIDER_REGISTRY[provider_type_lower] logger.info(f"Creating workload provider: {provider_class.__name__}") - # Special handling for BatchSandboxProvider - pass template file path - if provider_type_lower == PROVIDER_TYPE_BATCHSANDBOX: - template_file = k8s_config.batchsandbox_template_file if k8s_config else None - if template_file: - logger.info(f"Using BatchSandbox template file: {template_file}") - return provider_class( - k8s_client, - template_file_path=template_file, - ingress_config=ingress_config, - enable_informer=k8s_config.informer_enabled, - informer_resync_seconds=k8s_config.informer_resync_seconds, - informer_watch_timeout_seconds=k8s_config.informer_watch_timeout_seconds, - app_config=app_config, - execd_init_resources=k8s_config.execd_init_resources if k8s_config else None, - ) - - # Special handling for AgentSandboxProvider - pass agent-specific settings - if provider_type_lower == PROVIDER_TYPE_AGENT_SANDBOX: - agent_config = agent_sandbox_config or AgentSandboxRuntimeConfig() - return provider_class( - k8s_client, - template_file_path=agent_config.template_file, - shutdown_policy=agent_config.shutdown_policy, - service_account=k8s_config.service_account if k8s_config else None, - ingress_config=ingress_config, - enable_informer=k8s_config.informer_enabled if k8s_config else True, - informer_resync_seconds=( - k8s_config.informer_resync_seconds if k8s_config else 300 - ), - informer_watch_timeout_seconds=( - k8s_config.informer_watch_timeout_seconds if k8s_config else 60 - ), - app_config=app_config, - execd_init_resources=k8s_config.execd_init_resources if k8s_config else None, - ) + # Built-in providers accept (k8s_client, app_config) and read config internally + if provider_type_lower in (PROVIDER_TYPE_BATCHSANDBOX, PROVIDER_TYPE_AGENT_SANDBOX): + return provider_class(k8s_client, app_config=app_config) - # Providers without ingress-specific needs + # Custom/other providers may only accept k8s_client return provider_class(k8s_client) diff --git a/server/tests/k8s/fixtures/k8s_fixtures.py b/server/tests/k8s/fixtures/k8s_fixtures.py index bd882c03..42ec7a6c 100644 --- a/server/tests/k8s/fixtures/k8s_fixtures.py +++ b/server/tests/k8s/fixtures/k8s_fixtures.py @@ -22,7 +22,12 @@ import pytest from src.api.schema import CreateSandboxRequest, ImageSpec, ResourceLimits -from src.config import KubernetesRuntimeConfig +from src.config import ( + AppConfig, + AgentSandboxRuntimeConfig, + KubernetesRuntimeConfig, + RuntimeConfig, +) from src.services.k8s.client import K8sClient from src.services.k8s.provider_factory import PROVIDER_TYPE_BATCHSANDBOX @@ -62,6 +67,77 @@ def agent_sandbox_runtime_config(): ) +def _make_app_config( + *, + kubernetes: KubernetesRuntimeConfig, + agent_sandbox: AgentSandboxRuntimeConfig | None = None, +) -> AppConfig: + """Build AppConfig for K8s tests.""" + return AppConfig( + runtime=RuntimeConfig(type="kubernetes", execd_image="test-execd:latest"), + kubernetes=kubernetes, + agent_sandbox=agent_sandbox, + ) + + +@pytest.fixture +def app_config(k8s_runtime_config): + """Application config for batchsandbox provider tests.""" + return _make_app_config(kubernetes=k8s_runtime_config) + + +@pytest.fixture +def app_config_agent_sandbox(agent_sandbox_runtime_config, tmp_path): + """Application config for agent-sandbox provider tests.""" + template_file = tmp_path / "agent_sandbox_template.yaml" + template_file.write_text( + """ +metadata: + annotations: + managed-by: opensandbox +spec: + podTemplate: + spec: + nodeSelector: + workload: sandbox +""" + ) + agent_config = AgentSandboxRuntimeConfig( + template_file=str(template_file), + shutdown_policy="Retain", + ingress_enabled=True, + ) + return _make_app_config( + kubernetes=agent_sandbox_runtime_config, + agent_sandbox=agent_config, + ) + + +@pytest.fixture +def app_config_with_batch_template(tmp_path): + """Application config with BatchSandbox template file path set.""" + template_file = tmp_path / "test_template.yaml" + template_file.write_text(""" +apiVersion: execution.alibaba-inc.com/v1alpha1 +kind: BatchSandbox +metadata: + name: test-template +spec: + template: + spec: + nodeSelector: + gpu: "true" +""") + k8s_config = KubernetesRuntimeConfig( + kubeconfig_path="/tmp/test-kubeconfig", + namespace="test-namespace", + service_account="test-sa", + workload_provider=PROVIDER_TYPE_BATCHSANDBOX, + batchsandbox_template_file=str(template_file), + ) + return _make_app_config(kubernetes=k8s_config) + + @pytest.fixture def k8s_runtime_config_with_template(tmp_path): """Provide Kubernetes configuration with template file""" diff --git a/server/tests/k8s/test_agent_sandbox_provider.py b/server/tests/k8s/test_agent_sandbox_provider.py index 8fe31631..0d3acb65 100644 --- a/server/tests/k8s/test_agent_sandbox_provider.py +++ b/server/tests/k8s/test_agent_sandbox_provider.py @@ -24,10 +24,43 @@ from kubernetes.client import ApiException from src.api.schema import ImageSpec, NetworkPolicy, NetworkRule -from src.config import ExecdInitResources +from src.config import ( + AgentSandboxRuntimeConfig, + AppConfig, + ExecdInitResources, + KubernetesRuntimeConfig, + RuntimeConfig, +) from src.services.k8s.agent_sandbox_provider import AgentSandboxProvider +def _app_config_agent( + *, + shutdown_policy: str = "Delete", + service_account: str | None = "test-sa", + template_file: str | None = None, + execd_init_resources: ExecdInitResources | None = None, +) -> AppConfig: + """Build AppConfig for AgentSandboxProvider tests.""" + k8s = KubernetesRuntimeConfig( + kubeconfig_path="/tmp/test-kubeconfig", + namespace="test-namespace", + service_account=service_account, + workload_provider="agent-sandbox", + execd_init_resources=execd_init_resources, + ) + agent = AgentSandboxRuntimeConfig( + template_file=template_file, + shutdown_policy=shutdown_policy, + ingress_enabled=True, + ) + return AppConfig( + runtime=RuntimeConfig(type="kubernetes", execd_image="test-execd:latest"), + kubernetes=k8s, + agent_sandbox=agent, + ) + + class TestAgentSandboxProvider: """AgentSandboxProvider unit tests""" @@ -45,11 +78,11 @@ def test_create_workload_builds_correct_manifest_init_mode(self, mock_k8s_client """ Test case: Verify created manifest structure with init mode """ - provider = AgentSandboxProvider( - mock_k8s_client, + app_config = _app_config_agent( shutdown_policy="Delete", service_account="agent-sa", ) + provider = AgentSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "test-id", "uid": "test-uid"} @@ -494,13 +527,13 @@ def test_init_container_has_resources_when_configured(self, mock_k8s_client): """ Test case: Verify init container applies resources when execd_init_resources is set """ - provider = AgentSandboxProvider( - mock_k8s_client, + app_config = _app_config_agent( execd_init_resources=ExecdInitResources( limits={"cpu": "100m", "memory": "128Mi"}, requests={"cpu": "50m", "memory": "64Mi"}, ), ) + provider = AgentSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "test-id", "uid": "test-uid"} diff --git a/server/tests/k8s/test_batchsandbox_provider.py b/server/tests/k8s/test_batchsandbox_provider.py index 628642b4..08e79bbe 100644 --- a/server/tests/k8s/test_batchsandbox_provider.py +++ b/server/tests/k8s/test_batchsandbox_provider.py @@ -22,8 +22,34 @@ from kubernetes.client import ApiException from src.api.schema import ImageSpec, NetworkPolicy, NetworkRule -from src.config import ExecdInitResources +from src.config import ( + AppConfig, + ExecdInitResources, + KubernetesRuntimeConfig, + RuntimeConfig, +) from src.services.k8s.batchsandbox_provider import BatchSandboxProvider +from tests.k8s.fixtures.k8s_fixtures import PROVIDER_TYPE_BATCHSANDBOX + + +def _app_config_batch( + *, + template_file_path: str | None = None, + execd_init_resources: ExecdInitResources | None = None, +) -> AppConfig: + """Build AppConfig for BatchSandboxProvider tests.""" + k8s = KubernetesRuntimeConfig( + kubeconfig_path="/tmp/test-kubeconfig", + namespace="test-namespace", + service_account="test-sa", + workload_provider=PROVIDER_TYPE_BATCHSANDBOX, + batchsandbox_template_file=template_file_path, + execd_init_resources=execd_init_resources, + ) + return AppConfig( + runtime=RuntimeConfig(type="kubernetes", execd_image="test-execd:latest"), + kubernetes=k8s, + ) class TestBatchSandboxProvider: @@ -35,7 +61,7 @@ def test_init_without_template_creates_provider(self, mock_k8s_client): """ Test case: Verify normal initialization without template """ - provider = BatchSandboxProvider(mock_k8s_client, template_file_path=None) + provider = BatchSandboxProvider(mock_k8s_client) assert provider.k8s_client == mock_k8s_client assert provider.template_manager._template is None @@ -49,8 +75,8 @@ def test_init_with_template_loads_template(self, mock_k8s_client, tmp_path): """ template_file = tmp_path / "template.yaml" template_file.write_text("spec:\n replicas: 1") - - provider = BatchSandboxProvider(mock_k8s_client, str(template_file)) + app_config = _app_config_batch(template_file_path=str(template_file)) + provider = BatchSandboxProvider(mock_k8s_client, app_config=app_config) assert provider.template_manager._template is not None @@ -144,13 +170,13 @@ def test_create_workload_init_container_with_configured_resources(self, mock_k8s """ Test case: Verify init container applies resources when execd_init_resources is configured """ - provider = BatchSandboxProvider( - mock_k8s_client, + app_config = _app_config_batch( execd_init_resources=ExecdInitResources( limits={"cpu": "100m", "memory": "128Mi"}, requests={"cpu": "50m", "memory": "64Mi"}, ), ) + provider = BatchSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "test", "uid": "uid"} @@ -259,7 +285,8 @@ def test_create_workload_merges_template_volumes_and_mounts(self, mock_k8s_clien mountPath: /data """ ) - provider = BatchSandboxProvider(mock_k8s_client, str(template_file)) + app_config = _app_config_batch(template_file_path=str(template_file)) + provider = BatchSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "sandbox-test", "uid": "uid"} @@ -317,7 +344,8 @@ def test_create_workload_dedupes_template_volume_and_mount_names(self, mock_k8s_ mountPath: /data """ ) - provider = BatchSandboxProvider(mock_k8s_client, str(template_file)) + app_config = _app_config_batch(template_file_path=str(template_file)) + provider = BatchSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "sandbox-test", "uid": "uid"} @@ -504,7 +532,6 @@ def update_cache(self, obj): fake_informer = FakeInformer() provider = BatchSandboxProvider( mock_k8s_client, - enable_informer=True, informer_factory=lambda ns: fake_informer, ) @@ -545,7 +572,6 @@ def update_cache(self, obj): fake_informer = FakeInformer() provider = BatchSandboxProvider( mock_k8s_client, - enable_informer=True, informer_factory=lambda ns: fake_informer, ) mock_api = mock_k8s_client.get_custom_objects_api() @@ -592,7 +618,6 @@ def factory(ns): provider = BatchSandboxProvider( mock_k8s_client, - enable_informer=True, informer_factory=factory, ) @@ -1555,7 +1580,8 @@ def test_create_workload_with_network_policy_works_with_template(self, mock_k8s_ emptyDir: {} """ ) - provider = BatchSandboxProvider(mock_k8s_client, str(template_file)) + app_config = _app_config_batch(template_file_path=str(template_file)) + provider = BatchSandboxProvider(mock_k8s_client, app_config=app_config) mock_api = mock_k8s_client.get_custom_objects_api() mock_api.create_namespaced_custom_object.return_value = { "metadata": {"name": "test-id", "uid": "test-uid"} diff --git a/server/tests/k8s/test_provider_factory.py b/server/tests/k8s/test_provider_factory.py index bf0166ee..a97ea8a7 100644 --- a/server/tests/k8s/test_provider_factory.py +++ b/server/tests/k8s/test_provider_factory.py @@ -17,9 +17,7 @@ """ import pytest -from unittest.mock import patch -from src.config import AgentSandboxRuntimeConfig from src.services.k8s.provider_factory import ( register_provider, create_workload_provider, @@ -38,56 +36,38 @@ class TestProviderFactory: """provider_factory unit tests""" - def test_register_and_create_batchsandbox_provider(self, mock_k8s_client, k8s_runtime_config): + def test_register_and_create_batchsandbox_provider( + self, mock_k8s_client, app_config + ): """ Test case: Register and create BatchSandbox provider - + Purpose: Verify that BatchSandbox provider can be created through factory method """ provider = create_workload_provider( PROVIDER_TYPE_BATCHSANDBOX, mock_k8s_client, - k8s_runtime_config + app_config, ) - + assert isinstance(provider, BatchSandboxProvider) assert provider.k8s_client == mock_k8s_client def test_register_and_create_agent_sandbox_provider( self, mock_k8s_client, + app_config_agent_sandbox, agent_sandbox_runtime_config, - tmp_path, ): """ Test case: Register and create agent-sandbox provider Purpose: Verify that AgentSandbox provider can be created through factory method """ - template_file = tmp_path / "agent_sandbox_template.yaml" - template_file.write_text( - """ -metadata: - annotations: - managed-by: opensandbox -spec: - podTemplate: - spec: - nodeSelector: - workload: sandbox -""" - ) - - agent_config = AgentSandboxRuntimeConfig( - template_file=str(template_file), - shutdown_policy="Retain", - ingress_enabled=True, - ) provider = create_workload_provider( PROVIDER_TYPE_AGENT_SANDBOX, mock_k8s_client, - agent_sandbox_runtime_config, - agent_config, + app_config_agent_sandbox, ) assert isinstance(provider, AgentSandboxProvider) @@ -95,74 +75,77 @@ def test_register_and_create_agent_sandbox_provider( assert provider.shutdown_policy == "Retain" assert provider.service_account == agent_sandbox_runtime_config.service_account assert provider._enable_informer is True - - def test_create_provider_case_insensitive(self, mock_k8s_client, k8s_runtime_config): + + def test_create_provider_case_insensitive( + self, mock_k8s_client, app_config + ): """ Test case: Case-insensitive provider creation - + Purpose: Verify that provider type name is case-insensitive """ - provider1 = create_workload_provider("BatchSandbox", mock_k8s_client, k8s_runtime_config) - provider2 = create_workload_provider(PROVIDER_TYPE_BATCHSANDBOX, mock_k8s_client, k8s_runtime_config) - provider3 = create_workload_provider("BATCHSANDBOX", mock_k8s_client, k8s_runtime_config) - + provider1 = create_workload_provider( + "BatchSandbox", mock_k8s_client, app_config + ) + provider2 = create_workload_provider( + PROVIDER_TYPE_BATCHSANDBOX, mock_k8s_client, app_config + ) + provider3 = create_workload_provider( + "BATCHSANDBOX", mock_k8s_client, app_config + ) + assert isinstance(provider1, BatchSandboxProvider) assert isinstance(provider2, BatchSandboxProvider) assert isinstance(provider3, BatchSandboxProvider) - - def test_create_provider_with_none_type_uses_default(self, mock_k8s_client, k8s_runtime_config): + + def test_create_provider_with_none_type_uses_default( + self, mock_k8s_client, app_config + ): """ Test case: None type uses default provider - + Purpose: Verify that the first registered provider is used when provider_type is None """ - provider = create_workload_provider(None, mock_k8s_client, k8s_runtime_config) - + provider = create_workload_provider( + None, mock_k8s_client, app_config + ) + # Should use the first registered provider (batchsandbox) assert isinstance(provider, BatchSandboxProvider) - + def test_create_provider_with_invalid_type_raises_error(self, mock_k8s_client): """ Test case: Invalid provider type raises exception - + Purpose: Verify that ValueError is raised when passing unregistered provider type """ with pytest.raises(ValueError, match="Unsupported workload provider type"): create_workload_provider("invalid", mock_k8s_client) - - def test_create_batchsandbox_with_template_file(self, mock_k8s_client, k8s_runtime_config, tmp_path): + + def test_create_batchsandbox_with_template_file( + self, + mock_k8s_client, + app_config_with_batch_template, + ): """ Test case: Create BatchSandbox provider with template file - - Purpose: Verify that factory method correctly passes template file path to BatchSandboxProvider - """ - # Create temporary template file - template_file = tmp_path / "test_template.yaml" - template_file.write_text(""" -apiVersion: execution.alibaba-inc.com/v1alpha1 -kind: BatchSandbox -metadata: - name: test-template -spec: - template: - spec: - nodeSelector: - gpu: "true" -""") - - k8s_runtime_config.batchsandbox_template_file = str(template_file) - - with patch.object(BatchSandboxProvider, '__init__', return_value=None) as mock_init: - create_workload_provider(PROVIDER_TYPE_BATCHSANDBOX, mock_k8s_client, k8s_runtime_config) - - # Verify that template_file_path parameter was passed - mock_init.assert_called_once() - call_kwargs = mock_init.call_args.kwargs - assert 'template_file_path' in call_kwargs - assert call_kwargs['template_file_path'] == str(template_file) - assert call_kwargs['enable_informer'] is True - assert call_kwargs['informer_resync_seconds'] == k8s_runtime_config.informer_resync_seconds - assert call_kwargs['informer_watch_timeout_seconds'] == k8s_runtime_config.informer_watch_timeout_seconds + + Purpose: Verify that provider reads template file path from app_config + """ + app_config = app_config_with_batch_template + k8s_config = app_config.kubernetes + template_file = k8s_config.batchsandbox_template_file + + provider = create_workload_provider( + PROVIDER_TYPE_BATCHSANDBOX, + mock_k8s_client, + app_config, + ) + + assert isinstance(provider, BatchSandboxProvider) + assert provider.template_manager.template_file_path == template_file + assert provider._enable_informer is True + assert provider._informer_factory is not None def test_list_available_providers(self): """ @@ -215,34 +198,44 @@ def get_endpoint_info(self, *args, **kwargs): register_provider("custom", CustomProvider) # Verify that custom provider can be created - provider = create_workload_provider("custom", mock_k8s_client) + provider = create_workload_provider( + "custom", mock_k8s_client, app_config=None + ) assert isinstance(provider, CustomProvider) # Verify it's registered assert "custom" in list_available_providers() - def test_create_batchsandbox_with_config(self, mock_k8s_client, k8s_runtime_config): + def test_create_batchsandbox_with_config( + self, mock_k8s_client, app_config + ): """ - Test case: Create BatchSandbox provider with explicit config - - Purpose: Verify that provider creation works when k8s_config is provided + Test case: Create BatchSandbox provider with app_config + + Purpose: Verify that provider creation works when app_config is provided """ - provider = create_workload_provider(PROVIDER_TYPE_BATCHSANDBOX, mock_k8s_client, k8s_runtime_config) - + provider = create_workload_provider( + PROVIDER_TYPE_BATCHSANDBOX, + mock_k8s_client, + app_config, + ) + assert isinstance(provider, BatchSandboxProvider) assert provider.k8s_client == mock_k8s_client - - def test_create_provider_with_empty_registry_raises_error(self, mock_k8s_client, isolated_registry): + + def test_create_provider_with_empty_registry_raises_error( + self, mock_k8s_client, isolated_registry + ): """ Test case: Creating provider with empty registry raises exception - + Purpose: Verify that ValueError is raised when no provider is registered and type is None """ from src.services.k8s import provider_factory - + # Clear the registry to test empty registry scenario provider_factory._PROVIDER_REGISTRY.clear() - + # Verify that ValueError is raised when registry is empty and type is None with pytest.raises(ValueError, match="No workload providers are registered"): create_workload_provider(None, mock_k8s_client) diff --git a/server/tests/test_agent_sandbox_service.py b/server/tests/test_agent_sandbox_service.py index 490e1808..b563574a 100644 --- a/server/tests/test_agent_sandbox_service.py +++ b/server/tests/test_agent_sandbox_service.py @@ -126,9 +126,10 @@ def test_init_with_valid_config_succeeds(self, agent_sandbox_runtime_config): mock_provider_factory.assert_called_once() call_kwargs = mock_provider_factory.call_args.kwargs assert call_kwargs["provider_type"] == "agent-sandbox" - assert call_kwargs["agent_sandbox_config"].template_file == "/tmp/template.yaml" - assert call_kwargs["agent_sandbox_config"].shutdown_policy == "Retain" - assert call_kwargs["k8s_config"] == agent_sandbox_runtime_config + assert call_kwargs["app_config"] is config + assert call_kwargs["app_config"].agent_sandbox.template_file == "/tmp/template.yaml" + assert call_kwargs["app_config"].agent_sandbox.shutdown_policy == "Retain" + assert call_kwargs["app_config"].kubernetes == agent_sandbox_runtime_config def test_init_without_kubernetes_config_raises_error(self): """