From c2e23bb4365f2f64dbc4544f24def8f427926d7e Mon Sep 17 00:00:00 2001 From: Pablo Rodriguez Nava Date: Mon, 1 Dec 2025 13:41:27 +0100 Subject: [PATCH] MCO-1961: Add the OSImageStream Controller This change adds the OSImageStream controller and the additional logic required to introduce OSImageStreams into the MCO. --- pkg/controller/common/constants.go | 3 + pkg/controller/osimagestream/helpers.go | 60 +++ pkg/controller/osimagestream/helpers_test.go | 303 +++++++++++++ pkg/controller/osimagestream/osimagestream.go | 14 +- .../osimagestream/osimagestream_controller.go | 210 +++++++++ .../osimagestream_controller_test.go | 404 ++++++++++++++++++ .../osimagestream/osimagestream_test.go | 4 + pkg/helpers/helpers.go | 10 + pkg/helpers/helpers_test.go | 75 ++++ 9 files changed, 1078 insertions(+), 5 deletions(-) create mode 100644 pkg/controller/osimagestream/helpers.go create mode 100644 pkg/controller/osimagestream/helpers_test.go create mode 100644 pkg/controller/osimagestream/osimagestream_controller.go create mode 100644 pkg/controller/osimagestream/osimagestream_controller_test.go create mode 100644 pkg/helpers/helpers_test.go diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index a9f76eb232..5988016b3e 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -60,6 +60,9 @@ const ( // APIServerInstanceName is a singleton name for APIServer configuration APIServerInstanceName = "cluster" + // ClusterInstanceNameOSImageStream is the name of the singleton cluster-scoped OSImageStream instance. + ClusterInstanceNameOSImageStream = "cluster" + // APIServerInstanceName is a singleton name for APIServer configuration APIServerBootstrapFileLocation = "/etc/mcs/bootstrap/api-server/api-server.yaml" diff --git a/pkg/controller/osimagestream/helpers.go b/pkg/controller/osimagestream/helpers.go new file mode 100644 index 0000000000..063da9ff86 --- /dev/null +++ b/pkg/controller/osimagestream/helpers.go @@ -0,0 +1,60 @@ +package osimagestream + +import ( + "fmt" + "strings" + + v1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/machineconfiguration/v1alpha1" + "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/openshift/machine-config-operator/pkg/helpers" +) + +// GetStreamSetsNames extracts the names from a slice of OSImageStreamSets. +func GetStreamSetsNames(streamSet []v1alpha1.OSImageStreamSet) []string { + streams := make([]string, 0) + for _, stream := range streamSet { + streams = append(streams, stream.Name) + } + return streams +} + +// GetOSImageStreamSetByName retrieves an OSImageStreamSet by name from an OSImageStream. +// If name is empty, the default stream is returned. Returns an error if the stream is not found. +func GetOSImageStreamSetByName(osImageStream *v1alpha1.OSImageStream, name string) (*v1alpha1.OSImageStreamSet, error) { + if osImageStream == nil { + return nil, fmt.Errorf("requested OSImageStreamSet %s does not exist. OSImageStream cannot be nil", name) + } + if name == "" { + name = osImageStream.Status.DefaultStream + } + + for _, stream := range osImageStream.Status.AvailableStreams { + if stream.Name == name { + return &stream, nil + } + } + + return nil, fmt.Errorf("requested OSImageStream %s does not exist. Existing: %s", name, strings.Join(GetStreamSetsNames(osImageStream.Status.AvailableStreams), ",")) +} + +// TryGetOSImageStreamSetByName retrieves an OSImageStreamSet by name, returning nil if not found. +func TryGetOSImageStreamSetByName(osImageStream *v1alpha1.OSImageStream, name string) *v1alpha1.OSImageStreamSet { + stream, _ := GetOSImageStreamSetByName(osImageStream, name) + return stream +} + +// TryGetOSImageStreamFromPoolListByPoolName retrieves an OSImageStreamSet for a given pool name, +// returning nil if the pool or stream is not found. For custom pools (non-master, non-arbiter), +// falls back to the worker pool if the custom pool is not found. +func TryGetOSImageStreamFromPoolListByPoolName(osImageStream *v1alpha1.OSImageStream, pools []*v1.MachineConfigPool, poolName string) *v1alpha1.OSImageStreamSet { + targetPool := helpers.GetPoolByName(pools, poolName) + if targetPool == nil && (poolName != common.MachineConfigPoolMaster && poolName != common.MachineConfigPoolArbiter) { + targetPool = helpers.GetPoolByName(pools, common.MachineConfigPoolWorker) + } + if targetPool == nil { + return nil + } + + return TryGetOSImageStreamSetByName(osImageStream, targetPool.Spec.OSImageStream.Name) +} diff --git a/pkg/controller/osimagestream/helpers_test.go b/pkg/controller/osimagestream/helpers_test.go new file mode 100644 index 0000000000..62be5fe1b5 --- /dev/null +++ b/pkg/controller/osimagestream/helpers_test.go @@ -0,0 +1,303 @@ +// Assisted-by: Claude +package osimagestream + +import ( + "testing" + + v1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/machineconfiguration/v1alpha1" + "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetStreamSetsNames(t *testing.T) { + tests := []struct { + name string + input []v1alpha1.OSImageStreamSet + expected []string + }{ + { + name: "empty slice", + input: []v1alpha1.OSImageStreamSet{}, + expected: []string{}, + }, + { + name: "single stream", + input: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9"}, + }, + expected: []string{"rhel-9"}, + }, + { + name: "multiple streams", + input: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9"}, + {Name: "rhel-10"}, + {Name: "custom-stream"}, + }, + expected: []string{"rhel-9", "rhel-10", "custom-stream"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetStreamSetsNames(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetOSImageStreamSetByName(t *testing.T) { + osImageStream := &v1alpha1.OSImageStream{ + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + {Name: "rhel-10", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + }, + } + + tests := []struct { + name string + osImageStream *v1alpha1.OSImageStream + streamName string + expected *v1alpha1.OSImageStreamSet + errorContains string + }{ + { + name: "find existing stream", + osImageStream: osImageStream, + streamName: "rhel-9", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "find another existing stream", + osImageStream: osImageStream, + streamName: "rhel-10", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-10", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + { + name: "empty name returns default stream", + osImageStream: osImageStream, + streamName: "", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "non-existent stream", + osImageStream: osImageStream, + streamName: "non-existent", + errorContains: "does not exist", + }, + { + name: "nil osImageStream", + osImageStream: nil, + streamName: "rhel-9", + errorContains: "cannot be nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GetOSImageStreamSetByName(tt.osImageStream, tt.streamName) + if tt.errorContains != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errorContains) + assert.Nil(t, result) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestTryGetOSImageStreamSetByName(t *testing.T) { + osImageStream := &v1alpha1.OSImageStream{ + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + {Name: "rhel-10", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + }, + } + + tests := []struct { + name string + osImageStream *v1alpha1.OSImageStream + streamName string + expected *v1alpha1.OSImageStreamSet + }{ + { + name: "find existing stream", + osImageStream: osImageStream, + streamName: "rhel-9", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "non-existent stream returns nil", + osImageStream: osImageStream, + streamName: "non-existent", + expected: nil, + }, + { + name: "nil osImageStream returns nil", + osImageStream: nil, + streamName: "rhel-9", + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := TryGetOSImageStreamSetByName(tt.osImageStream, tt.streamName) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestTryGetOSImageStreamFromPoolListByPoolName(t *testing.T) { + osImageStream := &v1alpha1.OSImageStream{ + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "stream-master", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "stream-master", OSImage: "image1", OSExtensionsImage: "ext1"}, + {Name: "stream-worker", OSImage: "image2", OSExtensionsImage: "ext2"}, + {Name: "stream-arbiter", OSImage: "image3", OSExtensionsImage: "ext3"}, + {Name: "stream-custom", OSImage: "image4", OSExtensionsImage: "ext4"}, + }, + }, + } + + masterPool := &v1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: common.MachineConfigPoolMaster}, + Spec: v1.MachineConfigPoolSpec{ + OSImageStream: v1.OSImageStreamReference{Name: "stream-master"}, + }, + } + + workerPool := &v1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: common.MachineConfigPoolWorker}, + Spec: v1.MachineConfigPoolSpec{ + OSImageStream: v1.OSImageStreamReference{Name: "stream-worker"}, + }, + } + + arbiterPool := &v1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: common.MachineConfigPoolArbiter}, + Spec: v1.MachineConfigPoolSpec{ + OSImageStream: v1.OSImageStreamReference{Name: "stream-arbiter"}, + }, + } + + customPool := &v1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "custom"}, + Spec: v1.MachineConfigPoolSpec{ + OSImageStream: v1.OSImageStreamReference{Name: "stream-custom"}, + }, + } + + tests := []struct { + name string + osImageStream *v1alpha1.OSImageStream + pools []*v1.MachineConfigPool + poolName string + expected *v1alpha1.OSImageStreamSet + }{ + { + name: "find stream for master pool", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool}, + poolName: common.MachineConfigPoolMaster, + expected: &v1alpha1.OSImageStreamSet{Name: "stream-master", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "find stream for worker pool", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool}, + poolName: common.MachineConfigPoolWorker, + expected: &v1alpha1.OSImageStreamSet{Name: "stream-worker", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + { + name: "find stream for arbiter pool", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool, arbiterPool}, + poolName: common.MachineConfigPoolArbiter, + expected: &v1alpha1.OSImageStreamSet{Name: "stream-arbiter", OSImage: "image3", OSExtensionsImage: "ext3"}, + }, + { + name: "find stream for custom pool", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool, customPool}, + poolName: "custom", + expected: &v1alpha1.OSImageStreamSet{Name: "stream-custom", OSImage: "image4", OSExtensionsImage: "ext4"}, + }, + { + name: "custom pool not found - fallback to worker", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool}, + poolName: "non-existent-custom", + expected: &v1alpha1.OSImageStreamSet{Name: "stream-worker", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + { + name: "master pool not found - no fallback", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{workerPool}, + poolName: common.MachineConfigPoolMaster, + expected: nil, + }, + { + name: "arbiter pool not found - no fallback", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool, workerPool}, + poolName: common.MachineConfigPoolArbiter, + expected: nil, + }, + { + name: "worker pool not found - no fallback", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{masterPool}, + poolName: common.MachineConfigPoolWorker, + expected: nil, + }, + { + name: "pool found but stream not in osImageStream", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{ + { + ObjectMeta: metav1.ObjectMeta{Name: "custom"}, + Spec: v1.MachineConfigPoolSpec{ + OSImageStream: v1.OSImageStreamReference{Name: "non-existent-stream"}, + }, + }, + workerPool, + }, + poolName: "custom", + expected: nil, + }, + { + name: "nil osImageStream", + osImageStream: nil, + pools: []*v1.MachineConfigPool{masterPool, workerPool}, + poolName: common.MachineConfigPoolMaster, + expected: nil, + }, + { + name: "empty pools list", + osImageStream: osImageStream, + pools: []*v1.MachineConfigPool{}, + poolName: common.MachineConfigPoolMaster, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := TryGetOSImageStreamFromPoolListByPoolName(tt.osImageStream, tt.pools, tt.poolName) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/controller/osimagestream/osimagestream.go b/pkg/controller/osimagestream/osimagestream.go index 214b9f4e33..3c4cbeaf25 100644 --- a/pkg/controller/osimagestream/osimagestream.go +++ b/pkg/controller/osimagestream/osimagestream.go @@ -12,7 +12,9 @@ import ( imagev1 "github.com/openshift/api/image/v1" v1 "github.com/openshift/api/machineconfiguration/v1" "github.com/openshift/api/machineconfiguration/v1alpha1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/imageutils" + "github.com/openshift/machine-config-operator/pkg/version" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corelisterv1 "k8s.io/client-go/listers/core/v1" @@ -108,6 +110,7 @@ func BuildOsImageStreamRuntime( return factory.CreateRuntimeSources(ctx, releaseImage, sysCtx.SysContext) } +// BuildOSImageStreamFromSources aggregates streams from multiple sources into a single OSImageStream. func BuildOSImageStreamFromSources(ctx context.Context, sources []StreamSource) (*v1alpha1.OSImageStream, error) { streams := collect(ctx, sources) if len(streams) == 0 { @@ -123,7 +126,11 @@ func BuildOSImageStreamFromSources(ctx context.Context, sources []StreamSource) } return &v1alpha1.OSImageStream{ ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", + Name: ctrlcommon.ClusterInstanceNameOSImageStream, + Annotations: map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, + ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + }, }, Spec: &v1alpha1.OSImageStreamSpec{}, Status: v1alpha1.OSImageStreamStatus{ @@ -136,10 +143,7 @@ func BuildOSImageStreamFromSources(ctx context.Context, sources []StreamSource) func getDefaultStreamSet(streams []v1alpha1.OSImageStreamSet) (string, error) { // TODO This logic is temporal. For now, try to locate the RHEL 9 one in best effort // Make a copy to avoid modifying the input slice - streamNames := make([]string, 0, len(streams)) - for _, stream := range streams { - streamNames = append(streamNames, stream.Name) - } + streamNames := GetStreamSetsNames(streams) // Sort by name length (shortest first) to prefer simpler names slices.SortFunc(streamNames, func(a, b string) int { diff --git a/pkg/controller/osimagestream/osimagestream_controller.go b/pkg/controller/osimagestream/osimagestream_controller.go new file mode 100644 index 0000000000..afbe358cf8 --- /dev/null +++ b/pkg/controller/osimagestream/osimagestream_controller.go @@ -0,0 +1,210 @@ +package osimagestream + +import ( + "context" + "fmt" + "time" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/machineconfiguration/v1alpha1" + "github.com/openshift/machine-config-operator/pkg/version" + "k8s.io/apimachinery/pkg/api/errors" + corelisterv1 "k8s.io/client-go/listers/core/v1" + + configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" + configlisters "github.com/openshift/client-go/config/listers/config/v1" + mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" + mcfginformersv1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1" + mcfginformersv1alpha1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1alpha1" + mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1" + mcfglistersv1alpha1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1" + clientset "k8s.io/client-go/kubernetes" + + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + + coreinformersv1 "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/tools/cache" + + "k8s.io/klog/v2" +) + +// Controller manages the OSImageStream singleton resource lifecycle. +type Controller struct { + client mcfgclientset.Interface + kubeClient clientset.Interface + + ccLister mcfglistersv1.ControllerConfigLister + clusterVersionLister configlisters.ClusterVersionLister + osImageStreamLister mcfglistersv1alpha1.OSImageStreamLister + cmLister corelisterv1.ConfigMapLister + + cachesToSync []cache.InformerSynced + bootedChan chan error + // osImageStream holds the OSImageStream resource after successful boot + osImageStream *v1alpha1.OSImageStream + + imageStreamFactory ImageStreamFactory +} + +// NewController creates a new OSImageStream controller. +func NewController( + kubeClient clientset.Interface, + mcfgClient mcfgclientset.Interface, + ccInformer mcfginformersv1.ControllerConfigInformer, + mcoCmInformer coreinformersv1.ConfigMapInformer, + osImageStreamInformer mcfginformersv1alpha1.OSImageStreamInformer, + clusterVersionInformer configinformersv1.ClusterVersionInformer, + imageStreamFactory ImageStreamFactory, +) *Controller { + ctrl := &Controller{ + client: mcfgClient, + kubeClient: kubeClient, + ccLister: ccInformer.Lister(), + clusterVersionLister: clusterVersionInformer.Lister(), + osImageStreamLister: osImageStreamInformer.Lister(), + cmLister: mcoCmInformer.Lister(), + cachesToSync: []cache.InformerSynced{ + ccInformer.Informer().HasSynced, + mcoCmInformer.Informer().HasSynced, + clusterVersionInformer.Informer().HasSynced, + osImageStreamInformer.Informer().HasSynced, + }, + bootedChan: make(chan error, 1), + imageStreamFactory: imageStreamFactory, + } + + // Default to the "full/real" implementation if not factory was provided + if imageStreamFactory == nil { + ctrl.imageStreamFactory = NewDefaultStreamSourceFactory(ctrl.cmLister, &DefaultImagesInspectorFactory{}) + } + + return ctrl +} + +// Run starts the controller and boots the OSImageStream resource. +func (ctrl *Controller) Run(stopCh <-chan struct{}) { + defer utilruntime.HandleCrash() + + if !cache.WaitForCacheSync(stopCh, ctrl.cachesToSync...) { + utilruntime.HandleError(fmt.Errorf("caches did not sync")) + return + } + + klog.Info("Starting MachineConfigController-OSImageStreamController") + defer klog.Info("Shutting down MachineConfigController-OSImageStreamController") + + go func() { + err := ctrl.boot() + if err != nil { + klog.Errorf("Error booting OSImageStreamController: %v", err) + } + + if err != nil { + defer klog.Errorf("OSImageStreamController failed to boot: %v", err) + } else { + defer klog.Infof( + "OSImageStreamController booted successfully. Available streams: %s. Default stream: %s", + GetStreamSetsNames(ctrl.osImageStream.Status.AvailableStreams), + ctrl.osImageStream.Status.DefaultStream, + ) + } + + ctrl.bootedChan <- err + }() + <-stopCh +} + +// WaitBoot blocks until the boot process completes and returns any error encountered. +func (ctrl *Controller) WaitBoot() error { + return <-ctrl.bootedChan +} + +// boot initializes or updates the OSImageStream resource. +// It checks if an update is needed, fetches release images, and creates or updates the resource accordingly. +func (ctrl *Controller) boot() error { + existingOSImageStream, err := ctrl.getExistingOSImageStream() + if err != nil { + return err + } + + if !osImageStreamRequiresUpdate(existingOSImageStream) { + klog.Info("Skipping OSImageStream boot: OSImageStream is already up-to-date") + ctrl.osImageStream = existingOSImageStream + return nil + } + + image, err := GetReleasePayloadImage(ctrl.clusterVersionLister) + if err != nil { + return fmt.Errorf("error getting the Release Image digest from the ClusterVersion for the initial OSImageStream load: %w", err) + } + + secret, cc, err := ctrl.getSysContextObjects() + if err != nil { + return fmt.Errorf("error getting the required dependencies for the initial OSImageStream load: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + osImageStream, err := BuildOsImageStreamRuntime(ctx, secret, cc, image, ctrl.imageStreamFactory) + if err != nil { + return fmt.Errorf("error building the OSImageStream at runtime: %w", err) + } + + if existingOSImageStream == nil { + klog.V(4).Infof("Creating OSImageStream singleton instance as it doesn't exist") + if _, err = ctrl.client.MachineconfigurationV1alpha1().OSImageStreams().Create(context.TODO(), osImageStream, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("error creating the OSImageStream at runtime: %w", err) + } + } else { + oldVersion := existingOSImageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey] + klog.V(4).Infof("Updating the OSImageStream singleton as it was created by a previous version (%s). New version: %s", oldVersion, version.Hash) + if _, err = ctrl.client. + MachineconfigurationV1alpha1(). + OSImageStreams(). + UpdateStatus(context.TODO(), osImageStream, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("error updating the OSImageStream at runtime: %w", err) + } + } + ctrl.osImageStream = osImageStream + return nil +} + +// getExistingOSImageStream retrieves the existing OSImageStream from the lister. +// Returns nil if the OSImageStream does not exist. +func (ctrl *Controller) getExistingOSImageStream() (*v1alpha1.OSImageStream, error) { + osImageStream, err := ctrl.osImageStreamLister.Get(ctrlcommon.ClusterInstanceNameOSImageStream) + if err != nil { + if !errors.IsNotFound(err) { + return nil, fmt.Errorf("failed to retrieve existing OSImageStream: %v", err) + } + return nil, nil + } + return osImageStream, nil +} + +// osImageStreamRequiresUpdate checks if the OSImageStream needs to be created or updated. +// Returns true if osImageStream is nil or if its version annotation doesn't match the current version. +func osImageStreamRequiresUpdate(osImageStream *v1alpha1.OSImageStream) bool { + if osImageStream == nil { + return true + } + releaseVersion, ok := osImageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey] + return !ok || releaseVersion != version.Hash +} + +func (ctrl *Controller) getSysContextObjects() (*corev1.Secret, *mcfgv1.ControllerConfig, error) { + cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + return nil, nil, fmt.Errorf("could not get ControllerConfig for OSImageStream initial load: %v", err) + } + + clusterPullSecret, err := ctrl.kubeClient.CoreV1().Secrets(cc.Spec.PullSecret.Namespace).Get(context.TODO(), cc.Spec.PullSecret.Name, metav1.GetOptions{}) + if err != nil { + return nil, nil, fmt.Errorf("could not get the cluster PullSecret for OSImageStream initial load: %v", err) + } + return clusterPullSecret, cc, nil +} diff --git a/pkg/controller/osimagestream/osimagestream_controller_test.go b/pkg/controller/osimagestream/osimagestream_controller_test.go new file mode 100644 index 0000000000..ddb6e3d274 --- /dev/null +++ b/pkg/controller/osimagestream/osimagestream_controller_test.go @@ -0,0 +1,404 @@ +// Assisted-by: Claude +package osimagestream + +import ( + "context" + "testing" + "time" + + "github.com/containers/image/v5/types" + configv1 "github.com/openshift/api/config/v1" + imagev1 "github.com/openshift/api/image/v1" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/machineconfiguration/v1alpha1" + configfake "github.com/openshift/client-go/config/clientset/versioned/fake" + configinformers "github.com/openshift/client-go/config/informers/externalversions" + mcfgfake "github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake" + mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/openshift/machine-config-operator/pkg/version" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubeinformers "k8s.io/client-go/informers" + kubefake "k8s.io/client-go/kubernetes/fake" +) + +// mockImageStreamFactory is a test implementation of ImageStreamFactory +type mockImageStreamFactory struct { + runtimeStream *v1alpha1.OSImageStream + runtimeErr error + bootstrapStream *v1alpha1.OSImageStream + bootstrapErr error +} + +func (m *mockImageStreamFactory) CreateRuntimeSources(_ context.Context, _ string, _ *types.SystemContext) (*v1alpha1.OSImageStream, error) { + return m.runtimeStream, m.runtimeErr +} + +func (m *mockImageStreamFactory) CreateBootstrapSources(_ context.Context, _ *imagev1.ImageStream, _ *OSImageTuple, _ *types.SystemContext) (*v1alpha1.OSImageStream, error) { + return m.bootstrapStream, m.bootstrapErr +} + +func TestController_Run_BootSuccess(t *testing.T) { + // Create existing OSImageStream with current version (no update needed) + existingOSImageStream := &v1alpha1.OSImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ClusterInstanceNameOSImageStream, + Annotations: map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, + ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + }, + }, + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + }, + } + + // Create fake clients + mcfgObjs := []runtime.Object{existingOSImageStream} + fakeMcfgClient := mcfgfake.NewSimpleClientset(mcfgObjs...) + mcfgInformerFactory := mcfginformers.NewSharedInformerFactory(fakeMcfgClient, 0) + + configObjs := []runtime.Object{} + fakeConfigClient := configfake.NewSimpleClientset(configObjs...) + configInformerFactory := configinformers.NewSharedInformerFactory(fakeConfigClient, 0) + + fakeKubeClient := kubefake.NewSimpleClientset() + kubeInformerFactory := kubeinformers.NewSharedInformerFactory(fakeKubeClient, 0) + + // Setup informers + ccInformer := mcfgInformerFactory.Machineconfiguration().V1().ControllerConfigs() + cmInformer := kubeInformerFactory.Core().V1().ConfigMaps() + osImageStreamInformer := mcfgInformerFactory.Machineconfiguration().V1alpha1().OSImageStreams() + cvInformer := configInformerFactory.Config().V1().ClusterVersions() + + // Add objects to indexers + osImageStreamInformer.Informer().GetIndexer().Add(existingOSImageStream) + + // Mock factory (not used since no update is needed) + mockFactory := &mockImageStreamFactory{} + + // Create controller using constructor + ctrl := NewController( + fakeKubeClient, + fakeMcfgClient, + ccInformer, + cmInformer, + osImageStreamInformer, + cvInformer, + mockFactory, + ) + + // Start informers + stopCh := make(chan struct{}) + defer close(stopCh) + + mcfgInformerFactory.Start(stopCh) + configInformerFactory.Start(stopCh) + kubeInformerFactory.Start(stopCh) + + // Run controller in goroutine + go ctrl.Run(stopCh) + + // Wait for boot to complete using WaitBoot + done := make(chan error, 1) + go func() { + done <- ctrl.WaitBoot() + }() + + select { + case err := <-done: + require.NoError(t, err) + + // Verify the OSImageStream was not modified (remains as it was) + osImageStream, err := fakeMcfgClient.MachineconfigurationV1alpha1(). + OSImageStreams(). + Get(context.TODO(), ctrlcommon.ClusterInstanceNameOSImageStream, metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, existingOSImageStream, osImageStream) + case <-time.After(2 * time.Second): + t.Fatal("Boot did not complete in time") + } +} + +func TestController_Run_NoOSImageStream(t *testing.T) { + // No existing OSImageStream - controller should create one + + // New OSImageStream that will be returned by the mock factory and created + newOSImageStream := &v1alpha1.OSImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ClusterInstanceNameOSImageStream, + Annotations: map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, + ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + }, + }, + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + }, + } + + // Create fake clients + mcfgObjs := []runtime.Object{} + fakeMcfgClient := mcfgfake.NewSimpleClientset(mcfgObjs...) + mcfgInformerFactory := mcfginformers.NewSharedInformerFactory(fakeMcfgClient, 0) + + // Provide ClusterVersion + clusterVersion := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Status: configv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Image: "quay.io/openshift-release-dev/ocp-release@sha256:abc123", + }, + }, + } + configObjs := []runtime.Object{clusterVersion} + fakeConfigClient := configfake.NewSimpleClientset(configObjs...) + configInformerFactory := configinformers.NewSharedInformerFactory(fakeConfigClient, 0) + + // Provide ControllerConfig with PullSecret + controllerConfig := &mcfgv1.ControllerConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ControllerConfigName, + }, + Spec: mcfgv1.ControllerConfigSpec{ + PullSecret: &corev1.ObjectReference{ + Name: "test-pull-secret", + Namespace: "openshift-config", + }, + }, + } + mcfgObjs = append(mcfgObjs, controllerConfig) + fakeMcfgClient = mcfgfake.NewSimpleClientset(mcfgObjs...) + mcfgInformerFactory = mcfginformers.NewSharedInformerFactory(fakeMcfgClient, 0) + + // Provide PullSecret + pullSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pull-secret", + Namespace: "openshift-config", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{}}`), + }, + } + fakeKubeClient := kubefake.NewSimpleClientset(pullSecret) + kubeInformerFactory := kubeinformers.NewSharedInformerFactory(fakeKubeClient, 0) + + // Setup informers + ccInformer := mcfgInformerFactory.Machineconfiguration().V1().ControllerConfigs() + cmInformer := kubeInformerFactory.Core().V1().ConfigMaps() + osImageStreamInformer := mcfgInformerFactory.Machineconfiguration().V1alpha1().OSImageStreams() + cvInformer := configInformerFactory.Config().V1().ClusterVersions() + + // Add objects to indexers + cvInformer.Informer().GetIndexer().Add(clusterVersion) + ccInformer.Informer().GetIndexer().Add(controllerConfig) + + // Mock factory that returns the new OSImageStream + mockFactory := &mockImageStreamFactory{ + runtimeStream: newOSImageStream, + } + + // Create controller using constructor + ctrl := NewController( + fakeKubeClient, + fakeMcfgClient, + ccInformer, + cmInformer, + osImageStreamInformer, + cvInformer, + mockFactory, + ) + + // Start informers + stopCh := make(chan struct{}) + defer close(stopCh) + + mcfgInformerFactory.Start(stopCh) + configInformerFactory.Start(stopCh) + kubeInformerFactory.Start(stopCh) + + // Run controller in goroutine + go ctrl.Run(stopCh) + + // Wait for boot to complete using WaitBoot + done := make(chan error, 1) + go func() { + done <- ctrl.WaitBoot() + }() + + select { + case err := <-done: + require.NoError(t, err) + + // Verify the OSImageStream was created + created, err := fakeMcfgClient.MachineconfigurationV1alpha1(). + OSImageStreams(). + Get(context.TODO(), ctrlcommon.ClusterInstanceNameOSImageStream, metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, version.Hash, created.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]) + assert.Equal(t, "rhel-9", created.Status.DefaultStream) + assert.Len(t, created.Status.AvailableStreams, 1) + assert.Equal(t, v1alpha1.ImageDigestFormat("image1"), created.Status.AvailableStreams[0].OSImage) + case <-time.After(2 * time.Second): + t.Fatal("Boot did not complete in time") + } +} + +func TestController_Run_OldVersion(t *testing.T) { + // Create existing OSImageStream with old version (update needed) + existingOSImageStream := &v1alpha1.OSImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ClusterInstanceNameOSImageStream, + Annotations: map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: "old-version-hash", + }, + }, + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9-old", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9-old", OSImage: "old-image", OSExtensionsImage: "old-ext"}, + }, + }, + } + + // Updated OSImageStream that will be returned by the mock factory + updatedOSImageStream := &v1alpha1.OSImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ClusterInstanceNameOSImageStream, + Annotations: map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, + ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + }, + }, + Status: v1alpha1.OSImageStreamStatus{ + DefaultStream: "rhel-9", + AvailableStreams: []v1alpha1.OSImageStreamSet{ + {Name: "rhel-9", OSImage: "new-image", OSExtensionsImage: "new-ext"}, + }, + }, + } + + // Create fake clients + mcfgObjs := []runtime.Object{existingOSImageStream} + fakeMcfgClient := mcfgfake.NewSimpleClientset(mcfgObjs...) + mcfgInformerFactory := mcfginformers.NewSharedInformerFactory(fakeMcfgClient, 0) + + // Provide ClusterVersion + clusterVersion := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Status: configv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Image: "quay.io/openshift-release-dev/ocp-release@sha256:abc123", + }, + }, + } + configObjs := []runtime.Object{clusterVersion} + fakeConfigClient := configfake.NewSimpleClientset(configObjs...) + configInformerFactory := configinformers.NewSharedInformerFactory(fakeConfigClient, 0) + + // Provide ControllerConfig with PullSecret + controllerConfig := &mcfgv1.ControllerConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ControllerConfigName, + }, + Spec: mcfgv1.ControllerConfigSpec{ + PullSecret: &corev1.ObjectReference{ + Name: "test-pull-secret", + Namespace: "openshift-config", + }, + }, + } + mcfgObjs = append(mcfgObjs, controllerConfig) + fakeMcfgClient = mcfgfake.NewSimpleClientset(mcfgObjs...) + mcfgInformerFactory = mcfginformers.NewSharedInformerFactory(fakeMcfgClient, 0) + + // Provide PullSecret + pullSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pull-secret", + Namespace: "openshift-config", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{}}`), + }, + } + fakeKubeClient := kubefake.NewSimpleClientset(pullSecret) + kubeInformerFactory := kubeinformers.NewSharedInformerFactory(fakeKubeClient, 0) + + // Setup informers + ccInformer := mcfgInformerFactory.Machineconfiguration().V1().ControllerConfigs() + cmInformer := kubeInformerFactory.Core().V1().ConfigMaps() + osImageStreamInformer := mcfgInformerFactory.Machineconfiguration().V1alpha1().OSImageStreams() + cvInformer := configInformerFactory.Config().V1().ClusterVersions() + + // Add objects to indexers + osImageStreamInformer.Informer().GetIndexer().Add(existingOSImageStream) + cvInformer.Informer().GetIndexer().Add(clusterVersion) + ccInformer.Informer().GetIndexer().Add(controllerConfig) + + // Mock factory that returns the updated OSImageStream + mockFactory := &mockImageStreamFactory{ + runtimeStream: updatedOSImageStream, + } + + // Create controller using constructor + ctrl := NewController( + fakeKubeClient, + fakeMcfgClient, + ccInformer, + cmInformer, + osImageStreamInformer, + cvInformer, + mockFactory, + ) + + // Start informers + stopCh := make(chan struct{}) + defer close(stopCh) + + mcfgInformerFactory.Start(stopCh) + configInformerFactory.Start(stopCh) + kubeInformerFactory.Start(stopCh) + + // Run controller in goroutine + go ctrl.Run(stopCh) + + // Wait for boot to complete using WaitBoot + done := make(chan error, 1) + go func() { + done <- ctrl.WaitBoot() + }() + + select { + case err := <-done: + require.NoError(t, err) + + // Verify the OSImageStream was updated + updated, err := fakeMcfgClient.MachineconfigurationV1alpha1(). + OSImageStreams(). + Get(context.TODO(), ctrlcommon.ClusterInstanceNameOSImageStream, metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, version.Hash, updated.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]) + assert.Equal(t, "rhel-9", updated.Status.DefaultStream) + assert.Equal(t, v1alpha1.ImageDigestFormat("new-image"), updated.Status.AvailableStreams[0].OSImage) + case <-time.After(2 * time.Second): + t.Fatal("Boot did not complete in time") + } +} diff --git a/pkg/controller/osimagestream/osimagestream_test.go b/pkg/controller/osimagestream/osimagestream_test.go index d4b6206189..97132c9249 100644 --- a/pkg/controller/osimagestream/osimagestream_test.go +++ b/pkg/controller/osimagestream/osimagestream_test.go @@ -9,6 +9,8 @@ import ( "github.com/containers/image/v5/types" imagev1 "github.com/openshift/api/image/v1" "github.com/openshift/api/machineconfiguration/v1alpha1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/openshift/machine-config-operator/pkg/version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -171,6 +173,8 @@ func TestBuildOSImageStreamFromSources(t *testing.T) { require.NoError(t, err) assert.NotNil(t, result) assert.Equal(t, "cluster", result.Name) + assert.Equal(t, version.Hash, result.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]) + assert.Equal(t, version.Hash, result.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey]) if tt.expectedDefault != "" { assert.Equal(t, tt.expectedDefault, result.Status.DefaultStream) } diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 64624ba8f8..d72c98921b 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -229,3 +229,13 @@ func CanonicalizeKernelType(kernelType string) string { } return ctrlcommon.KernelTypeDefault } + +// GetPoolByName finds a MachineConfigPool by name from a list of pools. +func GetPoolByName(pools []*mcfgv1.MachineConfigPool, name string) *mcfgv1.MachineConfigPool { + for _, pool := range pools { + if pool.Name == name { + return pool + } + } + return nil +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go new file mode 100644 index 0000000000..609aad383b --- /dev/null +++ b/pkg/helpers/helpers_test.go @@ -0,0 +1,75 @@ +// Assisted-by: Claude +package helpers + +import ( + "testing" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetPoolByName(t *testing.T) { + masterPool := &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "master"}, + } + + workerPool := &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + } + + customPool := &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "custom"}, + } + + tests := []struct { + name string + pools []*mcfgv1.MachineConfigPool + poolName string + expected *mcfgv1.MachineConfigPool + }{ + { + name: "find master pool", + pools: []*mcfgv1.MachineConfigPool{masterPool, workerPool, customPool}, + poolName: "master", + expected: masterPool, + }, + { + name: "find worker pool", + pools: []*mcfgv1.MachineConfigPool{masterPool, workerPool, customPool}, + poolName: "worker", + expected: workerPool, + }, + { + name: "find custom pool", + pools: []*mcfgv1.MachineConfigPool{masterPool, workerPool, customPool}, + poolName: "custom", + expected: customPool, + }, + { + name: "pool not found", + pools: []*mcfgv1.MachineConfigPool{masterPool, workerPool}, + poolName: "non-existent", + expected: nil, + }, + { + name: "empty pools list", + pools: []*mcfgv1.MachineConfigPool{}, + poolName: "master", + expected: nil, + }, + { + name: "nil pools list", + pools: nil, + poolName: "master", + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetPoolByName(tt.pools, tt.poolName) + assert.Equal(t, tt.expected, result) + }) + } +}