Support a release controller for layered operators - #804
Open
JoelSpeed wants to merge 5 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughLayered releases now use stable-style discovery, synchronization, readiness, publishing, payload handling, and dashboard rendering. External registry publishing supports configured tag filters and per-tag mirror jobs. Payload controllers recognize pre-created images, and informer factories support namespace-scoped watches. ChangesLayered release lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseController
participant ReleasePayloadController
participant KubernetesJobs
participant ExternalRegistry
ReleaseController->>ReleasePayloadController: create layered ReleasePayload
ReleasePayloadController->>ReleasePayloadController: mark pre-created payload successful
ReleaseController->>KubernetesJobs: create filtered mirror jobs
KubernetesJobs->>ExternalRegistry: mirror release tags
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…d mirroring config
JoelSpeed
force-pushed
the
hypershift-release-controller
branch
from
August 6, 2026 16:17
7496ef7 to
c419063
Compare
JoelSpeed
marked this pull request as ready for review
August 7, 2026 13:13
JoelSpeed
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![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: 3
🤖 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 `@cmd/release-controller/sync_publish.go`:
- Around line 262-265: The release publishing flow must not call GetMirror for
Layered releases when OverrideCLIImage is empty. Validate this configuration
during parsing and reject it, or resolve the CLI image through a
mirror-independent path; preserve existing behavior for non-Layered releases and
add a regression test covering Layered external registry publishing without
overrideCLIImage.
- Around line 239-244: Update Job-name construction around jobName to normalize
tagName and config.Registry into DNS-safe components, then append a stable hash
derived from both original full values before applying the 63-character limit.
Ensure the final name contains only valid DNS characters and truncate in a way
that preserves the hash suffix, preventing distinct mirrors from colliding.
In `@pkg/cmd/release-payload-controller/cmd.go`:
- Line 79: Update the namespace filtering around the release payload
controller’s Job watch so it also includes the namespaces configured by
ReleaseCreationCoordinates.Namespace and ReleaseMirrorCoordinates.Namespace when
they differ from ReleaseNamespace. Alternatively, validate the configuration and
reject mismatched namespaces before starting the controllers; ensure Jobs are
never excluded and subsequently treated as missing.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d224e77f-1093-4918-b93e-d6f9843571d8
📒 Files selected for processing (15)
cmd/release-controller-api/http.gocmd/release-controller-api/http_candidate.gocmd/release-controller-api/http_helper.gocmd/release-controller/layered_mode_test.gocmd/release-controller/sync.gocmd/release-controller/sync_publish.gocmd/release-controller/sync_release_payload.gopkg/cmd/release-payload-controller/cmd.gopkg/cmd/release-payload-controller/layered_reference_test.gopkg/cmd/release-payload-controller/payload_creation_controller.gopkg/cmd/release-payload-controller/payload_mirror_controller.gopkg/cmd/release-payload-controller/payload_mirror_controller_test.gopkg/cmd/release-payload-controller/release_creation_job_controller.gopkg/release-controller/release.gopkg/release-controller/types.go
Comment on lines
+239
to
+244
| jobName := fmt.Sprintf("%s-external-mirror-%s", tagName, sanitizeRegistryForJobName(config.Registry)) | ||
|
|
||
| // Kubernetes limits job names to 63 characters | ||
| if len(jobName) > 63 { | ||
| jobName = jobName[:63] | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'ensureExternalRegistryMirrorJob|sanitizeRegistryForJobName|newReleaseJobBase|ensureJob' \
cmd/release-controller pkg
rg -n -C 3 'externalRegistry|registry:|tags:|excludeTags:' \
--glob '*.yaml' --glob '*.yml' --glob '*.json' .Repository: openshift/release-controller
Length of output: 16607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sync_publish.go ---'
sed -n '230,325p' cmd/release-controller/sync_publish.go
printf '%s\n' '--- related naming and configuration symbols ---'
rg -n -C 4 'sanitizeRegistryForJobName|PublishExternalRegistry|ExternalRegistry|finalTags|FindTagReference' \
--glob '*.go' --glob '*.yaml' --glob '*.yml' --glob '*.json' .
printf '%s\n' '--- tests mentioning external mirror jobs or naming ---'
rg -n -C 5 'external.*mirror|mirror.*job|sanitizeRegistry|PublishExternalRegistry' \
--glob '*_test.go' .Repository: openshift/release-controller
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '236,320p' cmd/release-controller/sync_publish.go
printf '%s\n' '--- helper usages and definitions ---'
rg -n -C 3 'sanitizeRegistryForJobName|PublishExternalRegistry|ExternalRegistry' --glob '*.go' .
printf '%s\n' '--- relevant tests ---'
rg -n -C 4 'external.*registry|mirror.*job|sanitizeRegistry|PublishExternalRegistry' --glob '*_test.go' .Repository: openshift/release-controller
Length of output: 29913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
def sanitize(registry):
result = registry.replace(".", "-").replace("/", "-").replace(":", "-")
if len(result) > 40:
result = result[:40]
return result
def job(tag, registry):
name = f"{tag}-external-mirror-{sanitize(registry)}"
return name[:63]
cases = [
("release_tag", "quay.io/example/image"),
("release_tag", "quay.io/example/image_with_underscore"),
("a"*50, "registry.example.com/team/one"),
("a"*50, "registry.example.com/team/two"),
]
for tag, registry in cases:
name = job(tag, registry)
print({"tag": tag, "registry": registry, "job": name,
"valid_dns_label": bool(re.fullmatch(r"[a-z0-9]([-a-z0-9]*[a-z0-9])?", name)),
"length": len(name)})
PYRepository: openshift/release-controller
Length of output: 956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n cmd/release-controller/sync_publish.go | sed -n '236,320p'Repository: openshift/release-controller
Length of output: 3849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '268,380p' cmd/release-controller/sync_release.goRepository: openshift/release-controller
Length of output: 4201
Use a DNS-safe, collision-resistant Job name.
Normalize tagName and config.Registry to DNS-safe components. Append a stable hash of both full values before enforcing the 63-character limit. The current code preserves _, which Kubernetes rejects. Truncation can also make different destinations share a Job name; ensureJob then returns the existing Job and skips the second mirror.
🤖 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 `@cmd/release-controller/sync_publish.go` around lines 239 - 244, Update
Job-name construction around jobName to normalize tagName and config.Registry
into DNS-safe components, then append a stable hash derived from both original
full values before applying the 63-character limit. Ensure the final name
contains only valid DNS characters and truncate in a way that preserves the hash
suffix, preventing distinct mirrors from colliding.
Comment on lines
+262
to
+265
| mirror, err := releasecontroller.GetMirror(release, tagName, c.releaseLister) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get mirror for %s: %v", tagName, err) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require a CLI image for Layered external publishing.
When config.OverrideCLIImage is empty, Line 262 calls GetMirror. Layered releases do not create a release mirror. The external publish step then fails on every reconciliation.
Reject this Layered configuration during parsing, or resolve the CLI image without a mirror. Add a regression test for a Layered external registry publish without overrideCLIImage.
🤖 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 `@cmd/release-controller/sync_publish.go` around lines 262 - 265, The release
publishing flow must not call GetMirror for Layered releases when
OverrideCLIImage is empty. Validate this configuration during parsing and reject
it, or resolve the CLI image through a mirror-independent path; preserve
existing behavior for non-Layered releases and add a regression test covering
Layered external registry publishing without overrideCLIImage.
| fs.StringVar(&o.GoogleProjectID, "google-project-id", os.Getenv("GOOGLE_PROJECT_ID"), "Google project name.") | ||
| fs.StringVar(&o.GoogleServiceAccountCredentialFile, "google-service-account-credential-file", os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"), "location of a credential file described by https://cloud.google.com/docs/authentication/production") | ||
| fs.DurationVar(&o.BigQueryCacheTTL, "bigquery-cache-ttl", o.BigQueryCacheTTL, "TTL for cached BigQuery query results (0 to disable caching)") | ||
| fs.StringVar(&o.ReleaseNamespace, "release-namespace", "", "Namespace to watch for releasepayloads. When unset, all namespaces will be watched. Useful for testing locally with a single namespace.") |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'ReleaseNamespace|WithNamespace|jobNamespace|ReleaseCreationCoordinates|ReleaseMirrorCoordinates' \
cmd pkg
rg -n -C 3 'release-namespace|job-namespace|jobNamespace' \
--glob '*.yaml' --glob '*.yml' --glob '*.json' --glob '*.go' .Repository: openshift/release-controller
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- release-payload-controller files ---'
fd -t f . | rg 'release-payload-controller|release-payload'
printf '%s\n' '--- command implementation ---'
cat -n pkg/cmd/release-payload-controller/cmd.go | sed -n '1,180p'
printf '%s\n' '--- Job informer and status handlers ---'
rg -n -C 5 'JobInformer|Batch\(\)\.V1\(\)\.Jobs|jobLister|ReleaseCreationCoordinates|ReleaseMirrorCoordinates|ReleaseNamespace' \
pkg/cmd/release-payload-controller pkg --glob '*.go' \
| head -n 500Repository: openshift/release-controller
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- status lookup implementations ---'
for f in \
pkg/cmd/release-payload-controller/release_creation_status_controller.go \
pkg/cmd/release-payload-controller/release_mirror_job_status_controller.go \
pkg/cmd/release-payload-controller/release_creation_job_controller.go \
pkg/cmd/release-payload-controller/release_mirror_job_controller.go; do
echo "### $f"
cat -n "$f" | sed -n '1,240p'
done
printf '%s\n' '--- controller deployment arguments ---'
rg -n -C 5 'release-payload-controller|release-namespace|job-namespace' \
. --glob '*.yaml' --glob '*.yml' --glob '*.json' --glob '*.go' \
| head -n 400Repository: openshift/release-controller
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
cmd = Path("pkg/cmd/release-payload-controller/cmd.go").read_text()
creation_status = Path(
"pkg/cmd/release-payload-controller/release_creation_status_controller.go"
).read_text()
mirror_status = Path(
"pkg/cmd/release-payload-controller/release_mirror_job_status_controller.go"
).read_text()
payload_builder = Path("cmd/release-controller/sync_release_payload.go").read_text()
release_main = Path("cmd/release-controller/main.go").read_text()
assert 'informers.WithNamespace(o.ReleaseNamespace)' in cmd
assert 'batchJobLister.Jobs(originalReleasePayload.Status.ReleaseCreationJobResult.Coordinates.Namespace)' in creation_status
assert 'batchJobLister.Jobs(originalReleasePayload.Status.ReleaseMirrorJobResult.Coordinates.Namespace)' in mirror_status
assert 'Namespace: jobNamespace' in payload_builder or 'Namespace: jobNamespace' in payload_builder
assert 'flagset.StringVar(&opt.JobNamespace, "job-namespace"' in release_main
assert 'flagset.StringSliceVar(&opt.ReleaseNamespaces, "release-namespace"' in release_main
release_namespace = "ocp"
job_namespace = "ci-release"
watched_jobs = {release_namespace}
creation_coordinate_namespace = job_namespace
mirror_coordinate_namespace = job_namespace
assert creation_coordinate_namespace not in watched_jobs
assert mirror_coordinate_namespace not in watched_jobs
print("PASS: a scoped release-payload controller does not cache Jobs in an independent job namespace.")
print("PASS: both status controllers look up Jobs by the payload coordinate namespace.")
print("PASS: release-controller exposes independent --release-namespace and --job-namespace settings.")
PYRepository: openshift/release-controller
Length of output: 436
Watch Jobs in their coordinate namespaces.
When --release-namespace differs from ReleaseCreationCoordinates.Namespace or ReleaseMirrorCoordinates.Namespace, line 102 excludes the Jobs. The status controllers then treat them as missing. Add a Job namespace scope that covers these namespaces, or reject mismatched configurations.
🤖 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 `@pkg/cmd/release-payload-controller/cmd.go` at line 79, Update the namespace
filtering around the release payload controller’s Job watch so it also includes
the namespaces configured by ReleaseCreationCoordinates.Namespace and
ReleaseMirrorCoordinates.Namespace when they differ from ReleaseNamespace.
Alternatively, validate the configuration and reject mismatched namespaces
before starting the controllers; ensure Jobs are never excluded and subsequently
treated as missing.
Contributor
|
@JoelSpeed: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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.
This PR adds a new
Layeredmode for the release controller and teaches the release-payload-controller how to handle a ReleasePayload that doesn't need any build or mirror step.Important things:
Summary by CodeRabbit
New Features
--release-namespaceoption to limit controller monitoring to a specific namespace.Bug Fixes