Skip to content

Optimize PR CI by deduplicating heavy workflow setup#6326

Open
Aias00 wants to merge 15 commits intoapache:masterfrom
Aias00:ci-workflow-dedupe-concurrency
Open

Optimize PR CI by deduplicating heavy workflow setup#6326
Aias00 wants to merge 15 commits intoapache:masterfrom
Aias00:ci-workflow-dedupe-concurrency

Conversation

@Aias00
Copy link
Copy Markdown
Contributor

@Aias00 Aias00 commented Apr 15, 2026

Summary

  • add workflow-level concurrency cancellation for heavy PR workflows
  • deduplicate e2e image preparation into a single upstream job
  • deduplicate it-k8s image preparation and remove repeated setup/cache steps

Testing

  • python3 YAML parse for modified workflows
  • actionlint on modified workflows with repo-specific false-positive filters
  • git diff --check

Aias00 added 4 commits March 31, 2026 19:18
The heavy PR workflows were rebuilding the same Docker images and example
artifacts in each matrix case, which multiplied machine time and increased
queue pressure. This change adds workflow-level concurrency cancellation and
restructures the e2e and it-k8s flows so expensive image preparation happens
once before fan-out.

Constraint: GitHub Actions matrix jobs do not share local Docker state across runners
Constraint: Changes must preserve existing test case topology and repository-local workflow behavior
Rejected: Keep per-case image builds and only add Maven cache tuning | leaves most duplicated runtime intact
Rejected: Push temporary images to an external registry | adds auth and lifecycle complexity not needed for first pass
Confidence: medium
Scope-risk: moderate
Reversibility: clean
Directive: If artifact transfer becomes the new bottleneck, split image bundles by case family before reintroducing per-case builds
Tested: YAML parse for modified workflows; actionlint on modified workflows with existing repo-specific false-positive filters; git diff --check
Not-tested: Live GitHub Actions execution time and artifact transfer overhead on hosted runners
Copilot AI review requested due to automatic review settings April 15, 2026 01:45
This empty commit exists to verify that the newly added workflow-level
concurrency keys cancel obsolete runs for the same pull request instead of
letting queued heavy workflows accumulate.

Constraint: Validation should exercise the real PR workflow path without altering CI logic again
Rejected: Manual re-run from the GitHub UI | does not verify cancel-in-progress behavior on new commits
Confidence: medium
Scope-risk: narrow
Reversibility: clean
Directive: Drop this commit from final history only if project history policy requires squashing PR validation noise
Tested: Commit creation only
Not-tested: Resulting run cancellation until GitHub reports updated statuses
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Optimizes PR CI runtime for heavy workflows by canceling superseded runs and deduplicating expensive Docker/Maven setup via shared “prepare/build images” jobs with artifact handoff.

Changes:

  • Added workflow-level concurrency with cancel-in-progress: true to several CI workflows.
  • Refactored e2e-k8s to build/export Docker images once and reuse them across downstream matrix jobs.
  • Refactored it-k8s ingress workflow to build/export required images once and reuse them across the matrix.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.github/workflows/k8s-examples-http.yml Adds workflow-level concurrency cancellation to reduce redundant runs.
.github/workflows/integrated-test.yml Adds workflow-level concurrency cancellation for integrated tests.
.github/workflows/integrated-test-k8s-ingress.yml Introduces changes + prepare-images upstream jobs and artifact-based image reuse for matrix runs; adds concurrency.
.github/workflows/e2e-k8s.yml Builds base/example Docker images once, uploads as artifacts, and reuses them in downstream e2e jobs; adds concurrency.
.github/workflows/codeql-analysis.yml Adds workflow-level concurrency cancellation for CodeQL runs.
.github/workflows/ci.yml Adds workflow-level concurrency cancellation for main CI workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 100 to 109
- name: Build with Maven
if: steps.filter.outputs.k8s-ingress == 'true'
shell: bash
run: |
if mvnd --version > /dev/null 2>&1; then
echo "Using mvnd for build"
mvnd -B clean install -Dmaven.javadoc.skip=true -Dmaven.test.skip=true
else
echo "Falling back to maven wrapper"
if [[ "${{ runner.os }}" == "Windows" ]]; then
./mvnw.cmd -B clean install -Dmaven.javadoc.skip=true -Dmaven.test.skip=true
else
./mvnw -B clean install -Dmaven.javadoc.skip=true -Dmaven.test.skip=true
fi
./mvnw -B clean install -Dmaven.javadoc.skip=true -Dmaven.test.skip=true
fi
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepare-images runs three separate Maven builds, each starting with clean, which forces redundant recompilation and reduces the CI savings from deduplicating this workflow. Consider removing the initial full clean install here and/or consolidating the needed modules/profiles into a single build so dependencies are built once (e.g., avoid multiple clean invocations).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped clean from the prepare-stage Maven commands so later invocations can reuse outputs from earlier phases instead of rebuilding from scratch.

Comment on lines 111 to 120
- name: Build integrated tests
if: steps.filter.outputs.k8s-ingress == 'true'
shell: bash
run: |
if mvnd --version > /dev/null 2>&1; then
echo "Using mvnd for build integrated tests"
mvnd -B clean install -Pit -DskipTests -am -f ./shenyu-integrated-test/pom.xml
else
echo "Falling back to maven wrapper for integrated tests"
if [[ "${{ runner.os }}" == "Windows" ]]; then
./mvnw.cmd -B clean install -Pit -DskipTests -am -f ./shenyu-integrated-test/pom.xml
else
./mvnw -B clean install -Pit -DskipTests -am -f ./shenyu-integrated-test/pom.xml
fi
./mvnw -B clean install -Pit -DskipTests -am -f ./shenyu-integrated-test/pom.xml
fi
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The follow-up Build integrated tests step also uses clean install (with -am), which will clean and rebuild upstream modules again even though they were already built earlier in this job. Dropping clean (or combining builds) would avoid extra work and speed up prepare-images.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this step to install so it reuses artifacts from the earlier prepare build instead of cleaning and recompiling upstream modules again.

Comment on lines 122 to 131
- name: Build examples
if: steps.filter.outputs.k8s-ingress == 'true'
shell: bash
run: |
if mvnd --version > /dev/null 2>&1; then
echo "Using mvnd for build examples"
mvnd -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
else
echo "Falling back to maven wrapper for examples"
if [[ "${{ runner.os }}" == "Windows" ]]; then
./mvnw.cmd -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
else
./mvnw -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
fi
./mvnw -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
fi
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Build examples step runs clean install with -am, which will trigger additional cleaning/rebuilding of dependencies and increases runtime. Prefer running without clean (or merging with the prior Maven invocation) so the example image build can reuse already-built artifacts.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this step to install as well. The example-image build now reuses outputs from the previous steps instead of wiping the workspace with another clean.

Comment thread .github/workflows/e2e-k8s.yml Outdated
Comment on lines +120 to +123
mvnd -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
else
echo "Falling back to maven wrapper for examples"
./mvnw -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build examples with Maven uses clean install after the main -Prelease,docker build, which will unnecessarily clean/rebuild any -am dependencies and can significantly increase CI time. Consider removing clean here so this step can reuse artifacts from the previous build (or combine the builds if feasible).

Suggested change
mvnd -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
else
echo "Falling back to maven wrapper for examples"
./mvnw -B clean install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
mvnd -B install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml
else
echo "Falling back to maven wrapper for examples"
./mvnw -B install -Pexample -Dmaven.javadoc.skip=true -Dmaven.test.skip=true -am -f ./shenyu-examples/pom.xml

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the follow-up example build from clean install to install so it can reuse artifacts produced by the earlier build in the same job.

Comment thread .github/workflows/e2e-k8s.yml Outdated
# script: e2e-logging-kafka-compose

steps:
- uses: actions/checkout@v2
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This job still uses actions/checkout@v2 while other jobs in this workflow use @v3. checkout@v2 is deprecated (Node 12-based) and may start failing as GitHub retires older runtimes; update this to match the newer version used elsewhere (e.g., actions/checkout@v3 or @v4).

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v3

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the e2e-k8s.yml checkout steps to actions/checkout@v4 so this workflow no longer relies on the deprecated v2 action.

Aias00 added 7 commits April 15, 2026 10:00
The first pass removed duplicated work across matrix jobs, but the prepare
jobs still used repeated `clean install` calls that threw away outputs from
prior steps. This follow-up keeps the workflow shape intact while letting the
later Maven invocations reuse artifacts from earlier phases. It also updates
`e2e-k8s.yml` checkout steps to v4 so the workflow no longer mixes in the
deprecated v2 action.

Constraint: Keep the new prepare-once/run-many workflow topology unchanged while addressing review feedback
Rejected: Merge all Maven/profile work into one invocation | higher risk without validating profile interactions in this repo
Confidence: medium
Scope-risk: narrow
Reversibility: clean
Directive: If prepare jobs remain the bottleneck after this change, profile consolidation is the next place to investigate
Tested: YAML parse for modified workflows; actionlint on modified workflows with repo-specific false-positive filters; git diff --check
Not-tested: Live GitHub Actions runtime after removing `clean` from prepare-stage Maven invocations
The prepare job was installing local SNAPSHOT artifacts into its own Maven
repository, but the matrix runners only restored the stale shared cache. That
left modules like `shenyu-integrated-test-common` unresolved during the test
step even though prepare-images had built them successfully. This change
packages the prepared `org/apache/shenyu` subtree and restores it in each test
runner before running Maven.

Constraint: Matrix runners are isolated and cannot see the prepare job's local ~/.m2 state
Rejected: Re-add per-case prepare builds | fixes the failure but gives back most of the deduplication win
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: If artifact size becomes problematic, switch from subtree tarball transfer to a run-scoped cache key or a narrower module subset
Tested: YAML parse; actionlint on the modified workflow with repo-specific false-positive filters; git diff --check
Not-tested: Live GitHub Actions rerun after adding the local Maven artifact handoff
The integrated-test workflow was still rebuilding the same repo artifacts,
example images, and integrated-test modules inside each of the 12 matrix
cases. This change splits it into a changes gate, a one-time prepare job, and a
matrix execution job that reuses prebuilt Docker images and local ShenYu
SNAPSHOT Maven artifacts.

Constraint: GitHub Actions matrix runners do not share local Docker or Maven state
Constraint: Preserve the existing 12-case matrix and compose/test failure logging behavior
Rejected: Keep per-case builds and optimize only cache keys | leaves the dominant repeated work in place
Rejected: Add per-case artifact fan-out in the first pass | lower download cost, but more YAML complexity and higher failure surface
Confidence: medium
Scope-risk: moderate
Reversibility: clean
Directive: If CI runtime shifts from build CPU to artifact transfer, next optimize the snapshot bundle and image artifact granularity
Tested: YAML parse for integrated-test.yml; bash -n for helper scripts; actionlint on integrated-test.yml with repo-specific false-positive filter; git diff --check; dry-run on both helper scripts
Not-tested: End-to-end GitHub Actions execution for the refactored integrated-test workflow
The first prepare-once/run-many pass made integrated-test correct but slower in
wall-clock because every matrix leg downloaded the same union image tar and the
same broad local Maven snapshot bundle. This change splits the Docker transfer
into a shared admin image plus per-case overlays and narrows the snapshot bundle
by default to only jars and poms that Maven resolves during the test phase.

Constraint: Preserve the 12-case matrix and existing docker-compose/test failure logging behavior
Constraint: Keep the successful prepare-once/run-many topology instead of reintroducing per-case full rebuilds
Rejected: Revert to per-case local image builds | likely improves wall-clock but restores the larger duplicated build path and changes the optimization direction
Rejected: Keep a single shared all-images tar and only trim Maven metadata | does not address the dominant long-tail case download cost
Confidence: medium
Scope-risk: moderate
Reversibility: clean
Directive: If this still does not recover enough wall-clock, the next experiment should be hybridizing image preparation rather than broadening artifact scope again
Tested: YAML parse; bash -n on helper scripts; actionlint on integrated-test.yml with repo-specific false-positive filter; git diff --check; dry-run counts for per-case image packaging and slim-vs-full Maven snapshot manifests
Not-tested: End-to-end GitHub Actions execution for the second-pass integrated-test artifact layout
with:
submodules: true
- name: Free disk space
run: |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the command may cause out of disk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants