feat(deployment): Deploy Presto with stock images and install the CLP connector as a plugin. - #2439
Draft
jackluo923 wants to merge 18 commits into
Draft
feat(deployment): Deploy Presto with stock images and install the CLP connector as a plugin.#2439jackluo923 wants to merge 18 commits into
jackluo923 wants to merge 18 commits into
Conversation
… connector as a plugin. Replace the connector-baked Presto images with the upstream `ghcr.io/y-scope/presto` and `ghcr.io/y-scope/presto-native` images, and install the CLP connector into them at startup from a dedicated connector image, so that only the connector artifact needs to be published. Docker Compose gains a one-shot `clp-plugin-presto-connector-init` service that populates shared plugin volumes; the Helm chart gains an equivalent `install-clp-plugin` init container backed by an `emptyDir`, selected via `image.clpConnector`. Both set the worker's `plugin.dir` and the coordinator's `use-connector-provided-serialization-codecs`, the latter being required for the worker to deserialize the connector's handles through its own codecs rather than through JSON, which the connector doesn't implement. The stock `presto-native` image is CentOS-based rather than Ubuntu, so the worker's config generation can no longer rely on `apt-get`, `jq`, `wget`, or `hostname`; use `curl` and `python3`, which the image already ships. Resolving the node identity via assignments also ensures a failure aborts the script instead of silently writing empty values. Mount the Compose config directory as tmpfs instead of a named volume so that it's rebuilt from the image and templates on every start, rather than retaining files seeded from a previously deployed image. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds Presto connector image resolution and validation, shared plugin installation for Docker Compose and Helm deployments, updated Presto configuration, and documentation for local connector image builds and testing. ChangesPresto connector deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant InitScript
participant Docker
participant ConnectorPlugin
participant PrestoCoordinator
participant PrestoWorker
InitScript->>Docker: Resolve and validate connector image
ConnectorPlugin->>Docker: Install plugin into shared volumes
PrestoCoordinator->>ConnectorPlugin: Wait for successful initialization
PrestoWorker->>ConnectorPlugin: Wait for successful initialization
PrestoCoordinator->>ConnectorPlugin: Mount coordinator plugin volume
PrestoWorker->>ConnectorPlugin: Mount worker plugin volume
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…in deployment changes.
… and document local testing. `init.py` now resolves `CLP_CONNECTOR_IMAGE`/`CLP_CONNECTOR_TAG` into `.env` instead of relying on the arm64-specific default in `docker-compose.yaml`. It tries, in order, a locally-built per-architecture tag (`<version>-<arch>`, via `docker image inspect`), a published multi-architecture tag (`<version>`, via `docker manifest inspect`), and a published per-architecture tag, erroring with an actionable message if none is found. This lets a connector built locally via `task package` (which only loads the building machine's `:<version>-<arch>` tag, since a multi-arch manifest can't be loaded into a local daemon) be picked up automatically, while published deployments use the multi-arch manifest. `CLP_CONNECTOR_PREFER_LOCAL=false` flips the order to prefer the published multi-architecture image, so an upstream release can be tested even when a local build of the same version is loaded; `CLP_CONNECTOR_TAG`/`CLP_CONNECTOR_IMAGE`/`CLP_CONNECTOR_VERSION` override the resolution directly. The inline default tag in `docker-compose.yaml` becomes the multi-arch `0.1.0-SNAPSHOT` (no arch suffix) since it's only used when `.env` is absent; `init.py`'s resolved tag is authoritative. Add a short README under `tools/deployment/presto-clp` covering setup, the resolution order and its overrides, building the connector locally, and Helm (kind) local testing via `set-up-test.sh --presto --clp-connector-image=...` (the local-image path the Helm set-up scripts already wire up).
Move the connector-image-resolution and local-testing documentation out of a code-adjacent README under tools/deployment/presto-clp and into the docs tree: the user-facing resolution/override notes and the Helm --clp-connector-image flag go into guides-using-presto.md, and the developer content (building the connector image locally, the CLP_CONNECTOR_PREFER_LOCAL toggle, and the Helm kind recipe) goes into a new dev-docs page.
…ector images Drop the :<version>-<arch> suffix scheme for locally-built connector images in favor of the conventional Docker pattern: local builds and the published multi-arch image share the same :<version> tag, and whatever is in the local daemon wins (docker pull to go back to the published image). Matches the corresponding change in clp-plugin-presto-connector. init.py's three-candidate tag resolution (local per-arch, published multi-arch, published per-arch) and CLP_CONNECTOR_PREFER_LOCAL collapse into a single existence check: error if the tag is in neither the local daemon nor the registry. The Helm default tag also loses its arm64 suffix, which was wrong on amd64 hosts. Docs updated; the --clp-connector-image examples also switch to the space-separated form the parser actually accepts.
CLP_CONNECTOR_IMAGE/TAG/VERSION were ambiguous (connector for what?); rename to CLP_PRESTO_CONNECTOR_* to match the image repo (clp-plugin-presto-connector) and the existing PRESTO_COORDINATOR_*/PRESTO_WORKER_* env var family. The vars are new in this PR so there's no backward-compat to preserve. Also renames the same-named internal shell variable in the Helm set-up scripts for consistency; the --clp-connector-image CLI flag is unchanged.
jackluo923
marked this pull request as ready for review
August 1, 2026 02:13
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/dev-docs/testing-presto-connector.md`:
- Around line 46-62: Update the Helm command examples in the
testing-presto-connector documentation to invoke set-up-test.sh from the
repository root using the tools/deployment/package-helm path, keeping the
existing arguments and image examples unchanged.
In `@docs/src/user-docs/guides-using-presto.md`:
- Around line 209-216: The note around set-up-config.sh should explicitly state
that setting CLP_PRESTO_CONNECTOR_TAG skips connector image validation. Clarify
that without an explicit tag, setup validates the image and rejects invalid tags
before writing .env, while an explicitly tagged image may fail later during
Compose startup.
In `@tools/deployment/package-helm/set-up-multi-dedicated-test.sh`:
- Around line 165-166: Update
tools/deployment/package-helm/set-up-multi-dedicated-test.sh at lines 165-166 to
resolve and validate get_image_helm_args output for the connector before
invoking helm install, exiting on failure instead of silently dropping the
override; apply the same checked-resolution pattern to
tools/deployment/package-helm/set-up-test.sh at lines 34-35.
In `@tools/deployment/presto-clp/scripts/init.py`:
- Around line 420-495: Introduce a shared Docker subprocess runner near
_image_exists_locally and _tag_exists_on_registry that applies a finite timeout,
catches OSError and subprocess.TimeoutExpired, logs the execution failure, and
returns False. Update both _image_exists_locally and _tag_exists_on_registry to
use this runner while preserving their existing command arguments and boolean
success behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7d2e39b-09f7-4c81-88ad-07d96e2801f9
📒 Files selected for processing (18)
docs/src/dev-docs/index.mddocs/src/dev-docs/testing-presto-connector.mddocs/src/user-docs/guides-using-presto.mdtools/deployment/package-helm/.set-up-common.shtools/deployment/package-helm/Chart.yamltools/deployment/package-helm/set-up-multi-dedicated-test.shtools/deployment/package-helm/set-up-multi-shared-test.shtools/deployment/package-helm/set-up-test.shtools/deployment/package-helm/templates/configmap.yamltools/deployment/package-helm/templates/presto-coordinator-deployment.yamltools/deployment/package-helm/templates/presto-worker-deployment.yamltools/deployment/package-helm/values.yamltools/deployment/presto-clp/coordinator/config-template/config.propertiestools/deployment/presto-clp/coordinator/scripts/generate-configs.shtools/deployment/presto-clp/docker-compose.yamltools/deployment/presto-clp/scripts/init.pytools/deployment/presto-clp/worker/config-template/config.propertiestools/deployment/presto-clp/worker/scripts/generate-configs.sh
…ample The Helm testing example used ./package-helm/set-up-test.sh, which only works from tools/deployment/. The script resolves its own directory via BASH_SOURCE, so use the repo-root-relative path tools/deployment/package-helm/set-up-test.sh instead — copy-pasteable from the repository root. Arguments and image examples unchanged.
The connector-image note in the user guide said set-up-config.sh writes the image into .env "after verifying it exists," which only holds for CLP_PRESTO_CONNECTOR_IMAGE/VERSION. An explicit CLP_PRESTO_CONNECTOR_TAG is written as-is and skips the existence check (init.py returns early without calling _connector_image_available), so an invalid tag surfaces later during docker compose up. Distinguish the two cases so users know when validation happens and when it does not.
get_image_helm_args was invoked inline as $(...) in the helm install command, so its non-zero exit (invalid image ref, or image absent from the local Docker daemon) was silently swallowed: helm install fell back to the chart-default image with no error. Resolve the overrides into Helm --set flags up front and exit 1 on failure in set-up-test.sh and set-up-multi-dedicated-test.sh. Also make get_image_helm_args check `kind load` explicitly. A bare `kind load ...` relied on errexit, but the caller's `$(...) ... || exit 1` pattern suspends errexit inside the command substitution, so a kind-load failure would be ignored and we would proceed with pullPolicy=Never for an image never loaded. Return 1 explicitly so every failure mode propagates.
Add _run_docker_probe: a shared runner for the Docker CLI probes used to check the connector image. It applies a finite timeout and catches OSError and subprocess.TimeoutExpired, logging the failure and returning False so a hung Docker daemon or an unresponsive registry does not stall set-up-config.sh indefinitely. Inline the two one-line wrappers (_image_exists_locally and _tag_exists_on_registry) into _connector_image_available — each was called once and the call site already distinguishes local daemon vs registry in its log messages. Command arguments and boolean success behavior are unchanged.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/src/dev-docs/testing-presto-connector.md (2)
27-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRun the Compose commands from the stack directory.
If a reader follows this page from the repository root,
./scripts/set-up-config.shanddocker compose up -ddo not resolve thepresto-clpfiles. Addcd tools/deployment/presto-clpbefore the commands, or use explicit repository-root paths.This repeats the command-path issue raised for the Helm examples in the previous review; the Helm block is corrected, but the Compose block still has the same failure mode.
Proposed fix
The `presto-clp` Compose stack is in `tools/deployment/presto-clp`. +cd tools/deployment/presto-clp + ```shell ./scripts/set-up-config.sh <clp-package-dir> docker compose up -d</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/src/dev-docs/testing-presto-connector.mdaround lines 27 - 41, Update
the Compose instructions around theset-up-config.shanddocker compose up -dcommands to first change intotools/deployment/presto-clp, so both
commands run from thepresto-clpstack directory when starting the Compose
stack.</details> <!-- cr-comment:v1:7e31864d9af2b2d4a05ad5bd --> --- `27-28`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **Add version-aware links for deployment directories.** Link `tools/deployment/presto-clp` and `tools/deployment/package-helm` to their GitHub directories with `DOCS_VAR_CLP_GIT_REF`. This lets readers open the directory for the documentation version that they are viewing. Based on learnings, directory references in `docs/` should use version-aware GitHub URLs with `DOCS_VAR_CLP_GIT_REF`. <details> <summary>Proposed fix</summary> ```diff -The `presto-clp` Compose stack is in `tools/deployment/presto-clp`. +The `presto-clp` Compose stack is in +[`tools/deployment/presto-clp`](https://github.com/y-scope/clp/tree/DOCS_VAR_CLP_GIT_REF/tools/deployment/presto-clp). -The Helm chart's local-image path is wired into the set-up scripts in -`tools/deployment/package-helm`, which load a local image into the `kind` cluster and set +The Helm chart's local-image path is wired into the set-up scripts in +[`tools/deployment/package-helm`](https://github.com/y-scope/clp/tree/DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm), which load a local image into the `kind` cluster and setAlso applies to: 48-50
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/dev-docs/testing-presto-connector.md` around lines 27 - 28, Update the deployment directory references in the testing Presto connector documentation, including the occurrences around the Compose stack and configuration script, to use GitHub directory links parameterized by DOCS_VAR_CLP_GIT_REF. Link tools/deployment/presto-clp and tools/deployment/package-helm while preserving the surrounding documentation text.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/src/dev-docs/testing-presto-connector.md`:
- Around line 27-41: Update the Compose instructions around the
`set-up-config.sh` and `docker compose up -d` commands to first change into
`tools/deployment/presto-clp`, so both commands run from the `presto-clp` stack
directory when starting the Compose stack.
- Around line 27-28: Update the deployment directory references in the testing
Presto connector documentation, including the occurrences around the Compose
stack and configuration script, to use GitHub directory links parameterized by
DOCS_VAR_CLP_GIT_REF. Link tools/deployment/presto-clp and
tools/deployment/package-helm while preserving the surrounding documentation
text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 85b83494-edfe-49f5-bea8-59e32a0ec3c8
📒 Files selected for processing (6)
docs/src/dev-docs/testing-presto-connector.mddocs/src/user-docs/guides-using-presto.mdtools/deployment/package-helm/.set-up-common.shtools/deployment/package-helm/set-up-multi-dedicated-test.shtools/deployment/package-helm/set-up-test.shtools/deployment/presto-clp/scripts/init.py
ruff TRY400 (stable under the pinned ruff<0.16) flags `logger.error` inside an except clause. Switch the _run_docker_probe failure log to `logger.exception(..., exc_info=e)`, matching the existing idiom in _add_worker_env_vars. Behavior is unchanged: the probe still returns False on OSError and subprocess.TimeoutExpired. The CPY001 copyright-header notices surfaced by newer local ruff builds are not failing CI — the repo caps ruff<0.16 (commit 40bee80, issue y-scope#2422) to defer them, so they belong in a separate PR.
`ghcr.io/y-scope/clp-plugin-presto-connector:0.1.0-SNAPSHOT` is now published as a multi-arch (linux/amd64 + linux/arm64) tag, so drop the three pre-merge TODOs asking for the defaults to point at a released tag. The default values in `init.py`, `docker-compose.yaml`, and `values.yaml` already named that tag and are unchanged. Also convert `set-up-multi-shared-test.sh` to resolve the image overrides into Helm flags before `helm install`, matching `set-up-test.sh` and `set-up-multi-dedicated-test.sh`. fed5c4c made an invalid or unloadable image override fail loudly in those two scripts but missed this one, where the override was still expanded inline inside the `helm install` command and a failure was swallowed, silently falling back to the chart-default image. Co-Authored-By: Claude <noreply@anthropic.com>
Both the dev page and the Compose set-up note in the user guide restated the same image-resolution rules at length. Trim them to what a reader needs and let the user guide defer to the dev page for the details. `testing-presto-connector.md`: fold the two Helm examples into one concrete command, and shorten the local-vs-published explanation. `guides-using-presto.md`: shorten the `set-up-config.sh` note, and match the surrounding notes' one-idea-per-sentence phrasing rather than chaining clauses. Both files are added by this PR, so neither change reflows text that exists on main. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/dev-docs/testing-presto-connector.md (1)
35-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a repository-root path for the Compose setup command.
./scripts/set-up-config.shworks only when the current directory istools/deployment/presto-clp. The page does not instruct the reader to change to that directory. From the repository root, the example fails. Usetools/deployment/presto-clp/scripts/set-up-config.shor add an explicit directory-change step.Proposed documentation fix
-./scripts/set-up-config.sh <clp-package-dir> +tools/deployment/presto-clp/scripts/set-up-config.sh <clp-package-dir>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/dev-docs/testing-presto-connector.md` at line 35, Update the Compose setup command in testing-presto-connector.md to use the repository-root path tools/deployment/presto-clp/scripts/set-up-config.sh, or explicitly instruct readers to change into tools/deployment/presto-clp before running the existing command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/src/dev-docs/testing-presto-connector.md`:
- Line 35: Update the Compose setup command in testing-presto-connector.md to
use the repository-root path
tools/deployment/presto-clp/scripts/set-up-config.sh, or explicitly instruct
readers to change into tools/deployment/presto-clp before running the existing
command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 28f844f4-ebc3-414e-8a56-f016ba3bb952
📒 Files selected for processing (6)
docs/src/dev-docs/testing-presto-connector.mddocs/src/user-docs/guides-using-presto.mdtools/deployment/package-helm/set-up-multi-shared-test.shtools/deployment/package-helm/values.yamltools/deployment/presto-clp/docker-compose.yamltools/deployment/presto-clp/scripts/init.py
💤 Files with no reviewable changes (3)
- tools/deployment/package-helm/values.yaml
- tools/deployment/presto-clp/scripts/init.py
- tools/deployment/presto-clp/docker-compose.yaml
The rationale comment for resolving image overrides before `helm install` was five lines repeated verbatim in all three set-up scripts, and the `kind load` check carried a five-line walkthrough of the `errexit` interaction. Keep the non-obvious part of each (that `errexit` is suspended inside the caller's command substitution, and that the failure mode is a silent fall back to the chart default) and drop the restatement. Also restore the blank line after `sleep 2` in `set-up-multi-shared-test.sh` so it matches its two sibling scripts. Co-Authored-By: Claude <noreply@anthropic.com>
Collapse the two exit paths in `_add_connector_image_env_vars` into one: both wrote the same pair of env vars, so resolve the tag first and assign once. Drop the `docker_executable` parameter from `_run_docker_probe`. It was always the same literal, threaded through two functions; hoist it to `_DOCKER_EXECUTABLE` alongside the timeout constant. Trim the comments to what isn't already evident from the code. The module constant comment restated the function's docstring, the docstring restated the local-vs-published behaviour already covered in `testing-presto-connector.md`, and the probe call sites were annotated with what their own log lines say. No behaviour change: defaults resolve, an explicit `CLP_PRESTO_CONNECTOR_TAG` still bypasses the check, an unavailable image still fails without writing a partial env var set, and the local and registry probes are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
…and select each with a single image reference.
The connector image was selected through three environment variables (`CLP_PRESTO_CONNECTOR_IMAGE`,
`_VERSION`, and `_TAG`) that were concatenated with a hardcoded ":", so a digest couldn't be
expressed at all. Each image is now chosen with one variable holding a complete reference, used
as-is, so `repository:tag`, `repository@digest`, and `repository:tag@digest` all work:
* `CLP_PRESTO_CONNECTOR_REF`
* `CLP_PRESTO_COORDINATOR_REF`
* `CLP_PRESTO_WORKER_REF`
All three now default to `tag@digest`: the digest pins what's actually pulled, while the tag stays
as a human-readable label. The digests are those of the multi-arch manifest lists, so both
linux/amd64 and linux/arm64 continue to resolve.
On the Helm side, `clp.imageRef` previously discarded the tag whenever a digest was set, so it
could render only two of the three forms; it now renders all three and fails if neither is given.
The Presto coordinator and worker templates were building their references by hand instead of
calling it, which meant `image.presto{Coordinator,Worker}.digest` was silently ignored; both now
go through the helper.
jackluo923
marked this pull request as draft
August 7, 2026 06:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Deploys Presto from the stock
ghcr.io/y-scope/presto/presto-nativeimages and installs the CLP connector into them at startup, so only the connector artifact needs to be published (no more CLP-specific Presto image builds).clp-plugin-presto-connector-initservice, Helm aninstall-clp-plugininit container (newimage.clpConnectorvalues block); coordinator and worker wait on it before starting. The coordinator setsuse-connector-provided-serialization-codecs, required for the worker to deserialize the connector's handles.repository:tag,repository@digest, andrepository:tag@digestall work:CLP_PRESTO_CONNECTOR_REF,CLP_PRESTO_COORDINATOR_REF, andCLP_PRESTO_WORKER_REF. For the connector,init.pyverifies the image exists (local Docker daemon or registry) and writes the resolved reference into.env. Details indocs/src/dev-docs/testing-presto-connector.md.tag@digest, so the digest fixes what's pulled while the tag stays a human-readable label. The digests are those of the multi-arch manifest lists, solinux/amd64andlinux/arm64still resolve per-platform. On the Helm side,clp.imageRefpreviously discarded the tag whenever a digest was set; it now renders all three forms and fails if neither is given. The Presto coordinator and worker templates were building references by hand rather than calling it, soimage.presto{Coordinator,Worker}.digestwas silently ignored — both now go through the helper.--clp-connector-imagemirrors--clp-package-image; overrides now resolve beforehelm install, so a bad ref fails loudly instead of silently using the chart default. Web UI and API server are exposed via NodePort so the kind cluster is reachable.curl/python3instead ofapt-get/jq/wget/hostname(stockpresto-nativeis CentOS-based) and fails loudly if node identity can't be resolved; the Compose Presto config dir istmpfs, rebuilt fresh each start.Connector version: deployment counterpart of y-scope/clp-plugin-presto-connector#36, now merged.
0.1.0-SNAPSHOTis published multi-arch (linux/amd64+linux/arm64) and is the default here, so the out-of-the-box pull works without overrides. CLP intentionally pins the connector version it onboards.Note that
0.1.0-SNAPSHOTis a moving tag, so each republish leaves the pinned digest valid but pointing at an image older than the label suggests. Re-pinning means updating the digest indocker-compose.yaml,init.py, andvalues.yamltogether;docs/src/dev-docs/testing-presto-connector.mdcovers taking the manifest-list digest (docker inspectreports the single-platform one and is the wrong source).Checklist
presto/presto-nativeimages are feature-compatible with the old CLP-baked ones they replace.)docs/src/user-docs/guides-using-presto.md, local testing indocs/src/dev-docs/testing-presto-connector.md.)Validation performed
Docker Compose (end to end, local connector image)
webui.query_engine: "presto", pre-compressed archives with 60 records).set-up-config.shfound the locally-built image and wrote it into.env.docker compose up -d: the installer exited 0, placingclp-plugin-presto-connector.jarin the coordinator's plugin dir andlibclp-plugin-velox-connector.so+lib/in the worker's; coordinator healthy, worker up.node.propertieshas a realnode.id/node.internal-address, so thecurl/python3path works.SHOW TABLES→default;SELECT COUNT(*)→ 60;SELECT * LIMIT 2returned real rows, so handle deserialization works.Helm / kind (end to end, local connector image)
./package-helm/set-up-test.sh --presto --clp-connector-image <repo>:0.1.0-SNAPSHOT, all other images at chart defaults; the script kind-loaded the image and setimage.clpConnector.{repository,tag,pullPolicy=Never}.install-clp-pluginexited 0 on both pods before their main containers, with both plugins in place (the worker's via the sharedpresto-pluginemptyDir).apt-get/jq/wget/hostname:node.propertieshas the real pod name/IP, and the coordinator hasuse-connector-provided-serialization-codecs=true. The query path itself was covered in Compose above, with the same image and flag.Published connector image
set-up-config.shre-run against the live package regenerated a byte-identical.env, and an unresolvable reference fails without writing a partial.env. The published multi-arch image was pulled by digest on anaarch64host and the installer ran against it, exiting 0 with both plugins in place.Image references
The end-to-end runs above predate the switch to single-variable, digest-pinned references. That change was re-verified as follows.
With the connector image removed from the local daemon,
docker compose up clp-plugin-presto-connector-initpulled it by the pinned reference (…:0.1.0-SNAPSHOT@sha256:d006b0ce…), the installer exited 0, and both plugins landed in the shared volumes (clp-plugin-presto-connector.jarfor the coordinator;libclp-plugin-velox-connector.soandlib/for the worker). The remaining checks:connector: amd64/arm64;presto: amd64/arm64/ppc64le;presto-native: amd64/arm64), so multi-arch resolution is preserved. A per-architecture digest would have broken the other architecture.docker-compose.yaml(×3),init.py, andvalues.yaml(×3) — agree exactly.helm templaterenderstag@digest, tag-only, and digest-only correctly, and fails withrequires "tag" or "digest"when both are absent. The pre-existing digest-onlykubectlpin is unchanged.docker compose configresolves the connector, coordinator, and worker images, with and without overrides.init.pyresolves the default and honours an overridingCLP_PRESTO_CONNECTOR_REF; a locally-built image is accepted without a network round-trip, and an image that exists nowhere fails at setup naming the ref it tried.task lint:yamlandtask lint:check-pypass.Summary by CodeRabbit