Skip to content

MCO-2116: Generate OSImageStream manifest #10357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Mar 23, 2026
+97 −1
Merged

MCO-2116: Generate OSImageStream manifest #10357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/asset/manifests/openshift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func (o *Openshift) Dependencies() []asset.Asset {
&openshift.BaremetalConfig{},
new(rhcos.Image),
&openshift.AzureCloudProviderSecret{},
&OSImageStream{},
}
}

Expand Down Expand Up @@ -260,13 +261,15 @@ func (o *Openshift) Generate(ctx context.Context, dependencies asset.Parents) er
roleCloudCredsSecretReader := &openshift.RoleCloudCredsSecretReader{}
baremetalConfig := &openshift.BaremetalConfig{}
rhcosImage := new(rhcos.Image)
osImageStream := &OSImageStream{}

dependencies.Get(
cloudCredsSecret,
kubeadminPasswordSecret,
roleCloudCredsSecretReader,
baremetalConfig,
rhcosImage)
rhcosImage,
osImageStream)

assetData := map[string][]byte{
"99_kubeadmin-password-secret.yaml": applyTemplateData(kubeadminPasswordSecret.Files()[0].Data, templateData),
Expand Down Expand Up @@ -299,6 +302,7 @@ func (o *Openshift) Generate(ctx context.Context, dependencies asset.Parents) er

o.FileList = append(o.FileList, openshiftInstall.Files()...)
o.FileList = append(o.FileList, featureGate.Files()...)
o.FileList = append(o.FileList, osImageStream.Files()...)

asset.SortFiles(o.FileList)

Expand Down
92 changes: 92 additions & 0 deletions pkg/asset/manifests/osimagestream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package manifests

import (
"context"
"fmt"
"path"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/yaml"

"github.com/openshift/api/features"
mcfgv1alpha "github.com/openshift/api/machineconfiguration/v1alpha1"
"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/rhcos"
)

var osImageStreamFileName = path.Join(openshiftManifestDir, "99_osimagestream.yaml")

// OSImageStream generates the OSImageStream manifest.
type OSImageStream struct {
FileList []*asset.File
}

var _ asset.WritableAsset = (*OSImageStream)(nil)

// Name returns a human-friendly name for the asset.
func (*OSImageStream) Name() string {
return "OS Image Stream"
}

// Dependencies returns all of the dependencies directly needed to generate
// the asset.
func (*OSImageStream) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
}
}

// Generate generates the OSImageStream CRD.
func (f *OSImageStream) Generate(_ context.Context, dependencies asset.Parents) error {
installConfig := &installconfig.InstallConfig{}
dependencies.Get(installConfig)

// If one of the following are true the OSImageStream CR is not generated
// 1. The feature is not enabled
// 2. The target is CentOS Stream CoreOS
if ic := installConfig.Config; !ic.Enabled(features.FeatureGateOSStreams) || ic.IsSCOS() {
// FG disabled or not OCP
return nil
}

// If no stream was set just report the default one for the current version
stream := installConfig.Config.OSImageStream
if stream == "" {
stream = rhcos.DefaultOSImageStream
}

osImageStream := &mcfgv1alpha.OSImageStream{
TypeMeta: metav1.TypeMeta{
APIVersion: mcfgv1alpha.SchemeGroupVersion.String(),
Kind: "OSImageStream",
},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: &mcfgv1alpha.OSImageStreamSpec{
DefaultStream: string(stream),
},
}
Comment on lines +45 to +70
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Skip OSImageStream generation when no stream was selected.

This currently emits 99_osimagestream.yaml whenever the feature gate is on, even if installConfig.Config.OSImageStream is empty. That produces a CR with an empty spec.defaultStream, which does not match the PR objective of generating the manifest only when a stream was chosen.

Suggested fix 
-	if !installConfig.Config.EnabledFeatureGates().Enabled(features.FeatureGateOSStreams) || installConfig.Config.IsSCOS() {
+	if !installConfig.Config.EnabledFeatureGates().Enabled(features.FeatureGateOSStreams) ||
+		installConfig.Config.IsSCOS() ||
+		installConfig.Config.OSImageStream == "" {
 		// FG disabled or not OCP
 		return nil
 	}
@@
 		Spec: &mcfgv1alpha.OSImageStreamSpec{
-			DefaultStream: string(installConfig.Config.OSImageStream),
+			DefaultStream: installConfig.Config.OSImageStream,
 		},
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// If one of the following are true the OSImageStream CR is not generated
// 1. The feature is not enabled
// 2. The target is OKD
if !installConfig.Config.EnabledFeatureGates().Enabled(features.FeatureGateOSStreams) || installConfig.Config.IsSCOS() {
// FG disabled or not OCP
return nil
}
osImageStream := &mcfgv1alpha.OSImageStream{
TypeMeta: metav1.TypeMeta{
APIVersion: mcfgv1alpha.SchemeGroupVersion.String(),
Kind: "OSImageStream",
},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: &mcfgv1alpha.OSImageStreamSpec{
DefaultStream: string(installConfig.Config.OSImageStream),
},
}
// If one of the following are true the OSImageStream CR is not generated
// 1. The feature is not enabled
// 2. The target is OKD
if !installConfig.Config.EnabledFeatureGates().Enabled(features.FeatureGateOSStreams) ||
installConfig.Config.IsSCOS() ||
installConfig.Config.OSImageStream == "" {
// FG disabled or not OCP
return nil
}
osImageStream := &mcfgv1alpha.OSImageStream{
TypeMeta: metav1.TypeMeta{
APIVersion: mcfgv1alpha.SchemeGroupVersion.String(),
Kind: "OSImageStream",
},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: &mcfgv1alpha.OSImageStreamSpec{
DefaultStream: installConfig.Config.OSImageStream,
},
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/asset/manifests/osimagestream.go` around lines 44 - 63, The OSImageStream
CR is being created even when no stream was chosen; update the generation logic
that currently checks features.FeatureGateOSStreams and
installConfig.Config.IsSCOS to also skip when installConfig.Config.OSImageStream
is empty (or only whitespace). Specifically, before constructing the
mcfgv1alpha.OSImageStream (the osImageStream variable and its
Spec.DefaultStream), add a guard that returns nil if
installConfig.Config.OSImageStream is blank so no 99_osimagestream.yaml is
emitted with an empty spec.defaultStream.

Copy link
Copy Markdown
Contributor Author

@pablintino pablintino Mar 16, 2026

Choose a reason for hiding this comment

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

This is expected. The CR should be generated always. If the user doesn't select a stream we should render the CR with the default stream the intaller uses.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

@pablintino, understood — always emit the CR and use the installer's default when no stream was chosen. My suggestion to skip was wrong.

That said, the current code still has a gap: when installConfig.Config.OSImageStream is empty it writes an empty spec.defaultStream instead of falling back to rhcos.DefaultOSImageStream. A small defaulting step before constructing the CR would cover that case:

stream := installConfig.Config.OSImageStream
if stream == "" {
    stream = rhcos.DefaultOSImageStream
}

osImageStream := &mcfgv1alpha.OSImageStream{
    ...
    Spec: &mcfgv1alpha.OSImageStreamSpec{
        DefaultStream: string(stream),
    },
}

This keeps the "always generate" intent while ensuring a valid default is written.

( •ᴗ• )っ

📝 Learnings were identified and not saved (knowledge base disabled). Enable


osImageStreamData, err := yaml.Marshal(osImageStream)
if err != nil {
return fmt.Errorf("failed to create %s manifests from InstallConfig. Error: %w", f.Name(), err)
}

f.FileList = append(f.FileList, &asset.File{
Filename: osImageStreamFileName,
Data: osImageStreamData,
})
return nil
}

// Files returns the files generated by the asset.
func (f *OSImageStream) Files() []*asset.File {
return f.FileList
}

// Load loads the already-rendered files back from disk.
func (f *OSImageStream) Load(_ asset.FileFetcher) (bool, error) {
return false, nil
}