Skip to content

Commit 6acb9de

Browse files
author
Pablo Rodriguez Nava
committed
MCO-2117: Allow default OSImageStream overrides
Previously the default stream was determined by matching the OS version in the stream name, without the possibility of telling the MCO which default stream to use. This change replaces that logic, allowing users to input the defaultStream they want to use and falling back to the builtin if necessary. The builtin default stream determination logic has been improved to avoid hard-coding versions and it now picks the OSImageStream that points to the images of the default OS tags in the payload ImageStream. Signed-off-by: Pablo Rodriguez Nava <git@amail.pablintino.com>
1 parent 39b1110 commit 6acb9de

74 files changed

Lines changed: 1558 additions & 7807 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ require (
3737
github.com/onsi/gomega v1.36.2
3838
github.com/opencontainers/go-digest v1.0.0
3939
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835
40-
github.com/openshift/api v0.0.0-20260213123447-0246c0ac1a77
41-
github.com/openshift/client-go v0.0.0-20260213141500-06efc6dce93b
40+
github.com/openshift/api v0.0.0-20260226052804-beff70de904a
41+
github.com/openshift/client-go v0.0.0-20260226152647-d8b2196ff0d9
4242
github.com/openshift/library-go v0.0.0-20251015151611-6fc7a74b67c5
4343
github.com/openshift/runtime-utils v0.0.0-20230921210328-7bdb5b9c177b
4444
github.com/prometheus/client_golang v1.22.0
@@ -248,7 +248,7 @@ require (
248248
github.com/ashanbrown/makezero v1.2.0 // indirect
249249
github.com/beorn7/perks v1.0.1 // indirect
250250
github.com/bkielbasa/cyclop v1.2.3 // indirect
251-
github.com/blang/semver/v4 v4.0.0 // indirect
251+
github.com/blang/semver/v4 v4.0.0
252252
github.com/blizzy78/varnamelen v0.8.0 // indirect
253253
github.com/breml/bidichk v0.3.2 // indirect
254254
github.com/breml/errchkjson v0.4.0 // indirect

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,10 @@ github.com/opencontainers/selinux v1.12.0 h1:6n5JV4Cf+4y0KNXW48TLj5DwfXpvWlxXplU
609609
github.com/opencontainers/selinux v1.12.0/go.mod h1:BTPX+bjVbWGXw7ZZWUbdENt8w0htPSrlgOOysQaU62U=
610610
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835 h1:rkqIIfdYYkasXbF2XKVgh/3f1mhjSQK9By8WtVMgYo8=
611611
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250916161632-d81c09058835/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M=
612-
github.com/openshift/api v0.0.0-20260213123447-0246c0ac1a77 h1:kAS0KmsQ+HzWUhX6pkGsrrKX8Dx5OKAj93cPNj+RGNs=
613-
github.com/openshift/api v0.0.0-20260213123447-0246c0ac1a77/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
614-
github.com/openshift/client-go v0.0.0-20260213141500-06efc6dce93b h1:7rTnxq+haKrzUFKQQDylkklyFRtGFox73dlawRlnTA8=
615-
github.com/openshift/client-go v0.0.0-20260213141500-06efc6dce93b/go.mod h1:V3s8weD4bKXGXaN7d9g3V1QS2gmlYBRI2i/lfIwmroM=
612+
github.com/openshift/api v0.0.0-20260226052804-beff70de904a h1:3mBAecolw1crGJR1pUk4YVmFbwt6upeyALOsu6L0MCE=
613+
github.com/openshift/api v0.0.0-20260226052804-beff70de904a/go.mod h1:ZYAxo9t1AALeEotN07tNzIvqqqWSxcZIqMUKnY/xCeQ=
614+
github.com/openshift/client-go v0.0.0-20260226152647-d8b2196ff0d9 h1:5gHG1T3PUm5Ly7vILl9+byccl9eAUiV2RZuWXVVNnB4=
615+
github.com/openshift/client-go v0.0.0-20260226152647-d8b2196ff0d9/go.mod h1:TeUQXSUW0UPXSmEtMZLfTYIE3MajxnziS3lbdH0WSwk=
616616
github.com/openshift/kubernetes v1.30.1-0.20251028145634-9e794b89909a h1:uaeiYAYOVlXChnGxvsziVTkzaSlBV7h8Y2U2Bc81UKM=
617617
github.com/openshift/kubernetes v1.30.1-0.20251028145634-9e794b89909a/go.mod h1:w3+IfrXNp5RosdDXg3LB55yijJqR/FwouvVntYHQf0o=
618618
github.com/openshift/kubernetes/staging/src/k8s.io/api v0.0.0-20251028145634-9e794b89909a h1:hZUZg/qpvT23oUoCkFWe/Q4VNu5zOeqmDOl3f/F6uRk=

pkg/controller/bootstrap/bootstrap.go

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ func (b *Bootstrap) Run(destDir string) error {
115115
imageStream *imagev1.ImageStream
116116
iri *mcfgv1alpha1.InternalReleaseImage
117117
iriTLSCert *corev1.Secret
118+
osImageStream *mcfgv1alpha1.OSImageStream
118119
)
119120
for _, info := range infos {
120121
if info.IsDir() {
@@ -199,6 +200,9 @@ func (b *Bootstrap) Run(destDir string) error {
199200
if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName {
200201
iriTLSCert = obj
201202
}
203+
case *mcfgv1alpha1.OSImageStream:
204+
// If given, it's treated as user input with config such as the default stream
205+
osImageStream = obj
202206
default:
203207
klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji)
204208
}
@@ -224,10 +228,17 @@ func (b *Bootstrap) Run(destDir string) error {
224228
return fmt.Errorf("error filtering pools: %w", err)
225229
}
226230

227-
var osImageStream *mcfgv1alpha1.OSImageStream
228231
// Enable OSImageStreams if the FeatureGate is active and the deployment is not OKD
229232
if osimagestream.IsFeatureEnabled(fgHandler) {
230-
osImageStream, err = b.fetchOSImageStream(imageStream, cconfig, icspRules, idmsRules, itmsRules, imgCfg, pullSecret)
233+
osImageStream, err = b.fetchOSImageStream(
234+
imageStream,
235+
cconfig,
236+
icspRules,
237+
idmsRules,
238+
itmsRules,
239+
imgCfg,
240+
pullSecret,
241+
osImageStream)
231242
if err != nil {
232243
return err
233244
}
@@ -240,6 +251,9 @@ func (b *Bootstrap) Run(destDir string) error {
240251
}
241252
cconfig.Spec.BaseOSContainerImage = string(defaultStreamSet.OSImage)
242253
cconfig.Spec.BaseOSExtensionsContainerImage = string(defaultStreamSet.OSExtensionsImage)
254+
} else {
255+
// Just ensuring the osImageStream is nil if the FG is disabled
256+
osImageStream = nil
243257
}
244258

245259
pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey]
@@ -462,6 +476,7 @@ func (b *Bootstrap) fetchOSImageStream(
462476
itmsRules []*apicfgv1.ImageTagMirrorSet,
463477
imgCfg *apicfgv1.Image,
464478
pullSecret *corev1.Secret,
479+
existingOSImageStream *mcfgv1alpha1.OSImageStream,
465480
) (*mcfgv1alpha1.OSImageStream, error) {
466481

467482
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
@@ -479,15 +494,25 @@ func (b *Bootstrap) fetchOSImageStream(
479494
sysCtxBuilder.WithRegistriesConfig(registriesConfig)
480495
}
481496

482-
osImageStream, err := osimagestream.BuildOsImageStreamBootstrap(ctx,
483-
sysCtxBuilder,
484-
imageStream,
485-
&osimagestream.OSImageTuple{
497+
sysCtx, err := sysCtxBuilder.Build()
498+
if err != nil {
499+
return nil, fmt.Errorf("could not prepare for OSImageStream inspection: %w", err)
500+
}
501+
defer func() {
502+
if err := sysCtx.Cleanup(); err != nil {
503+
klog.Warningf("Unable to clean resources after OSImageStream inspection: %s", err)
504+
}
505+
}()
506+
507+
factory := osimagestream.NewDefaultStreamSourceFactory(&osimagestream.DefaultImagesInspectorFactory{})
508+
osImageStream, err := factory.Create(ctx, sysCtx.SysContext, osimagestream.CreateOptions{
509+
ExistingOSImageStream: existingOSImageStream,
510+
ReleaseImageStream: imageStream,
511+
CliImages: &osimagestream.OSImageTuple{
486512
OSImage: cconfig.Spec.BaseOSContainerImage,
487513
OSExtensionsImage: cconfig.Spec.BaseOSExtensionsContainerImage,
488514
},
489-
osimagestream.NewDefaultStreamSourceFactory(nil, &osimagestream.DefaultImagesInspectorFactory{}),
490-
)
515+
})
491516
if err != nil {
492517
return nil, fmt.Errorf("error inspecting available OSImageStreams: %w", err)
493518
}

pkg/controller/common/constants.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ const (
2727
// ReleaseImageVersionAnnotationKey is used to tag the rendered machineconfigs & controller config with the release image version.
2828
ReleaseImageVersionAnnotationKey = "machineconfiguration.openshift.io/release-image-version"
2929

30+
// BuiltinDefaultStreamAnnotationKey is the MCO fallback default stream.
31+
BuiltinDefaultStreamAnnotationKey = "machineconfiguration.openshift.io/builtin-default-stream"
32+
3033
// OSImageURLOverriddenKey is used to tag a rendered machineconfig when OSImageURL has been overridden from default using machineconfig
3134
OSImageURLOverriddenKey = "machineconfiguration.openshift.io/os-image-url-overridden"
3235

pkg/operator/operator.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,8 @@ func New(
372372
if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) {
373373
optr.osImageStreamLister = osImageStreamInformer.Lister()
374374
optr.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced
375+
// TODO: Move this AddEventHandler to the main loop when the FG is removed
376+
osImageStreamInformer.Informer().AddEventHandler(optr.eventHandler())
375377
}
376378
if iriInformer != nil {
377379
optr.iriLister = iriInformer.Lister()

pkg/operator/osimagestream_ocp.go

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,46 @@ func (optr *Operator) syncOSImageStream(_ *renderConfig, _ *configv1.ClusterOper
3232
// This sync runs once per version. Before performing the streams fetching
3333
// process, that takes time as it requires inspecting images, ensure this function
3434
// needs to build the stream.
35-
existingOSImageStream, updateRequired, err := optr.isOSImageStreamBuildRequired()
36-
if !updateRequired || err != nil {
35+
existingOSImageStream, rebuildRequired, err := optr.isOSImageStreamBuildRequired()
36+
if err != nil {
3737
return err
3838
}
3939

40-
// If the code reaches this point the OSImageStream CR is not
41-
// present (new cluster) or it's out-dated (cluster update).
42-
// Build the new OSImageStream and push it.
43-
return optr.buildOSImageStream(existingOSImageStream)
40+
if rebuildRequired {
41+
// If the code reaches this point the OSImageStream status containing
42+
// the available OS Image Streams is outdated or doesn't exist.
43+
return optr.buildOSImageStream(existingOSImageStream)
44+
} else if existingOSImageStream != nil {
45+
// We didn't require a rebuild, but check if we need to update the default
46+
return optr.updateOSImageStream(existingOSImageStream)
47+
}
4448

49+
return err
50+
}
51+
52+
func (optr *Operator) updateOSImageStream(existingOSImageStream *v1alpha1.OSImageStream) error {
53+
requestedDefault := osimagestream.GetOSImageStreamSpecDefault(existingOSImageStream)
54+
if requestedDefault == "" {
55+
requestedDefault = osimagestream.GetBuiltinDefault(existingOSImageStream)
56+
}
57+
58+
currentDefault := existingOSImageStream.Status.DefaultStream
59+
if currentDefault != requestedDefault {
60+
if _, err := osimagestream.GetOSImageStreamSetByName(existingOSImageStream, requestedDefault); err != nil {
61+
return fmt.Errorf("error syncing default OSImageStream with OSImageStream %s: %v", requestedDefault, err)
62+
}
63+
64+
existingOSImageStream.Status.DefaultStream = requestedDefault
65+
if _, err := optr.client.
66+
MachineconfigurationV1alpha1().
67+
OSImageStreams().
68+
UpdateStatus(context.TODO(), existingOSImageStream, metav1.UpdateOptions{}); err != nil {
69+
return fmt.Errorf("error updating the default OSImageStream status: %w", err)
70+
}
71+
72+
klog.Infof("OSImageStream default has changed from %s to %s", currentDefault, requestedDefault)
73+
}
74+
return nil
4575
}
4676

4777
func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImageStream) error {
@@ -76,8 +106,25 @@ func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImage
76106
return fmt.Errorf("could not build SysContext for OSImageStream build: %w", err)
77107
}
78108

79-
imageStreamFactory := osimagestream.NewDefaultStreamSourceFactory(optr.mcoCmLister, &osimagestream.DefaultImagesInspectorFactory{})
80-
osImageStream, err := osimagestream.BuildOsImageStreamRuntime(buildCtx, sysCtxBuilder, image, imageStreamFactory)
109+
sysCtx, err := sysCtxBuilder.Build()
110+
if err != nil {
111+
return fmt.Errorf("could not prepare for OSImageStream inspection: %w", err)
112+
}
113+
defer func() {
114+
if err := sysCtx.Cleanup(); err != nil {
115+
klog.Warningf("Unable to clean resources after OSImageStream inspection: %s", err)
116+
}
117+
}()
118+
119+
factory := osimagestream.NewDefaultStreamSourceFactory(&osimagestream.DefaultImagesInspectorFactory{})
120+
osImageStream, err := factory.Create(
121+
buildCtx,
122+
sysCtx.SysContext,
123+
osimagestream.CreateOptions{
124+
ReleaseImage: image,
125+
ConfigMapLister: optr.mcoCmLister,
126+
ExistingOSImageStream: existingOSImageStream,
127+
})
81128
if err != nil {
82129
return fmt.Errorf("error building the OSImageStream: %w", err)
83130
}
@@ -179,9 +226,9 @@ func (optr *Operator) isOSImageStreamBuildRequired() (*v1alpha1.OSImageStream, b
179226
}
180227

181228
// Check if an update is needed
182-
if !osImageStreamRequiresUpdate(existingOSImageStream) {
229+
if !osImageStreamRequiresRebuild(existingOSImageStream) {
183230
klog.V(4).Info("OSImageStream is already up-to-date, skipping sync")
184-
return nil, false, nil
231+
return existingOSImageStream, false, nil
185232
}
186233
return existingOSImageStream, true, nil
187234
}
@@ -235,12 +282,13 @@ func (optr *Operator) getExistingOSImageStream() (*v1alpha1.OSImageStream, error
235282
return osImageStream, nil
236283
}
237284

238-
// osImageStreamRequiresUpdate checks if the OSImageStream needs to be created or updated.
239-
// Returns true if osImageStream is nil or if its version annotation doesn't match the current version.
240-
func osImageStreamRequiresUpdate(osImageStream *v1alpha1.OSImageStream) bool {
285+
// osImageStreamRequiresRebuild checks if the OSImageStream needs to be created or updated.
286+
// Returns true if osImageStream is nil, if its version annotation doesn't match the current version,
287+
// or if the builtin default stream annotation is missing.
288+
func osImageStreamRequiresRebuild(osImageStream *v1alpha1.OSImageStream) bool {
241289
if osImageStream == nil {
242290
return true
243291
}
244292
releaseVersion, ok := osImageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]
245-
return !ok || releaseVersion != version.Hash
293+
return !ok || releaseVersion != version.Hash || osimagestream.GetBuiltinDefault(osImageStream) == ""
246294
}

pkg/osimagestream/helpers.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,19 @@ func GetOSImageStreamSetByName(osImageStream *v1alpha1.OSImageStream, name strin
4343
func IsFeatureEnabled(fgHandler common.FeatureGatesHandler) bool {
4444
return fgHandler.Enabled(features.FeatureGateOSStreams) && !version.IsSCOS() && !version.IsFCOS()
4545
}
46+
47+
// GetBuiltinDefault returns the MCO fallback default stream, or empty string if not available.
48+
func GetBuiltinDefault(osImageStream *v1alpha1.OSImageStream) string {
49+
if osImageStream == nil {
50+
return ""
51+
}
52+
return osImageStream.Annotations[common.BuiltinDefaultStreamAnnotationKey]
53+
}
54+
55+
// GetOSImageStreamSpecDefault returns the user-requested default stream override, or empty string if not set.
56+
func GetOSImageStreamSpecDefault(osImageStream *v1alpha1.OSImageStream) string {
57+
if osImageStream != nil && osImageStream.Spec != nil {
58+
return osImageStream.Spec.DefaultStream
59+
}
60+
return ""
61+
}

pkg/osimagestream/imagestream_provider.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package osimagestream
33
import (
44
"context"
55
"fmt"
6+
"sync"
67

78
imagev1 "github.com/openshift/api/image/v1"
89
"github.com/openshift/client-go/image/clientset/versioned/scheme"
@@ -37,6 +38,8 @@ func (i *ImageStreamProviderResource) ReadImageStream(_ context.Context) (*image
3738
type ImageStreamProviderNetwork struct {
3839
imagesInspector ImagesInspector
3940
imageName string
41+
cacheLock sync.Mutex
42+
imageStream *imagev1.ImageStream
4043
}
4144

4245
// NewImageStreamProviderNetwork creates a new ImageStreamProviderNetwork that will fetch
@@ -46,14 +49,27 @@ func NewImageStreamProviderNetwork(imagesInspector ImagesInspector, imageName st
4649
}
4750

4851
// ReadImageStream fetches the ImageStream manifest from the container image and decodes it.
49-
// Returns an error if the file cannot be fetched, is empty, contains invalid YAML,
50-
// or does not contain an ImageStream resource.
52+
// The result is cached on success; failures are retried on subsequent calls.
5153
func (i *ImageStreamProviderNetwork) ReadImageStream(ctx context.Context) (*imagev1.ImageStream, error) {
54+
i.cacheLock.Lock()
55+
defer i.cacheLock.Unlock()
56+
if i.imageStream != nil {
57+
return i.imageStream, nil
58+
}
59+
is, err := i.fetchImageStream(ctx)
60+
if err != nil {
61+
return nil, err
62+
}
63+
i.imageStream = is
64+
return is, nil
65+
}
66+
67+
func (i *ImageStreamProviderNetwork) fetchImageStream(ctx context.Context) (*imagev1.ImageStream, error) {
5268
imageStreamBytes, err := i.imagesInspector.FetchImageFile(ctx, i.imageName, releaseImageStreamLocation)
5369
if err != nil {
5470
return nil, err
5571
}
56-
if imageStreamBytes == nil || len(imageStreamBytes) == 0 {
72+
if len(imageStreamBytes) == 0 {
5773
return nil, fmt.Errorf("no ImageStream found for %s", i.imageName)
5874
}
5975

0 commit comments

Comments
 (0)