From 0aa797d3bfbaba95fa57e692007ee455bf60c7c1 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Mon, 13 May 2024 15:49:17 +0800 Subject: [PATCH 01/30] Add support for submitting a prebuilt image Rename submit_dockerfile to submit_container_image submit_container_image now accepts a docker_config of a more generic WorkerConfig type which could be DockerWorkerConfig or PrebuiltWorkerConfig Add unit tests --- .../admin/Custom API + Custom Worker.ipynb | 2 +- notebooks/api/0.8/10-container-images.ipynb | 4 +- .../api/0.8/11-container-images-k8s.ipynb | 4 +- packages/syft/src/syft/__init__.py | 1 + .../syft/src/syft/service/request/request.py | 36 +++++---- .../service/worker/worker_image_service.py | 9 ++- .../worker_pool/worker_pool_service_test.py | 77 ++++++++++++------- .../tests/syft/worker_pool/worker_test.py | 2 +- .../container_workload/pool_image_test.py | 4 +- 9 files changed, 87 insertions(+), 52 deletions(-) diff --git a/notebooks/admin/Custom API + Custom Worker.ipynb b/notebooks/admin/Custom API + Custom Worker.ipynb index bbf47476301..eea3152e71f 100644 --- a/notebooks/admin/Custom API + Custom Worker.ipynb +++ b/notebooks/admin/Custom API + Custom Worker.ipynb @@ -114,7 +114,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_dockerfile(\n", + "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", " docker_config=docker_config\n", ")\n", "submit_result" diff --git a/notebooks/api/0.8/10-container-images.ipynb b/notebooks/api/0.8/10-container-images.ipynb index 84fd1a340ab..92b4db793a3 100644 --- a/notebooks/api/0.8/10-container-images.ipynb +++ b/notebooks/api/0.8/10-container-images.ipynb @@ -229,7 +229,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_dockerfile(\n", + "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", " docker_config=docker_config\n", ")" ] @@ -1097,7 +1097,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_dockerfile(\n", + "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", " docker_config=docker_config_2\n", ")\n", "submit_result" diff --git a/notebooks/api/0.8/11-container-images-k8s.ipynb b/notebooks/api/0.8/11-container-images-k8s.ipynb index c9663acd3ad..4251a0ddb8e 100644 --- a/notebooks/api/0.8/11-container-images-k8s.ipynb +++ b/notebooks/api/0.8/11-container-images-k8s.ipynb @@ -265,7 +265,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_dockerfile(\n", + "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", " docker_config=docker_config\n", ")\n", "submit_result" @@ -935,7 +935,7 @@ "outputs": [], "source": [ "submit_result = None\n", - "submit_result = domain_client.api.services.worker_image.submit_dockerfile(\n", + "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", " docker_config=docker_config_opendp\n", ")\n", "submit_result" diff --git a/packages/syft/src/syft/__init__.py b/packages/syft/src/syft/__init__.py index 6ebbad50708..8ebca195ebc 100644 --- a/packages/syft/src/syft/__init__.py +++ b/packages/syft/src/syft/__init__.py @@ -26,6 +26,7 @@ from .client.user_settings import UserSettings # noqa: F401 from .client.user_settings import settings # noqa: F401 from .custom_worker.config import DockerWorkerConfig # noqa: F401 +from .custom_worker.config import PrebuiltWorkerConfig # noqa: F401 from .node.credentials import SyftSigningKey # noqa: F401 from .node.domain import Domain # noqa: F401 from .node.enclave import Enclave # noqa: F401 diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index 5d10f6cc75e..04845c8664d 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -205,7 +205,7 @@ def _run( worker_image_service = context.node.get_service("SyftWorkerImageService") service_context = context.to_service_ctx() - result = worker_image_service.submit_dockerfile( + result = worker_image_service.submit_container_image( service_context, docker_config=self.config ) @@ -219,18 +219,28 @@ def _run( if result.is_err(): return Err(SyftError(message=f"{result.err()}")) - worker_image = result.ok() + if (worker_image := result.ok()) is None: + return Err(SyftError(message="The worker image does not exist.")) - build_result = worker_image_service.build( - service_context, - image_uid=worker_image.id, - tag=self.tag, - registry_uid=self.registry_uid, - pull=self.pull_image, - ) + build_success_message = "Image was pre-built." + + if not worker_image.is_prebuilt: + build_result = worker_image_service.build( + service_context, + image_uid=worker_image.id, + tag=self.tag, + registry_uid=self.registry_uid, + pull=self.pull_image, + ) - if isinstance(build_result, SyftError): - return Err(build_result) + if isinstance(build_result, SyftError): + return Err(build_result) + + build_success_message = build_result.message + + build_success = SyftSuccess( + message=f"Build result: {build_success_message}" + ) if IN_KUBERNETES: push_result = worker_image_service.push( @@ -245,11 +255,11 @@ def _run( return Ok( SyftSuccess( - message=f"Build Result: {build_result.message} \n Push Result: {push_result.message}" + message=f"{build_success}\nPush result: {push_result.message}" ) ) - return Ok(build_result) + return Ok(build_success) except Exception as e: return Err(SyftError(message=f"Failed to create/build image: {e}")) diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index da7c39f4582..ede93af1a55 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -7,6 +7,7 @@ # relative from ...custom_worker.config import DockerWorkerConfig +from ...custom_worker.config import WorkerConfig from ...custom_worker.k8s import IN_KUBERNETES from ...serde.serializable import serializable from ...store.document_store import DocumentStore @@ -39,12 +40,12 @@ def __init__(self, store: DocumentStore) -> None: self.stash = SyftWorkerImageStash(store=store) @service_method( - path="worker_image.submit_dockerfile", - name="submit_dockerfile", + path="worker_image.submit_container_image", + name="submit_container_image", roles=DATA_OWNER_ROLE_LEVEL, ) - def submit_dockerfile( - self, context: AuthedServiceContext, docker_config: DockerWorkerConfig + def submit_container_image( + self, context: AuthedServiceContext, docker_config: WorkerConfig ) -> SyftSuccess | SyftError: worker_image = SyftWorkerImage( config=docker_config, diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index 8aad9f5a27e..b7d50d48f43 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -1,9 +1,12 @@ # third party from faker import Faker +import pytest # syft absolute import syft as sy from syft.custom_worker.config import DockerWorkerConfig +from syft.custom_worker.config import PrebuiltWorkerConfig +from syft.custom_worker.config import WorkerConfig from syft.node.worker import Worker from syft.service.request.request import CreateCustomWorkerPoolChange from syft.service.response import SyftSuccess @@ -13,8 +16,34 @@ # relative from ..request.request_code_accept_deny_test import get_ds_client +PREBUILT_IMAGE_TAG = f"openmined/grid-backend:{sy.__version__}" -def test_create_image_and_pool_request_accept(faker: Faker, worker: Worker): +CUSTOM_DOCKERFILE = f""" +FROM {PREBUILT_IMAGE_TAG} + +RUN pip install recordlinkage +""" + +CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage:latest" + +DOCKER_CONFIG_TEST_CASES = [ + ( + CUSTOM_IMAGE_TAG, + DockerWorkerConfig(dockerfile=CUSTOM_DOCKERFILE), + 2, # total number of images. + # 2 since we pull a pre-built image (1) as the base image to build a custom image (2) + ), + (PREBUILT_IMAGE_TAG, PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG), 1), +] + + +@pytest.mark.parametrize( + "docker_tag,docker_config", + [test_case[:2] for test_case in DOCKER_CONFIG_TEST_CASES], +) +def test_create_image_and_pool_request_accept( + faker: Faker, worker: Worker, docker_tag: str, docker_config: WorkerConfig +) -> None: """ Test the functionality of `SyftWorkerPoolService.create_image_and_pool_request` when the request is accepted @@ -25,13 +54,6 @@ def test_create_image_and_pool_request_accept(faker: Faker, worker: Worker): assert root_client.credentials != ds_client.credentials # the DS makes a request to create an image and a pool based on the image - custom_dockerfile = f""" - FROM openmined/grid-backend:{sy.__version__} - - RUN pip install recordlinkage - """ - docker_config = DockerWorkerConfig(dockerfile=custom_dockerfile) - docker_tag = "openmined/custom-worker-recordlinkage:latest" request = ds_client.api.services.worker_pool.create_image_and_pool_request( pool_name="recordlinkage-pool", num_workers=2, @@ -61,7 +83,14 @@ def test_create_image_and_pool_request_accept(faker: Faker, worker: Worker): assert len(launched_pool.worker_list) == 2 -def test_create_pool_request_accept(faker: Faker, worker: Worker): +@pytest.mark.parametrize("docker_tag,docker_config,n_images", DOCKER_CONFIG_TEST_CASES) +def test_create_pool_request_accept( + faker: Faker, + worker: Worker, + docker_tag: str, + docker_config: WorkerConfig, + n_images: int, +) -> None: """ Test the functionality of `SyftWorkerPoolService.create_pool_request` when the request is accepted @@ -72,29 +101,23 @@ def test_create_pool_request_accept(faker: Faker, worker: Worker): assert root_client.credentials != ds_client.credentials # the DO submits the docker config to build an image - custom_dockerfile_str = f""" - FROM openmined/grid-backend:{sy.__version__} - - RUN pip install opendp - """ - docker_config = DockerWorkerConfig(dockerfile=custom_dockerfile_str) - submit_result = root_client.api.services.worker_image.submit_dockerfile( + submit_result = root_client.api.services.worker_image.submit_container_image( docker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) - assert len(root_client.images.get_all()) == 2 + assert len(root_client.images.get_all()) == n_images # The root client builds the image - worker_image: SyftWorkerImage = root_client.images[1] - docker_tag = "openmined/custom-worker-opendp:latest" - docker_build_result = root_client.api.services.worker_image.build( - image_uid=worker_image.id, - tag=docker_tag, - ) - # update the worker image variable after the image was built - worker_image: SyftWorkerImage = root_client.images[1] - assert isinstance(docker_build_result, SyftSuccess) - assert worker_image.image_identifier.repo_with_tag == docker_tag + worker_image: SyftWorkerImage = root_client.images[-1] + if not worker_image.is_prebuilt: + docker_build_result = root_client.api.services.worker_image.build( + image_uid=worker_image.id, + tag=docker_tag, + ) + # update the worker image variable after the image was built + worker_image: SyftWorkerImage = root_client.images[-1] + assert isinstance(docker_build_result, SyftSuccess) + assert worker_image.image_identifier.repo_with_tag == docker_tag # The DS client submits a request to create a pool from an existing image request = ds_client.api.services.worker_pool.pool_creation_request( diff --git a/packages/syft/tests/syft/worker_pool/worker_test.py b/packages/syft/tests/syft/worker_pool/worker_test.py index 6503e51eb66..91399083a56 100644 --- a/packages/syft/tests/syft/worker_pool/worker_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_test.py @@ -23,7 +23,7 @@ def test_syft_worker(worker: Worker): """ root_client = worker.root_client docker_config = get_docker_config() - submit_result = root_client.api.services.worker_image.submit_dockerfile( + submit_result = root_client.api.services.worker_image.submit_container_image( docker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index d8cb59f8f1f..775c2964ccc 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -38,7 +38,7 @@ def test_image_build(domain_1_port) -> None: docker_config = DockerWorkerConfig(dockerfile=docker_config_rl) # Submit Worker Image - submit_result = domain_client.api.services.worker_image.submit_dockerfile( + submit_result = domain_client.api.services.worker_image.submit_container_image( docker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) @@ -92,7 +92,7 @@ def test_pool_launch(domain_1_port) -> None: docker_config = DockerWorkerConfig(dockerfile=docker_config_opendp) # Submit Worker Image - submit_result = domain_client.api.services.worker_image.submit_dockerfile( + submit_result = domain_client.api.services.worker_image.submit_container_image( docker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) From 870f591f9e21e615cf241b2dadcdd25aaac99d1b Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Mon, 13 May 2024 16:21:30 +0800 Subject: [PATCH 02/30] Remove the nbqa-black pre-commit hook ruff has already been configured to format notebook files as well. --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d243ca60d91..c99e2d96bba 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -82,7 +82,6 @@ repos: # files: "^notebooks/(api|tutorials|admin)" hooks: - id: nbqa-isort - - id: nbqa-black - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. From 6c482186bf6c72d32107ba2ff4fdc478f3bf22a8 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 14 May 2024 14:46:24 +0800 Subject: [PATCH 03/30] Rename get_by_docker_config to get_by_worker_config get_by_worker_config now accepts a config of a more generic WorkerConfig type which could be DockerWorkerConfig or PrebuiltWorkerConfig --- packages/syft/src/syft/service/request/request.py | 4 ++-- packages/syft/src/syft/service/worker/utils.py | 2 +- .../syft/src/syft/service/worker/worker_image_service.py | 5 ++--- packages/syft/src/syft/service/worker/worker_image_stash.py | 6 +++--- .../syft/src/syft/service/worker/worker_pool_service.py | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index 04845c8664d..cf02db70cd4 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -212,7 +212,7 @@ def _run( if isinstance(result, SyftError): return Err(result) - result = worker_image_service.stash.get_by_docker_config( + result = worker_image_service.stash.get_by_worker_config( service_context.credentials, config=self.config ) @@ -301,7 +301,7 @@ def _run( service_context: AuthedServiceContext = context.to_service_ctx() if self.config is not None: - result = worker_pool_service.image_stash.get_by_docker_config( + result = worker_pool_service.image_stash.get_by_worker_config( service_context.credentials, self.config ) if result.is_err(): diff --git a/packages/syft/src/syft/service/worker/utils.py b/packages/syft/src/syft/service/worker/utils.py index 75d1c36d459..2f647069ca6 100644 --- a/packages/syft/src/syft/service/worker/utils.py +++ b/packages/syft/src/syft/service/worker/utils.py @@ -564,7 +564,7 @@ def create_default_image( image_identifier=SyftWorkerImageIdentifier.from_str(tag), ) - result = image_stash.get_by_docker_config( + result = image_stash.get_by_worker_config( credentials=credentials, config=worker_config, ) diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index ede93af1a55..255702e5b44 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -6,7 +6,6 @@ import pydantic # relative -from ...custom_worker.config import DockerWorkerConfig from ...custom_worker.config import WorkerConfig from ...custom_worker.k8s import IN_KUBERNETES from ...serde.serializable import serializable @@ -279,9 +278,9 @@ def get_by_uid( roles=DATA_SCIENTIST_ROLE_LEVEL, ) def get_by_config( - self, context: AuthedServiceContext, docker_config: DockerWorkerConfig + self, context: AuthedServiceContext, docker_config: WorkerConfig ) -> SyftWorkerImage | SyftError: - res = self.stash.get_by_docker_config( + res = self.stash.get_by_worker_config( credentials=context.credentials, config=docker_config ) if res.is_err(): diff --git a/packages/syft/src/syft/service/worker/worker_image_stash.py b/packages/syft/src/syft/service/worker/worker_image_stash.py index 900bcdd7cd6..ab9dde6701f 100644 --- a/packages/syft/src/syft/service/worker/worker_image_stash.py +++ b/packages/syft/src/syft/service/worker/worker_image_stash.py @@ -48,7 +48,7 @@ def set( ) if isinstance(obj.config, DockerWorkerConfig): - result = self.get_by_docker_config( + result = self.get_by_worker_config( credentials=credentials, config=obj.config ) if result.is_ok() and result.ok() is not None: @@ -62,8 +62,8 @@ def set( ignore_duplicates=ignore_duplicates, ) - def get_by_docker_config( - self, credentials: SyftVerifyKey, config: DockerWorkerConfig + def get_by_worker_config( + self, credentials: SyftVerifyKey, config: WorkerConfig ) -> Result[SyftWorkerImage | None, str]: qks = QueryKeys(qks=[WorkerConfigPK.with_obj(config)]) return self.query_one(credentials=credentials, qks=qks) diff --git a/packages/syft/src/syft/service/worker/worker_pool_service.py b/packages/syft/src/syft/service/worker/worker_pool_service.py index a8a91965d3c..a9c65d5a5b2 100644 --- a/packages/syft/src/syft/service/worker/worker_pool_service.py +++ b/packages/syft/src/syft/service/worker/worker_pool_service.py @@ -257,7 +257,7 @@ def create_image_and_pool_request( return SyftError(message="Registry UID is required in Kubernetes mode.") # Check if an image already exists for given docker config - search_result = self.image_stash.get_by_docker_config( + search_result = self.image_stash.get_by_worker_config( credentials=context.credentials, config=config ) From 85935b36ba90ecf3d1b6fcadd8958c90020fff08 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 14 May 2024 15:18:17 +0800 Subject: [PATCH 04/30] Add unit test for services.worker_image.get_by_config --- .../worker_pool/worker_pool_service_test.py | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index b7d50d48f43..bdab8f37eb9 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -26,7 +26,7 @@ CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage:latest" -DOCKER_CONFIG_TEST_CASES = [ +DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES = [ ( CUSTOM_IMAGE_TAG, DockerWorkerConfig(dockerfile=CUSTOM_DOCKERFILE), @@ -36,11 +36,12 @@ (PREBUILT_IMAGE_TAG, PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG), 1), ] +DOCKER_CONFIG_TEST_CASES = [ + test_case[:2] for test_case in DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES +] -@pytest.mark.parametrize( - "docker_tag,docker_config", - [test_case[:2] for test_case in DOCKER_CONFIG_TEST_CASES], -) + +@pytest.mark.parametrize("docker_tag,docker_config", DOCKER_CONFIG_TEST_CASES) def test_create_image_and_pool_request_accept( faker: Faker, worker: Worker, docker_tag: str, docker_config: WorkerConfig ) -> None: @@ -83,7 +84,10 @@ def test_create_image_and_pool_request_accept( assert len(launched_pool.worker_list) == 2 -@pytest.mark.parametrize("docker_tag,docker_config,n_images", DOCKER_CONFIG_TEST_CASES) +@pytest.mark.parametrize( + "docker_tag,docker_config,n_images", + DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES, +) def test_create_pool_request_accept( faker: Faker, worker: Worker, @@ -136,3 +140,21 @@ def test_create_pool_request_accept( launched_pool = root_client.worker_pools["opendp-pool"] assert isinstance(launched_pool, WorkerPool) assert len(launched_pool.worker_list) == 3 + + +WORKER_CONFIGS = [test_case[1] for test_case in DOCKER_CONFIG_TEST_CASES] + + +@pytest.mark.parametrize("docker_config", WORKER_CONFIGS) +def test_get_by_worker_config( + worker: Worker, + docker_config: WorkerConfig, +) -> None: + root_client = worker.root_client + for config in WORKER_CONFIGS: + root_client.api.services.worker_image.submit_container_image( + docker_config=config + ) + + worker_image = root_client.api.services.worker_image.get_by_config(docker_config) + assert worker_image.config == docker_config From 81ae3ea0221a4dcd4475e829c0abe24f7806801d Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 14 May 2024 15:20:58 +0800 Subject: [PATCH 05/30] Rename --- .../worker_pool/worker_pool_service_test.py | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index bdab8f37eb9..381c60c107d 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -26,7 +26,7 @@ CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage:latest" -DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES = [ +WORKER_CONFIG_TEST_CASES_WITH_N_IMAGES = [ ( CUSTOM_IMAGE_TAG, DockerWorkerConfig(dockerfile=CUSTOM_DOCKERFILE), @@ -36,14 +36,14 @@ (PREBUILT_IMAGE_TAG, PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG), 1), ] -DOCKER_CONFIG_TEST_CASES = [ - test_case[:2] for test_case in DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES +WORKER_CONFIG_TEST_CASES = [ + test_case[:2] for test_case in WORKER_CONFIG_TEST_CASES_WITH_N_IMAGES ] -@pytest.mark.parametrize("docker_tag,docker_config", DOCKER_CONFIG_TEST_CASES) +@pytest.mark.parametrize("docker_tag,worker_config", WORKER_CONFIG_TEST_CASES) def test_create_image_and_pool_request_accept( - faker: Faker, worker: Worker, docker_tag: str, docker_config: WorkerConfig + faker: Faker, worker: Worker, docker_tag: str, worker_config: WorkerConfig ) -> None: """ Test the functionality of `SyftWorkerPoolService.create_image_and_pool_request` @@ -59,11 +59,11 @@ def test_create_image_and_pool_request_accept( pool_name="recordlinkage-pool", num_workers=2, tag=docker_tag, - config=docker_config, + config=worker_config, reason="I want to do some more cool data science with PySyft and Recordlinkage", ) assert len(request.changes) == 2 - assert request.changes[0].config == docker_config + assert request.changes[0].config == worker_config assert request.changes[1].num_workers == 2 assert request.changes[1].pool_name == "recordlinkage-pool" @@ -85,14 +85,14 @@ def test_create_image_and_pool_request_accept( @pytest.mark.parametrize( - "docker_tag,docker_config,n_images", - DOCKER_CONFIG_TEST_CASES_WITH_N_IMAGES, + "docker_tag,worker_config,n_images", + WORKER_CONFIG_TEST_CASES_WITH_N_IMAGES, ) def test_create_pool_request_accept( faker: Faker, worker: Worker, docker_tag: str, - docker_config: WorkerConfig, + worker_config: WorkerConfig, n_images: int, ) -> None: """ @@ -106,7 +106,7 @@ def test_create_pool_request_accept( # the DO submits the docker config to build an image submit_result = root_client.api.services.worker_image.submit_container_image( - docker_config=docker_config + docker_config=worker_config ) assert isinstance(submit_result, SyftSuccess) assert len(root_client.images.get_all()) == n_images @@ -142,13 +142,13 @@ def test_create_pool_request_accept( assert len(launched_pool.worker_list) == 3 -WORKER_CONFIGS = [test_case[1] for test_case in DOCKER_CONFIG_TEST_CASES] +WORKER_CONFIGS = [test_case[1] for test_case in WORKER_CONFIG_TEST_CASES] -@pytest.mark.parametrize("docker_config", WORKER_CONFIGS) +@pytest.mark.parametrize("worker_config", WORKER_CONFIGS) def test_get_by_worker_config( worker: Worker, - docker_config: WorkerConfig, + worker_config: WorkerConfig, ) -> None: root_client = worker.root_client for config in WORKER_CONFIGS: @@ -156,5 +156,5 @@ def test_get_by_worker_config( docker_config=config ) - worker_image = root_client.api.services.worker_image.get_by_config(docker_config) - assert worker_image.config == docker_config + worker_image = root_client.api.services.worker_image.get_by_config(worker_config) + assert worker_image.config == worker_config From 56a8ad646a118775321aac0b6ebacdcaa5f346c1 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 14 May 2024 15:22:31 +0800 Subject: [PATCH 06/30] Use get_by_image to retrieve the image in test --- .../tests/syft/worker_pool/worker_pool_service_test.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index 381c60c107d..e7a0b4e4042 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -112,14 +112,18 @@ def test_create_pool_request_accept( assert len(root_client.images.get_all()) == n_images # The root client builds the image - worker_image: SyftWorkerImage = root_client.images[-1] + worker_image: SyftWorkerImage = root_client.api.services.worker_image.get_by_config( + worker_config + ) if not worker_image.is_prebuilt: docker_build_result = root_client.api.services.worker_image.build( image_uid=worker_image.id, tag=docker_tag, ) # update the worker image variable after the image was built - worker_image: SyftWorkerImage = root_client.images[-1] + worker_image: SyftWorkerImage = ( + root_client.api.services.worker_image.get_by_config(worker_config) + ) assert isinstance(docker_build_result, SyftSuccess) assert worker_image.image_identifier.repo_with_tag == docker_tag From ce219876c279a8f2ffffce7e2b38117e1bf3dcf2 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 14 May 2024 15:26:36 +0800 Subject: [PATCH 07/30] Rename arg docker_config to worker_config in services.worker_image.submit_container_image and services.worker_image.get_by_config --- notebooks/admin/Custom API + Custom Worker.ipynb | 2 +- notebooks/api/0.8/10-container-images.ipynb | 4 ++-- notebooks/api/0.8/11-container-images-k8s.ipynb | 4 ++-- packages/syft/src/syft/service/request/request.py | 2 +- .../src/syft/service/worker/worker_image_service.py | 10 +++++----- .../tests/syft/worker_pool/worker_pool_service_test.py | 4 ++-- packages/syft/tests/syft/worker_pool/worker_test.py | 2 +- .../integration/container_workload/pool_image_test.py | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/notebooks/admin/Custom API + Custom Worker.ipynb b/notebooks/admin/Custom API + Custom Worker.ipynb index eea3152e71f..4dc9cdd124c 100644 --- a/notebooks/admin/Custom API + Custom Worker.ipynb +++ b/notebooks/admin/Custom API + Custom Worker.ipynb @@ -115,7 +115,7 @@ "outputs": [], "source": [ "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", - " docker_config=docker_config\n", + " worker_config=docker_config\n", ")\n", "submit_result" ] diff --git a/notebooks/api/0.8/10-container-images.ipynb b/notebooks/api/0.8/10-container-images.ipynb index 92b4db793a3..fc0f71f0871 100644 --- a/notebooks/api/0.8/10-container-images.ipynb +++ b/notebooks/api/0.8/10-container-images.ipynb @@ -230,7 +230,7 @@ "outputs": [], "source": [ "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", - " docker_config=docker_config\n", + " worker_config=docker_config\n", ")" ] }, @@ -1098,7 +1098,7 @@ "outputs": [], "source": [ "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", - " docker_config=docker_config_2\n", + " worker_config=docker_config_2\n", ")\n", "submit_result" ] diff --git a/notebooks/api/0.8/11-container-images-k8s.ipynb b/notebooks/api/0.8/11-container-images-k8s.ipynb index 4251a0ddb8e..794574f6758 100644 --- a/notebooks/api/0.8/11-container-images-k8s.ipynb +++ b/notebooks/api/0.8/11-container-images-k8s.ipynb @@ -266,7 +266,7 @@ "outputs": [], "source": [ "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", - " docker_config=docker_config\n", + " worker_config=docker_config\n", ")\n", "submit_result" ] @@ -936,7 +936,7 @@ "source": [ "submit_result = None\n", "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", - " docker_config=docker_config_opendp\n", + " worker_config=docker_config_opendp\n", ")\n", "submit_result" ] diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index cf02db70cd4..16c12a9cfdd 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -206,7 +206,7 @@ def _run( service_context = context.to_service_ctx() result = worker_image_service.submit_container_image( - service_context, docker_config=self.config + service_context, worker_config=self.config ) if isinstance(result, SyftError): diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index 255702e5b44..1e2391132ab 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -44,10 +44,10 @@ def __init__(self, store: DocumentStore) -> None: roles=DATA_OWNER_ROLE_LEVEL, ) def submit_container_image( - self, context: AuthedServiceContext, docker_config: WorkerConfig + self, context: AuthedServiceContext, worker_config: WorkerConfig ) -> SyftSuccess | SyftError: worker_image = SyftWorkerImage( - config=docker_config, + config=worker_config, created_by=context.credentials, ) res = self.stash.set(context.credentials, worker_image) @@ -278,14 +278,14 @@ def get_by_uid( roles=DATA_SCIENTIST_ROLE_LEVEL, ) def get_by_config( - self, context: AuthedServiceContext, docker_config: WorkerConfig + self, context: AuthedServiceContext, worker_config: WorkerConfig ) -> SyftWorkerImage | SyftError: res = self.stash.get_by_worker_config( - credentials=context.credentials, config=docker_config + credentials=context.credentials, config=worker_config ) if res.is_err(): return SyftError( - message=f"Failed to get image with docker config {docker_config}. Error: {res.err()}" + message=f"Failed to get image with docker config {worker_config}. Error: {res.err()}" ) image: SyftWorkerImage = res.ok() return image diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index e7a0b4e4042..dd6a5a9453d 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -106,7 +106,7 @@ def test_create_pool_request_accept( # the DO submits the docker config to build an image submit_result = root_client.api.services.worker_image.submit_container_image( - docker_config=worker_config + worker_config=worker_config ) assert isinstance(submit_result, SyftSuccess) assert len(root_client.images.get_all()) == n_images @@ -157,7 +157,7 @@ def test_get_by_worker_config( root_client = worker.root_client for config in WORKER_CONFIGS: root_client.api.services.worker_image.submit_container_image( - docker_config=config + worker_config=config ) worker_image = root_client.api.services.worker_image.get_by_config(worker_config) diff --git a/packages/syft/tests/syft/worker_pool/worker_test.py b/packages/syft/tests/syft/worker_pool/worker_test.py index 91399083a56..55909b5599d 100644 --- a/packages/syft/tests/syft/worker_pool/worker_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_test.py @@ -24,7 +24,7 @@ def test_syft_worker(worker: Worker): root_client = worker.root_client docker_config = get_docker_config() submit_result = root_client.api.services.worker_image.submit_container_image( - docker_config=docker_config + worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 775c2964ccc..4367f26be0e 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -39,7 +39,7 @@ def test_image_build(domain_1_port) -> None: # Submit Worker Image submit_result = domain_client.api.services.worker_image.submit_container_image( - docker_config=docker_config + worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) assert len(domain_client.images.get_all()) == 2 @@ -93,7 +93,7 @@ def test_pool_launch(domain_1_port) -> None: # Submit Worker Image submit_result = domain_client.api.services.worker_image.submit_container_image( - docker_config=docker_config + worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) From df646eaef2a0bd8820a9f98e0b35e81da3f1c8c7 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 18:00:32 +0800 Subject: [PATCH 08/30] Add integration tests for prebuilt images --- .../container_workload/pool_image_test.py | 96 ++++++++++--------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 4367f26be0e..a155b529d17 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -11,6 +11,8 @@ import syft as sy from syft.client.domain_client import DomainClient from syft.custom_worker.config import DockerWorkerConfig +from syft.custom_worker.config import PrebuiltWorkerConfig +from syft.custom_worker.config import WorkerConfig from syft.node.node import get_default_worker_tag_by_env from syft.service.request.request import Request from syft.service.response import SyftSuccess @@ -24,33 +26,42 @@ SYFT_BASE_TAG = get_default_worker_tag_by_env(dev_mode=True) +PREBUILT_IMAGE_TAG = f"docker.io/openmined/grid-backend:{sy.__version__}" + +PREBUILT_WORKER_CONFIG = PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG) + +CUSTOM_DOCKERFILE = f""" +FROM openmined/grid-backend:{SYFT_BASE_TAG} + +RUN pip install recordlinkage +""" + +CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage" + +CUSTOM_DOCKER_WORKER_CONFIG = DockerWorkerConfig(dockerfile=CUSTOM_DOCKERFILE) + + @pytest.mark.container_workload def test_image_build(domain_1_port) -> None: domain_client: DomainClient = sy.login( port=domain_1_port, email="info@openmined.org", password="changethis" ) - # Submit Docker Worker Config - docker_config_rl = f""" - FROM openmined/grid-backend:{SYFT_BASE_TAG} - RUN pip install recordlinkage - """ - docker_config = DockerWorkerConfig(dockerfile=docker_config_rl) - - # Submit Worker Image submit_result = domain_client.api.services.worker_image.submit_container_image( - worker_config=docker_config + worker_config=CUSTOM_DOCKER_WORKER_CONFIG ) assert isinstance(submit_result, SyftSuccess) assert len(domain_client.images.get_all()) == 2 # Validate if we can get the worker image object from its config - workerimage = domain_client.api.services.worker_image.get_by_config(docker_config) + workerimage = domain_client.api.services.worker_image.get_by_config( + CUSTOM_DOCKER_WORKER_CONFIG + ) assert not isinstance(workerimage, sy.SyftError) # Build docker image tag_version = sy.UID().short() - docker_tag = f"openmined/custom-worker-rl:{tag_version}" + docker_tag = f"{CUSTOM_IMAGE_TAG}:{tag_version}" docker_build_result = domain_client.api.services.worker_image.build( image_uid=workerimage.id, tag=docker_tag, @@ -74,43 +85,41 @@ def test_image_build(domain_1_port) -> None: # Validate the image is successfully deleted assert len(domain_client.images.get_all()) == 1 workerimage = domain_client.images.get_all()[0] - assert workerimage.config != docker_config + assert workerimage.config != CUSTOM_IMAGE_TAG @pytest.mark.container_workload -def test_pool_launch(domain_1_port) -> None: +@pytest.mark.parametrize( + "worker_config", [PREBUILT_WORKER_CONFIG, CUSTOM_DOCKER_WORKER_CONFIG] +) +def test_pool_launch(domain_1_port: int, worker_config: WorkerConfig) -> None: domain_client: DomainClient = sy.login( port=domain_1_port, email="info@openmined.org", password="changethis" ) assert len(domain_client.worker_pools.get_all()) == 1 - # Submit Docker Worker Config - docker_config_opendp = f""" - FROM openmined/grid-backend:{SYFT_BASE_TAG} - RUN pip install opendp - """ - docker_config = DockerWorkerConfig(dockerfile=docker_config_opendp) - # Submit Worker Image submit_result = domain_client.api.services.worker_image.submit_container_image( - worker_config=docker_config + worker_config=worker_config ) assert isinstance(submit_result, SyftSuccess) - worker_image = domain_client.api.services.worker_image.get_by_config(docker_config) + worker_image = domain_client.api.services.worker_image.get_by_config(worker_config) assert not isinstance(worker_image, sy.SyftError) assert worker_image is not None - assert not worker_image.is_built - # Build docker image - tag_version = sy.UID().short() - docker_tag = f"openmined/custom-worker-opendp:{tag_version}" - docker_build_result = domain_client.api.services.worker_image.build( - image_uid=worker_image.id, - tag=docker_tag, - pull=False, - ) - assert isinstance(docker_build_result, SyftSuccess) + if not worker_image.is_prebuilt: + assert not worker_image.is_built + + # Build docker image + tag_version = sy.UID().short() + docker_tag = f"{CUSTOM_IMAGE_TAG}:{tag_version}" + docker_build_result = domain_client.api.services.worker_image.build( + image_uid=worker_image.id, + tag=docker_tag, + pull=False, + ) + assert isinstance(docker_build_result, SyftSuccess) # Launch a worker pool pool_version = sy.UID().short() @@ -163,7 +172,12 @@ def test_pool_launch(domain_1_port) -> None: @pytest.mark.container_workload -def test_pool_image_creation_job_requests(domain_1_port) -> None: +@pytest.mark.parametrize( + "worker_config", [PREBUILT_WORKER_CONFIG, CUSTOM_DOCKER_WORKER_CONFIG] +) +def test_pool_image_creation_job_requests( + domain_1_port: int, worker_config: WorkerConfig +) -> None: """ Test register ds client, ds requests to create an image and pool creation, do approves, then ds creates a function attached to the worker pool, then creates another @@ -186,26 +200,20 @@ def test_pool_image_creation_job_requests(domain_1_port) -> None: ds_client = sy.login(email=ds_email, password="secret_pw", port=domain_1_port) # the DS makes a request to create an image and a pool based on the image - docker_config_np = f""" - FROM openmined/grid-backend:{SYFT_BASE_TAG} - RUN pip install numpy - """ - docker_config = DockerWorkerConfig(dockerfile=docker_config_np) tag_version = sy.UID().short() - docker_tag = f"openmined/custom-worker-np:{tag_version}" pool_version = sy.UID().short() worker_pool_name = f"custom_worker_pool_ver{pool_version}" request = ds_client.api.services.worker_pool.create_image_and_pool_request( pool_name=worker_pool_name, num_workers=1, - tag=docker_tag, - config=docker_config, - reason="I want to do some more cool data science with PySyft and Recordlinkage", + tag=f"{CUSTOM_IMAGE_TAG}:{tag_version}", + config=worker_config, + reason="I want to do some more cool data science with PySyft", pull_image=False, ) assert isinstance(request, Request) assert len(request.changes) == 2 - assert request.changes[0].config == docker_config + assert request.changes[0].config == worker_config assert request.changes[1].num_workers == 1 assert request.changes[1].pool_name == worker_pool_name @@ -230,7 +238,7 @@ def test_pool_image_creation_job_requests(domain_1_port) -> None: assert isinstance(worker.logs, str) assert worker.job_id is None - built_image = ds_client.api.services.worker_image.get_by_config(docker_config) + built_image = ds_client.api.services.worker_image.get_by_config(worker_config) assert isinstance(built_image, SyftWorkerImage) assert built_image.id == launched_pool.image.id assert worker.image.id == built_image.id From 16bcf91ff8ad7ed3daa803e1a1c6da84b5e03d32 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 18:38:10 +0800 Subject: [PATCH 09/30] Fix worker pool integration tests Revert to use a different dockerfile for each test --- .../container_workload/pool_image_test.py | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index ffef5cbe5aa..c01532bcf49 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -12,7 +12,6 @@ from syft.client.domain_client import DomainClient from syft.custom_worker.config import DockerWorkerConfig from syft.custom_worker.config import PrebuiltWorkerConfig -from syft.custom_worker.config import WorkerConfig from syft.service.request.request import Request from syft.service.response import SyftSuccess from syft.service.worker.worker_image import SyftWorkerImage @@ -63,15 +62,14 @@ def external_registry_uid(domain_1_port: int) -> UID: PREBUILT_WORKER_CONFIG = PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG) -CUSTOM_DOCKERFILE = f""" -FROM {registry}/{repo}:{tag} -RUN pip install recordlinkage -""" - -CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage" - -CUSTOM_DOCKER_WORKER_CONFIG = DockerWorkerConfig(dockerfile=CUSTOM_DOCKERFILE) +def make_docker_config_test_case(pkg: str) -> tuple[str, str]: + return ( + DockerWorkerConfig( + dockerfile=(f"FROM {registry}/{repo}:{tag}\nRUN pip install {pkg}\n") + ), + f"openmined/custom-worker-{pkg}:latest", + ) @pytest.mark.container_workload @@ -80,20 +78,19 @@ def test_image_build(domain_1_port: int, external_registry_uid: UID) -> None: port=domain_1_port, email="info@openmined.org", password="changethis" ) + docker_config, docker_tag = make_docker_config_test_case("recordlinkage") + submit_result = domain_client.api.services.worker_image.submit_container_image( - worker_config=CUSTOM_DOCKER_WORKER_CONFIG + worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) assert len(domain_client.images.get_all()) == 2 # Validate if we can get the worker image object from its config - workerimage = domain_client.api.services.worker_image.get_by_config( - CUSTOM_DOCKER_WORKER_CONFIG - ) + workerimage = domain_client.api.services.worker_image.get_by_config(docker_config) assert not isinstance(workerimage, sy.SyftError) # Build docker image - docker_tag = f"{CUSTOM_IMAGE_TAG}:{sy.UID().short()}" docker_build_result = domain_client.api.services.worker_image.build( image_uid=workerimage.id, tag=docker_tag, @@ -112,11 +109,9 @@ def test_image_build(domain_1_port: int, external_registry_uid: UID) -> None: @pytest.mark.container_workload -@pytest.mark.parametrize( - "worker_config", [PREBUILT_WORKER_CONFIG, CUSTOM_DOCKER_WORKER_CONFIG] -) +@pytest.mark.parametrize("prebuilt", [True, False]) def test_pool_launch( - domain_1_port: int, external_registry_uid: UID, worker_config: WorkerConfig + domain_1_port: int, external_registry_uid: UID, prebuilt: bool ) -> None: domain_client: DomainClient = sy.login( port=domain_1_port, email="info@openmined.org", password="changethis" @@ -124,6 +119,11 @@ def test_pool_launch( assert len(domain_client.worker_pools.get_all()) == 1 # Submit Worker Image + worker_config, docker_tag = ( + (PREBUILT_WORKER_CONFIG, PREBUILT_IMAGE_TAG) + if prebuilt + else make_docker_config_test_case("opendp") + ) submit_result = domain_client.api.services.worker_image.submit_container_image( worker_config=worker_config ) @@ -137,7 +137,6 @@ def test_pool_launch( assert not worker_image.is_built # Build docker image - docker_tag = f"{CUSTOM_IMAGE_TAG}:{sy.UID().short()}" docker_build_result = domain_client.api.services.worker_image.build( image_uid=worker_image.id, tag=docker_tag, @@ -145,13 +144,13 @@ def test_pool_launch( ) assert isinstance(docker_build_result, SyftSuccess) - # Push Image to External registry - push_result = domain_client.api.services.worker_image.push( - worker_image.id, - username=external_registry_username, - password=external_registry_password, - ) - assert isinstance(push_result, sy.SyftSuccess), str(push_result) + # Push Image to External registry + push_result = domain_client.api.services.worker_image.push( + worker_image.id, + username=external_registry_username, + password=external_registry_password, + ) + assert isinstance(push_result, sy.SyftSuccess), str(push_result) # Launch a worker pool worker_pool_name = "custom-worker-pool-opendp" @@ -198,11 +197,9 @@ def test_pool_launch( @pytest.mark.container_workload -@pytest.mark.parametrize( - "worker_config", [PREBUILT_WORKER_CONFIG, CUSTOM_DOCKER_WORKER_CONFIG] -) +@pytest.mark.parametrize("prebuilt", [True, False]) def test_pool_image_creation_job_requests( - domain_1_port: int, external_registry_uid: UID, worker_config: WorkerConfig + domain_1_port: int, external_registry_uid: UID, prebuilt: bool ) -> None: """ Test register ds client, ds requests to create an image and pool creation, @@ -226,11 +223,17 @@ def test_pool_image_creation_job_requests( ds_client = sy.login(email=ds_email, password="secret_pw", port=domain_1_port) # the DS makes a request to create an image and a pool based on the image + worker_config, docker_tag = ( + (PREBUILT_WORKER_CONFIG, PREBUILT_IMAGE_TAG) + if prebuilt + else make_docker_config_test_case("numpy") + ) + worker_pool_name = "custom_worker_pool" request = ds_client.api.services.worker_pool.create_image_and_pool_request( pool_name=worker_pool_name, num_workers=1, - tag=f"{CUSTOM_IMAGE_TAG}:{sy.UID().short()}", + tag=docker_tag, config=worker_config, reason="I want to do some more cool data science with PySyft", pull_image=False, From 87861a3d80248454b465e539d0ebcb44f76d1627 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 19:28:41 +0800 Subject: [PATCH 10/30] Fix worker pool integration tests set registry_uid --- tests/integration/container_workload/pool_image_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index c01532bcf49..d4be44bbf01 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -140,7 +140,7 @@ def test_pool_launch( docker_build_result = domain_client.api.services.worker_image.build( image_uid=worker_image.id, tag=docker_tag, - pull=False, + registry_uid=external_registry_uid, ) assert isinstance(docker_build_result, SyftSuccess) @@ -236,7 +236,7 @@ def test_pool_image_creation_job_requests( tag=docker_tag, config=worker_config, reason="I want to do some more cool data science with PySyft", - pull_image=False, + registry_uid=external_registry_uid, ) assert isinstance(request, Request) assert len(request.changes) == 2 From 749bc1e304bde494bbd7802e47f92adbcea06c50 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 19:47:11 +0800 Subject: [PATCH 11/30] Use a different prebuilt image for each tag --- .../container_workload/pool_image_test.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index d4be44bbf01..95e8f8f7177 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -13,6 +13,7 @@ from syft.custom_worker.config import DockerWorkerConfig from syft.custom_worker.config import PrebuiltWorkerConfig from syft.service.request.request import Request +from syft.service.response import SyftError from syft.service.response import SyftSuccess from syft.service.worker.worker_image import SyftWorkerImage from syft.service.worker.worker_pool import SyftWorker @@ -120,7 +121,7 @@ def test_pool_launch( # Submit Worker Image worker_config, docker_tag = ( - (PREBUILT_WORKER_CONFIG, PREBUILT_IMAGE_TAG) + (PrebuiltWorkerConfig(tag=(_tag := "docker.io/python:3-slim")), _tag) if prebuilt else make_docker_config_test_case("opendp") ) @@ -159,6 +160,7 @@ def test_pool_launch( image_uid=worker_image.id, num_workers=3, ) + assert not isinstance(worker_pool_res, SyftError) assert len(worker_pool_res) == 3 assert all(worker.error is None for worker in worker_pool_res) @@ -224,12 +226,17 @@ def test_pool_image_creation_job_requests( # the DS makes a request to create an image and a pool based on the image worker_config, docker_tag = ( - (PREBUILT_WORKER_CONFIG, PREBUILT_IMAGE_TAG) + ( + PrebuiltWorkerConfig( + tag=(_tag := f"docker.io/openmined/grid-backend:{sy.__version__}") + ), + _tag, + ) if prebuilt else make_docker_config_test_case("numpy") ) - worker_pool_name = "custom_worker_pool" + worker_pool_name = "custom-worker-pool-numpy" request = ds_client.api.services.worker_pool.create_image_and_pool_request( pool_name=worker_pool_name, num_workers=1, From f2c4d5cc66d89b2e17a4a69613dc3ca579fd32ff Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 21:46:39 +0800 Subject: [PATCH 12/30] Add tag for images from PrebuiltWorkerConfig --- .../syft/src/syft/service/worker/worker_image_service.py | 6 ++++++ tests/integration/container_workload/pool_image_test.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index 1e2391132ab..7f1ea910acb 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -6,6 +6,7 @@ import pydantic # relative +from ...custom_worker.config import PrebuiltWorkerConfig from ...custom_worker.config import WorkerConfig from ...custom_worker.k8s import IN_KUBERNETES from ...serde.serializable import serializable @@ -49,6 +50,11 @@ def submit_container_image( worker_image = SyftWorkerImage( config=worker_config, created_by=context.credentials, + image_identifier=( + SyftWorkerImageIdentifier.from_str(worker_config.tag) + if isinstance(worker_config, PrebuiltWorkerConfig) + else None + ), ) res = self.stash.set(context.credentials, worker_image) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 95e8f8f7177..935e3ad16fd 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -121,7 +121,7 @@ def test_pool_launch( # Submit Worker Image worker_config, docker_tag = ( - (PrebuiltWorkerConfig(tag=(_tag := "docker.io/python:3-slim")), _tag) + (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/python:3-slim")), _tag) if prebuilt else make_docker_config_test_case("opendp") ) From 9f24babc7cedea47f3e69e92612987c93a8ff39a Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 22:19:53 +0800 Subject: [PATCH 13/30] No longer try to push prebuilt image to local registry --- packages/syft/src/syft/service/request/request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index 16c12a9cfdd..61a50496df7 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -242,7 +242,7 @@ def _run( message=f"Build result: {build_success_message}" ) - if IN_KUBERNETES: + if IN_KUBERNETES and not worker_image.is_prebuilt: push_result = worker_image_service.push( service_context, image=worker_image.id, From b0b658a3cd2491ef83f211612a03cd5610991bb3 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 22:21:58 +0800 Subject: [PATCH 14/30] Use a different worker pool name for each test --- tests/integration/container_workload/pool_image_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 935e3ad16fd..9d200fef198 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -154,7 +154,7 @@ def test_pool_launch( assert isinstance(push_result, sy.SyftSuccess), str(push_result) # Launch a worker pool - worker_pool_name = "custom-worker-pool-opendp" + worker_pool_name = f"custom-worker-pool-opendp{'-prebuilt' if prebuilt else ''}" worker_pool_res = domain_client.api.services.worker_pool.launch( name=worker_pool_name, image_uid=worker_image.id, @@ -236,7 +236,7 @@ def test_pool_image_creation_job_requests( else make_docker_config_test_case("numpy") ) - worker_pool_name = "custom-worker-pool-numpy" + worker_pool_name = f"custom-worker-pool-numpy{'-prebuilt' if prebuilt else ''}" request = ds_client.api.services.worker_pool.create_image_and_pool_request( pool_name=worker_pool_name, num_workers=1, From 74038fededab7e73e71d8aee791c30221c6f17f9 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 23:04:23 +0800 Subject: [PATCH 15/30] Get error while launching worker pool --- packages/syft/src/syft/custom_worker/runner_k8s.py | 2 -- packages/syft/src/syft/service/worker/utils.py | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/syft/src/syft/custom_worker/runner_k8s.py b/packages/syft/src/syft/custom_worker/runner_k8s.py index eeddacd9ebf..c2ded231bf7 100644 --- a/packages/syft/src/syft/custom_worker/runner_k8s.py +++ b/packages/syft/src/syft/custom_worker/runner_k8s.py @@ -60,8 +60,6 @@ def create_pool( f"jsonpath='{JSONPATH_AVAILABLE_REPLICAS}'={replicas}", timeout=CREATE_POOL_TIMEOUT_SEC, ) - except Exception: - raise finally: if pull_secret: pull_secret.delete(propagation_policy="Foreground") diff --git a/packages/syft/src/syft/service/worker/utils.py b/packages/syft/src/syft/service/worker/utils.py index 2f647069ca6..3ff5a76482b 100644 --- a/packages/syft/src/syft/service/worker/utils.py +++ b/packages/syft/src/syft/service/worker/utils.py @@ -331,7 +331,6 @@ def create_kubernetes_pool( **kwargs: Any, ) -> list[Pod] | SyftError: pool = None - error = False try: print( @@ -366,11 +365,9 @@ def create_kubernetes_pool( reg_url=reg_url, ) except Exception as e: - error = True - return SyftError(message=f"Failed to start workers {e}") - finally: - if error and pool: + if pool: pool.delete() + return SyftError(message=f"Failed to start workers {e}.") return runner.get_pool_pods(pool_name=pool_name) From b9554e549e5a6a5c3e841d27e2468ded18c3bc62 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 16 May 2024 23:30:50 +0800 Subject: [PATCH 16/30] debug --- packages/syft/src/syft/service/worker/utils.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/syft/src/syft/service/worker/utils.py b/packages/syft/src/syft/service/worker/utils.py index 3ff5a76482b..181774da837 100644 --- a/packages/syft/src/syft/service/worker/utils.py +++ b/packages/syft/src/syft/service/worker/utils.py @@ -367,7 +367,12 @@ def create_kubernetes_pool( except Exception as e: if pool: pool.delete() - return SyftError(message=f"Failed to start workers {e}.") + # stdlib + import traceback + + return SyftError( + message=f"Failed to start workers {e} {e.__class__} {e.args} {traceback.format_exc()}." + ) return runner.get_pool_pods(pool_name=pool_name) From 6d6a6a629719aa6f83177103d4f4201caffa7944 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 17 May 2024 00:35:03 +0800 Subject: [PATCH 17/30] Increase k8s pool creation timeout --- packages/syft/src/syft/custom_worker/runner_k8s.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/syft/src/syft/custom_worker/runner_k8s.py b/packages/syft/src/syft/custom_worker/runner_k8s.py index c2ded231bf7..f9c48c6e394 100644 --- a/packages/syft/src/syft/custom_worker/runner_k8s.py +++ b/packages/syft/src/syft/custom_worker/runner_k8s.py @@ -13,7 +13,7 @@ from .k8s import get_kr8s_client JSONPATH_AVAILABLE_REPLICAS = "{.status.availableReplicas}" -CREATE_POOL_TIMEOUT_SEC = 60 +CREATE_POOL_TIMEOUT_SEC = 180 SCALE_POOL_TIMEOUT_SEC = 60 From 5636bf1800e836e89f2c1ffb16c43e39640ffb88 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 17 May 2024 00:54:55 +0800 Subject: [PATCH 18/30] Test using a smaller image --- tests/integration/container_workload/pool_image_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 9d200fef198..0264aa92cb0 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -121,7 +121,7 @@ def test_pool_launch( # Submit Worker Image worker_config, docker_tag = ( - (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/python:3-slim")), _tag) + (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/python:3-alpine")), _tag) if prebuilt else make_docker_config_test_case("opendp") ) From ece69c1b268ca0a2b128a642b280a84d93fca7fa Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 21 May 2024 11:21:18 +0800 Subject: [PATCH 19/30] Remove unused variables --- tests/integration/container_workload/pool_image_test.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 0264aa92cb0..d9d0caf27f2 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -59,11 +59,6 @@ def external_registry_uid(domain_1_port: int) -> UID: return image_registry_list[0].id -PREBUILT_IMAGE_TAG = f"docker.io/openmined/grid-backend:{sy.__version__}" - -PREBUILT_WORKER_CONFIG = PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG) - - def make_docker_config_test_case(pkg: str) -> tuple[str, str]: return ( DockerWorkerConfig( From 9e84ea92c2db33376705d00e80971e0b7873e87b Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 21 May 2024 11:21:49 +0800 Subject: [PATCH 20/30] Remove unnecessary check --- tests/integration/container_workload/pool_image_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index d9d0caf27f2..69c551134f6 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -112,7 +112,6 @@ def test_pool_launch( domain_client: DomainClient = sy.login( port=domain_1_port, email="info@openmined.org", password="changethis" ) - assert len(domain_client.worker_pools.get_all()) == 1 # Submit Worker Image worker_config, docker_tag = ( @@ -156,10 +155,8 @@ def test_pool_launch( num_workers=3, ) assert not isinstance(worker_pool_res, SyftError) - assert len(worker_pool_res) == 3 assert all(worker.error is None for worker in worker_pool_res) - assert len(domain_client.worker_pools.get_all()) == 2 worker_pool = domain_client.worker_pools[worker_pool_name] assert len(worker_pool.worker_list) == 3 From f9a35e136d9e6b33f901b328133cbdbd36fb2548 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 21 May 2024 20:27:31 +0800 Subject: [PATCH 21/30] Use different prebuilt images for tests --- tests/integration/container_workload/pool_image_test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 69c551134f6..f0a7da83c51 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -115,7 +115,7 @@ def test_pool_launch( # Submit Worker Image worker_config, docker_tag = ( - (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/python:3-alpine")), _tag) + (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/nginx:latest")), _tag) if prebuilt else make_docker_config_test_case("opendp") ) @@ -219,9 +219,7 @@ def test_pool_image_creation_job_requests( # the DS makes a request to create an image and a pool based on the image worker_config, docker_tag = ( ( - PrebuiltWorkerConfig( - tag=(_tag := f"docker.io/openmined/grid-backend:{sy.__version__}") - ), + PrebuiltWorkerConfig(tag=(_tag := f"{registry}/{repo}:{tag}")), _tag, ) if prebuilt From dd138f52f4ae81a711df89454896885229829a60 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 22 May 2024 11:45:18 +0800 Subject: [PATCH 22/30] Avoid username collision in tests --- tests/integration/container_workload/pool_image_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index f0a7da83c51..032a9f5b706 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -1,8 +1,8 @@ # stdlib import os +from uuid import uuid4 # third party -from faker import Faker import numpy as np import pytest import requests @@ -204,8 +204,7 @@ def test_pool_image_creation_job_requests( domain_client: DomainClient = sy.login( port=domain_1_port, email="info@openmined.org", password="changethis" ) - fake = Faker() - ds_username = fake.user_name() + ds_username = uuid4().hex[:8] ds_email = ds_username + "@example.com" res = domain_client.register( name=ds_username, From 128694050dfd55a9363d02fbcda04456d2d30c50 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 22 May 2024 11:46:53 +0800 Subject: [PATCH 23/30] Get the correct job by id instead of the last job --- tests/integration/container_workload/pool_image_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 032a9f5b706..04c633c5259 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -296,7 +296,7 @@ def custom_worker_func(x): job.wait() assert job.status.value == "completed" - job = domain_client.jobs[-1] + job = domain_client.jobs.get_by_user_code_id(job.user_code_id)[-1] assert job.job_worker_id == worker.id # Validate the result received from the syft function From 7c535c7b01f6739289963baad3379e9c84b45067 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 22 May 2024 12:10:00 +0800 Subject: [PATCH 24/30] Only require tag for DockerWorkerConfig pool request --- .../syft/src/syft/service/request/request.py | 10 ++++- .../service/worker/worker_pool_service.py | 39 ++++++++++++------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index 646536b7d7f..df952c11778 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -6,6 +6,7 @@ from typing import Any # third party +from pydantic import model_validator from result import Err from result import Ok from result import Result @@ -15,6 +16,7 @@ from ...abstract_node import NodeSideType from ...client.api import APIRegistry from ...client.client import SyftClient +from ...custom_worker.config import DockerWorkerConfig from ...custom_worker.config import WorkerConfig from ...custom_worker.k8s import IN_KUBERNETES from ...node.credentials import SyftVerifyKey @@ -192,12 +194,18 @@ class CreateCustomImageChange(Change): __version__ = SYFT_OBJECT_VERSION_2 config: WorkerConfig - tag: str + tag: str | None = None registry_uid: UID | None = None pull_image: bool = True __repr_attrs__ = ["config", "tag"] + @model_validator(mode="after") + def _tag_required_for_dockerworkerconfig(self) -> Self: + if isinstance(self.config, DockerWorkerConfig) and self.tag is None: + raise ValueError("`tag` is required for `DockerWorkerConfig`.") + return self + def _run( self, context: ChangeContext, apply: bool ) -> Result[SyftSuccess, SyftError]: diff --git a/packages/syft/src/syft/service/worker/worker_pool_service.py b/packages/syft/src/syft/service/worker/worker_pool_service.py index a9c65d5a5b2..cef16fb5b72 100644 --- a/packages/syft/src/syft/service/worker/worker_pool_service.py +++ b/packages/syft/src/syft/service/worker/worker_pool_service.py @@ -6,7 +6,8 @@ from result import OkErr # relative -from ...custom_worker.config import CustomWorkerConfig +from ...custom_worker.config import DockerWorkerConfig +from ...custom_worker.config import PrebuiltWorkerConfig from ...custom_worker.config import WorkerConfig from ...custom_worker.k8s import IN_KUBERNETES from ...custom_worker.runner_k8s import KubernetesRunner @@ -232,8 +233,8 @@ def create_image_and_pool_request( context: AuthedServiceContext, pool_name: str, num_workers: int, - tag: str, config: WorkerConfig, + tag: str | None = None, registry_uid: UID | None = None, reason: str | None = "", pull_image: bool = True, @@ -246,15 +247,31 @@ def create_image_and_pool_request( pool_name (str): The name of the worker pool. num_workers (int): The number of workers in the pool. config: (WorkerConfig): Config of the image to be built. - tag (str): human-readable manifest identifier that is typically a specific version or variant of an image - reason (Optional[str], optional): The reason for creating the worker image and pool. Defaults to "". + tag (str | None, optional): + a human-readable manifest identifier that is typically a specific version or variant of an image, + only needed for `DockerWorkerConfig` to tag the image after it is built. + reason (str | None, optional): The reason for creating the worker image and pool. Defaults to "". """ - if isinstance(config, CustomWorkerConfig): - return SyftError(message="We only support DockerWorkerConfig.") + if not isinstance(config, DockerWorkerConfig | PrebuiltWorkerConfig): + return SyftError( + message="We only support either `DockerWorkerConfig` or `PrebuiltWorkerConfig`." + ) - if IN_KUBERNETES and registry_uid is None: - return SyftError(message="Registry UID is required in Kubernetes mode.") + if isinstance(config, DockerWorkerConfig): + if tag is None: + return SyftError(message="`tag` is required for `DockerWorkerConfig`.") + + # Validate image tag + try: + SyftWorkerImageIdentifier.from_str(tag=tag) + except pydantic.ValidationError as e: + return SyftError(message=f"Invalid `tag`: {e}.") + + if IN_KUBERNETES and registry_uid is None: + return SyftError( + message="`registry_uid` is required in Kubernetes mode for `DockerWorkerConfig`." + ) # Check if an image already exists for given docker config search_result = self.image_stash.get_by_worker_config( @@ -272,12 +289,6 @@ def create_image_and_pool_request( Please use `worker_pool.create_pool_request` to request pool creation." ) - # Validate Image Tag - try: - SyftWorkerImageIdentifier.from_str(tag=tag) - except pydantic.ValidationError as e: - return SyftError(message=f"Failed to create tag: {e}") - # create a list of Change objects and submit a # request for these changes for approval changes: list[Change] = [] From 5ecf05b101319882aeb2baf87c9d848ebc5d74f9 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 22 May 2024 12:42:59 +0800 Subject: [PATCH 25/30] Update CreateCustomImageChange protocol version --- packages/syft/src/syft/service/request/request.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index df952c11778..f5eaed2eb46 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -189,10 +189,23 @@ def __repr_syft_nested__(self) -> str: @serializable() -class CreateCustomImageChange(Change): +class CreateCustomImageChangeV2(Change): __canonical_name__ = "CreateCustomImageChange" __version__ = SYFT_OBJECT_VERSION_2 + config: WorkerConfig + tag: str + registry_uid: UID | None = None + pull_image: bool = True + + __repr_attrs__ = ["config", "tag"] + + +@serializable() +class CreateCustomImageChange(Change): + __canonical_name__ = "CreateCustomImageChange" + __version__ = SYFT_OBJECT_VERSION_3 + config: WorkerConfig tag: str | None = None registry_uid: UID | None = None From 135fbb227c50b98cf9fdfaea40630ecf42e5427e Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 22 May 2024 12:54:12 +0800 Subject: [PATCH 26/30] Update tests --- .../worker_pool/worker_pool_service_test.py | 17 +++++++----- .../container_workload/pool_image_test.py | 26 +++++++++---------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index dd6a5a9453d..4fb500b11c6 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -16,7 +16,7 @@ # relative from ..request.request_code_accept_deny_test import get_ds_client -PREBUILT_IMAGE_TAG = f"openmined/grid-backend:{sy.__version__}" +PREBUILT_IMAGE_TAG = f"docker.io/openmined/grid-backend:{sy.__version__}" CUSTOM_DOCKERFILE = f""" FROM {PREBUILT_IMAGE_TAG} @@ -24,7 +24,7 @@ RUN pip install recordlinkage """ -CUSTOM_IMAGE_TAG = "openmined/custom-worker-recordlinkage:latest" +CUSTOM_IMAGE_TAG = "docker.io/openmined/custom-worker-recordlinkage:latest" WORKER_CONFIG_TEST_CASES_WITH_N_IMAGES = [ ( @@ -33,7 +33,7 @@ 2, # total number of images. # 2 since we pull a pre-built image (1) as the base image to build a custom image (2) ), - (PREBUILT_IMAGE_TAG, PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG), 1), + (None, PrebuiltWorkerConfig(tag=PREBUILT_IMAGE_TAG), 1), ] WORKER_CONFIG_TEST_CASES = [ @@ -74,11 +74,16 @@ def test_create_image_and_pool_request_accept( assert root_client.requests[-1].status.value == 2 all_image_tags = [ - im.image_identifier.repo_with_tag + im.image_identifier.full_name_with_tag for im in root_client.images.get_all() if im.image_identifier ] - assert docker_tag in all_image_tags + tag = ( + worker_config.tag + if isinstance(worker_config, PrebuiltWorkerConfig) + else docker_tag + ) + assert tag in all_image_tags launched_pool = root_client.worker_pools["recordlinkage-pool"] assert isinstance(launched_pool, WorkerPool) assert len(launched_pool.worker_list) == 2 @@ -125,7 +130,7 @@ def test_create_pool_request_accept( root_client.api.services.worker_image.get_by_config(worker_config) ) assert isinstance(docker_build_result, SyftSuccess) - assert worker_image.image_identifier.repo_with_tag == docker_tag + assert worker_image.image_identifier.full_name_with_tag == docker_tag # The DS client submits a request to create a pool from an existing image request = ds_client.api.services.worker_pool.pool_creation_request( diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 04c633c5259..2754eb650b5 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -115,7 +115,7 @@ def test_pool_launch( # Submit Worker Image worker_config, docker_tag = ( - (PrebuiltWorkerConfig(tag=(_tag := "docker.io/library/nginx:latest")), _tag) + (PrebuiltWorkerConfig(tag="docker.io/library/nginx:latest"), None) if prebuilt else make_docker_config_test_case("opendp") ) @@ -217,23 +217,23 @@ def test_pool_image_creation_job_requests( # the DS makes a request to create an image and a pool based on the image worker_config, docker_tag = ( - ( - PrebuiltWorkerConfig(tag=(_tag := f"{registry}/{repo}:{tag}")), - _tag, - ) + (PrebuiltWorkerConfig(tag=f"{registry}/{repo}:{tag}"), None) if prebuilt else make_docker_config_test_case("numpy") ) worker_pool_name = f"custom-worker-pool-numpy{'-prebuilt' if prebuilt else ''}" - request = ds_client.api.services.worker_pool.create_image_and_pool_request( - pool_name=worker_pool_name, - num_workers=1, - tag=docker_tag, - config=worker_config, - reason="I want to do some more cool data science with PySyft", - registry_uid=external_registry_uid, - ) + + kwargs = { + "pool_name": worker_pool_name, + "num_workers": 1, + "config": worker_config, + "reason": "I want to do some more cool data science with PySyft", + } + if not prebuilt: + kwargs.update({"tag": docker_tag, "registry_uid": external_registry_uid}) + + request = ds_client.api.services.worker_pool.create_image_and_pool_request(**kwargs) assert isinstance(request, Request) assert len(request.changes) == 2 assert request.changes[0].config == worker_config From 3618ad478f31916720dbe374b8aebf3f54834759 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 24 May 2024 14:26:40 +0800 Subject: [PATCH 27/30] Return an error in case of invalid container name --- .../syft/service/worker/worker_image_service.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index 7f1ea910acb..dd5cf4bfcd0 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -47,14 +47,20 @@ def __init__(self, store: DocumentStore) -> None: def submit_container_image( self, context: AuthedServiceContext, worker_config: WorkerConfig ) -> SyftSuccess | SyftError: + image_identifier: SyftWorkerImageIdentifier | None = None + if isinstance(worker_config, PrebuiltWorkerConfig): + try: + image_identifier = SyftWorkerImageIdentifier.from_str(worker_config.tag) + except Exception: + return SyftError( + f"Invalid Docker image name: {worker_config.tag}.\n" + + "Please specify the image name in this format /:." + ) + worker_image = SyftWorkerImage( config=worker_config, created_by=context.credentials, - image_identifier=( - SyftWorkerImageIdentifier.from_str(worker_config.tag) - if isinstance(worker_config, PrebuiltWorkerConfig) - else None - ), + image_identifier=image_identifier, ) res = self.stash.set(context.credentials, worker_image) From 3cb2a0a84a654fa4f6afb4f944c4070d5ad69bdf Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Mon, 27 May 2024 14:16:50 +0800 Subject: [PATCH 28/30] Rename submit_container_image to just submit --- notebooks/admin/Custom API + Custom Worker.ipynb | 2 +- notebooks/api/0.8/10-container-images.ipynb | 4 ++-- notebooks/api/0.8/11-container-images-k8s.ipynb | 4 ++-- packages/syft/src/syft/service/request/request.py | 2 +- .../syft/src/syft/service/worker/worker_image_service.py | 6 +++--- .../syft/tests/syft/worker_pool/worker_pool_service_test.py | 6 ++---- packages/syft/tests/syft/worker_pool/worker_test.py | 2 +- tests/integration/container_workload/pool_image_test.py | 4 ++-- 8 files changed, 14 insertions(+), 16 deletions(-) diff --git a/notebooks/admin/Custom API + Custom Worker.ipynb b/notebooks/admin/Custom API + Custom Worker.ipynb index a7bdf91c5ef..a27bb7a23c4 100644 --- a/notebooks/admin/Custom API + Custom Worker.ipynb +++ b/notebooks/admin/Custom API + Custom Worker.ipynb @@ -114,7 +114,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", + "submit_result = domain_client.api.services.worker_image.submit(\n", " worker_config=docker_config\n", ")\n", "submit_result" diff --git a/notebooks/api/0.8/10-container-images.ipynb b/notebooks/api/0.8/10-container-images.ipynb index 1c682ad1b96..870cb655d93 100644 --- a/notebooks/api/0.8/10-container-images.ipynb +++ b/notebooks/api/0.8/10-container-images.ipynb @@ -227,7 +227,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", + "submit_result = domain_client.api.services.worker_image.submit(\n", " worker_config=docker_config\n", ")" ] @@ -1095,7 +1095,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", + "submit_result = domain_client.api.services.worker_image.submit(\n", " worker_config=docker_config_2\n", ")\n", "submit_result" diff --git a/notebooks/api/0.8/11-container-images-k8s.ipynb b/notebooks/api/0.8/11-container-images-k8s.ipynb index 003c4d79f2a..24f4a7c33df 100644 --- a/notebooks/api/0.8/11-container-images-k8s.ipynb +++ b/notebooks/api/0.8/11-container-images-k8s.ipynb @@ -265,7 +265,7 @@ "metadata": {}, "outputs": [], "source": [ - "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", + "submit_result = domain_client.api.services.worker_image.submit(\n", " worker_config=docker_config\n", ")\n", "submit_result" @@ -935,7 +935,7 @@ "outputs": [], "source": [ "submit_result = None\n", - "submit_result = domain_client.api.services.worker_image.submit_container_image(\n", + "submit_result = domain_client.api.services.worker_image.submit(\n", " worker_config=docker_config_opendp\n", ")\n", "submit_result" diff --git a/packages/syft/src/syft/service/request/request.py b/packages/syft/src/syft/service/request/request.py index 114140bc704..3afbafee914 100644 --- a/packages/syft/src/syft/service/request/request.py +++ b/packages/syft/src/syft/service/request/request.py @@ -226,7 +226,7 @@ def _run( worker_image_service = context.node.get_service("SyftWorkerImageService") service_context = context.to_service_ctx() - result = worker_image_service.submit_container_image( + result = worker_image_service.submit( service_context, worker_config=self.config ) diff --git a/packages/syft/src/syft/service/worker/worker_image_service.py b/packages/syft/src/syft/service/worker/worker_image_service.py index dd5cf4bfcd0..afd21af9e50 100644 --- a/packages/syft/src/syft/service/worker/worker_image_service.py +++ b/packages/syft/src/syft/service/worker/worker_image_service.py @@ -40,11 +40,11 @@ def __init__(self, store: DocumentStore) -> None: self.stash = SyftWorkerImageStash(store=store) @service_method( - path="worker_image.submit_container_image", - name="submit_container_image", + path="worker_image.submit", + name="submit", roles=DATA_OWNER_ROLE_LEVEL, ) - def submit_container_image( + def submit( self, context: AuthedServiceContext, worker_config: WorkerConfig ) -> SyftSuccess | SyftError: image_identifier: SyftWorkerImageIdentifier | None = None diff --git a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py index 4fb500b11c6..2b65105c160 100644 --- a/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_pool_service_test.py @@ -110,7 +110,7 @@ def test_create_pool_request_accept( assert root_client.credentials != ds_client.credentials # the DO submits the docker config to build an image - submit_result = root_client.api.services.worker_image.submit_container_image( + submit_result = root_client.api.services.worker_image.submit( worker_config=worker_config ) assert isinstance(submit_result, SyftSuccess) @@ -161,9 +161,7 @@ def test_get_by_worker_config( ) -> None: root_client = worker.root_client for config in WORKER_CONFIGS: - root_client.api.services.worker_image.submit_container_image( - worker_config=config - ) + root_client.api.services.worker_image.submit(worker_config=config) worker_image = root_client.api.services.worker_image.get_by_config(worker_config) assert worker_image.config == worker_config diff --git a/packages/syft/tests/syft/worker_pool/worker_test.py b/packages/syft/tests/syft/worker_pool/worker_test.py index 55909b5599d..3c05875a009 100644 --- a/packages/syft/tests/syft/worker_pool/worker_test.py +++ b/packages/syft/tests/syft/worker_pool/worker_test.py @@ -23,7 +23,7 @@ def test_syft_worker(worker: Worker): """ root_client = worker.root_client docker_config = get_docker_config() - submit_result = root_client.api.services.worker_image.submit_container_image( + submit_result = root_client.api.services.worker_image.submit( worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 2754eb650b5..73f841c3f56 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -76,7 +76,7 @@ def test_image_build(domain_1_port: int, external_registry_uid: UID) -> None: docker_config, docker_tag = make_docker_config_test_case("recordlinkage") - submit_result = domain_client.api.services.worker_image.submit_container_image( + submit_result = domain_client.api.services.worker_image.submit( worker_config=docker_config ) assert isinstance(submit_result, SyftSuccess) @@ -119,7 +119,7 @@ def test_pool_launch( if prebuilt else make_docker_config_test_case("opendp") ) - submit_result = domain_client.api.services.worker_image.submit_container_image( + submit_result = domain_client.api.services.worker_image.submit( worker_config=worker_config ) assert isinstance(submit_result, SyftSuccess) From a8d86ed9948a31b14c15055043c0060d6a43fcf5 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Mon, 27 May 2024 16:55:58 +0800 Subject: [PATCH 29/30] Use correct account client this should be ds_client instead of domain_client --- tests/integration/container_workload/pool_image_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 73f841c3f56..19d8ff6b67d 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -269,7 +269,7 @@ def test_pool_image_creation_job_requests( # Dataset data = np.array([1, 2, 3]) data_action_obj = sy.ActionObject.from_obj(data) - data_pointer = domain_client.api.services.action.set(data_action_obj) + data_pointer = ds_client.api.services.action.set(data_action_obj) # Function @sy.syft_function( From 19d627f30f2796eda89fd6065d49072716e8e46e Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Mon, 27 May 2024 17:52:02 +0800 Subject: [PATCH 30/30] Fix user permission in test --- tests/integration/container_workload/pool_image_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/container_workload/pool_image_test.py b/tests/integration/container_workload/pool_image_test.py index 19d8ff6b67d..53f0b8b10a1 100644 --- a/tests/integration/container_workload/pool_image_test.py +++ b/tests/integration/container_workload/pool_image_test.py @@ -213,6 +213,11 @@ def test_pool_image_creation_job_requests( password_verify="secret_pw", ) assert isinstance(res, SyftSuccess) + + # Grant user permission to request code execution + ds = next(u for u in domain_client.users if u.email == ds_email) + ds.allow_mock_execution() + ds_client = sy.login(email=ds_email, password="secret_pw", port=domain_1_port) # the DS makes a request to create an image and a pool based on the image