From c37a6f5f062adb0a223c76c7d38331990b2ffaae Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 20 Jul 2026 16:16:51 +0100 Subject: [PATCH 1/5] Add Layered mode for Release Controller to handle pre-built layered operator images --- cmd/release-controller-api/http.go | 32 ++++++--- cmd/release-controller-api/http_candidate.go | 2 +- cmd/release-controller-api/http_helper.go | 6 +- cmd/release-controller/layered_mode_test.go | 70 +++++++++++++++++++ cmd/release-controller/sync.go | 66 +++++++++++++---- cmd/release-controller/sync_publish.go | 17 +++-- .../sync_release_payload.go | 38 +++++----- pkg/release-controller/release.go | 13 +++- pkg/release-controller/types.go | 1 + 9 files changed, 196 insertions(+), 49 deletions(-) create mode 100644 cmd/release-controller/layered_mode_test.go diff --git a/cmd/release-controller-api/http.go b/cmd/release-controller-api/http.go index 3699c0980..0c60dbf8e 100644 --- a/cmd/release-controller-api/http.go +++ b/cmd/release-controller-api/http.go @@ -107,7 +107,7 @@ func (c *Controller) findReleaseStreamTags(includeStableTags bool, tags ...strin // TODO: should be refactored to be unsortedSemanticReleaseTags releaseTags := releasecontroller.SortedReleaseTags(r) if includeStableTags { - if version, err := releasecontroller.SemverParseTolerant(r.Config.Name); err == nil || r.Config.As == releasecontroller.ReleaseConfigModeStable { + if version, err := releasecontroller.SemverParseTolerant(r.Config.Name); err == nil || r.Config.As == releasecontroller.ReleaseConfigModeStable || r.Config.As == releasecontroller.ReleaseConfigModeLayered { stable.Releases = append(stable.Releases, releasecontroller.StableRelease{ Release: r, Version: version, @@ -1702,7 +1702,7 @@ func (c *Controller) tableLink(config *releasecontroller.ReleaseConfig, tag imag } if strings.Contains(tag.Name, "nightly") && c.doesInconsistencyExist(tag.Name) { return fmt.Sprintf(`%s `, alert, template.HTMLEscapeString(config.Name), template.HTMLEscapeString(tag.Name), template.HTMLEscapeString(tag.Name), template.HTMLEscapeString(config.Name), template.HTMLEscapeString(tag.Name)) - } else if config.As == releasecontroller.ReleaseConfigModeStable { + } else if config.As == releasecontroller.ReleaseConfigModeStable || config.As == releasecontroller.ReleaseConfigModeLayered { return fmt.Sprintf(`%s`, alert, template.HTMLEscapeString(config.Name), template.HTMLEscapeString(tag.Name), template.HTMLEscapeString(tag.Name)) } else { return fmt.Sprintf(`%s`, alert, template.HTMLEscapeString(config.Name), template.HTMLEscapeString(tag.Name), template.HTMLEscapeString(tag.Name)) @@ -1747,7 +1747,7 @@ func (c *Controller) httpReleases(w http.ResponseWriter, req *http.Request) { "publishDescription": func(r *ReleaseStream) string { streamMessage := generateStreamMessage(r) if len(streamMessage) > 0 { - if r.Release.Config.As == releasecontroller.ReleaseConfigModeStable { + if r.Release.Config.As == releasecontroller.ReleaseConfigModeStable || r.Release.Config.As == releasecontroller.ReleaseConfigModeLayered { searchFunctionPrefix := removeSpecialCharacters(r.Release.Config.Name) searchFunction := fmt.Sprintf("searchTable_%s('%s')", searchFunctionPrefix, searchFunctionPrefix) return fmt.Sprintf("
\n
\n

%s

\n
\n
\n
", streamMessage, searchFunctionPrefix, searchFunction) @@ -1760,6 +1760,10 @@ func (c *Controller) httpReleases(w http.ResponseWriter, req *http.Request) { if len(streamMessage) == 0 { out = append(out, `stable tags`) } + case releasecontroller.ReleaseConfigModeLayered: + if len(streamMessage) == 0 { + out = append(out, `layered releases`) + } default: out = append(out, fmt.Sprintf(`updated when %s/%s changes`, r.Release.Source.Namespace, r.Release.Source.Name)) } @@ -1846,7 +1850,7 @@ func (c *Controller) httpReleases(w http.ResponseWriter, req *http.Request) { Tags: releasecontroller.SortedReleaseTags(r), } var delays []string - if r.Config.As != releasecontroller.ReleaseConfigModeStable && len(s.Tags) > 0 { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered && len(s.Tags) > 0 { if ok, _, queueAfter := releasecontroller.IsReleaseDelayedForInterval(r, s.Tags[0]); ok { delays = append(delays, fmt.Sprintf("waiting for %s", queueAfter.Truncate(time.Second))) } @@ -1857,7 +1861,7 @@ func (c *Controller) httpReleases(w http.ResponseWriter, req *http.Request) { if len(delays) > 0 { s.Delayed = &ReleaseDelay{Message: fmt.Sprintf("Next release may not start: %s", strings.Join(delays, ", "))} } - if r.Config.As != releasecontroller.ReleaseConfigModeStable { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered { s.Upgrades = calculateReleaseUpgrades(r, s.Tags, c.graph, false) } page.Streams = append(page.Streams, s) @@ -2214,7 +2218,7 @@ func (c *Controller) httpReleaseStreamTable(w http.ResponseWriter, req *http.Req "publishDescription": func(r *ReleaseStream) string { streamMessage := generateStreamMessage(r) if len(streamMessage) > 0 { - if r.Release.Config.As == releasecontroller.ReleaseConfigModeStable { + if r.Release.Config.As == releasecontroller.ReleaseConfigModeStable || r.Release.Config.As == releasecontroller.ReleaseConfigModeLayered { searchFunctionPrefix := removeSpecialCharacters(r.Release.Config.Name) searchFunction := fmt.Sprintf("searchTable_%s('%s')", searchFunctionPrefix, searchFunctionPrefix) return fmt.Sprintf("
\n
\n

%s

\n
\n
\n
", streamMessage, searchFunctionPrefix, searchFunction) @@ -2227,6 +2231,10 @@ func (c *Controller) httpReleaseStreamTable(w http.ResponseWriter, req *http.Req if len(streamMessage) == 0 { out = append(out, `stable tags`) } + case releasecontroller.ReleaseConfigModeLayered: + if len(streamMessage) == 0 { + out = append(out, `layered releases`) + } default: out = append(out, fmt.Sprintf(`updated when %s/%s changes`, r.Release.Source.Namespace, r.Release.Source.Name)) } @@ -2305,7 +2313,7 @@ func (c *Controller) httpReleaseStreamTable(w http.ResponseWriter, req *http.Req Tags: releasecontroller.SortedReleaseTags(r), } var delays []string - if r.Config.As != releasecontroller.ReleaseConfigModeStable && len(s.Tags) > 0 { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered && len(s.Tags) > 0 { if ok, _, queueAfter := releasecontroller.IsReleaseDelayedForInterval(r, s.Tags[0]); ok { delays = append(delays, fmt.Sprintf("waiting for %s", queueAfter.Truncate(time.Second))) } @@ -2316,7 +2324,7 @@ func (c *Controller) httpReleaseStreamTable(w http.ResponseWriter, req *http.Req if len(delays) > 0 { s.Delayed = &ReleaseDelay{Message: fmt.Sprintf("Next release may not start: %s", strings.Join(delays, ", "))} } - if r.Config.As != releasecontroller.ReleaseConfigModeStable { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered { s.Upgrades = calculateReleaseUpgrades(r, s.Tags, c.graph, false) } page.TargetStream = s @@ -2377,6 +2385,10 @@ func (c *Controller) httpDashboardOverview(w http.ResponseWriter, req *http.Requ if len(streamMessage) == 0 { out = append(out, `stable tags`) } + case releasecontroller.ReleaseConfigModeLayered: + if len(streamMessage) == 0 { + out = append(out, `layered releases`) + } default: out = append(out, fmt.Sprintf(`updated when %s/%s changes`, r.Release.Source.Namespace, r.Release.Source.Name)) } @@ -2444,7 +2456,7 @@ func (c *Controller) httpDashboardOverview(w http.ResponseWriter, req *http.Requ Tags: releasecontroller.SortedReleaseTags(r), } var delays []string - if r.Config.As != releasecontroller.ReleaseConfigModeStable && len(s.Tags) > 0 { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered && len(s.Tags) > 0 { if ok, _, queueAfter := releasecontroller.IsReleaseDelayedForInterval(r, s.Tags[0]); ok { delays = append(delays, fmt.Sprintf("waiting for %s", queueAfter.Truncate(time.Second))) } @@ -2463,7 +2475,7 @@ func (c *Controller) httpDashboardOverview(w http.ResponseWriter, req *http.Requ if len(delays) > 0 { s.Delayed = &ReleaseDelay{Message: fmt.Sprintf("Next release may not start: %s", strings.Join(delays, ", "))} } - if r.Config.As != releasecontroller.ReleaseConfigModeStable { + if r.Config.As != releasecontroller.ReleaseConfigModeStable && r.Config.As != releasecontroller.ReleaseConfigModeLayered { s.Upgrades = calculateReleaseUpgrades(r, s.Tags, c.graph, true) } page.Streams = append(page.Streams, s) diff --git a/cmd/release-controller-api/http_candidate.go b/cmd/release-controller-api/http_candidate.go index cad5bd21d..a3e7351ae 100644 --- a/cmd/release-controller-api/http_candidate.go +++ b/cmd/release-controller-api/http_candidate.go @@ -303,7 +303,7 @@ func (c *Controller) findReleaseByName(includeStableTags bool, names ...string) } if includeStableTags { - if version, err := releasecontroller.SemverParseTolerant(r.Config.Name); err == nil || r.Config.As == releasecontroller.ReleaseConfigModeStable { + if version, err := releasecontroller.SemverParseTolerant(r.Config.Name); err == nil || r.Config.As == releasecontroller.ReleaseConfigModeStable || r.Config.As == releasecontroller.ReleaseConfigModeLayered { stable.Releases = append(stable.Releases, releasecontroller.StableRelease{ Release: r, Version: version, diff --git a/cmd/release-controller-api/http_helper.go b/cmd/release-controller-api/http_helper.go index 45eada6a3..200d31c8a 100644 --- a/cmd/release-controller-api/http_helper.go +++ b/cmd/release-controller-api/http_helper.go @@ -717,7 +717,7 @@ func renderAlerts(release ReleaseStream) string { func releaseJoin(streams []ReleaseStream, showStableReleases bool) string { releases := []string{} for _, s := range streams { - if !showStableReleases && s.Release.Config.As == releasecontroller.ReleaseConfigModeStable { + if !showStableReleases && (s.Release.Config.As == releasecontroller.ReleaseConfigModeStable || s.Release.Config.As == releasecontroller.ReleaseConfigModeLayered) { continue } releases = append(releases, fmt.Sprintf("%s", template.HTMLEscapeString(s.Release.Config.Name), template.HTMLEscapeString(s.Release.Config.Name))) @@ -939,7 +939,7 @@ func (r preferredReleases) Less(i, j int) bool { if a.Release.Config.Hide && !b.Release.Config.Hide { return false } - aStable, bStable := a.Release.Config.As == releasecontroller.ReleaseConfigModeStable, b.Release.Config.As == releasecontroller.ReleaseConfigModeStable + aStable, bStable := (a.Release.Config.As == releasecontroller.ReleaseConfigModeStable || a.Release.Config.As == releasecontroller.ReleaseConfigModeLayered), (b.Release.Config.As == releasecontroller.ReleaseConfigModeStable || b.Release.Config.As == releasecontroller.ReleaseConfigModeLayered) if aStable && !bStable { return true } @@ -1104,7 +1104,7 @@ func isStaleStatusTag(tag imagev1.NamedTagEventList, target *imagev1.ImageStream func pruneEndOfLifeTags(page *ReleasePage, endOfLifePrefixes sets.Set[string]) { for i := range page.Streams { stream := &page.Streams[i] - if stream.Release.Config.As == releasecontroller.ReleaseConfigModeStable { + if stream.Release.Config.As == releasecontroller.ReleaseConfigModeStable || stream.Release.Config.As == releasecontroller.ReleaseConfigModeLayered { var tags []*imagev1.TagReference for _, tag := range stream.Tags { if version, err := releasecontroller.SemverParseTolerant(tag.Name); err == nil { diff --git a/cmd/release-controller/layered_mode_test.go b/cmd/release-controller/layered_mode_test.go new file mode 100644 index 000000000..6e55aa765 --- /dev/null +++ b/cmd/release-controller/layered_mode_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "testing" + + releasecontroller "github.com/openshift/release-controller/pkg/release-controller" +) + +func TestLayeredModeConfiguration(t *testing.T) { + testCases := []struct { + name string + configJSON string + expectError bool + errorMsg string + }{ + { + name: "Valid layered mode without 'to' field", + configJSON: `{"name": "test-layered", "as": "Layered"}`, + expectError: false, + }, + { + name: "Layered mode with optional 'to' field is allowed", + configJSON: `{"name": "test-layered", "as": "Layered", "to": "releases"}`, + expectError: false, + }, + { + name: "Stable mode without 'to' field is valid", + configJSON: `{"name": "test-stable", "as": "Stable"}`, + expectError: false, + }, + { + name: "Integration mode without 'to' field should error", + configJSON: `{"name": "test-integration"}`, + expectError: true, + errorMsg: "release must specify 'to' unless 'as' is 'Stable' or 'Layered'", + }, + { + name: "Integration mode with 'to' field is valid", + configJSON: `{"name": "test-integration", "to": "releases"}`, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config, err := releasecontroller.ParseReleaseConfig(tc.configJSON, nil) + + if tc.expectError { + if err == nil { + t.Errorf("Expected error but got none") + return + } + if tc.errorMsg != "" && err.Error() != tc.errorMsg { + t.Errorf("Expected error message %q, got %q", tc.errorMsg, err.Error()) + } + return + } + + if err != nil { + t.Errorf("Expected no error but got: %v", err) + return + } + + if config == nil { + t.Errorf("Expected valid config but got nil") + return + } + }) + } +} diff --git a/cmd/release-controller/sync.go b/cmd/release-controller/sync.go index f1d91c63b..d5a36497d 100644 --- a/cmd/release-controller/sync.go +++ b/cmd/release-controller/sync.go @@ -154,7 +154,8 @@ func calculateSyncActions(release *releasecontroller.Release, now time.Time) (ad ) target := release.Target - shouldAdopt := release.Config.As == releasecontroller.ReleaseConfigModeStable + shouldAdopt := release.Config.As == releasecontroller.ReleaseConfigModeStable || + release.Config.As == releasecontroller.ReleaseConfigModeLayered tags := make([]*imagev1.TagReference, 0, len(target.Spec.Tags)) for i := range target.Spec.Tags { @@ -177,7 +178,9 @@ func calculateSyncActions(release *releasecontroller.Release, now time.Time) (ad continue } // check annotations when using the target as tag source - if release.Config.As != releasecontroller.ReleaseConfigModeStable && tag.Annotations[releasecontroller.ReleaseAnnotationSource] != fmt.Sprintf("%s/%s", release.Source.Namespace, release.Source.Name) { + if release.Config.As != releasecontroller.ReleaseConfigModeStable && + release.Config.As != releasecontroller.ReleaseConfigModeLayered && + tag.Annotations[releasecontroller.ReleaseAnnotationSource] != fmt.Sprintf("%s/%s", release.Source.Namespace, release.Source.Name) { continue } // if the name has changed, consider the tag abandoned (admin is responsible for cleaning it up) @@ -244,7 +247,7 @@ func calculateSyncActions(release *releasecontroller.Release, now time.Time) (ad } switch release.Config.As { - case releasecontroller.ReleaseConfigModeStable: + case releasecontroller.ReleaseConfigModeStable, releasecontroller.ReleaseConfigModeLayered: hasNewImages = false inputImageHash = "" removeTags = nil @@ -399,6 +402,32 @@ func (c *Controller) syncPending(release *releasecontroller.Release, pendingTags } } return nil + + case releasecontroller.ReleaseConfigModeLayered: + // New layered mode - skip payload building, go directly to ready + for _, tag := range pendingTags { + if len(tag.Annotations[releasecontroller.ReleaseAnnotationImageHash]) == 0 { + // Set a hash based on the single input image + hash := fmt.Sprintf("layered-%s-%d", tag.Name, *tag.Generation) + if err := c.setReleaseAnnotation(release, tag.Annotations[releasecontroller.ReleaseAnnotationPhase], + map[string]string{releasecontroller.ReleaseAnnotationImageHash: hash}, tag.Name); err != nil { + return err + } + continue + } + + // Create ReleasePayload object for verification tracking + _, err = c.ensureReleasePayload(release, tag) + if err != nil { + return err + } + + // Mark as ready immediately since we skip payload building + if err := c.markReleaseReady(release, nil, tag.Name); err != nil { + return err + } + } + return nil } if len(pendingTags) > 1 { @@ -465,15 +494,21 @@ func (c *Controller) syncReady(release *releasecontroller.Release) error { } for _, releaseTag := range readyTags { - mirror, err := releasecontroller.GetMirror(release, releaseTag.Name, c.releaseLister) - if err != nil { - klog.Errorf("Failed to identify `from` mirror for creation of release mirror job: %v", err) - } else if _, err := c.ensureReleaseMirrorJob(release, releaseTag.Name, mirror); err != nil { - klog.Errorf("Failed to create release mirror job: %v", err) + // Skip mirroring for layered releases since they use pre-existing single images + if release.Config.As != releasecontroller.ReleaseConfigModeLayered { + mirror, err := releasecontroller.GetMirror(release, releaseTag.Name, c.releaseLister) + if err != nil { + klog.Errorf("Failed to identify `from` mirror for creation of release mirror job: %v", err) + } else if _, err := c.ensureReleaseMirrorJob(release, releaseTag.Name, mirror); err != nil { + klog.Errorf("Failed to create release mirror job: %v", err) + } } - if err := c.ensureReleaseUpgradeJobs(release, releaseTag); err != nil { - klog.Errorf("unable to launch release upgrade jobs for %q: %v", releaseTag.Name, err) + // Skip upgrade jobs for layered releases since they represent single components + if release.Config.As != releasecontroller.ReleaseConfigModeLayered { + if err := c.ensureReleaseUpgradeJobs(release, releaseTag); err != nil { + klog.Errorf("unable to launch release upgrade jobs for %q: %v", releaseTag.Name, err) + } } payload, verifyStatus, err := c.getReleasePayloadVerificationState(release, releaseTag.Name) @@ -540,7 +575,15 @@ func (c *Controller) syncAccepted(release *releasecontroller.Release) error { if len(ns) == 0 { ns = release.Target.Namespace } - if err := c.ensureImageStreamMatchesRelease(release, ns, publishType.ImageStreamRef.Name, newestAccepted.Name, publishType.ImageStreamRef.Tags, publishType.ImageStreamRef.ExcludeTags); err != nil { + + // For layered releases, we need to publish the specific tag that was verified + // rather than all tags within the image stream. + tagNames := publishType.ImageStreamRef.Tags + if len(tagNames) == 0 && release.Config.As == releasecontroller.ReleaseConfigModeLayered { + tagNames = []string{newestAccepted.Name} + } + + if err := c.ensureImageStreamMatchesRelease(release, ns, publishType.ImageStreamRef.Name, newestAccepted.Name, tagNames, publishType.ImageStreamRef.ExcludeTags); err != nil { errs = append(errs, fmt.Errorf("unable to update image stream for publish step %s: %v", name, err)) continue } @@ -636,4 +679,3 @@ func getRejectionDetails(payload *v1alpha1.ReleasePayload) (string, string) { } return "VerificationFailed", "release verification failed" } - diff --git a/cmd/release-controller/sync_publish.go b/cmd/release-controller/sync_publish.go index f93f6af4d..41c5ce7e3 100644 --- a/cmd/release-controller/sync_publish.go +++ b/cmd/release-controller/sync_publish.go @@ -71,10 +71,19 @@ func (c *Controller) ensureImageStreamMatchesRelease(release *releasecontroller. return nil } - mirror, err := releasecontroller.GetMirror(release, from, c.releaseLister) - if err != nil { - klog.V(2).Infof("Error getting release mirror image stream: %v", err) - return nil + var mirror *imagev1.ImageStream + var err error + + // For layered releases, use the release target imagestream directly since there's no separate mirror + if release.Config.As == releasecontroller.ReleaseConfigModeLayered { + mirror = release.Target + klog.V(4).Infof("Using release target imagestream directly for layered release publishing: %s/%s", release.Target.Namespace, release.Target.Name) + } else { + mirror, err = releasecontroller.GetMirror(release, from, c.releaseLister) + if err != nil { + klog.V(2).Infof("Error getting release mirror image stream: %v", err) + return nil + } } lister := c.publishLister.ImageStreams(toNamespace) diff --git a/cmd/release-controller/sync_release_payload.go b/cmd/release-controller/sync_release_payload.go index a8fd90935..da79dbc02 100644 --- a/cmd/release-controller/sync_release_payload.go +++ b/cmd/release-controller/sync_release_payload.go @@ -44,14 +44,7 @@ func newReleasePayload(release *releasecontroller.Release, tag *imagev1.TagRefer Namespace: release.Target.Namespace, ImagestreamName: release.Target.Name, ImagestreamTagName: name, - StreamName: release.Config.Name, - }, - PayloadCreationConfig: v1alpha1.PayloadCreationConfig{ - ReleaseCreationCoordinates: v1alpha1.ReleaseCreationCoordinates{ - Namespace: jobNamespace, - ReleaseCreationJobName: name, - }, - ProwCoordinates: v1alpha1.ProwCoordinates{Namespace: prowNamespace}, + StreamName: release.Config.Name, }, PayloadVerificationConfig: v1alpha1.PayloadVerificationConfig{ BlockingJobs: []v1alpha1.CIConfiguration{}, @@ -63,6 +56,27 @@ func newReleasePayload(release *releasecontroller.Release, tag *imagev1.TagRefer }, } + // Only add PayloadCreationConfig for releases that need payload creation jobs + // Layered releases use pre-existing images and don't need creation jobs + if release.Config.As != releasecontroller.ReleaseConfigModeLayered { + payload.Spec.PayloadCreationConfig = v1alpha1.PayloadCreationConfig{ + ReleaseCreationCoordinates: v1alpha1.ReleaseCreationCoordinates{ + Namespace: jobNamespace, + ReleaseCreationJobName: name, + }, + ProwCoordinates: v1alpha1.ProwCoordinates{Namespace: prowNamespace}, + } + + // We should only be populating the ReleaseMirrorCoordinates if/when they are actually defined... + // And only for releases that have PayloadCreationConfig (not layered releases) + if release.Config.AlternateImageRepository != "" && release.Config.AlternateImageRepositorySecretName != "" { + payload.Spec.PayloadCreationConfig.ReleaseMirrorCoordinates = v1alpha1.ReleaseMirrorCoordinates{ + Namespace: jobNamespace, + ReleaseMirrorJobName: releaseMirrorJobName(name), + } + } + } + if releasecontroller.IsReferenceReleaseTag(release, tag) { payload.Spec.ReleaseCoordinates = []v1alpha1.ReleaseCoordinates{{ Repository: release.Config.ReferenceRelease.PullRepository, @@ -75,14 +89,6 @@ func newReleasePayload(release *releasecontroller.Release, tag *imagev1.TagRefer }} } - // We should only be populating the ReleaseMirrorCoordinates if/when they are actually defined... - if release.Config.AlternateImageRepository != "" && release.Config.AlternateImageRepositorySecretName != "" { - payload.Spec.PayloadCreationConfig.ReleaseMirrorCoordinates = v1alpha1.ReleaseMirrorCoordinates{ - Namespace: jobNamespace, - ReleaseMirrorJobName: releaseMirrorJobName(name), - } - } - // Sort the ReleaseVerification items into a consistent order var sortedKeys []string for key := range verificationJobs { diff --git a/pkg/release-controller/release.go b/pkg/release-controller/release.go index 534977161..b4cf86387 100644 --- a/pkg/release-controller/release.go +++ b/pkg/release-controller/release.go @@ -114,6 +114,13 @@ func ReleaseDefinition(is *imagev1.ImageStream, releaseConfigCache *lru.Cache, e Config: cfg, } return r, true, nil + case ReleaseConfigModeLayered: + r := &Release{ + Source: is, + Target: is, + Config: cfg, + } + return r, true, nil default: targetImageStream, err := releaseLister.ImageStreams(is.Namespace).Get(cfg.To) if errors.IsNotFound(err) { @@ -152,8 +159,8 @@ func ParseReleaseConfig(data string, configCache *lru.Cache) (*ReleaseConfig, er if len(cfg.Name) == 0 { return nil, fmt.Errorf("release config must have a valid name") } - if len(cfg.To) == 0 && cfg.As != ReleaseConfigModeStable { - return nil, fmt.Errorf("release must specify 'to' unless 'as' is 'Stable'") + if len(cfg.To) == 0 && cfg.As != ReleaseConfigModeStable && cfg.As != ReleaseConfigModeLayered { + return nil, fmt.Errorf("release must specify 'to' unless 'as' is 'Stable' or 'Layered'") } for name, verify := range cfg.Verify { if len(name) == 0 { @@ -608,7 +615,7 @@ func GetImageInfo(releaseInfo ReleaseInfo, architecture, pullSpec string) (*imag } func GetVerificationJobs(rcCache *lru.Cache, eventRecorder record.EventRecorder, lister *MultiImageStreamLister, release *Release, releaseTag *imagev1.TagReference, artSuffix string) (map[string]ReleaseVerification, error) { - if release.Config.As != ReleaseConfigModeStable || artSuffix == "" { + if release.Config.As != ReleaseConfigModeStable && release.Config.As != ReleaseConfigModeLayered || artSuffix == "" { return release.Config.Verify, nil } jobs := make(map[string]ReleaseVerification) diff --git a/pkg/release-controller/types.go b/pkg/release-controller/types.go index 535b7cc45..72bad8fc5 100644 --- a/pkg/release-controller/types.go +++ b/pkg/release-controller/types.go @@ -556,6 +556,7 @@ const ( ReleaseVerificationStatePending = "Pending" ReleaseConfigModeStable = "Stable" + ReleaseConfigModeLayered = "Layered" // ReferencePayloadTagPrefix is prepended to release names when pushing // to ReferenceRepository, so that image cleanup tooling can identify From 50a9300c57d08d1c432fb3794d5b62f1d1d47b15 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 24 Jul 2026 14:48:31 +0100 Subject: [PATCH 2/5] Add release-namespace flag to release-payload-controller --- pkg/cmd/release-payload-controller/cmd.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/release-payload-controller/cmd.go b/pkg/cmd/release-payload-controller/cmd.go index e5d81ee0d..2dc3f09b1 100644 --- a/pkg/cmd/release-payload-controller/cmd.go +++ b/pkg/cmd/release-payload-controller/cmd.go @@ -30,6 +30,7 @@ import ( type Options struct { controllerContext *controllercmd.ControllerContext ReleaseQualifiersConfigPath string + ReleaseNamespace string // BigQuery Options GoogleProjectID string @@ -75,6 +76,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { 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.") goFlagSet := flag.NewFlagSet("jira", flag.ContinueOnError) o.jira.AddFlags(goFlagSet) @@ -97,7 +99,7 @@ func (o *Options) Run(ctx context.Context) error { } // Batch Job Informers - kubeFactory := informers.NewSharedInformerFactory(kubeClient, controllerDefaultResyncDuration) + kubeFactory := informers.NewSharedInformerFactoryWithOptions(kubeClient, controllerDefaultResyncDuration, informers.WithNamespace(o.ReleaseNamespace)) batchJobInformer := kubeFactory.Batch().V1().Jobs() // ReleasePayload Informers @@ -106,7 +108,7 @@ func (o *Options) Run(ctx context.Context) error { klog.Fatalf("Error building releasePayload clientset: %s", err.Error()) } - releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactory(releasePayloadClient, controllerDefaultResyncDuration) + releasePayloadInformerFactory := releasepayloadinformers.NewSharedInformerFactoryWithOptions(releasePayloadClient, controllerDefaultResyncDuration, releasepayloadinformers.WithNamespace(o.ReleaseNamespace)) releasePayloadInformer := releasePayloadInformerFactory.Release().V1alpha1().ReleasePayloads() // ProwJob Informers @@ -124,7 +126,7 @@ func (o *Options) Run(ctx context.Context) error { klog.Fatalf("Error building imagestream clientset: %s", err.Error()) } - imageStreamInformerFactory := imageinformers.NewSharedInformerFactory(imageStreamClient, controllerDefaultResyncDuration) + imageStreamInformerFactory := imageinformers.NewSharedInformerFactoryWithOptions(imageStreamClient, controllerDefaultResyncDuration, imageinformers.WithNamespace(o.ReleaseNamespace)) imageStreamInformer := imageStreamInformerFactory.Image().V1().ImageStreams() // Initialize release qualifiers config loader From 9483dbf21ae8ed72ee391b5e4ddbcf0ccc9db610 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 24 Jul 2026 15:23:51 +0100 Subject: [PATCH 3/5] Fixup prow coordinates in layered --- cmd/release-controller/sync_release_payload.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/release-controller/sync_release_payload.go b/cmd/release-controller/sync_release_payload.go index da79dbc02..c702ef1af 100644 --- a/cmd/release-controller/sync_release_payload.go +++ b/cmd/release-controller/sync_release_payload.go @@ -40,6 +40,9 @@ func newReleasePayload(release *releasecontroller.Release, tag *imagev1.TagRefer Namespace: release.Target.Namespace, }, Spec: v1alpha1.ReleasePayloadSpec{ + PayloadCreationConfig: v1alpha1.PayloadCreationConfig{ + ProwCoordinates: v1alpha1.ProwCoordinates{Namespace: prowNamespace}, + }, PayloadCoordinates: v1alpha1.PayloadCoordinates{ Namespace: release.Target.Namespace, ImagestreamName: release.Target.Name, @@ -56,15 +59,12 @@ func newReleasePayload(release *releasecontroller.Release, tag *imagev1.TagRefer }, } - // Only add PayloadCreationConfig for releases that need payload creation jobs + // Only add ReleaseCreationCoordinates for releases that need payload creation jobs // Layered releases use pre-existing images and don't need creation jobs if release.Config.As != releasecontroller.ReleaseConfigModeLayered { - payload.Spec.PayloadCreationConfig = v1alpha1.PayloadCreationConfig{ - ReleaseCreationCoordinates: v1alpha1.ReleaseCreationCoordinates{ - Namespace: jobNamespace, - ReleaseCreationJobName: name, - }, - ProwCoordinates: v1alpha1.ProwCoordinates{Namespace: prowNamespace}, + payload.Spec.PayloadCreationConfig.ReleaseCreationCoordinates = v1alpha1.ReleaseCreationCoordinates{ + Namespace: jobNamespace, + ReleaseCreationJobName: name, } // We should only be populating the ReleaseMirrorCoordinates if/when they are actually defined... From f370695a47397081df9ff476e2d5d9c05865d833 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 24 Jul 2026 17:15:52 +0100 Subject: [PATCH 4/5] Update release-payload-controller to accept empty release creation and mirroring config --- .../layered_reference_test.go | 121 ++++++++++++++++++ .../payload_creation_controller.go | 48 +++++++ .../payload_mirror_controller.go | 40 ++++++ .../payload_mirror_controller_test.go | 113 ++++++++++++++++ .../release_creation_job_controller.go | 4 +- 5 files changed, 324 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/release-payload-controller/layered_reference_test.go diff --git a/pkg/cmd/release-payload-controller/layered_reference_test.go b/pkg/cmd/release-payload-controller/layered_reference_test.go new file mode 100644 index 000000000..ce64fbdff --- /dev/null +++ b/pkg/cmd/release-payload-controller/layered_reference_test.go @@ -0,0 +1,121 @@ +package release_payload_controller + +import ( + "context" + "testing" + + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" + "github.com/openshift/release-controller/pkg/apis/release/v1alpha1" + "github.com/openshift/release-controller/pkg/client/clientset/versioned/fake" + releasepayloadinformers "github.com/openshift/release-controller/pkg/client/informers/externalversions" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/clock" +) + +func TestPayloadCreationWithoutCreationConfig(t *testing.T) { + testCases := []struct { + name string + hasCreationConfig bool + expectCreatedCondition metav1.ConditionStatus + expectFailedCondition metav1.ConditionStatus + expectMessage string + }{ + { + name: "Payload with creation config follows normal job-based flow", + hasCreationConfig: true, + expectCreatedCondition: metav1.ConditionUnknown, // Waits for job completion + expectFailedCondition: metav1.ConditionUnknown, + }, + { + name: "Payload without creation config marked as created immediately", + hasCreationConfig: false, + expectCreatedCondition: metav1.ConditionTrue, // Immediate success + expectFailedCondition: metav1.ConditionFalse, + expectMessage: "Release payload using pre-existing image, no creation job needed", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a test payload with coordinates (like a real release payload) + payload := &v1alpha1.ReleasePayload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-release", + Namespace: "test-namespace", + }, + Spec: v1alpha1.ReleasePayloadSpec{ + PayloadCoordinates: v1alpha1.PayloadCoordinates{ + Namespace: "test-namespace", + ImagestreamName: "releases", + ImagestreamTagName: "test-release", + }, + PayloadType: v1alpha1.PayloadTypeLocal, + }, + } + + if tc.hasCreationConfig { + payload.Spec.PayloadCreationConfig = v1alpha1.PayloadCreationConfig{ + ReleaseCreationCoordinates: v1alpha1.ReleaseCreationCoordinates{ + Namespace: "test-jobs", + ReleaseCreationJobName: "test-release", + }, + } + } + + // Set up fake client and controller + fakeClient := fake.NewSimpleClientset(payload) + informerFactory := releasepayloadinformers.NewSharedInformerFactory(fakeClient, 0) + payloadInformer := informerFactory.Release().V1alpha1().ReleasePayloads() + + controller, err := NewPayloadCreationController( + payloadInformer, + fakeClient.ReleaseV1alpha1(), + events.NewInMemoryRecorder("test", clock.RealClock{}), + ) + if err != nil { + t.Fatalf("Failed to create controller: %v", err) + } + + // Start informer and wait for cache sync + informerFactory.Start(make(chan struct{})) + cache.WaitForCacheSync(make(chan struct{}), payloadInformer.Informer().HasSynced) + + // Sync the payload + err = controller.sync(context.Background(), "test-namespace/test-release") + if err != nil { + t.Fatalf("Sync failed: %v", err) + } + + // Get the updated payload + updatedPayload, err := fakeClient.ReleaseV1alpha1().ReleasePayloads("test-namespace").Get(context.Background(), "test-release", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get updated payload: %v", err) + } + + // Check the conditions + createdCond := v1helpers.FindCondition(updatedPayload.Status.Conditions, v1alpha1.ConditionPayloadCreated) + failedCond := v1helpers.FindCondition(updatedPayload.Status.Conditions, v1alpha1.ConditionPayloadFailed) + + if createdCond == nil { + t.Fatalf("PayloadCreated condition not found") + } + if failedCond == nil { + t.Fatalf("PayloadFailed condition not found") + } + + if createdCond.Status != tc.expectCreatedCondition { + t.Errorf("Expected PayloadCreated condition status %s, got %s", tc.expectCreatedCondition, createdCond.Status) + } + + if failedCond.Status != tc.expectFailedCondition { + t.Errorf("Expected PayloadFailed condition status %s, got %s", tc.expectFailedCondition, failedCond.Status) + } + + if tc.expectMessage != "" && createdCond.Message != tc.expectMessage { + t.Errorf("Expected message %q, got %q", tc.expectMessage, createdCond.Message) + } + }) + } +} \ No newline at end of file diff --git a/pkg/cmd/release-payload-controller/payload_creation_controller.go b/pkg/cmd/release-payload-controller/payload_creation_controller.go index b20335b73..d7432ed01 100644 --- a/pkg/cmd/release-payload-controller/payload_creation_controller.go +++ b/pkg/cmd/release-payload-controller/payload_creation_controller.go @@ -115,6 +115,37 @@ func (c *PayloadCreationController) sync(ctx context.Context, key string) error return nil } + // If we already have coordinates and no creation config, then we skip the release creation phase + // and assume that the release has already been compiled and is ready to be used. + if originalReleasePayload.Spec.PayloadCoordinates != (v1alpha1.PayloadCoordinates{}) && + originalReleasePayload.Spec.PayloadCreationConfig.ReleaseCreationCoordinates == (v1alpha1.ReleaseCreationCoordinates{}) { + preCreatedImageConditions := getPreCreatedImageCreationConditions() + + releasePayload := originalReleasePayload.DeepCopy() + for _, condition := range preCreatedImageConditions { + v1helpers.SetCondition(&releasePayload.Status.Conditions, condition) + } + releasepayloadhelpers.CanonicalizeReleasePayloadStatus(releasePayload) + + // Set the job status to success to indicate that the release has already been created. + // This short-circuits the release creation job controller and the release creation status controller. + // It also indicates to the release payload accepted controller that the release has already been created. + releasePayload.Status.ReleaseCreationJobResult = v1alpha1.ReleaseCreationJobResult{ + Status: v1alpha1.ReleaseCreationJobSuccess, + Message: "Release payload using pre-existing image, no creation job needed", + } + + if reflect.DeepEqual(originalReleasePayload, releasePayload) { + return nil + } + + _, err := c.releasePayloadClient.ReleasePayloads(releasePayload.Namespace).UpdateStatus(ctx, releasePayload, metav1.UpdateOptions{}) + if errors.IsNotFound(err) { + return nil + } + return err + } + createdCondition := &metav1.Condition{ Type: v1alpha1.ConditionPayloadCreated, Status: metav1.ConditionUnknown, @@ -161,3 +192,20 @@ func (c *PayloadCreationController) sync(ctx context.Context, key string) error return nil } + +func getPreCreatedImageCreationConditions() []metav1.Condition { + createdCondition := metav1.Condition{ + Type: v1alpha1.ConditionPayloadCreated, + Status: metav1.ConditionTrue, + Reason: ReleasePayloadCreatedReason, + Message: "Release payload using pre-existing image, no creation job needed", + } + failedCondition := metav1.Condition{ + Type: v1alpha1.ConditionPayloadFailed, + Status: metav1.ConditionFalse, + Reason: ReleasePayloadFailedReason, + Message: "Release payload using pre-existing image, no creation job needed", + } + + return []metav1.Condition{createdCondition, failedCondition} +} diff --git a/pkg/cmd/release-payload-controller/payload_mirror_controller.go b/pkg/cmd/release-payload-controller/payload_mirror_controller.go index af475488c..ab88c2891 100644 --- a/pkg/cmd/release-payload-controller/payload_mirror_controller.go +++ b/pkg/cmd/release-payload-controller/payload_mirror_controller.go @@ -115,6 +115,30 @@ func (c *PayloadMirrorController) sync(ctx context.Context, key string) error { return nil } + // If we already have coordinates and no mirror config, then we skip the release mirror phase + // and assume that the release uses pre-existing images and doesn't need mirroring. + if originalReleasePayload.Spec.PayloadCoordinates != (v1alpha1.PayloadCoordinates{}) && + originalReleasePayload.Spec.PayloadCreationConfig.ReleaseMirrorCoordinates == (v1alpha1.ReleaseMirrorCoordinates{}) { + + preCreatedImageConditions := getPreCreatedImageMirrorConditions() + + releasePayload := originalReleasePayload.DeepCopy() + for _, condition := range preCreatedImageConditions { + v1helpers.SetCondition(&releasePayload.Status.Conditions, condition) + } + releasepayloadhelpers.CanonicalizeReleasePayloadStatus(releasePayload) + + if reflect.DeepEqual(originalReleasePayload, releasePayload) { + return nil + } + + _, err := c.releasePayloadClient.ReleasePayloads(releasePayload.Namespace).UpdateStatus(ctx, releasePayload, metav1.UpdateOptions{}) + if errors.IsNotFound(err) { + return nil + } + return err + } + createdCondition := &metav1.Condition{ Type: v1alpha1.ConditionPayloadMirrored, Status: metav1.ConditionUnknown, @@ -161,3 +185,19 @@ func (c *PayloadMirrorController) sync(ctx context.Context, key string) error { return nil } + +func getPreCreatedImageMirrorConditions() []metav1.Condition { + createdCondition := metav1.Condition{ + Type: v1alpha1.ConditionPayloadMirrored, + Status: metav1.ConditionTrue, + Reason: ReleasePayloadMirroredReason, + Message: "Release payload using pre-existing image, no mirror job needed", + } + failedCondition := metav1.Condition{ + Type: v1alpha1.ConditionPayloadMirrorFailed, + Status: metav1.ConditionFalse, + Reason: ReleasePayloadMirrorFailedReason, + Message: "Release payload using pre-existing image, no mirror job needed", + } + return []metav1.Condition{createdCondition, failedCondition} +} diff --git a/pkg/cmd/release-payload-controller/payload_mirror_controller_test.go b/pkg/cmd/release-payload-controller/payload_mirror_controller_test.go index fbed62694..ae91c6c6b 100644 --- a/pkg/cmd/release-payload-controller/payload_mirror_controller_test.go +++ b/pkg/cmd/release-payload-controller/payload_mirror_controller_test.go @@ -277,3 +277,116 @@ func TestPayloadMirrorSync(t *testing.T) { }) } } + +func TestPayloadMirrorWithoutMirrorConfig(t *testing.T) { + testCases := []struct { + name string + hasMirrorConfig bool + expectMirroredCondition metav1.ConditionStatus + expectFailedCondition metav1.ConditionStatus + expectMessage string + }{ + { + name: "Payload with mirror config follows normal job-based flow", + hasMirrorConfig: true, + expectMirroredCondition: metav1.ConditionUnknown, // Waits for job completion + expectFailedCondition: metav1.ConditionUnknown, + }, + { + name: "Payload without mirror config marked as mirrored immediately", + hasMirrorConfig: false, + expectMirroredCondition: metav1.ConditionTrue, // Immediate success + expectFailedCondition: metav1.ConditionFalse, + expectMessage: "Release payload using pre-existing image, no mirror job needed", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a test payload with coordinates (like a real release payload) + payload := &v1alpha1.ReleasePayload{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-release", + Namespace: "test-namespace", + }, + Spec: v1alpha1.ReleasePayloadSpec{ + PayloadCoordinates: v1alpha1.PayloadCoordinates{ + Namespace: "test-namespace", + ImagestreamName: "releases", + ImagestreamTagName: "test-release", + }, + PayloadType: v1alpha1.PayloadTypeLocal, + }, + } + + if tc.hasMirrorConfig { + payload.Spec.PayloadCreationConfig = v1alpha1.PayloadCreationConfig{ + ReleaseMirrorCoordinates: v1alpha1.ReleaseMirrorCoordinates{ + Namespace: "test-jobs", + ReleaseMirrorJobName: "test-release-mirror", + }, + } + } + + // Set up fake client and controller + fakeClient := fake.NewSimpleClientset(payload) + informerFactory := releasepayloadinformers.NewSharedInformerFactory(fakeClient, 0) + payloadInformer := informerFactory.Release().V1alpha1().ReleasePayloads() + + controller, err := NewPayloadMirrorController( + payloadInformer, + fakeClient.ReleaseV1alpha1(), + events.NewInMemoryRecorder("test", clock.RealClock{}), + ) + if err != nil { + t.Fatalf("Failed to create controller: %v", err) + } + + // Start informer and wait for cache sync + informerFactory.Start(make(chan struct{})) + cache.WaitForCacheSync(make(chan struct{}), payloadInformer.Informer().HasSynced) + + // Sync the payload + err = controller.sync(context.Background(), "test-namespace/test-release") + if err != nil { + t.Fatalf("Sync failed: %v", err) + } + + // Get the updated payload + updatedPayload, err := fakeClient.ReleaseV1alpha1().ReleasePayloads("test-namespace").Get(context.Background(), "test-release", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get updated payload: %v", err) + } + + // Check the conditions + var mirroredCond, failedCond *metav1.Condition + for _, cond := range updatedPayload.Status.Conditions { + if cond.Type == v1alpha1.ConditionPayloadMirrored { + mirroredCond = &cond + } + if cond.Type == v1alpha1.ConditionPayloadMirrorFailed { + failedCond = &cond + } + } + + if mirroredCond == nil { + t.Fatalf("PayloadMirrored condition not found") + } + if failedCond == nil { + t.Fatalf("PayloadMirrorFailed condition not found") + } + + if mirroredCond.Status != tc.expectMirroredCondition { + t.Errorf("Expected PayloadMirrored condition status %s, got %s", tc.expectMirroredCondition, mirroredCond.Status) + } + + if failedCond.Status != tc.expectFailedCondition { + t.Errorf("Expected PayloadMirrorFailed condition status %s, got %s", tc.expectFailedCondition, failedCond.Status) + } + + if tc.expectMessage != "" && mirroredCond.Message != tc.expectMessage { + t.Errorf("Expected message %q, got %q", tc.expectMessage, mirroredCond.Message) + } + }) + } +} diff --git a/pkg/cmd/release-payload-controller/release_creation_job_controller.go b/pkg/cmd/release-payload-controller/release_creation_job_controller.go index d88a8952c..3f25207f9 100644 --- a/pkg/cmd/release-payload-controller/release_creation_job_controller.go +++ b/pkg/cmd/release-payload-controller/release_creation_job_controller.go @@ -85,8 +85,8 @@ func (c *ReleaseCreationJobController) sync(ctx context.Context, key string) err return err } - // If the Coordinates are already set, then don't do anything... - if len(originalReleasePayload.Status.ReleaseCreationJobResult.Coordinates.Namespace) > 0 && len(originalReleasePayload.Status.ReleaseCreationJobResult.Coordinates.Name) > 0 { + // If the Coordinates are already set, or the job has otherwise been set to success, then don't do anything... + if len(originalReleasePayload.Status.ReleaseCreationJobResult.Coordinates.Namespace) > 0 && len(originalReleasePayload.Status.ReleaseCreationJobResult.Coordinates.Name) > 0 || originalReleasePayload.Status.ReleaseCreationJobResult.Status == v1alpha1.ReleaseCreationJobSuccess { return nil } From c41906351147fd5133390dc1ef1667c90834560f Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 6 Aug 2026 17:14:59 +0100 Subject: [PATCH 5/5] Add a new external mirroring publish strategy for pushing accepted releases --- cmd/release-controller/layered_mode_test.go | 79 +++++++++++++ cmd/release-controller/sync.go | 5 + cmd/release-controller/sync_publish.go | 124 ++++++++++++++++++++ pkg/release-controller/release.go | 8 ++ pkg/release-controller/types.go | 18 +++ 5 files changed, 234 insertions(+) diff --git a/cmd/release-controller/layered_mode_test.go b/cmd/release-controller/layered_mode_test.go index 6e55aa765..10737e910 100644 --- a/cmd/release-controller/layered_mode_test.go +++ b/cmd/release-controller/layered_mode_test.go @@ -68,3 +68,82 @@ func TestLayeredModeConfiguration(t *testing.T) { }) } } + +func TestExternalRegistryPublishValidation(t *testing.T) { + testCases := []struct { + name string + configJSON string + expectError bool + errorMsg string + }{ + { + name: "Valid external registry publish configuration", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/repo", "secretName": "test-secret"}}}}`, + expectError: false, + }, + { + name: "External registry publish missing registry", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"secretName": "test-secret"}}}}`, + expectError: true, + errorMsg: "externalRegistry publish for ext-mirror has no registry", + }, + { + name: "External registry publish missing secretName", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/repo"}}}}`, + expectError: true, + errorMsg: "externalRegistry publish for ext-mirror has no secretName", + }, + { + name: "External registry publish with empty registry", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "", "secretName": "test-secret"}}}}`, + expectError: true, + errorMsg: "externalRegistry publish for ext-mirror has no registry", + }, + { + name: "External registry publish with empty secretName", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/repo", "secretName": ""}}}}`, + expectError: true, + errorMsg: "externalRegistry publish for ext-mirror has no secretName", + }, + { + name: "External registry publish with optional fields", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/repo", "secretName": "test-secret", "tags": ["latest", "v1.0"], "excludeTags": ["dev"]}}}}`, + expectError: false, + }, + { + name: "External registry publish with override CLI image", + configJSON: `{"name": "test", "to": "releases", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/repo", "secretName": "test-secret", "overrideCLIImage": "quay.io/openshift/cli:latest"}}}}`, + expectError: false, + }, + { + name: "Layered mode with external registry and override CLI image", + configJSON: `{"name": "test-layered", "as": "Layered", "publish": {"ext-mirror": {"externalRegistry": {"registry": "quay.io/test/layered", "secretName": "test-secret", "overrideCLIImage": "registry.ci.openshift.org/ocp/4.17:cli"}}}}`, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config, err := releasecontroller.ParseReleaseConfig(tc.configJSON, nil) + + if tc.expectError { + if err == nil { + t.Errorf("Expected error but got none") + return + } + + if tc.errorMsg != "" && err.Error() != tc.errorMsg { + t.Errorf("Expected error message %q, got %q", tc.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("Expected no error but got: %v", err) + } + + if config == nil { + t.Errorf("Expected valid config but got nil") + } + } + }) + } +} diff --git a/cmd/release-controller/sync.go b/cmd/release-controller/sync.go index d5a36497d..63e7a2ac7 100644 --- a/cmd/release-controller/sync.go +++ b/cmd/release-controller/sync.go @@ -587,6 +587,11 @@ func (c *Controller) syncAccepted(release *releasecontroller.Release) error { errs = append(errs, fmt.Errorf("unable to update image stream for publish step %s: %v", name, err)) continue } + case publishType.ExternalRegistry != nil: + if err := c.ensureExternalRegistryMirror(release, publishType.ExternalRegistry, newestAccepted.Name); err != nil { + errs = append(errs, fmt.Errorf("unable to mirror to external registry for publish step %s: %v", name, err)) + continue + } } } if len(errs) > 0 { diff --git a/cmd/release-controller/sync_publish.go b/cmd/release-controller/sync_publish.go index 41c5ce7e3..869aa8132 100644 --- a/cmd/release-controller/sync_publish.go +++ b/cmd/release-controller/sync_publish.go @@ -5,11 +5,14 @@ import ( "fmt" "reflect" "sort" + "strconv" "strings" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog" @@ -185,3 +188,124 @@ func (c *Controller) ensureImageStreamMatchesRelease(release *releasecontroller. } return nil } + +// ensureExternalRegistryMirror handles mirroring of a release to an external registry +func (c *Controller) ensureExternalRegistryMirror(release *releasecontroller.Release, config *releasecontroller.PublishExternalRegistry, releaseTagName string) error { + // Validation + if len(config.Registry) == 0 { + return fmt.Errorf("external registry config has no registry specified") + } + if len(config.SecretName) == 0 { + return fmt.Errorf("external registry config has no secretName specified") + } + + // Determine tags to mirror (default: release tag only) + tagsToMirror := config.Tags + if len(tagsToMirror) == 0 { + tagsToMirror = []string{releaseTagName} + } + + // Apply exclusions + excludeSet := sets.NewString(config.ExcludeTags...) + var finalTags []string + for _, tag := range tagsToMirror { + if !excludeSet.Has(tag) { + finalTags = append(finalTags, tag) + } + } + + klog.V(2).Infof("Creating external registry mirror jobs for %s to %s (tags: %v)", releaseTagName, config.Registry, finalTags) + + // Create mirror jobs for each tag + var errs []error + for _, tag := range finalTags { + if err := c.ensureExternalRegistryMirrorJob(release, config, tag); err != nil { + klog.Errorf("Failed to create external registry mirror job for %s: %v", tag, err) + errs = append(errs, fmt.Errorf("failed to create mirror job for tag %s: %v", tag, err)) + } + } + + if len(errs) > 0 { + return utilerrors.NewAggregate(errs) + } + + klog.V(2).Infof("External registry mirror jobs created successfully for %s", releaseTagName) + return nil +} + +// ensureExternalRegistryMirrorJob creates a single mirror job for a specific tag +func (c *Controller) ensureExternalRegistryMirrorJob(release *releasecontroller.Release, config *releasecontroller.PublishExternalRegistry, tagName string) error { + // Create unique job name for this registry and tag combination + 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] + } + + _, err := c.ensureJob(jobName, nil, func() (*batchv1.Job, error) { + // Get tag reference and validate it exists + tag := releasecontroller.FindTagReference(release.Target, tagName) + if tag == nil { + return nil, fmt.Errorf("tag %q not found in target imagestream %s/%s", tagName, release.Target.Namespace, release.Target.Name) + } + + // Construct image references + fromImage := releasecontroller.ReleasePullSpec(release, tag) + toImage := fmt.Sprintf("%s:%s", config.Registry, tagName) + + var cliImage string + if len(config.OverrideCLIImage) > 0 { + cliImage = config.OverrideCLIImage + klog.V(2).Infof("Using override CLI image for external registry mirror: %s", cliImage) + } else { + mirror, err := releasecontroller.GetMirror(release, tagName, c.releaseLister) + if err != nil { + return nil, fmt.Errorf("failed to get mirror for %s: %v", tagName, err) + } + + cliImage, err = releasecontroller.ResolveCLIImage(release, mirror) + if err != nil { + return nil, fmt.Errorf("failed to resolve CLI image: %v", err) + } + } + + // Create job using existing patterns + job, prefix := newReleaseJobBase(jobName, cliImage, config.SecretName) + + // Configure mirror command (reuse manifest list logic) + manifestListMode := "false" + if c.manifestListMode && !release.Config.DisableManifestListMode { + manifestListMode = "true" + } + + job.Spec.Template.Spec.Containers[0].Command = []string{ + "/bin/bash", "-c", + prefix + ` + oc image mirror --keep-manifest-list=$1 $2 $3 + `, + "", + manifestListMode, fromImage, toImage, + } + + // Add standard annotations using release object directly (consistent with other job creation patterns) + job.Annotations[releasecontroller.ReleaseAnnotationSource] = fmt.Sprintf("%s/%s", release.Source.Namespace, release.Source.Name) + job.Annotations[releasecontroller.ReleaseAnnotationTarget] = fmt.Sprintf("%s/%s", release.Target.Namespace, release.Target.Name) + job.Annotations[releasecontroller.ReleaseAnnotationGeneration] = strconv.FormatInt(release.Target.Generation, 10) + job.Annotations[releasecontroller.ReleaseAnnotationReleaseTag] = tagName + + klog.V(2).Infof("Creating external registry mirror job %s/%s for %s to %s", c.jobNamespace, job.Name, tagName, toImage) + return job, nil + }) + return err +} + +// sanitizeRegistryForJobName converts a registry URL to a job-name-safe string +func sanitizeRegistryForJobName(registry string) string { + // Replace invalid characters with dashes and truncate if needed + result := strings.ReplaceAll(registry, ".", "-") + result = strings.ReplaceAll(result, "/", "-") + result = strings.ReplaceAll(result, ":", "-") + + return result +} diff --git a/pkg/release-controller/release.go b/pkg/release-controller/release.go index b4cf86387..0a9f308bd 100644 --- a/pkg/release-controller/release.go +++ b/pkg/release-controller/release.go @@ -191,6 +191,14 @@ func ParseReleaseConfig(data string, configCache *lru.Cache) (*ReleaseConfig, er return nil, fmt.Errorf("imageStreamRef publish for %s has no name", name) } } + if publish.ExternalRegistry != nil { + if len(publish.ExternalRegistry.Registry) == 0 { + return nil, fmt.Errorf("externalRegistry publish for %s has no registry", name) + } + if len(publish.ExternalRegistry.SecretName) == 0 { + return nil, fmt.Errorf("externalRegistry publish for %s has no secretName", name) + } + } } copied := *cfg if configCache != nil { diff --git a/pkg/release-controller/types.go b/pkg/release-controller/types.go index 72bad8fc5..b6ff6741d 100644 --- a/pkg/release-controller/types.go +++ b/pkg/release-controller/types.go @@ -227,6 +227,8 @@ type ReleasePublish struct { TagRef *PublishTagReference `json:"tagRef"` // ImageStreamRef copies all images to another image stream in one transaction. ImageStreamRef *PublishStreamReference `json:"imageStreamRef"` + // ExternalRegistry mirrors the release to an external image registry. + ExternalRegistry *PublishExternalRegistry `json:"externalRegistry"` // VerifyIssue marks jira issues fixed by this tag as VERIFIED in Jira if the QA contact reviewed and approved the bugfix PR VerifyIssues *PublishVerifyIssues `json:"verifyIssues"` } @@ -254,6 +256,22 @@ type PublishStreamReference struct { ExcludeTags []string `json:"excludeTags"` } +// PublishExternalRegistry defines configuration for mirroring a release to an external registry +type PublishExternalRegistry struct { + // Registry is the full path to the external image registry (e.g., "quay.io/openshift-release-dev/ocp-release") + Registry string `json:"registry"` + // SecretName is the name of the secret containing credentials to the registry. + // Secret must exist in job namespace and contain a config.json with valid Docker auths. + SecretName string `json:"secretName"` + // OverrideCLIImage may be used to override the CLI image used for the mirror job. + // This is particularly useful for layered releases where no CLI image is available in the mirror stream. + OverrideCLIImage string `json:"overrideCLIImage,omitempty"` + // Tags if set will limit mirroring to specific release tags. Defaults to the accepted release tag. + Tags []string `json:"tags,omitempty"` + // ExcludeTags if set will explicitly exclude these tags from mirroring. + ExcludeTags []string `json:"excludeTags,omitempty"` +} + // PublishVerifyIssues marks jira issue fixed by this tag as VERIFIED in Jira if the QA contact reviewed and approved the bugfix PR type PublishVerifyIssues struct { // PreviousRelease points to the last release created before the imagestream