Skip to content

Commit 776ed5a

Browse files
ci(oss): write the image tag once, pull it in CI
Ports javascript-sdk#176's resolution of the same review feedback, so the OSS harness stays diffable across the SDKs. csharp-sdk#171 carries the identical change. One tag, one home. The tag was written twice: FORK_PR_FALLBACK_VERSION in the integration-tests-oss job, and `latest` as the local script's default. A local run therefore could not reproduce a CI failure, which is most of why the script exists. Rather than teach one of them to read the other, make the `image:` line of docker-compose-oss.yaml the only place it is written and let both fall through to it: the script applies no default of its own and exports OSS_CONDUCTOR_VERSION only when --version actually supplied one, and the fork-PR branch of the resolve step now leaves the variable unset instead of pinning its own copy. Fork CI and a plain local run reach the identical image by the identical path, with no YAML parsing on either side and one hardcode removed rather than a mechanism added. The non-fork empty case still fails loudly, and the workflow_run nuance is untouched -- that trigger runs in the base repo's context and still gets the org variable even for a fork PR. The script's "Using ..." and "Pulling ..." lines now come from `compose config --images conductor-server` instead of reconstructing the tag, so they stay honest whichever source supplied it. The compose header already noted that E2E_TEST_OSS_CONDUCTOR_VERSION is set to `latest` org-wide, so a normal CI run is not really pinned. That stays, with the addition that this default is now what actually pins fork PRs and local runs until someone sets the org variable to a real tag. Pull in CI. The script pulls before `up`, CI did not. On a GitHub-hosted runner the VM is ephemeral and starts with no cached copy, so `up` pulls anyway and the step is redundant today — kept regardless, because it costs no extra network pull (`up` then finds the image locally), it splits "couldn't pull the image" from "the stack didn't come up" into two distinct red steps, and it is what stops a mutable tag going stale the day this job moves to a self-hosted runner with a warm Docker daemon. It also prints the tag in use, which matters now that a fork PR's tag is not spelled out in the workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent cebf93e commit 776ed5a

3 files changed

Lines changed: 60 additions & 27 deletions

File tree

‎.github/workflows/integration-tests.yml‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ on:
88
workflow_dispatch:
99
inputs:
1010
oss_conductor_version:
11-
description: 'OSS Conductor image tag (falls back to E2E_TEST_OSS_CONDUCTOR_VERSION org var, then to a pinned default on fork PRs)'
11+
description: 'OSS Conductor image tag (falls back to the E2E_TEST_OSS_CONDUCTOR_VERSION org var, then to the default in scripts/docker-compose-oss.yaml on fork PRs)'
1212
required: false
1313
type: string
1414

@@ -99,9 +99,6 @@ jobs:
9999
env:
100100
CONDUCTOR_SERVER_URL: http://localhost:8080/api
101101
CONDUCTOR_SERVER_TYPE: oss
102-
# Used only when the org variable is unreachable because the run is a fork
103-
# PR -- see the resolve step below.
104-
FORK_PR_FALLBACK_VERSION: '3.32.3'
105102

106103
steps:
107104
# OSS_CONDUCTOR_VERSION is resolved here rather than in the job `env` so
@@ -110,30 +107,31 @@ jobs:
110107
# - Fork PR: GitHub withholds org/repo variables from pull_request runs
111108
# on forks exactly as it withholds secrets, so vars.* is always "" for
112109
# an outside contributor (observed in csharp-sdk#178). This job needs
113-
# no secrets, only a tag, so pin one and keep running. Note that the
110+
# no secrets, only a tag, so leave the var unset and let the default
111+
# baked into the `image:` line of scripts/docker-compose-oss.yaml
112+
# apply. That is the same tag a plain local run of
113+
# scripts/run-integration-oss.sh gets, and the one place it is
114+
# written -- no second copy to drift out of sync here. Note that the
114115
# workflow_run trigger above executes in the base repo's context and
115116
# does get the org variable even for a fork PR; the fork branch here
116117
# matters for the push/dispatch paths and for any future direct
117118
# pull_request trigger.
118119
# - Anything else: the org variable is genuinely missing or its
119120
# repository access policy no longer covers this repo. Fail loudly
120-
# rather than silently drifting onto the pin.
121+
# rather than silently drifting onto the default.
121122
- name: Resolve OSS Conductor version
122123
env:
123124
REQUESTED_VERSION: ${{ inputs.oss_conductor_version || vars.E2E_TEST_OSS_CONDUCTOR_VERSION }}
124125
IS_FORK_PR: ${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) || (github.event_name == 'workflow_run' && github.event.workflow_run.head_repository.full_name != github.repository) }}
125126
run: |
126127
if [ -n "$REQUESTED_VERSION" ]; then
127-
resolved="$REQUESTED_VERSION"
128+
echo "OSS_CONDUCTOR_VERSION=${REQUESTED_VERSION}" >> "$GITHUB_ENV"
128129
elif [ "$IS_FORK_PR" = "true" ]; then
129-
resolved="$FORK_PR_FALLBACK_VERSION"
130-
echo "::notice::Fork PR: org variables are withheld, pinning conductoross/conductor:${resolved}"
130+
echo "::notice::Fork PR: org variables are withheld, falling back to the default tag in scripts/docker-compose-oss.yaml"
131131
else
132132
echo "::error::No Conductor OSS image tag resolved. Set the E2E_TEST_OSS_CONDUCTOR_VERSION organization variable (and ensure its repository access policy includes this repo), or pass the oss_conductor_version input via workflow_dispatch."
133133
exit 1
134134
fi
135-
echo "OSS_CONDUCTOR_VERSION=${resolved}" >> "$GITHUB_ENV"
136-
echo "Using conductoross/conductor:${resolved}"
137135
138136
- name: Checkout
139137
uses: actions/checkout@v6
@@ -149,6 +147,21 @@ jobs:
149147
distribution: "zulu"
150148
java-version: "21"
151149

150+
# `docker compose up` only pulls an image when it is missing locally. On a
151+
# GitHub-hosted runner the VM is ephemeral and starts with no cached copy
152+
# of this image, so `up` would pull anyway and this step is redundant
153+
# today. It is here deliberately: it costs no extra network pull (`up`
154+
# then finds the image locally), it separates "couldn't pull the image"
155+
# from "the stack didn't come up" into two distinct red steps, and it is
156+
# what keeps a mutable tag from going stale if this job ever moves to a
157+
# self-hosted runner with a warm Docker daemon -- the same reason
158+
# scripts/run-integration-oss.sh pulls. It also prints the tag actually in
159+
# use, which for a fork PR comes from the compose file's default.
160+
- name: Pull Conductor OSS image
161+
run: |
162+
echo "Using $(docker compose -f scripts/docker-compose-oss.yaml config --images | grep -m1 '^conductoross/conductor:')"
163+
docker compose -f scripts/docker-compose-oss.yaml pull conductor-server
164+
152165
- name: Start Conductor OSS stack
153166
run: docker compose -f scripts/docker-compose-oss.yaml up -d
154167

‎scripts/docker-compose-oss.yaml‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@
22
# Shared by scripts/run-integration-oss.sh and the integration-tests-oss job in
33
# .github/workflows/integration-tests.yml.
44
#
5-
# OSS_CONDUCTOR_VERSION defaults to `latest` for local runs. CI resolves it from the
6-
# E2E_TEST_OSS_CONDUCTOR_VERSION org variable (or a workflow_dispatch input); that variable is
7-
# currently set to `latest` too, so CI tracks whatever `latest` resolves to at run time rather
8-
# than a fixed version. Set the org variable to a real tag if the job needs to be deterministic.
5+
# The `image:` default below is the SINGLE place the Conductor OSS image tag is written.
6+
# Everything that does not override OSS_CONDUCTOR_VERSION lands on it: a plain
7+
# scripts/run-integration-oss.sh run, and the integration-tests-oss job on a fork PR (where
8+
# GitHub withholds org variables). Overrides are the script's --version flag and, in CI, the
9+
# E2E_TEST_OSS_CONDUCTOR_VERSION org variable or a workflow_dispatch input. Bump the tag here
10+
# and both follow.
11+
#
12+
# Note that the org variable is currently set to `latest`, so a normal CI run still tracks
13+
# whatever `latest` resolves to at run time. Set it to a real tag if the job needs to be
14+
# deterministic; until then this default is what actually pins fork PRs and local runs.
915

1016
services:
1117
conductor-server:
12-
image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-latest}
18+
image: conductoross/conductor:${OSS_CONDUCTOR_VERSION:-3.32.3}
1319
environment:
1420
- CONFIG_PROP=config-postgres.properties
1521
ports:

‎scripts/run-integration-oss.sh‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,17 @@
99
# the empirically-confirmed gaps).
1010
#
1111
# The stack (Conductor OSS + Postgres) is defined in
12-
# scripts/docker-compose-oss.yaml and is torn down automatically on exit. The
13-
# image is always pulled before starting, since `latest` (the local default)
14-
# is a mutable tag and a cached copy would otherwise go stale silently.
12+
# scripts/docker-compose-oss.yaml and is torn down automatically on exit. That
13+
# file's `image:` line is also where the default tag lives -- this script
14+
# applies no default of its own, so a plain run and a fork-PR CI run land on
15+
# the identical image. The image is always pulled before starting, since a tag
16+
# can be mutable and a cached copy would otherwise go stale silently.
1517
#
1618
# Usage:
1719
# scripts/run-integration-oss.sh [--keep-up] [--version <tag>] [--include-gated] [-- gradle args]
1820
# Examples:
19-
# scripts/run-integration-oss.sh
20-
# scripts/run-integration-oss.sh --version 3.32.0-rc18
21+
# scripts/run-integration-oss.sh # default tag from the compose file
22+
# scripts/run-integration-oss.sh --version 3.33.0-rc1
2123
# scripts/run-integration-oss.sh --keep-up
2224
# scripts/run-integration-oss.sh --include-gated # also run tests normally skipped as Orkes-only
2325
# scripts/run-integration-oss.sh -- --tests "*WorkflowClientTests"
@@ -41,7 +43,14 @@ while [[ $# -gt 0 ]]; do
4143
esac
4244
done
4345

44-
export OSS_CONDUCTOR_VERSION="${OSS_CONDUCTOR_VERSION:-latest}"
46+
# No default is applied here on purpose. The default tag is written once, in the
47+
# `image:` line of scripts/docker-compose-oss.yaml, so leaving OSS_CONDUCTOR_VERSION
48+
# unset lets compose supply it -- the same path a fork PR takes in CI. Only export
49+
# it when the caller actually asked for a specific tag, otherwise a value set but
50+
# not exported in the caller's shell would never reach compose anyway.
51+
if [[ -n "${OSS_CONDUCTOR_VERSION:-}" ]]; then
52+
export OSS_CONDUCTOR_VERSION
53+
fi
4554

4655
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
4756
REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)"
@@ -66,13 +75,18 @@ cleanup() {
6675
}
6776
trap cleanup EXIT
6877

69-
echo "Using conductoross/conductor:${OSS_CONDUCTOR_VERSION}"
78+
# Ask compose what it resolved rather than reconstructing the tag here, so this
79+
# stays correct whether the tag came from --version or from the compose default.
80+
# `--images` lists every service's image and does not reliably honour a service
81+
# filter, so select the server's by name rather than by position.
82+
SERVER_IMAGE="$(compose config --images | grep -m1 '^conductoross/conductor:')"
83+
echo "Using ${SERVER_IMAGE}"
7084

7185
# `docker compose up` only pulls an image when it is missing locally, so a
72-
# previously-cached `latest` (or any other mutable tag) would silently be
73-
# reused instead of getting the current version. Pull unconditionally so the
74-
# stack always reflects the tag we just printed.
75-
echo "Pulling conductoross/conductor:${OSS_CONDUCTOR_VERSION} to ensure it's current..."
86+
# previously-cached mutable tag (a re-pushed rc, or `latest` if that is what was
87+
# asked for) would silently be reused instead of getting the current version.
88+
# Pull unconditionally so the stack always reflects the tag we just printed.
89+
echo "Pulling ${SERVER_IMAGE} to ensure it's current..."
7690
compose pull conductor-server
7791

7892
echo "Starting Conductor OSS stack..."

0 commit comments

Comments
 (0)