Skip to content

Commit

Permalink
chore: parametrize user image docker repository (#888)
Browse files Browse the repository at this point in the history
Signed-off-by: ThibaultFy <[email protected]>
  • Loading branch information
ThibaultFy authored Apr 18, 2024
1 parent d030646 commit 9210bc3
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 40 deletions.
1 change: 1 addition & 0 deletions backend/backend/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@
REGISTRY_PULL_DOMAIN = os.getenv("REGISTRY_PULL_DOMAIN")
REGISTRY_IS_LOCAL = to_bool(os.environ.get("REGISTRY_IS_LOCAL"))
REGISTRY_SERVICE_NAME = os.environ.get("REGISTRY_SERVICE_NAME")
USER_IMAGE_REPOSITORY = os.environ.get("USER_IMAGE_REPOSITORY")

COMPUTE_POD_RUN_AS_USER = os.environ.get("COMPUTE_POD_RUN_AS_USER")
COMPUTE_POD_RUN_AS_GROUP = os.environ.get("COMPUTE_POD_RUN_AS_GROUP")
Expand Down
16 changes: 14 additions & 2 deletions backend/image_transfer/decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def push_payload(
strict: bool = False,
registry: str = "registry-1.docker.io",
secure: bool = True,
repository: Optional[str] = None,
username: Optional[str] = None,
password: Optional[str] = None,
) -> list[str]:
Expand All @@ -49,6 +50,7 @@ def push_payload(
registry: the registry to push to. It defaults to `registry-1.docker.io` (dockerhub).
secure: whether to use TLS (HTTPS) or not to connect to the registry,
default is True.
repository: the repository to push the images to. Optional.
username: the username to use to connect to the registry. Optional
if the registry does not require authentication.
password: the password to use to connect to the registry. Optional
Expand All @@ -65,7 +67,9 @@ def push_payload(

with DXFBase(host=registry, auth=authenticator.auth, insecure=not secure) as dxf_base:
with safezip.ZipFile(zip_file, "r") as zip_file:
return list(load_zip_images_in_registry(dxf_base, zip_file, strict))
return list(
load_zip_images_in_registry(dxf_base=dxf_base, zip_file=zip_file, repository=repository, strict=strict)
)


def push_all_blobs_from_manifest(
Expand Down Expand Up @@ -135,12 +139,20 @@ def check_if_the_docker_image_is_in_the_registry(dxf_base: DXFBase, docker_image
print(f"Skipping {docker_image} as its already in the registry", file=sys.stderr)


def load_zip_images_in_registry(dxf_base: DXFBase, zip_file: safezip.ZipFile, strict: bool) -> Iterator[str]:
def load_zip_images_in_registry(
dxf_base: DXFBase, zip_file: safezip.ZipFile, strict: bool, repository: Optional[str] = None
) -> Iterator[str]:
payload_descriptor = get_payload_descriptor(zip_file)
for (
docker_image,
manifest_path_in_zip,
) in payload_descriptor.manifests_paths.items():

if repository is not None:
# The repository depends on the organization registry.
_, tag = get_repo_and_tag(docker_image)
docker_image = f"{repository}:{tag}"

if manifest_path_in_zip is None:
check_if_the_docker_image_is_in_the_registry(dxf_base, docker_image, strict)
else:
Expand Down
20 changes: 20 additions & 0 deletions backend/image_transfer/tests/test_decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,26 @@ def test_end_to_end_single_image(
docker_client.images.remove(f"localhost:{destination_registry_local_port}/{ubuntu_base_image}", force=True)


@pytest.mark.usefixtures("add_destination_registry")
@pytest.mark.serial
def test_end_to_end_single_image_different_repository(
tmp_path, ubuntu_base_image, base_registry_local_port, destination_registry_local_port
):
payload_path = tmp_path / "payload.zip"
make_payload(
payload_path,
[ubuntu_base_image],
registry=f"localhost:{base_registry_local_port}",
secure=False,
)

images_pushed = push_payload(
payload_path, registry=f"localhost:{destination_registry_local_port}", secure=False, repository="custom_ubuntu"
)
expected_image = f"custom_ubuntu:{ubuntu_base_image.split(':',1)[1]}"
assert images_pushed == [expected_image]


@pytest.mark.usefixtures("add_destination_registry")
@pytest.mark.serial
def test_end_to_end_multiple_images(
Expand Down
3 changes: 2 additions & 1 deletion backend/substrapp/compute_tasks/image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
IMAGE_BUILD_CHECK_DELAY = settings.IMAGE_BUILD_CHECK_DELAY
REGISTRY = settings.REGISTRY
SUBTUPLE_TMP_DIR = settings.SUBTUPLE_TMP_DIR
USER_IMAGE_REPOSITORY = settings.USER_IMAGE_REPOSITORY


def load_remote_function_image(function: orchestrator.Function, channel: str) -> None:
Expand All @@ -33,4 +34,4 @@ def load_remote_function_image(function: orchestrator.Function, channel: str) ->
with TemporaryDirectory(dir=SUBTUPLE_TMP_DIR) as tmp_dir:
storage_path = pathlib.Path(tmp_dir) / f"{container_image_tag}.zip"
storage_path.write_bytes(function_image_content)
push_payload(storage_path, registry=REGISTRY, secure=False)
push_payload(storage_path, registry=REGISTRY, repository=USER_IMAGE_REPOSITORY, secure=False)
27 changes: 1 addition & 26 deletions backend/substrapp/docker_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
REGISTRY_IS_LOCAL = settings.REGISTRY_IS_LOCAL
REGISTRY_SERVICE_NAME = settings.REGISTRY_SERVICE_NAME
HTTP_CLIENT_TIMEOUT_SECONDS = settings.HTTP_CLIENT_TIMEOUT_SECONDS
USER_IMAGE_REPOSITORY = "substrafoundation/user-image"
USER_IMAGE_REPOSITORY = settings.USER_IMAGE_REPOSITORY


class ImageNotFoundError(Exception):
Expand Down Expand Up @@ -59,31 +59,6 @@ def get_container_image(image_name: str) -> dict:
return response.json()


def get_container_images() -> list[dict]:
response = requests.get(
f"{REGISTRY_SCHEME}://{REGISTRY}/v2/_catalog",
headers={"Accept": "application/vnd.docker.distribution.manifest.v2+json"},
timeout=HTTP_CLIENT_TIMEOUT_SECONDS,
)

response.raise_for_status()
res = response.json()

for repository in res["repositories"]:
# get only user-image repo, images built by substra-backend
if repository == USER_IMAGE_REPOSITORY:
response = requests.get(
f"{REGISTRY_SCHEME}://{REGISTRY}/v2/{repository}/tags/list",
headers={"Accept": "application/vnd.docker.distribution.manifest.v2+json"},
timeout=HTTP_CLIENT_TIMEOUT_SECONDS,
)

response.raise_for_status()
return response.json()

return None


def run_garbage_collector() -> None:
logger.info("Launch garbage collect on docker-registry")

Expand Down
1 change: 1 addition & 0 deletions changes/888.added
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Parametrize user docker repository for user docker images through environment variable.
6 changes: 5 additions & 1 deletion charts/substra-backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

<!-- towncrier release notes start -->

## [26.2.0] - 2024-04-18

### Added

- User docker registry in configurable through value in `containerRegistry.userImageRepository`

## [26.1.0] - 2024-04-17

### Added

- Resources limits and requests (CPU and memory) for all containers.


## [26.0.0] - 2024-04-16

### Changed
Expand Down
2 changes: 1 addition & 1 deletion charts/substra-backend/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: substra-backend
home: https://github.com/Substra
version: 26.1.0
version: 26.2.0
appVersion: 0.45.0
kubeVersion: ">= 1.19.0-0"
description: Main package for Substra
Expand Down
17 changes: 9 additions & 8 deletions charts/substra-backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,15 @@ See [UPGRADE.md](https://github.com/Substra/substra-backend/blob/main/charts/sub

### Substra container registry settings

| Name | Description | Value |
| ------------------------------- | ----------------------------------------------------------------------------------------------- | ----------- |
| `containerRegistry.local` | Whether the registry is exposed as a _nodePort_ and located in the same _Namespace_ as Substra. | `true` |
| `containerRegistry.host` | Hostname of the container registry | `127.0.0.1` |
| `containerRegistry.port` | Port of the container registry | `32000` |
| `containerRegistry.scheme` | Communication scheme of the container registry | `http` |
| `containerRegistry.pullDomain` | Hostname from which the cluster should pull container images | `127.0.0.1` |
| `containerRegistry.prepopulate` | Images to add to the container registry | `[]` |
| Name | Description | Value |
| --------------------------------------- | ----------------------------------------------------------------------------------------------- | -------------------- |
| `containerRegistry.local` | Whether the registry is exposed as a _nodePort_ and located in the same _Namespace_ as Substra. | `true` |
| `containerRegistry.host` | Hostname of the container registry | `127.0.0.1` |
| `containerRegistry.port` | Port of the container registry | `32000` |
| `containerRegistry.scheme` | Communication scheme of the container registry | `http` |
| `containerRegistry.userImageRepository` | Name of the repository where to push and pull user docker images | `substra/user-image` |
| `containerRegistry.pullDomain` | Hostname from which the cluster should pull container images | `127.0.0.1` |
| `containerRegistry.prepopulate` | Images to add to the container registry | `[]` |

### Api event app settings

Expand Down
1 change: 1 addition & 0 deletions charts/substra-backend/templates/configmap-registry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ data:
{{- end }}
REGISTRY_SCHEME: {{ .Values.containerRegistry.scheme}}
REGISTRY_PULL_DOMAIN: {{ .Values.containerRegistry.pullDomain}}
USER_IMAGE_REPOSITORY: {{ .Values.containerRegistry.userImageRepository}}
5 changes: 4 additions & 1 deletion charts/substra-backend/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ privateCa:
name: substra-private-ca
data:
fileName: private-ca.crt


## @section Server settings
## @param server.replicaCount Number of server replicas
Expand Down Expand Up @@ -613,6 +613,9 @@ containerRegistry:
## @param containerRegistry.scheme Communication scheme of the container registry
##
scheme: http
## @param containerRegistry.userImageRepository Name of the repository where to push and pull user docker images
##
userImageRepository: substra/user-image
## @param containerRegistry.pullDomain Hostname from which the cluster should pull container images
##
pullDomain: 127.0.0.1
Expand Down
1 change: 1 addition & 0 deletions docs/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Accepted true values for `bool` are: `1`, `ON`, `On`, `on`, `T`, `t`, `TRUE`, `T
| bool | `TASK_CACHE_DOCKER_IMAGES` | `False` | |
| bool | `TASK_CHAINKEYS_ENABLED` | `False` | |
| bool | `TASK_LIST_WORKSPACE` | `True` | |
| string | `USER_IMAGE_REPOSITORY` | nil | |
| string | `WORKER_PVC_DOCKER_CACHE` | nil | |
| bool | `WORKER_PVC_IS_HOSTPATH` | nil | |
| string | `WORKER_PVC_SUBTUPLE` | nil | |
Expand Down
1 change: 1 addition & 0 deletions examples/values/backend-org-1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ containerRegistry:
- image: substra/substra-tools:latest
sourceRegistry: ghcr.io
dstImage: substra/substra-tools:latest
userImageRepository: substra-user-images

worker:
concurrency: 3
Expand Down
1 change: 1 addition & 0 deletions examples/values/backend-org-2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ containerRegistry:
- image: substra/substra-tools:latest
dstImage: substra/substra-tools:latest
sourceRegistry: ghcr.io
userImageRepository: substra/user-image

worker:
concurrency: 3
Expand Down
1 change: 1 addition & 0 deletions examples/values/backend-org-3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ containerRegistry:
- image: substra/substra-tools:latest
dstImage: substra/substra-tools:latest
sourceRegistry: ghcr.io
userImageRepository: substra/user-image

worker:
concurrency: 3
Expand Down

0 comments on commit 9210bc3

Please sign in to comment.