-
Notifications
You must be signed in to change notification settings - Fork 373
feat(connectors): ship connector plugins in the iggy-connect docker image #3658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
c819f17
eff5b48
6cea26f
a0719d3
7cf2d08
1dde3c2
dd6f794
c5d9f6e
a56baf9
22cbe3b
ddcf3f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,8 +46,8 @@ on: | |
| connector_plugins: | ||
| type: string | ||
| required: false | ||
| default: "iggy_connector_elasticsearch_sink,iggy_connector_elasticsearch_source,iggy_connector_iceberg_sink,iggy_connector_postgres_sink,iggy_connector_postgres_source,iggy_connector_quickwit_sink,iggy_connector_random_source,iggy_connector_s3_sink,iggy_connector_stdout_sink,iggy_connector_surrealdb_sink" | ||
| description: "Comma-separated list of connector plugin crates to build as shared libraries" | ||
| default: "" | ||
| description: "Comma-separated list of connector plugin crates to build as shared libraries. Empty (default) derives the full cdylib set from scripts/ci/connector-plugins.sh so new connectors are picked up automatically." | ||
| outputs: | ||
| artifact_name: | ||
| description: "Name of the uploaded artifact containing all artifacts" | ||
|
|
@@ -216,12 +216,25 @@ jobs: | |
| - name: Add Rust target | ||
| run: rustup target add ${{ matrix.target }} | ||
|
|
||
| - name: Resolve connector plugin list | ||
| env: | ||
| OVERRIDE: ${{ inputs.connector_plugins }} | ||
| run: | | ||
| if [[ -n "$OVERRIDE" ]]; then | ||
| plugins="$OVERRIDE" | ||
| echo "Using connector plugin list from workflow input (override)" | ||
| else | ||
| plugins="$(scripts/ci/connector-plugins.sh --comma-names)" | ||
| echo "Derived connector plugin list from cargo metadata" | ||
| fi | ||
| echo "Plugins: $plugins" | ||
| echo "CONNECTOR_PLUGINS=$plugins" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Build connector plugins | ||
| run: | | ||
| plugins="${{ inputs.connector_plugins }}" | ||
| pkg_flags=() | ||
|
|
||
| IFS=',' read -ra pkgs <<< "$plugins" | ||
| IFS=',' read -ra pkgs <<< "$CONNECTOR_PLUGINS" | ||
| for pkg in "${pkgs[@]}"; do | ||
| name="$(echo "$pkg" | xargs)" | ||
| [[ -z "$name" ]] && continue | ||
|
|
@@ -238,13 +251,16 @@ jobs: | |
| outdir="dist/${target}" | ||
| mkdir -p "${outdir}" | ||
|
|
||
| plugins="${{ inputs.connector_plugins }}" | ||
| IFS=',' read -ra pkgs <<< "$plugins" | ||
| IFS=',' read -ra pkgs <<< "$CONNECTOR_PLUGINS" | ||
| for pkg in "${pkgs[@]}"; do | ||
| lib_name="$(echo "$pkg" | xargs)" | ||
| [[ -z "$lib_name" ]] && continue | ||
| so_file="target/${target}/release/lib${lib_name}.so" | ||
| [[ -f "$so_file" ]] && cp "$so_file" "${outdir}/" | ||
| if [[ ! -f "$so_file" ]]; then | ||
| echo "::error::expected connector plugin artifact missing: ${so_file}. The plugin crate built no cdylib, or its name does not match lib<crate>.so." >&2 | ||
| exit 1 | ||
|
Comment on lines
+259
to
+261
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This turns a previously tolerated miss into a hard failure at the same time as the plugin set silently grows, so the first master push after merge is the first real exercise of the new set. Old behaviour was The hard failure is the right call, the concern is that it is unverified for the 7 additions. Per the PR description the local validation was the docker image build, which is a different job and a different build graph. Worth running the |
||
| fi | ||
| cp "$so_file" "${outdir}/" | ||
| done | ||
|
|
||
| tarball="iggy-connectors-${target}-${version}.tar.gz" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targetnow changes the build graph, but the buildx cache ref is still only component- and arch-scoped, so the two flavors fight over one cache entry.The cache composition step above builds
cache-to/cache-fromastype=registry,ref=${img}:buildcache-${arch},mode=max(and the shared${shared_deps}:buildcache-${arch}) with no flavor component. Sincedocker_matrixis now component x flavor x platform, the fat and slim jobs for the same arch run concurrently and both pushmode=maxtoapache/iggy-connect:buildcache-amd64. The cache index is last-writer-wins, so each run one flavor's cache export is effectively lost, andcache-frommay pull an index describing the other flavor's graph.Layer integrity is fine (BuildKit verifies digests), but the fat build is the expensive one and is exactly the one that needs a warm cache.
${img}:latestincache-fromis also the fat image under the new tag scheme, which the slim build will now pull as inline cache.Suggestion: thread the flavor into the cache ref, e.g. pass the flavor suffix into this action and use
${img}:buildcache${suffix}-${arch}, so each flavor keeps its own cache the same way it now keeps its own digest artifacts.