diff --git a/docs/src/dev-docs/index.md b/docs/src/dev-docs/index.md index f22ae29f37..6036563ba9 100644 --- a/docs/src/dev-docs/index.md +++ b/docs/src/dev-docs/index.md @@ -15,6 +15,13 @@ Building Docs about building CLP. ::: +:::{grid-item-card} +:link: testing-presto-connector +Presto connector +^^^ +Building and testing the CLP Presto connector locally. +::: + :::{grid-item-card} :link: testing/index Testing @@ -55,6 +62,7 @@ Any design docs describing parts of this project. :hidden: building-package +testing-presto-connector ::: :::{toctree} diff --git a/docs/src/dev-docs/testing-presto-connector.md b/docs/src/dev-docs/testing-presto-connector.md new file mode 100644 index 0000000000..4406e71fdb --- /dev/null +++ b/docs/src/dev-docs/testing-presto-connector.md @@ -0,0 +1,73 @@ +# Building and testing the Presto connector + +The Presto integration installs the [CLP connector][clp-connector] at startup from the +`ghcr.io/y-scope/clp-plugin-presto-connector` image, so the stock Presto images stay unmodified. +This page covers building that image locally and pointing the [Docker Compose](#docker-compose) and +[Helm](#helm-kind) stacks at it. + +The default connector image is pinned by digest, so a local build of the same tag won't be picked +up on its own — point `CLP_PRESTO_CONNECTOR_REF` at your image to use it. + +## Building the connector image + +In the [`clp-plugin-presto-connector`][clp-connector] repository, run: + +```shell +task package +``` + +This builds `ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT` and loads it into your +local Docker daemon. + +## Docker Compose + +The `presto-clp` stack is in `tools/deployment/presto-clp`. `scripts/set-up-config.sh` verifies the +connector image exists (locally or on the registry) and writes it into `.env`, erroring with the +ref it tried if neither exists. + +Each image is selected by a single environment variable holding a complete reference, which is used +as-is. Any of `repository:tag`, `repository@digest`, or `repository:tag@digest` works; unset +variables fall back to the pinned defaults. + +* `CLP_PRESTO_CONNECTOR_REF`: the CLP connector. +* `CLP_PRESTO_COORDINATOR_REF`: the Presto coordinator. +* `CLP_PRESTO_WORKER_REF`: the Presto worker. + +To run against the image you just built: + +```shell +CLP_PRESTO_CONNECTOR_REF=ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT \ + ./scripts/set-up-config.sh +docker compose up -d +``` + +Omitting the digest is what makes the local image win: Docker resolves a tag against the local +daemon first, but a digest only ever matches the exact published image. + +See the [Using Presto with CLP][using-presto] user guide for the full setup. + +## Helm (kind) + +`--clp-connector-image` loads a local image into the `kind` cluster and sets +`image.clpConnector.{repository,tag,pullPolicy=Never}` for you: + +```shell +tools/deployment/package-helm/set-up-test.sh --presto \ + --clp-connector-image ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT +``` + +## Pinning a new default + +When bumping a pinned default, take the digest of the *manifest list*, not of a per-architecture +manifest — the connector is published for both `linux/amd64` and `linux/arm64`, and a per-arch +digest would break the other architecture: + +```shell +docker buildx imagetools inspect --format '{{.Manifest.Digest}}' +``` + +`docker inspect` reports the digest of the single-platform image you happen to have pulled, so it's +the wrong source here. + +[clp-connector]: https://github.com/y-scope/clp-plugin-presto-connector +[using-presto]: ../user-docs/guides-using-presto.md diff --git a/docs/src/user-docs/guides-using-presto.md b/docs/src/user-docs/guides-using-presto.md index 0a66152356..fe897e0777 100644 --- a/docs/src/user-docs/guides-using-presto.md +++ b/docs/src/user-docs/guides-using-presto.md @@ -102,6 +102,13 @@ When using Kubernetes, Presto worker scheduling can be configured using the Kubernetes deployment guide for details. ::: +:::{note} +The CLP connector is installed at startup from the image under `image.clpConnector` (default +`ghcr.io/y-scope/clp-plugin-presto-connector`). To test a locally-built connector image in a `kind` +cluster, use the set-up script's `--clp-connector-image` flag; see +[Testing the Presto connector locally][testing-presto-connector]. +::: + ## Docker Compose ### Requirements @@ -199,6 +206,15 @@ Using Presto with CLP via Docker Compose requires: * Replace `` with the location of the clp-json package you set up in the previous section. + :::{note} + `set-up-config.sh` also verifies that the CLP connector image exists, locally or on the + registry, and writes it into `.env`. The default is pinned by digest, so the image can't change + underneath you. To use a different one, export `CLP_PRESTO_CONNECTOR_REF` before running the + script; it's used as-is, so it may be `repository:tag`, `repository@digest`, or + `repository:tag@digest`. See [Testing the Presto connector locally][testing-presto-connector] + for building the image yourself. + ::: + 4. Configure Presto to use CLP's metadata database as follows: * Open and edit `coordinator/config-template/split-filter.json`. @@ -332,3 +348,4 @@ These limitations will be addressed in a future release of the Presto integratio [Presto]: https://prestodb.io/ [y-scope/presto#8]: https://github.com/y-scope/presto/issues/8 [yscope-presto]: https://github.com/y-scope/presto +[testing-presto-connector]: ../dev-docs/testing-presto-connector.md diff --git a/tools/deployment/package-helm/.set-up-common.sh b/tools/deployment/package-helm/.set-up-common.sh index 79e785888e..fc80e5f067 100755 --- a/tools/deployment/package-helm/.set-up-common.sh +++ b/tools/deployment/package-helm/.set-up-common.sh @@ -58,7 +58,12 @@ get_image_helm_args() { fi echo "Loading local image '${image}' into kind cluster..." >&2 - kind load docker-image "${image}" --name "${cluster_name}" >&2 + # `errexit` is suspended inside the caller's `$(...) || exit 1`, so check explicitly; + # otherwise a failed load silently yields `pullPolicy=Never` for an unloaded image. + if ! kind load docker-image "${image}" --name "${cluster_name}" >&2; then + echo "Error: failed to load local image '${image}' into kind cluster '${cluster_name}'." >&2 + return 1 + fi # Split "repo:tag" on the last colon whose right-hand side contains no '/' # (so registry ports like localhost:5000/repo are not mistaken for tags). @@ -75,11 +80,12 @@ get_image_helm_args() { } # Parses common arguments shared across set-up scripts. -# Sets CLP_PACKAGE_IMAGE and ENABLE_PRESTO global variables. +# Sets CLP_PACKAGE_IMAGE, CLP_PRESTO_CONNECTOR_IMAGE, and ENABLE_PRESTO global variables. # # @param {string[]} args Script arguments parse_common_args() { CLP_PACKAGE_IMAGE="" + CLP_PRESTO_CONNECTOR_IMAGE="" ENABLE_PRESTO="false" while [[ $# -gt 0 ]]; do case "$1" in @@ -91,6 +97,14 @@ parse_common_args() { CLP_PACKAGE_IMAGE="$2" shift 2 ;; + --clp-connector-image) + if [[ $# -lt 2 || "$2" == --* ]]; then + echo "Error: '--clp-connector-image' requires a value." >&2 + exit 1 + fi + CLP_PRESTO_CONNECTOR_IMAGE="$2" + shift 2 + ;; --presto) ENABLE_PRESTO="true" shift diff --git a/tools/deployment/package-helm/Chart.yaml b/tools/deployment/package-helm/Chart.yaml index 5e6fef4af4..82ef2f2b56 100644 --- a/tools/deployment/package-helm/Chart.yaml +++ b/tools/deployment/package-helm/Chart.yaml @@ -1,6 +1,6 @@ apiVersion: "v2" name: "clp" -version: "0.4.1-dev.3" +version: "0.4.1-dev.4" description: "A Helm chart for CLP's (Compressed Log Processor) package deployment" type: "application" appVersion: "0.13.1-dev" diff --git a/tools/deployment/package-helm/set-up-multi-dedicated-test.sh b/tools/deployment/package-helm/set-up-multi-dedicated-test.sh index b838b05196..2e31ae4a1c 100755 --- a/tools/deployment/package-helm/set-up-multi-dedicated-test.sh +++ b/tools/deployment/package-helm/set-up-multi-dedicated-test.sh @@ -136,8 +136,14 @@ EOF echo "Installing Helm chart..." helm uninstall test --ignore-not-found sleep 2 + +# Resolve image overrides up front so an invalid or unloadable image exits loudly instead +# of silently falling back to the chart default. An empty override resolves to no flags. +clp_package_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") || exit 1 +clp_connector_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpConnector" "${CLP_PRESTO_CONNECTOR_IMAGE}") || exit 1 + # Word splitting is intentional: helper functions return multiple --set flags. -# shellcheck disable=SC2046 +# shellcheck disable=SC2086,SC2046 helm install test "${script_dir}" \ --set "distributedDeployment=true" \ --set "scheduling.compressionWorker.replicas=${COMPRESSION_WORKER_REPLICAS}" \ @@ -162,6 +168,7 @@ helm install test "${script_dir}" \ --set "scheduling.mcpServer.nodeSelector.yscope\.io/nodeType=core" \ $(get_service_exposure_helm_args) \ $(get_presto_helm_args) \ - $(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") + ${clp_package_args} \ + ${clp_connector_args} wait_for_cluster_ready diff --git a/tools/deployment/package-helm/set-up-multi-shared-test.sh b/tools/deployment/package-helm/set-up-multi-shared-test.sh index 45a498d9a9..69a7fda473 100755 --- a/tools/deployment/package-helm/set-up-multi-shared-test.sh +++ b/tools/deployment/package-helm/set-up-multi-shared-test.sh @@ -42,8 +42,14 @@ generate_kind_config "${NUM_WORKER_NODES}" | kind create cluster --name "${CLUST echo "Installing Helm chart..." helm uninstall test --ignore-not-found sleep 2 + +# Resolve image overrides up front so an invalid or unloadable image exits loudly instead +# of silently falling back to the chart default. An empty override resolves to no flags. +clp_package_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") || exit 1 +clp_connector_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpConnector" "${CLP_PRESTO_CONNECTOR_IMAGE}") || exit 1 + # Word splitting is intentional: helper functions return multiple --set flags. -# shellcheck disable=SC2046 +# shellcheck disable=SC2086,SC2046 helm install test "${script_dir}" \ --set "distributedDeployment=true" \ --set "scheduling.compressionWorker.replicas=${COMPRESSION_WORKER_REPLICAS}" \ @@ -52,6 +58,7 @@ helm install test "${script_dir}" \ --set "scheduling.prestoWorker.replicas=${PRESTO_WORKER_REPLICAS}" \ $(get_service_exposure_helm_args) \ $(get_presto_helm_args) \ - $(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") + ${clp_package_args} \ + ${clp_connector_args} wait_for_cluster_ready diff --git a/tools/deployment/package-helm/set-up-test.sh b/tools/deployment/package-helm/set-up-test.sh index de7238363a..f3e1a3be49 100755 --- a/tools/deployment/package-helm/set-up-test.sh +++ b/tools/deployment/package-helm/set-up-test.sh @@ -26,11 +26,18 @@ generate_kind_config 0 | kind create cluster --name "${CLUSTER_NAME}" --config=- echo "Installing Helm chart..." helm uninstall test --ignore-not-found sleep 2 + +# Resolve image overrides up front so an invalid or unloadable image exits loudly instead +# of silently falling back to the chart default. An empty override resolves to no flags. +clp_package_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") || exit 1 +clp_connector_args=$(get_image_helm_args "${CLUSTER_NAME}" "clpConnector" "${CLP_PRESTO_CONNECTOR_IMAGE}") || exit 1 + # Word splitting is intentional: helper functions return multiple --set flags. -# shellcheck disable=SC2046 +# shellcheck disable=SC2086,SC2046 helm install test "${script_dir}" \ $(get_service_exposure_helm_args) \ $(get_presto_helm_args) \ - $(get_image_helm_args "${CLUSTER_NAME}" "clpPackage" "${CLP_PACKAGE_IMAGE}") + ${clp_package_args} \ + ${clp_connector_args} wait_for_cluster_ready diff --git a/tools/deployment/package-helm/templates/_helpers.tpl b/tools/deployment/package-helm/templates/_helpers.tpl index 26d03d8523..308365fb69 100644 --- a/tools/deployment/package-helm/templates/_helpers.tpl +++ b/tools/deployment/package-helm/templates/_helpers.tpl @@ -156,28 +156,32 @@ app.kubernetes.io/instance: {{ .Release.Name }} {{/* Creates a container image reference from .Values.image. -Renders repository@digest when "digest" is set; otherwise, renders repository:tag. clpPackage -defaults to Chart.AppVersion when "tag" is omitted; other components require "tag". +Renders repository:tag, repository@digest, or repository:tag@digest, depending on which of "tag" +and "digest" are set. Setting both keeps the tag as a human-readable label while the digest is what +actually gets pulled. clpPackage defaults to Chart.AppVersion when "tag" is omitted; other +components require at least one of "tag" or "digest". @param {object} root Root template context (required) @param {string} component Key under .Values.image (e.g., "clpPackage", "redis") -@return {string} Full image reference (repository@digest or repository:tag) +@return {string} Full image reference */}} {{- define "clp.imageRef" -}} {{- $img := index .root.Values.image .component -}} -{{- if $img.digest -}} -{{- printf "%s@%s" $img.repository $img.digest -}} -{{- else -}} {{- $tag := $img.tag -}} -{{- if not $tag -}} - {{- if eq .component "clpPackage" -}} - {{- $tag = .root.Chart.AppVersion -}} - {{- else -}} - {{- fail (printf "image.%s.tag is required" .component) -}} - {{- end -}} +{{- if and (not $tag) (eq .component "clpPackage") -}} + {{- $tag = .root.Chart.AppVersion -}} +{{- end -}} +{{- if not (or $tag $img.digest) -}} + {{- fail (printf "image.%s requires \"tag\" or \"digest\"" .component) -}} {{- end -}} -{{- printf "%s:%s" $img.repository $tag -}} +{{- $ref := $img.repository -}} +{{- if $tag -}} + {{- $ref = printf "%s:%s" $ref $tag -}} +{{- end -}} +{{- if $img.digest -}} + {{- $ref = printf "%s@%s" $ref $img.digest -}} {{- end -}} +{{- $ref -}} {{- end }} {{/* diff --git a/tools/deployment/package-helm/templates/configmap.yaml b/tools/deployment/package-helm/templates/configmap.yaml index ee49584a06..6880755589 100644 --- a/tools/deployment/package-helm/templates/configmap.yaml +++ b/tools/deployment/package-helm/templates/configmap.yaml @@ -391,6 +391,7 @@ data: inline-sql-functions=false nested-data-serialization-enabled=false native-execution-enabled=true + use-connector-provided-serialization-codecs=true presto-coordinator-config-jvm.config: | -server @@ -438,6 +439,7 @@ data: system-memory-gb={{ .worker.system_memory_gb }} register-test-functions=false runtime-metrics-collection-enabled=false + plugin.dir=/opt/presto-server/plugin/clp presto-worker-config-node.properties: | node.environment=production diff --git a/tools/deployment/package-helm/templates/presto-coordinator-deployment.yaml b/tools/deployment/package-helm/templates/presto-coordinator-deployment.yaml index 8b5e56e5c0..adaf67f7b9 100644 --- a/tools/deployment/package-helm/templates/presto-coordinator-deployment.yaml +++ b/tools/deployment/package-helm/templates/presto-coordinator-deployment.yaml @@ -30,9 +30,22 @@ spec: "type" "job" "name" "db-table-creator" ) | nindent 10 }} + - name: "install-clp-plugin" + image: {{ include "clp.imageRef" (dict "root" . "component" "clpConnector") | quote }} + imagePullPolicy: {{ .Values.image.clpConnector.pullPolicy | quote }} + env: + - name: "COORDINATOR_PLUGIN_INSTALL_PATH" + value: "/install/coordinator" + {{- include "clp.createResourceLimits" (dict + "root" . + "component" "prestoCoordinatorInstallPlugin" + ) | nindent 10 }} + volumeMounts: + - name: "presto-plugin" + mountPath: "/install/coordinator" containers: - name: "presto-coordinator" - image: "{{ .Values.image.prestoCoordinator.repository }}:{{ .Values.image.prestoCoordinator.tag }}" + image: {{ include "clp.imageRef" (dict "root" . "component" "prestoCoordinator") | quote }} imagePullPolicy: "{{ .Values.image.prestoCoordinator.pullPolicy }}" {{- include "clp.createResourceLimits" (dict "root" . @@ -65,6 +78,9 @@ spec: mountPath: "/opt/presto-server/etc/split-filter.json" subPath: "presto-coordinator-config-split-filter.json" readOnly: true + - name: "presto-plugin" + mountPath: "/opt/presto-server/plugin/clp" + readOnly: true readinessProbe: {{- include "clp.readinessProbeTimings" . | nindent 12 }} httpGet: &presto-coordinator-health-check @@ -83,4 +99,6 @@ spec: - name: "presto-config" configMap: name: {{ include "clp.fullname" . }}-config + - name: "presto-plugin" + emptyDir: {} {{- end }} diff --git a/tools/deployment/package-helm/templates/presto-worker-deployment.yaml b/tools/deployment/package-helm/templates/presto-worker-deployment.yaml index 2c9337c04c..6688669c22 100644 --- a/tools/deployment/package-helm/templates/presto-worker-deployment.yaml +++ b/tools/deployment/package-helm/templates/presto-worker-deployment.yaml @@ -30,6 +30,19 @@ spec: "type" "service" "name" "presto-coordinator" ) | nindent 10 }} + - name: "install-clp-plugin" + image: {{ include "clp.imageRef" (dict "root" . "component" "clpConnector") | quote }} + imagePullPolicy: {{ .Values.image.clpConnector.pullPolicy | quote }} + env: + - name: "WORKER_PLUGIN_INSTALL_PATH" + value: "/install/worker" + {{- include "clp.createResourceLimits" (dict + "root" . + "component" "prestoWorkerInstallPlugin" + ) | nindent 10 }} + volumeMounts: + - name: "presto-plugin" + mountPath: "/install/worker" - name: "setup-configs" image: {{ include "clp.imageRef" (dict "root" . "component" "kubectl") | quote }} imagePullPolicy: {{ .Values.image.kubectl.pullPolicy | quote }} @@ -49,7 +62,7 @@ spec: readOnly: true containers: - name: "presto-worker" - image: "{{ .Values.image.prestoWorker.repository }}:{{ .Values.image.prestoWorker.tag }}" + image: {{ include "clp.imageRef" (dict "root" . "component" "prestoWorker") | quote }} imagePullPolicy: "{{ .Values.image.prestoWorker.pullPolicy }}" {{- include "clp.createResourceLimits" (dict "root" . @@ -68,6 +81,9 @@ spec: readOnly: true - name: "presto-etc" mountPath: "/opt/presto-server/etc" + - name: "presto-plugin" + mountPath: "/opt/presto-server/plugin/clp" + readOnly: true {{- if eq .Values.clpConfig.archive_output.storage.type "fs" }} - name: {{ include "clp.volumeName" (dict "component_category" "shared-data" @@ -95,6 +111,8 @@ spec: name: {{ include "clp.fullname" . }}-config - name: "presto-etc" emptyDir: {} + - name: "presto-plugin" + emptyDir: {} - name: "presto-scripts" configMap: name: {{ include "clp.fullname" . }}-config diff --git a/tools/deployment/package-helm/values.yaml b/tools/deployment/package-helm/values.yaml index 27412fdbb0..ca7dff0c4e 100644 --- a/tools/deployment/package-helm/values.yaml +++ b/tools/deployment/package-helm/values.yaml @@ -5,6 +5,13 @@ fullnameOverride: "" allowHostAccessForSbinScripts: true image: + # Installs the CLP Presto connector plugin into the Presto coordinator and worker pods. Override + # the repository to use a different edition of the connector. + clpConnector: + repository: "ghcr.io/y-scope/clp-plugin-presto-connector" + pullPolicy: "IfNotPresent" + tag: "0.1.0-SNAPSHOT" + digest: "sha256:d006b0ce7830b6932eea66f1edc8dedbc54dbd661943eb38147e62c847fe0c32" clpPackage: repository: "ghcr.io/y-scope/clp/clp-package" pullPolicy: "Always" @@ -31,13 +38,15 @@ image: pullPolicy: "IfNotPresent" tag: "0.152.0" prestoCoordinator: - repository: "ghcr.io/y-scope/presto/coordinator" + repository: "ghcr.io/y-scope/presto" pullPolicy: "IfNotPresent" - tag: "clp-v0.10.0" + tag: "0.299" + digest: "sha256:87e9ce10a1184f1b5ce0dd74591ef753c3afe763d860ef412a331684ec653753" prestoWorker: - repository: "ghcr.io/y-scope/presto/prestissimo-worker" + repository: "ghcr.io/y-scope/presto-native" pullPolicy: "IfNotPresent" - tag: "clp-v0.10.0-fix.1" + tag: "0.299" + digest: "sha256:8f9ca4ba7dd844385b85e7e3bd9568128096e2a7ad09432c6a982ea11279a13b" queue: repository: "rabbitmq" pullPolicy: "Always" @@ -142,6 +151,8 @@ resources: # Jobs dbTableCreator: {} + prestoCoordinatorInstallPlugin: {} + prestoWorkerInstallPlugin: {} prestoWorkerSetupConfigs: {} resultsCacheIndicesCreator: {} topologyMetricsEmitter: {} diff --git a/tools/deployment/presto-clp/coordinator/config-template/config.properties b/tools/deployment/presto-clp/coordinator/config-template/config.properties index b9da2234f4..d0bd7bb8ad 100644 --- a/tools/deployment/presto-clp/coordinator/config-template/config.properties +++ b/tools/deployment/presto-clp/coordinator/config-template/config.properties @@ -11,3 +11,4 @@ use-alternative-function-signatures=true inline-sql-functions=false nested-data-serialization-enabled=false native-execution-enabled=true +use-connector-provided-serialization-codecs=true diff --git a/tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh b/tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh index d2d05cf041..802e5bf61f 100755 --- a/tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh @@ -15,6 +15,6 @@ find /configs -type f | while read -r f; do ) | sh >"${PRESTO_CONFIG_DIR}/$(basename "$f")" done -# Remove existing catalog files that exist in the image and add the CLP catalog -rm -f "${PRESTO_CONFIG_DIR}/catalog/"* +# Create the catalog directory and add the CLP catalog +mkdir -p "${PRESTO_CONFIG_DIR}/catalog" mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" diff --git a/tools/deployment/presto-clp/docker-compose.yaml b/tools/deployment/presto-clp/docker-compose.yaml index fdea3c7294..cb156e8651 100644 --- a/tools/deployment/presto-clp/docker-compose.yaml +++ b/tools/deployment/presto-clp/docker-compose.yaml @@ -1,7 +1,35 @@ services: + clp-plugin-presto-connector-init: + # One-shot: installs the CLP Presto connector plugin into shared volumes that the + # coordinator and worker mount into their plugin dirs (replacing the connector-baked + # Presto images). The connector image's own entrypoint does the install; we only supply + # the target paths via *_PLUGIN_INSTALL_PATH. + # init.py verifies the image exists (locally or on the registry) and writes the resolved + # reference into .env as CLP_PRESTO_CONNECTOR_REF, which may be repository:tag, + # repository@digest, or repository:tag@digest. Locally-built and published images share the + # same : tag; whatever is in the local daemon wins. The default below is only used + # when .env is absent. + image: "${CLP_PRESTO_CONNECTOR_REF:-\ + ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT@sha256:d006b0ce7830b6932eea66f1edc8dedbc54dbd661943eb38147e62c847fe0c32}" + restart: "no" + environment: + COORDINATOR_PLUGIN_INSTALL_PATH: "/install/coordinator" + WORKER_PLUGIN_INSTALL_PATH: "/install/worker" + volumes: + - "coordinator-plugin:/install/coordinator" + - "worker-plugin:/install/worker" + networks: + - "clp-package" + presto-coordinator: - image: "ghcr.io/y-scope/presto/coordinator:${CLP_PRESTO_COORDINATOR_IMAGE_TAG:-clp-v0.10.0}" + # Pinned by digest so the image can't change under us; the tag is kept as a + # human-readable label. Override with a full reference. + image: "${CLP_PRESTO_COORDINATOR_REF:-\ + ghcr.io/y-scope/presto:0.299@sha256:87e9ce10a1184f1b5ce0dd74591ef753c3afe763d860ef412a331684ec653753}" entrypoint: ["/bin/bash", "-c", "/scripts/generate-configs.sh && /opt/entrypoint.sh"] + depends_on: + clp-plugin-presto-connector-init: + condition: "service_completed_successfully" env_file: - ".env" - "coordinator-common.env" @@ -9,7 +37,9 @@ services: volumes: - "./coordinator/config-template:/configs:ro" - "./coordinator/scripts:/scripts:ro" - - "coordinator-config:/opt/presto-server/etc" + - type: "tmpfs" + target: "/opt/presto-server/etc" + - "coordinator-plugin:/opt/presto-server/plugin/clp" networks: - "clp-package" ports: @@ -24,11 +54,14 @@ services: retries: 30 presto-worker: - image: >- - ghcr.io/y-scope/presto/prestissimo-worker:${CLP_PRESTO_WORKER_IMAGE_TAG:-clp-v0.10.0-fix.1} + # Pinned by digest; see the coordinator's image above. + image: "${CLP_PRESTO_WORKER_REF:-\ + ghcr.io/y-scope/presto-native:0.299@sha256:8f9ca4ba7dd844385b85e7e3bd9568128096e2a7ad09432c6a982ea11279a13b}" depends_on: presto-coordinator: condition: "service_healthy" + clp-plugin-presto-connector-init: + condition: "service_completed_successfully" entrypoint: ["/bin/bash", "-c", "/scripts/generate-configs.sh && /opt/entrypoint.sh"] env_file: - ".env" @@ -39,7 +72,9 @@ services: - "${CLP_STAGED_ARCHIVES_DIR:-empty}:/var/data/staged-archives:ro" - "./worker/config-template:/configs:ro" - "./worker/scripts:/scripts:ro" - - "worker-config:/opt/presto-server/etc" + - type: "tmpfs" + target: "/opt/presto-server/etc" + - "worker-plugin:/opt/presto-server/plugin/clp" networks: - "clp-package" @@ -47,8 +82,10 @@ volumes: # Dummy volume to use when a bind mount is not desired. empty: - coordinator-config: - worker-config: + # Populated by clp-plugin-presto-connector-init; mounted into the coordinator/worker + # plugin directories. + coordinator-plugin: + worker-plugin: networks: clp-package: diff --git a/tools/deployment/presto-clp/scripts/init.py b/tools/deployment/presto-clp/scripts/init.py index 4cd357931d..04c51910d6 100755 --- a/tools/deployment/presto-clp/scripts/init.py +++ b/tools/deployment/presto-clp/scripts/init.py @@ -3,6 +3,9 @@ import argparse import logging +import os +import shutil +import subprocess import sys from pathlib import Path from typing import Any @@ -24,6 +27,20 @@ # S3 URL constant AWS_S3_DOMAIN = "amazonaws.com" +# Default CLP Presto connector image. Pinned by digest so the image can't change under us; +# the tag is kept as a human-readable label and must be updated alongside the digest. +DEFAULT_CONNECTOR_REF = ( + "ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT" + "@sha256:d006b0ce7830b6932eea66f1edc8dedbc54dbd661943eb38147e62c847fe0c32" +) + +# Silence Ruff S607: the absolute path of the Docker binary may vary depending on the installation +# method. +_DOCKER_EXECUTABLE = "docker" + +# Bounds each Docker probe so a hung daemon or an unresponsive registry can't stall setup. +_DOCKER_PROBE_TIMEOUT_SECONDS = 30 + # Set up console logging logging_console_handler = logging.StreamHandler() logging_formatter = logging.Formatter( @@ -40,7 +57,7 @@ logger = logging.getLogger(__name__) -def main(argv: list[str] | None = None) -> int: +def main(argv: list[str] | None = None) -> int: # noqa: PLR0911 """Initializes Presto worker configuration based on CLP package settings.""" if argv is None: argv = sys.argv @@ -89,6 +106,9 @@ def main(argv: list[str] | None = None) -> int: ): return 1 + if not _add_connector_image_env_vars(env_vars): + return 1 + with output_file.open("w") as output_file_handle: output_file_handle.writelines(f"{key}={value}\n" for key, value in env_vars.items()) @@ -363,6 +383,80 @@ def _add_worker_env_vars(coordinator_common_env_file_path: Path, env_vars: dict[ return True +def _add_connector_image_env_vars(env_vars: dict[str, str]) -> bool: + """ + Resolves the CLP Presto connector image and adds `CLP_PRESTO_CONNECTOR_REF` to `env_vars`, + which `docker-compose.yaml` consumes. + + `CLP_PRESTO_CONNECTOR_REF` overrides the default and is used as-is, so it may be + `repository:tag`, `repository@digest`, or `repository:tag@digest`. + + :param env_vars: Dictionary to populate with the connector image environment variable. + :return: Whether the reference was successfully resolved. + """ + ref = os.environ.get("CLP_PRESTO_CONNECTOR_REF", DEFAULT_CONNECTOR_REF) + if not _connector_image_available(ref): + return False + + env_vars["CLP_PRESTO_CONNECTOR_REF"] = ref + return True + + +def _connector_image_available(ref: str) -> bool: + """ + Returns whether `ref` exists in the local Docker daemon or on its registry. A local image takes + precedence at `docker compose up`. + + :param ref: The full image reference. + :return: Whether the image is available. + """ + if shutil.which(_DOCKER_EXECUTABLE) is None: + logger.error( + "Docker isn't installed or isn't on PATH, so the CLP Presto connector image can't be" + " checked. Install Docker to continue." + ) + return False + + # Check the local daemon before the registry so a locally-built image is accepted without a + # network round-trip. + if _run_docker_probe(["image", "inspect", ref]) or _run_docker_probe( + ["manifest", "inspect", ref] + ): + logger.info("Found CLP connector image '%s'.", ref) + return True + + logger.error( + "Couldn't find CLP Presto connector image '%s' locally or on the registry. Build it" + " (e.g. via `task package` in clp-plugin-presto-connector) or set" + " CLP_PRESTO_CONNECTOR_REF to an image that exists.", + ref, + ) + return False + + +def _run_docker_probe(args: list[str]) -> bool: + """ + Runs `docker `, suppressing its output. + + :param args: Arguments to pass to Docker (e.g. `["image", "inspect", ref]`). + :return: Whether the command exited 0. False if Docker couldn't be run or timed out. + """ + try: + completed_process = subprocess.run( + [_DOCKER_EXECUTABLE, *args], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + timeout=_DOCKER_PROBE_TIMEOUT_SECONDS, + ) + except (OSError, subprocess.TimeoutExpired) as e: + logger.exception( + "Docker command '%s %s' failed.", _DOCKER_EXECUTABLE, " ".join(args), exc_info=e + ) + return False + return completed_process.returncode == 0 + + def _generate_worker_clp_properties( worker_config_template_path: Path, env_vars: dict[str, str] ) -> bool: diff --git a/tools/deployment/presto-clp/worker/config-template/config.properties b/tools/deployment/presto-clp/worker/config-template/config.properties index 2b0d386b1b..2fbf419c84 100644 --- a/tools/deployment/presto-clp/worker/config-template/config.properties +++ b/tools/deployment/presto-clp/worker/config-template/config.properties @@ -5,3 +5,4 @@ shutdown-onset-sec=1 system-memory-gb=${PRESTO_WORKER_CONFIGPROPERTIES_SYSTEM_MEMORY_GB} register-test-functions=false runtime-metrics-collection-enabled=false +plugin.dir=/opt/presto-server/plugin/clp diff --git a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh index a2205f5502..a9ab197a28 100755 --- a/tools/deployment/presto-clp/worker/scripts/generate-configs.sh +++ b/tools/deployment/presto-clp/worker/scripts/generate-configs.sh @@ -24,11 +24,13 @@ get_coordinator_version() { local discovery_uri discovery_uri=$(awk -F "=" '/^discovery.uri=/ {print $2}' "$config_properties_file") - if response=$( - wget --quiet --output-document - --timeout 10 "${discovery_uri}/v1/info" 2>/dev/null - ); then - version=$(echo "$response" | jq --raw-output '.nodeVersion.version') - if [[ "$version" = "null" ]]; then + if response=$(curl --fail --silent --max-time 10 "${discovery_uri}/v1/info"); then + if ! version=$( + echo "$response" \ + | python3 -c \ + "import json, sys; print(json.load(sys.stdin)['nodeVersion']['version'])" \ + 2>/dev/null + ); then log "ERROR" "Presto response is empty or doesn't contain version info." exit 1 fi @@ -58,8 +60,6 @@ update_config_file() { log "INFO" "Set ${key}=${value} in ${file_path}" } -apt-get update && apt-get install --assume-yes --no-install-recommends jq wget - readonly PRESTO_CONFIG_DIR="/opt/presto-server/etc" # Substitute environment variables in config template @@ -71,8 +71,8 @@ find /configs -type f | while read -r f; do ) | sh >"${PRESTO_CONFIG_DIR}/$(basename "$f")" done -# Remove existing catalog files that exist in the image and add the CLP catalog -rm -f "${PRESTO_CONFIG_DIR}/catalog/"* +# Create the catalog directory and add the CLP catalog +mkdir -p "${PRESTO_CONFIG_DIR}/catalog" mv "${PRESTO_CONFIG_DIR}/clp.properties" "${PRESTO_CONFIG_DIR}/catalog" # Update config.properties @@ -82,6 +82,12 @@ log "INFO" "Detected Presto version: $version" update_config_file "$CONFIG_PROPERTIES_FILE" "presto.version" "$version" # Update node.properties +# +# NOTE: These are resolved through Python rather than `hostname`, which the Presto worker image +# doesn't ship. Assigning them first ensures a resolution failure aborts the script instead of +# silently writing empty values. readonly NODE_PROPERTIES_FILE="/opt/presto-server/etc/node.properties" -update_config_file "$NODE_PROPERTIES_FILE" "node.internal-address" "$(hostname -i)" -update_config_file "$NODE_PROPERTIES_FILE" "node.id" "$(hostname)" +node_internal_address=$(python3 -c "import socket; print(socket.gethostbyname(socket.gethostname()))") +node_id=$(python3 -c "import socket; print(socket.gethostname())") +update_config_file "$NODE_PROPERTIES_FILE" "node.internal-address" "$node_internal_address" +update_config_file "$NODE_PROPERTIES_FILE" "node.id" "$node_id"