feat: implement RFC-006 registry lifecycle - #66
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new build workflow and Makefile have behavior that can break common contribution/use cases (fork PR runs without secrets; Makefile prevents overriding the target registry), and documentation should be aligned with the checked-in pipeline defaults.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements the examples-repo side of RFC-006 registry lifecycle by standardizing how container images are built/published (development vs release) and updating example manifests/docs to reference the new registries.
Changes:
- Adds a single GitHub Actions workflow that builds a six-image matrix and publishes to
prokube/developmentfor non-release builds andprokube/releasesfor release tags (with immutability checks). - Introduces a local
Makefilefor building/pushing the same image set to the development registry by default. - Updates example YAMLs and READMEs to reference
europe-west3-docker.pkg.dev/prokube/...instead ofprokube-internal, and removes stale GitLab / molecules workflows.
File summaries
| File | Description |
|---|---|
| serving/minimal-example-shadow-deployment/tripler-inference-service.yaml | Switches predictor/transformer images to prokube/development. |
| serving/minimal-example-shadow-deployment/doubler-inference-service.yaml | Switches predictor/transformer images to prokube/development. |
| serving/minimal-example-shadow-deployment/README.md | Updates image references and build/push instructions for the new registry. |
| serving/minimal-custom-kserve-predictor/README.md | Updates build example to use the prokube/development registry. |
| serving/minimal-custom-kserve-predictor/inference-service.yaml | Updates image reference and clarifies the replacement comment for registry access. |
| README.md | Adds documentation describing development vs release image publishing behavior. |
| pipelines/lightweight-python-package/README.md | Updates example image references and build/push instructions for the new registry. |
| pipelines/lightweight-python-package/pipeline.py | Switches default COMPONENTS_IMAGE to prokube/development. |
| Makefile | Adds local build/push targets for the six images. |
| images/streamlit-example/README.md | Updates build/push docs and default image reference to prokube/development. |
| images/streamlit-example/k8s/streamlit-manifests.yaml | Updates Deployment image to prokube/development. |
| images/README.md | Replaces legacy GitLab/GitHub CI notes with the new registry lifecycle behavior and local make usage. |
| hparam-tuning/minimal-mnist/README.md | Updates default registry references to prokube/development. |
| hparam-tuning/minimal-mnist/katib-experiment.yaml | Updates training image reference to prokube/development. |
| .gitlab-ci.yml | Removes legacy GitLab CI image build pipeline. |
| .github/workflows/molecules.yaml | Removes obsolete molecules image workflow. |
| .github/workflows/minimal-mnist.yaml | Removes obsolete minimal-mnist image workflow. |
| .github/workflows/build-images.yaml | Adds consolidated build/publish workflow with release immutability checks. |
| .github/testing/test_build_images_workflow.py | Adds contract tests to keep workflow matrix/tag rules aligned with repository Dockerfiles. |
Review details
Suppressed comments (1)
pipelines/lightweight-python-package/README.md:73
- This snippet suggests updating
pipeline.pyto:my-development-tag, but the committedpipeline.pyuses:latest. If:latestis the intended default, update this snippet to match to avoid confusion.
COMPONENTS_IMAGE = "europe-west3-docker.pkg.dev/prokube/development/mobile-price-classification:my-development-tag"
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The new workflow’s tags composition can produce an empty tag entry and the workflow-level permissions omit actions: write required for type=gha caching, both of which can break CI/publishing runs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
serving/minimal-custom-kserve-predictor/README.md:52
- This example builds the image directly into the shared
prokube/developmentregistry, but the next step talks about pushing to a private registry (docker login <your-registry>). Use a<your-registry>placeholder here to keep the instructions consistent and avoid implying everyone can push to the official registry.
serving/minimal-example-shadow-deployment/README.md:126 - These commands push to the shared
prokube/developmentregistry, but the surrounding text doesn’t mention that write access is required. For “build your own”, using a<your-registry>placeholder avoids implying that all readers can push to the official registry.
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
Note
AI Generated text, the ideas are mine though.
The registry split and release behavior make sense to me. My concern is mainly that the implementation seems more complicated than this repository needs, so I would prefer that we simplify it before merging. I mean this as a proposal and am happy to discuss the tradeoffs rather than prescribing the exact implementation.
As noted inline, these images change extremely rarely, so I do not think building all six on every pull request is worth the CI cost. The same likely applies to every push to main; release tags plus workflow_dispatch should be sufficient.
With those triggers, I think this can be a single self-contained workflow without the two Python helpers or the bespoke 208-line unit/contract test suite. The event/tag resolution and release-tag validation can live in a short shell step, and the existing-image check can use standard Docker or gcloud tooling. We should retain the important behavior: the six-image matrix, manual builds going to development, release tags going to releases with exactly one unchanged tag, and safe handling of immutable release reruns.
The current implementation is careful, but for six infrequently changed example images I do not think we need to overcomplicate the maintenance surface this much.
AI suggestion is something like:
env:
IS_RELEASE: ${{ github.event_name == 'push' && startsWith(github.ref, 'refs/tags/') }}
REGISTRY: ${{ startsWith(github.ref, 'refs/tags/') && 'europe-west3-docker.pkg.dev/prokube/releases' || 'europe-west3-docker.pkg.dev/prokube/development' }}
TAG: ${{ startsWith(github.ref, 'refs/tags/') && github.ref_name || format('commit-{0}', github.sha) }}
- name: Validate release tag
if: env.IS_RELEASE == 'true'
run: |
if [[ ! "$TAG" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-rc(0|[1-9][0-9]*))?$ ]]; then
echo "Release tag must match vX.Y.Z or vX.Y.Z-rcN"
exit 1
fi
- name: Check existing release
id: existing
if: env.IS_RELEASE == 'true'
run: |
if docker manifest inspect "$REGISTRY/${{ matrix.image }}:$TAG" >/dev/null 2>&1; then
echo "exists=true" >> "$GITHUB_OUTPUT"
fi|
we have never done a release in this repo until now, so we probably need to build on something else. I'd be fine with only building on merges to main that touch the relevant folders. |
There was a problem hiding this comment.
🟡 Changes recommended
Per-image preflight checks can leave a partial release when one artifact already exists.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
|
@copilot review |
There was a problem hiding this comment.
🟢 Approval recommended
The workflow matches the stated lifecycle requirements and covers all six Dockerfiles without identified correctness issues.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
prokube/developmentwith commit andlatesttagsvX.Y.Z/vX.Y.Z-rcNtag pushes and publish only the exact tag toprokube/releasesimages/README.mdThis intentionally leaves example defaults, GitLab CI, and the molecules workflow unchanged.
Testing
go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.7 .github/workflows/build-images.yamluvx --from yamllint==1.37.1 yamllint .github/workflows/build-images.yamlpre-commit run --files .github/workflows/build-images.yaml images/README.mdgit diff --check origin/mainImplements the examples repository portion of RFC-006.
Related to prokube/dev-tracker#78; this PR does not close the cross-repository epic.