diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index 08abb66217..527e5c2ea4 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -73,6 +73,15 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx := ctrlcommon.CreateControllerContext(ctx, cb) + // Early start the config informer because feature gate depends on it + ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop) + if fgErr := ctrlctx.FeatureGatesHandler.Connect(ctx); fgErr != nil { + klog.Error(fmt.Errorf("failed to connect to feature gates %w", fgErr)) + runCancel() + <-ctx.Done() + return + } + go ctrlcommon.StartMetricsListener(startOpts.promMetricsListenAddress, ctrlctx.Stop, ctrlcommon.RegisterMCCMetrics) controllers := createControllers(ctrlctx) @@ -111,10 +120,6 @@ func runStartCmd(_ *cobra.Command, _ []string) { close(ctrlctx.InformersStarted) - if fgErr := ctrlctx.FeatureGatesHandler.Connect(ctx); fgErr != nil { - klog.Fatal(fmt.Errorf("failed to connect to feature gates %w", fgErr)) - } - if ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGatePinnedImages) && ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateMachineConfigNodes) { pinnedImageSet := pinnedimageset.New( ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(), @@ -245,6 +250,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.InformerFactory.Machineconfiguration().V1().ContainerRuntimeConfigs(), ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(), ctx.OperatorInformerFactory.Operator().V1().MachineConfigurations(), + ctx.InformerFactory.Machineconfiguration().V1alpha1().OSImageStreams(), ctx.ClientBuilder.KubeClientOrDie("render-controller"), ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"), ctx.FeatureGatesHandler, diff --git a/cmd/machine-config-operator/start.go b/cmd/machine-config-operator/start.go index 22d7323dea..b9cbca3265 100644 --- a/cmd/machine-config-operator/start.go +++ b/cmd/machine-config-operator/start.go @@ -67,6 +67,12 @@ func runStartCmd(_ *cobra.Command, _ []string) { go common.SignalHandler(runCancel) ctrlctx := ctrlcommon.CreateControllerContext(ctx, cb) + // Early start the config informer because feature gate depends on it + ctrlctx.ConfigInformerFactory.Start(ctrlctx.Stop) + if fgErr := ctrlctx.FeatureGatesHandler.Connect(ctx); fgErr != nil { + klog.Fatal(fmt.Errorf("failed to connect to feature gates %w", fgErr)) + } + controller := operator.New( ctrlcommon.MCONamespace, componentName, startOpts.imagesFile, @@ -107,6 +113,8 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.ConfigInformerFactory.Config().V1().Nodes(), ctrlctx.ConfigInformerFactory.Config().V1().APIServers(), ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineOSConfigs(), + ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(), + ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().OSImageStreams(), ctrlctx, ) @@ -124,10 +132,6 @@ func runStartCmd(_ *cobra.Command, _ []string) { close(ctrlctx.InformersStarted) - if fgErr := ctrlctx.FeatureGatesHandler.Connect(ctx); fgErr != nil { - klog.Fatal(fmt.Errorf("failed to connect to feature gates %w", fgErr)) - } - go controller.Run(2, ctrlctx.Stop) // wait here in this function until the context gets cancelled (which tells us whe were being shut down) diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index cd256b5e9c..85a452481c 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -2,12 +2,15 @@ package bootstrap import ( "bytes" + "context" "errors" "fmt" "io" "os" "path/filepath" + "time" + "github.com/openshift/machine-config-operator/pkg/osimagestream" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1 "k8s.io/api/core/v1" @@ -23,6 +26,7 @@ import ( apicfgv1 "github.com/openshift/api/config/v1" apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" "github.com/openshift/api/features" + imagev1 "github.com/openshift/api/image/v1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" @@ -70,7 +74,7 @@ func (b *Bootstrap) Run(destDir string) error { return err } - psraw, err := getPullSecretFromSecret(psfraw) + pullSecret, err := getValidPullSecretFromBytes(psfraw) if err != nil { return err } @@ -82,8 +86,13 @@ func (b *Bootstrap) Run(destDir string) error { apicfgv1.Install(scheme) apicfgv1alpha1.Install(scheme) corev1.AddToScheme(scheme) + imagev1.AddToScheme(scheme) codecFactory := serializer.NewCodecFactory(scheme) - decoder := codecFactory.UniversalDecoder(mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, apicfgv1.GroupVersion, apicfgv1alpha1.GroupVersion, corev1.SchemeGroupVersion, mcfgv1alpha1.GroupVersion) + decoder := codecFactory.UniversalDecoder( + mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, + apicfgv1.GroupVersion, apicfgv1alpha1.GroupVersion, + corev1.SchemeGroupVersion, mcfgv1alpha1.GroupVersion, + imagev1.SchemeGroupVersion) var ( cconfig *mcfgv1.ControllerConfig @@ -101,6 +110,7 @@ func (b *Bootstrap) Run(destDir string) error { imagePolicies []*apicfgv1.ImagePolicy imgCfg *apicfgv1.Image apiServer *apicfgv1.APIServer + imageStream *imagev1.ImageStream iri *mcfgv1alpha1.InternalReleaseImage iriTLSCert *corev1.Secret ) @@ -172,6 +182,17 @@ func (b *Bootstrap) Run(destDir string) error { if obj.GetName() == ctrlcommon.InternalReleaseImageInstanceName { iri = obj } + case *imagev1.ImageStream: + for _, tag := range obj.Spec.Tags { + if tag.Name == "machine-config-operator" { + if imageStream != nil { + klog.Infof("multiple ImageStream found. Previous ImageStream %s replaced by %s", imageStream.Name, obj.Name) + } + imageStream = obj + + } + } + // It's an ImageStream that doesn't look like the Release one (doesn't have our tag) case *corev1.Secret: if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName { iriTLSCert = obj @@ -196,7 +217,40 @@ func (b *Bootstrap) Run(destDir string) error { return fmt.Errorf("error creating feature gates handler: %w", err) } - iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, psraw, apiServer) + var osImageStream *mcfgv1alpha1.OSImageStream + // Enable OSImageStreams if the FeatureGate is active and the deployment is not OKD + if osimagestream.IsFeatureEnabled(fgHandler) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + osImageStream, err = osimagestream.BuildOsImageStreamBootstrap(ctx, + pullSecret, + cconfig, + imageStream, + &osimagestream.OSImageTuple{ + OSImage: cconfig.Spec.BaseOSContainerImage, + OSExtensionsImage: cconfig.Spec.BaseOSExtensionsContainerImage, + }, + osimagestream.NewDefaultStreamSourceFactory(nil, &osimagestream.DefaultImagesInspectorFactory{}), + ) + if err != nil { + return fmt.Errorf("error inspecting available OSImageStreams: %w", err) + } + + // If no error happened override the ControllerConfig URLs with the default stream ones + if err == nil { + defaultStreamSet, err := osimagestream.GetOSImageStreamSetByName(osImageStream, "") + if err != nil { + // Should never happen + return fmt.Errorf("error getting default OSImageStreamSet: %w", err) + } + cconfig.Spec.BaseOSContainerImage = string(defaultStreamSet.OSImage) + cconfig.Spec.BaseOSExtensionsContainerImage = string(defaultStreamSet.OSExtensionsImage) + } + } + + pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey] + iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, pullSecretBytes, apiServer) if err != nil { return err } @@ -279,7 +333,7 @@ func (b *Bootstrap) Run(destDir string) error { klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs)) } - fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig) + fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig, osImageStream) if err != nil { return err } @@ -362,7 +416,7 @@ func (b *Bootstrap) Run(destDir string) error { } -func getPullSecretFromSecret(sData []byte) ([]byte, error) { +func getValidPullSecretFromBytes(sData []byte) (*corev1.Secret, error) { obji, err := runtime.Decode(kscheme.Codecs.UniversalDecoder(corev1.SchemeGroupVersion), sData) if err != nil { return nil, err @@ -374,7 +428,7 @@ func getPullSecretFromSecret(sData []byte) ([]byte, error) { if s.Type != corev1.SecretTypeDockerConfigJson { return nil, fmt.Errorf("expected secret type %s found %s", corev1.SecretTypeDockerConfigJson, s.Type) } - return s.Data[corev1.DockerConfigJsonKey], nil + return s, nil } type manifest struct { diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index acac5425a1..1657c738fd 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -28,6 +28,7 @@ import ( ign2types "github.com/coreos/ignition/config/v2_2/types" validate2 "github.com/coreos/ignition/config/validate" ign3error "github.com/coreos/ignition/v2/config/shared/errors" + "github.com/openshift/api/machineconfiguration/v1alpha1" ign3 "github.com/coreos/ignition/v2/config/v3_5" ign3types "github.com/coreos/ignition/v2/config/v3_5/types" @@ -68,7 +69,7 @@ func boolToPtr(b bool) *bool { // It uses the Ignition config from first object as base and appends all the rest. // Kernel arguments are concatenated. // It defaults to the OSImageURL provided by the CVO but allows a MC provided OSImageURL to take precedence. -func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) { +func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, imageStream *v1alpha1.OSImageStreamSet) (*mcfgv1.MachineConfig, error) { if len(configs) == 0 { return nil, nil } @@ -187,7 +188,7 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.Contro // For layering, we want to let the user override OSImageURL again // The template configs always match what's in controllerconfig because they get rendered from there, // so the only way we get an override here is if the user adds something different - osImageURL := GetDefaultBaseImageContainer(&cconfig.Spec) + osImageURL := GetBaseImageContainer(&cconfig.Spec, imageStream) for _, cfg := range configs { // Ignore generated MCs, only the rendered MC or a user provided MC can set this field if cfg.Annotations[GeneratedByControllerVersionAnnotationKey] != "" { @@ -199,7 +200,7 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.Contro } // Allow overriding the extensions container - baseOSExtensionsContainerImage := cconfig.Spec.BaseOSExtensionsContainerImage + baseOSExtensionsContainerImage := GetBaseExtensionsImageContainer(&cconfig.Spec, imageStream) for _, cfg := range configs { // Ignore generated MCs, only the rendered MC or a user provided MC can set this field if cfg.Annotations[GeneratedByControllerVersionAnnotationKey] != "" { @@ -1043,9 +1044,20 @@ func GetIgnitionFileDataByPath(config *ign3types.Config, path string) ([]byte, e return nil, nil } -// GetDefaultBaseImageContainer returns the default bootable host base image. -func GetDefaultBaseImageContainer(cconfigspec *mcfgv1.ControllerConfigSpec) string { - return cconfigspec.BaseOSContainerImage +// GetBaseImageContainer returns the default bootable host base image. +func GetBaseImageContainer(cconfigspec *mcfgv1.ControllerConfigSpec, imageStream *v1alpha1.OSImageStreamSet) string { + if imageStream == nil { + return cconfigspec.BaseOSContainerImage + } + return string(imageStream.OSImage) +} + +// GetBaseExtensionsImageContainer returns the default bootable host base image. +func GetBaseExtensionsImageContainer(cconfigspec *mcfgv1.ControllerConfigSpec, imageStream *v1alpha1.OSImageStreamSet) string { + if imageStream == nil { + return cconfigspec.BaseOSExtensionsContainerImage + } + return string(imageStream.OSExtensionsImage) } // Configures common template FuncMaps used across all renderers. diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index c1f2f9e103..a1ff947dff 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -383,7 +383,7 @@ func TestMergeMachineConfigs(t *testing.T) { }, } inMachineConfigs := []*mcfgv1.MachineConfig{machineConfigFIPS} - mergedMachineConfig, err := MergeMachineConfigs(inMachineConfigs, cconfig) + mergedMachineConfig, err := MergeMachineConfigs(inMachineConfigs, cconfig, nil) require.Nil(t, err) // check that the outgoing config does have the version string set, @@ -397,7 +397,7 @@ func TestMergeMachineConfigs(t *testing.T) { require.Nil(t, err) expectedMachineConfig := &mcfgv1.MachineConfig{ Spec: mcfgv1.MachineConfigSpec{ - OSImageURL: GetDefaultBaseImageContainer(&cconfig.Spec), + OSImageURL: GetBaseImageContainer(&cconfig.Spec, nil), KernelArguments: []string{}, Config: runtime.RawExtension{ Raw: rawOutIgn, @@ -504,7 +504,7 @@ func TestMergeMachineConfigs(t *testing.T) { machineConfigIgnV2Merge, } - mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig) + mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig, nil) require.Nil(t, err) expectedMachineConfig = &mcfgv1.MachineConfig{ @@ -588,7 +588,7 @@ func TestMergeMachineConfigs(t *testing.T) { } cconfig = &mcfgv1.ControllerConfig{} - mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig) + mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig, nil) require.Nil(t, err) // The expectation here is that the merged config contains the MCs with name bbb (overrides aaa due to name) and ccc (overrides ddd due to pool) @@ -795,7 +795,7 @@ func TestSetDefaultFileOverwrite(t *testing.T) { require.Nil(t, err) cconfig := &mcfgv1.ControllerConfig{} - mergedMachineConfig, err := MergeMachineConfigs([]*mcfgv1.MachineConfig{machineConfigPreMerge}, cconfig) + mergedMachineConfig, err := MergeMachineConfigs([]*mcfgv1.MachineConfig{machineConfigPreMerge}, cconfig, nil) require.Nil(t, err) // Convert and create the expected post-merge config diff --git a/pkg/controller/node/status.go b/pkg/controller/node/status.go index fe748a4ebd..95724085ee 100644 --- a/pkg/controller/node/status.go +++ b/pkg/controller/node/status.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/openshift/machine-config-operator/pkg/osimagestream" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -250,6 +251,13 @@ func (ctrl *Controller) calculateStatus(mcns []*mcfgv1.MachineConfigNode, cconfi unavailableMachineCount == 0 && !isLayeredPoolBuilding(isLayeredPool, mosc, mosb) if allUpdated { + // When the pool is fully updated & the `OSStreams` FeatureGate is enabled, set the + // `OSImageStream` reference in the MCP status to be consistent to what is defined in the + // MCP spec + if osimagestream.IsFeatureEnabled(ctrl.fgHandler) { + status.OSImageStream = pool.Spec.OSImageStream + } + //TODO: update api to only have one condition regarding status of update. updatedMsg := fmt.Sprintf("All nodes are updated with %s", getPoolUpdateLine(pool, mosc, isLayeredPool)) supdated := apihelpers.NewMachineConfigPoolCondition(mcfgv1.MachineConfigPoolUpdated, corev1.ConditionTrue, "", updatedMsg) diff --git a/pkg/controller/node/status_test.go b/pkg/controller/node/status_test.go index 6ae7e06620..aa118b5692 100644 --- a/pkg/controller/node/status_test.go +++ b/pkg/controller/node/status_test.go @@ -437,6 +437,7 @@ func TestCalculateStatus(t *testing.T) { nodes []*corev1.Node currentConfig string paused bool + osStream mcfgv1.OSImageStreamReference verify func(mcfgv1.MachineConfigPoolStatus, *testing.T) }{{ name: "0 nodes updated, 0 nodes updating, 0 nodes degraded", @@ -884,6 +885,157 @@ func TestCalculateStatus(t *testing.T) { t.Fatalf("mismatch conddegraded.Status: got %s want: %s", got, want) } }, + }, { + name: "all nodes updated, OSStream defined in MCP Spec", + nodes: []*corev1.Node{ + helpers.NewNodeWithReady("node-0", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-1", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + }, + currentConfig: machineConfigV0, + osStream: mcfgv1.OSImageStreamReference{ + Name: "rhel-10", + }, + verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) { + if got, want := status.MachineCount, int32(3); got != want { + t.Fatalf("mismatch MachineCount: got %d want: %d", got, want) + } + + if got, want := status.UpdatedMachineCount, int32(3); got != want { + t.Fatalf("mismatch UpdatedMachineCount: got %d want: %d", got, want) + } + + if got, want := status.ReadyMachineCount, int32(3); got != want { + t.Fatalf("mismatch ReadyMachineCount: got %d want: %d", got, want) + } + + if got, want := status.UnavailableMachineCount, int32(0); got != want { + t.Fatalf("mismatch UnavailableMachineCount: got %d want: %d", got, want) + } + + condupdated := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdated) + if condupdated == nil { + t.Fatal("updated condition not found") + } + + condupdating := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdating) + if condupdating == nil { + t.Fatal("updating condition not found") + } + + if got, want := condupdated.Status, corev1.ConditionTrue; got != want { + t.Fatalf("mismatch condupdated.Status: got %s want: %s", got, want) + } + + if got, want := condupdating.Status, corev1.ConditionFalse; got != want { + t.Fatalf("mismatch condupdating.Status: got %s want: %s", got, want) + } + + statusOSStreamName := status.OSImageStream.Name + if statusOSStreamName != "rhel-10" { + t.Fatal("OSImageStreamReference in MCP status not updated correctly") + } + }, + }, { + name: "some nodes still updating, OSStream defined in MCP Spec", + nodes: []*corev1.Node{ + helpers.NewNodeWithReady("node-0", machineConfigV0, machineConfigV1, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-1", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + }, + currentConfig: machineConfigV1, + osStream: mcfgv1.OSImageStreamReference{ + Name: "rhel-10", + }, + verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) { + if got, want := status.MachineCount, int32(3); got != want { + t.Fatalf("mismatch MachineCount: got %d want: %d", got, want) + } + + if got, want := status.UpdatedMachineCount, int32(0); got != want { + t.Fatalf("mismatch UpdatedMachineCount: got %d want: %d", got, want) + } + + if got, want := status.ReadyMachineCount, int32(0); got != want { + t.Fatalf("mismatch ReadyMachineCount: got %d want: %d", got, want) + } + + if got, want := status.UnavailableMachineCount, int32(1); got != want { + t.Fatalf("mismatch UnavailableMachineCount: got %d want: %d", got, want) + } + + condupdated := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdated) + if condupdated == nil { + t.Fatal("updated condition not found") + } + + condupdating := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdating) + if condupdating == nil { + t.Fatal("updating condition not found") + } + + if got, want := condupdated.Status, corev1.ConditionFalse; got != want { + t.Fatalf("mismatch condupdated.Status: got %s want: %s", got, want) + } + + if got, want := condupdating.Status, corev1.ConditionTrue; got != want { + t.Fatalf("mismatch condupdating.Status: got %s want: %s", got, want) + } + + statusOSStreamName := status.OSImageStream.Name + if statusOSStreamName == "rhel-10" { + t.Fatal("OSImageStreamReference updated in MCP status, but should not be") + } + }, + }, { + name: "all nodes updated, OSStream removed from MCP Spec", + nodes: []*corev1.Node{ + helpers.NewNodeWithReady("node-0", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-1", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue), + }, + currentConfig: machineConfigV0, + osStream: mcfgv1.OSImageStreamReference{}, + verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) { + if got, want := status.MachineCount, int32(3); got != want { + t.Fatalf("mismatch MachineCount: got %d want: %d", got, want) + } + + if got, want := status.UpdatedMachineCount, int32(3); got != want { + t.Fatalf("mismatch UpdatedMachineCount: got %d want: %d", got, want) + } + + if got, want := status.ReadyMachineCount, int32(3); got != want { + t.Fatalf("mismatch ReadyMachineCount: got %d want: %d", got, want) + } + + if got, want := status.UnavailableMachineCount, int32(0); got != want { + t.Fatalf("mismatch UnavailableMachineCount: got %d want: %d", got, want) + } + + condupdated := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdated) + if condupdated == nil { + t.Fatal("updated condition not found") + } + + condupdating := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdating) + if condupdating == nil { + t.Fatal("updating condition not found") + } + + if got, want := condupdated.Status, corev1.ConditionTrue; got != want { + t.Fatalf("mismatch condupdated.Status: got %s want: %s", got, want) + } + + if got, want := condupdating.Status, corev1.ConditionFalse; got != want { + t.Fatalf("mismatch condupdating.Status: got %s want: %s", got, want) + } + + statusOSStream := status.OSImageStream + if statusOSStream.Name != "" { + t.Fatal("OSImageStreamReference in MCP status not cleared correctly") + } + }, }} for idx, test := range tests { idx := idx @@ -894,12 +1046,14 @@ func TestCalculateStatus(t *testing.T) { Spec: mcfgv1.MachineConfigPoolSpec{ Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ObjectReference: corev1.ObjectReference{Name: test.currentConfig}}, Paused: test.paused, + OSImageStream: test.osStream, }, } f := newFixtureWithFeatureGates(t, []apicfgv1.FeatureGateName{ features.FeatureGateMachineConfigNodes, features.FeatureGatePinnedImages, + features.FeatureGateOSStreams, }, []apicfgv1.FeatureGateName{}, ) diff --git a/pkg/controller/osimagestream/helpers_test.go b/pkg/controller/osimagestream/helpers_test.go deleted file mode 100644 index f1503c4543..0000000000 --- a/pkg/controller/osimagestream/helpers_test.go +++ /dev/null @@ -1,313 +0,0 @@ -// 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" - apierrors "k8s.io/apimachinery/pkg/api/errors" - 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 getStubOSImageStream() *v1alpha1.OSImageStream { - return &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"}, - }, - }, - } -} - -func TestGetOSImageStreamSetByName(t *testing.T) { - tests := []struct { - name string - osImageStreamFactory func() *v1alpha1.OSImageStream - streamName string - expected *v1alpha1.OSImageStreamSet - errorContains string - errorCheckFn func(*testing.T, error) - }{ - { - name: "find existing stream", - osImageStreamFactory: getStubOSImageStream, - streamName: "rhel-9", - expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, - }, - { - name: "find another existing stream", - osImageStreamFactory: getStubOSImageStream, - streamName: "rhel-10", - expected: &v1alpha1.OSImageStreamSet{Name: "rhel-10", OSImage: "image2", OSExtensionsImage: "ext2"}, - }, - { - name: "empty name returns default stream", - osImageStreamFactory: getStubOSImageStream, - streamName: "", - expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, - }, - { - name: "non-existent stream", - osImageStreamFactory: getStubOSImageStream, - streamName: "non-existent", - errorContains: "not found", - errorCheckFn: func(t *testing.T, err error) { - assert.True(t, apierrors.IsNotFound(err)) - }, - }, - { - name: "nil osImageStream", - osImageStreamFactory: nil, - streamName: "rhel-9", - errorContains: "cannot be nil", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var osImageStream *v1alpha1.OSImageStream - if tt.osImageStreamFactory != nil { - osImageStream = tt.osImageStreamFactory() - } - - result, err := GetOSImageStreamSetByName(osImageStream, tt.streamName) - if tt.errorContains != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.errorContains) - assert.Nil(t, result) - if tt.errorCheckFn != nil { - tt.errorCheckFn(t, err) - } - } else { - require.NoError(t, err) - assert.Equal(t, tt.expected, result) - } - }) - } -} - -func TestTryGetOSImageStreamSetByName(t *testing.T) { - - tests := []struct { - name string - osImageStreamFactory func() *v1alpha1.OSImageStream - streamName string - expected *v1alpha1.OSImageStreamSet - }{ - { - name: "find existing stream", - osImageStreamFactory: getStubOSImageStream, - streamName: "rhel-9", - expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, - }, - { - name: "non-existent stream returns nil", - osImageStreamFactory: getStubOSImageStream, - streamName: "non-existent", - expected: nil, - }, - { - name: "nil osImageStream returns nil", - osImageStreamFactory: nil, - streamName: "rhel-9", - expected: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var osImageStream *v1alpha1.OSImageStream - if tt.osImageStreamFactory != nil { - osImageStream = tt.osImageStreamFactory() - } - result := TryGetOSImageStreamSetByName(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_controller.go b/pkg/controller/osimagestream/osimagestream_controller.go deleted file mode 100644 index abeeb9d530..0000000000 --- a/pkg/controller/osimagestream/osimagestream_controller.go +++ /dev/null @@ -1,206 +0,0 @@ -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) - } else { - 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 deleted file mode 100644 index ddb6e3d274..0000000000 --- a/pkg/controller/osimagestream/osimagestream_controller_test.go +++ /dev/null @@ -1,404 +0,0 @@ -// 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/render/render_controller.go b/pkg/controller/render/render_controller.go index 6cfef18944..1d78e369ea 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -10,17 +10,21 @@ import ( "github.com/openshift/api/features" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/machineconfiguration/v1alpha1" opv1 "github.com/openshift/api/operator/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" 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" mcopinformersv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" mcoplistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" mcoResourceApply "github.com/openshift/machine-config-operator/lib/resourceapply" "github.com/openshift/machine-config-operator/pkg/apihelpers" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" + "github.com/openshift/machine-config-operator/pkg/osimagestream" "github.com/openshift/machine-config-operator/pkg/version" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -65,11 +69,13 @@ type Controller struct { syncHandler func(mcp string) error enqueueMachineConfigPool func(*mcfgv1.MachineConfigPool) - mcpLister mcfglistersv1.MachineConfigPoolLister - mcLister mcfglistersv1.MachineConfigLister + mcpLister mcfglistersv1.MachineConfigPoolLister + mcLister mcfglistersv1.MachineConfigLister + osImageStreamLister mcfglistersv1alpha1.OSImageStreamLister - mcpListerSynced cache.InformerSynced - mcListerSynced cache.InformerSynced + mcpListerSynced cache.InformerSynced + mcListerSynced cache.InformerSynced + osImageStreamListerSynced cache.InformerSynced ccLister mcfglistersv1.ControllerConfigLister ccListerSynced cache.InformerSynced @@ -96,6 +102,7 @@ func New( crcInformer mcfginformersv1.ContainerRuntimeConfigInformer, mckInformer mcfginformersv1.KubeletConfigInformer, mcopInformer mcopinformersv1.MachineConfigurationInformer, + osImageStreamInformer mcfginformersv1alpha1.OSImageStreamInformer, kubeClient clientset.Interface, mcfgClient mcfgclientset.Interface, featureGatesHandler ctrlcommon.FeatureGatesHandler, @@ -140,6 +147,10 @@ func New( ctrl.mcopLister = mcopInformer.Lister() ctrl.mcopListerSynced = mcopInformer.Informer().HasSynced + if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(ctrl.fgHandler) { + ctrl.osImageStreamLister = osImageStreamInformer.Lister() + ctrl.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced + } return ctrl } @@ -148,7 +159,13 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() - if !cache.WaitForCacheSync(stopCh, ctrl.mcpListerSynced, ctrl.mcListerSynced, ctrl.ccListerSynced) { + listerCaches := []cache.InformerSynced{ctrl.mcpListerSynced, ctrl.mcListerSynced, ctrl.ccListerSynced} + + // OSImageStreams and MCPs fetched only if FeatureGateOSStreams active + if ctrl.osImageStreamListerSynced != nil { + listerCaches = append(listerCaches, ctrl.osImageStreamListerSynced) + } + if !cache.WaitForCacheSync(stopCh, listerCaches...) { return } @@ -512,12 +529,12 @@ func (ctrl *Controller) garbageCollectRenderedConfigs(_ *mcfgv1.MachineConfigPoo return nil } -func (ctrl *Controller) getRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cc *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) { +func (ctrl *Controller) getRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cc *mcfgv1.ControllerConfig, osImageStreamSet *v1alpha1.OSImageStreamSet) (*mcfgv1.MachineConfig, error) { // If we don't yet have a rendered MachineConfig on the pool, we cannot // perform reconciliation. So we must solely generate the rendered // MachineConfig. if pool.Spec.Configuration.Name == "" { - return generateRenderedMachineConfig(pool, configs, cc) + return generateRenderedMachineConfig(pool, configs, cc, osImageStreamSet) } // The pool has a rendered MachineConfig, so we can do more advanced @@ -526,7 +543,7 @@ func (ctrl *Controller) getRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, // Degenerate case: When the renderedMC that the MCP is currently pointing to is deleted if apierrors.IsNotFound(err) { - generated, err := generateRenderedMachineConfig(pool, configs, cc) + generated, err := generateRenderedMachineConfig(pool, configs, cc, osImageStreamSet) if err != nil { return nil, err } @@ -545,7 +562,24 @@ func (ctrl *Controller) getRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, mcop = *mcopPtr } - return generateAndValidateRenderedMachineConfig(currentMC, pool, configs, cc, &mcop.Spec.IrreconcilableValidationOverrides) + return generateAndValidateRenderedMachineConfig(currentMC, pool, configs, cc, &mcop.Spec.IrreconcilableValidationOverrides, osImageStreamSet) +} + +func (ctrl *Controller) getOSImageStreamForPool(pool *mcfgv1.MachineConfigPool) (*v1alpha1.OSImageStreamSet, error) { + if !osimagestream.IsFeatureEnabled(ctrl.fgHandler) || ctrl.osImageStreamLister == nil { + return nil, nil + } + + imageStream, err := ctrl.osImageStreamLister.Get(ctrlcommon.ClusterInstanceNameOSImageStream) + if err != nil { + return nil, fmt.Errorf("could not get OSImageStream for pool %s: %w", pool.Name, err) + } + + imageStreamSet, err := osimagestream.GetOSImageStreamSetByName(imageStream, pool.Spec.OSImageStream.Name) + if err != nil { + return nil, fmt.Errorf("could not get OSImageStreamSet for pool %s: %w", pool.Name, err) + } + return imageStreamSet, nil } func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig) error { @@ -558,14 +592,19 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo return err } - generated, err := ctrl.getRenderedMachineConfig(pool, configs, cc) + osImageStreamSet, err := ctrl.getOSImageStreamForPool(pool) + if err != nil { + return err + } + + generated, err := ctrl.getRenderedMachineConfig(pool, configs, cc, osImageStreamSet) if err != nil { return fmt.Errorf("could not generate rendered MachineConfig: %w", err) } // Collect metric when OSImageURL was overridden var isOSImageURLOverridden bool - if generated.Spec.OSImageURL != ctrlcommon.GetDefaultBaseImageContainer(&cc.Spec) { + if generated.Spec.OSImageURL != ctrlcommon.GetBaseImageContainer(&cc.Spec, osImageStreamSet) { ctrlcommon.OSImageURLOverride.WithLabelValues(pool.Name).Set(1) isOSImageURLOverridden = true } else { @@ -616,7 +655,7 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo } // generateRenderedMachineConfig takes all MCs for a given pool and returns a single rendered MC. For ex master-XXXX or worker-XXXX -func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) { +func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, osImageStreamSet *v1alpha1.OSImageStreamSet) (*mcfgv1.MachineConfig, error) { // Suppress rendered config generation until a corresponding new controller can roll out too. // https://bugzilla.redhat.com/show_bug.cgi?id=1879099 if genver, ok := cconfig.Annotations[daemonconsts.GeneratedByVersionAnnotationKey]; ok { @@ -647,7 +686,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc } } - merged, err := ctrlcommon.MergeMachineConfigs(configs, cconfig) + merged, err := ctrlcommon.MergeMachineConfigs(configs, cconfig, osImageStreamSet) if err != nil { return nil, err @@ -669,7 +708,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc // The operator needs to know the user overrode this, so it knows if it needs to skip the // OSImageURL check during upgrade -- if the user took over managing OS upgrades this way, // the operator shouldn't stop the rest of the upgrade from progressing/completing. - if merged.Spec.OSImageURL != ctrlcommon.GetDefaultBaseImageContainer(&cconfig.Spec) { + if merged.Spec.OSImageURL != ctrlcommon.GetBaseImageContainer(&cconfig.Spec, osImageStreamSet) { merged.Annotations[ctrlcommon.OSImageURLOverriddenKey] = "true" // Log a warning if the osImageURL is set using a tag instead of a digest if !strings.Contains(merged.Spec.OSImageURL, "sha256:") { @@ -692,11 +731,12 @@ func generateAndValidateRenderedMachineConfig( pool *mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, - validationOverrides *opv1.IrreconcilableValidationOverrides) (*mcfgv1.MachineConfig, error) { + validationOverrides *opv1.IrreconcilableValidationOverrides, + osImageStreamSet *v1alpha1.OSImageStreamSet) (*mcfgv1.MachineConfig, error) { source := getMachineConfigRefs(configs) klog.V(4).Infof("Considering %d configs %s for MachineConfig generation", len(source), source) - generated, err := generateRenderedMachineConfig(pool, configs, cconfig) + generated, err := generateRenderedMachineConfig(pool, configs, cconfig, osImageStreamSet) if err != nil { return nil, err } @@ -741,7 +781,7 @@ func generateAndValidateRenderedMachineConfig( // RunBootstrap runs the render controller in bootstrap mode. // For each pool, it matches the machineconfigs based on label selector and // returns the generated machineconfigs and pool with CurrentMachineConfig status field set. -func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { +func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, osImageStream *v1alpha1.OSImageStream) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { var ( opools []*mcfgv1.MachineConfigPool oconfigs []*mcfgv1.MachineConfig @@ -751,8 +791,15 @@ func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineCo if err != nil { return nil, nil, err } + var osImageStreamSet *v1alpha1.OSImageStreamSet + if osImageStream != nil { + osImageStreamSet, err = osimagestream.GetOSImageStreamSetByName(osImageStream, pool.Spec.OSImageStream.Name) + if err != nil { + return nil, nil, fmt.Errorf("couldn't get the OSImageStream for pool %s %w", pool.Name, err) + } + } - generated, err := generateRenderedMachineConfig(pool, pcs, cconfig) + generated, err := generateRenderedMachineConfig(pool, pcs, cconfig, osImageStreamSet) if err != nil { return nil, nil, err } diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index a6aa38e56e..cc8f96b3ba 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -8,6 +8,7 @@ import ( "github.com/clarketm/json" ign3types "github.com/coreos/ignition/v2/config/v3_5/types" + apicfgv1 "github.com/openshift/api/config/v1" configv1 "github.com/openshift/api/config/v1" mcopfake "github.com/openshift/client-go/operator/clientset/versioned/fake" operatorinformer "github.com/openshift/client-go/operator/informers/externalversions" @@ -64,6 +65,10 @@ func newFixture(t *testing.T) *fixture { f.t = t f.objects = []runtime.Object{} f.oObjects = []runtime.Object{} + f.fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler( + []apicfgv1.FeatureGateName{}, + []apicfgv1.FeatureGateName{}, + ) return f } @@ -76,7 +81,7 @@ func (f *fixture) newController() *Controller { c := New(i.Machineconfiguration().V1().MachineConfigPools(), i.Machineconfiguration().V1().MachineConfigs(), i.Machineconfiguration().V1().ControllerConfigs(), i.Machineconfiguration().V1().ContainerRuntimeConfigs(), i.Machineconfiguration().V1().KubeletConfigs(), oi.Operator().V1().MachineConfigurations(), - k8sfake.NewSimpleClientset(), f.client, f.fgHandler) + i.Machineconfiguration().V1alpha1().OSImageStreams(), k8sfake.NewSimpleClientset(), f.client, f.fgHandler) c.mcpListerSynced = alwaysReady c.mcListerSynced = alwaysReady @@ -299,7 +304,7 @@ func TestCreatesGeneratedMachineConfig(t *testing.T) { f.objects = append(f.objects, mcs[idx]) } - gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) assert.NoError(t, err) mcpNew := mcp.DeepCopy() @@ -331,7 +336,7 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { } cc := newControllerConfig(ctrlcommon.ControllerConfigName) - _, err := generateRenderedMachineConfig(mcp, mcs, cc) + _, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) require.Nil(t, err) // verify that an invalid ignition config (here a config with content and an empty version, @@ -343,7 +348,7 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { require.Nil(t, err) mcs[1].Spec.Config.Raw = rawIgnCfg - _, err = generateRenderedMachineConfig(mcp, mcs, cc) + _, err = generateRenderedMachineConfig(mcp, mcs, cc, nil) require.NotNil(t, err) // verify that a machine config with no ignition content will not fail validation @@ -352,7 +357,7 @@ func TestIgnValidationGenerateRenderedMachineConfig(t *testing.T) { require.Nil(t, err) mcs[1].Spec.Config.Raw = rawEmptyIgnCfg mcs[1].Spec.KernelArguments = append(mcs[1].Spec.KernelArguments, "test1") - _, err = generateRenderedMachineConfig(mcp, mcs, cc) + _, err = generateRenderedMachineConfig(mcp, mcs, cc, nil) require.Nil(t, err) } @@ -377,7 +382,7 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { } cc := newControllerConfig(ctrlcommon.ControllerConfigName) - gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) if err != nil { t.Fatal(err) } @@ -400,7 +405,7 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) { f.mcLister = append(f.mcLister, gmc) f.objects = append(f.objects, gmc) - expmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + expmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) if err != nil { t.Fatal(err) } @@ -424,7 +429,7 @@ func TestGenerateMachineConfigOverrideOSImageURL(t *testing.T) { cc := newControllerConfig(ctrlcommon.ControllerConfigName) - gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) if err != nil { t.Fatal(err) } @@ -432,7 +437,7 @@ func TestGenerateMachineConfigOverrideOSImageURL(t *testing.T) { mcs = append(mcs, helpers.NewMachineConfig("00-test-cluster-master-1", map[string]string{"node-role/master": ""}, "dummy-change-2", []ign3types.File{})) - gmc, err = generateAndValidateRenderedMachineConfig(gmc, mcp, mcs, cc, nil) + gmc, err = generateAndValidateRenderedMachineConfig(gmc, mcp, mcs, cc, nil, nil) assert.NoError(t, err) assert.Equal(t, "dummy-change-2", gmc.Spec.OSImageURL) } @@ -446,12 +451,12 @@ func TestVersionSkew(t *testing.T) { cc := newControllerConfig(ctrlcommon.ControllerConfigName) cc.Annotations[daemonconsts.GeneratedByVersionAnnotationKey] = "different-version" - _, err := generateRenderedMachineConfig(mcp, mcs, cc) + _, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) require.NotNil(t, err) // Now the same thing without overriding the version cc = newControllerConfig(ctrlcommon.ControllerConfigName) - gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) require.Nil(t, err) require.NotNil(t, gmc) } @@ -464,14 +469,14 @@ func TestGenerateRenderedConfigOnLatestControllerVersionOnly(t *testing.T) { } version.Hash = "2" cc := newControllerConfig(ctrlcommon.ControllerConfigName) - _, err := generateRenderedMachineConfig(mcp, mcs, cc) + _, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) require.NotNil(t, err) mcs = []*mcfgv1.MachineConfig{ helpers.NewMachineConfigWithAnnotation("00-updated-conf", map[string]string{"node-role/master": ""}, map[string]string{ctrlcommon.GeneratedByControllerVersionAnnotationKey: "2"}, "dummy-test-1", []ign3types.File{}), helpers.NewMachineConfigWithAnnotation("99-user-conf", map[string]string{"node-role/master": ""}, map[string]string{ctrlcommon.GeneratedByControllerVersionAnnotationKey: ""}, "user-data", []ign3types.File{}), } - _, err = generateRenderedMachineConfig(mcp, mcs, cc) + _, err = generateRenderedMachineConfig(mcp, mcs, cc, nil) require.Nil(t, err) } @@ -495,7 +500,7 @@ func TestDoNothing(t *testing.T) { } cc := newControllerConfig(ctrlcommon.ControllerConfigName) - gmc, err := generateRenderedMachineConfig(mcp, mcs, cc) + gmc, err := generateRenderedMachineConfig(mcp, mcs, cc, nil) if err != nil { t.Fatal(err) } @@ -604,7 +609,7 @@ func TestGenerateMachineConfigValidation(t *testing.T) { cc := newControllerConfig(ctrlcommon.ControllerConfigName) - gmc, err := generateAndValidateRenderedMachineConfig(currentMC, mcp, mcs, cc, nil) + gmc, err := generateAndValidateRenderedMachineConfig(currentMC, mcp, mcs, cc, nil, nil) assert.Error(t, err) assert.Nil(t, gmc) } diff --git a/pkg/imageutils/layer_reader.go b/pkg/imageutils/layer_reader.go index 6e2f334bde..d121dc5655 100644 --- a/pkg/imageutils/layer_reader.go +++ b/pkg/imageutils/layer_reader.go @@ -21,7 +21,7 @@ type ReadImageFileContentFn func(*tar.Header) bool // It iterates through the image layers (starting from the last) and uses matcherFn // to identify the target file. When found, the file content is returned as a byte slice. // If no matching file is found, (nil, nil) is returned. -func ReadImageFileContent(ctx context.Context, sysCtx *types.SystemContext, imageName string, matcherFn ReadImageFileContentFn) (content []byte, err error) { +func ReadImageFileContent(ctx context.Context, sysCtx *types.SystemContext, imageName string, matcherFn ReadImageFileContentFn, retryOpts *retry.Options) (content []byte, err error) { ref, err := ParseImageName(imageName) if err != nil { return nil, err @@ -36,7 +36,11 @@ func ReadImageFileContent(ctx context.Context, sysCtx *types.SystemContext, imag } }() - src, err := GetImageSourceFromReference(ctx, sysCtx, ref, &retry.Options{MaxRetry: 2}) + retries := retryOpts + if retries == nil { + retries = &retry.Options{MaxRetry: 2} + } + src, err := GetImageSourceFromReference(ctx, sysCtx, ref, retries) if err != nil { return nil, err } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index da0e9cacb2..b859220b98 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -6,6 +6,7 @@ import ( "time" opv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/machine-config-operator/pkg/osimagestream" "k8s.io/klog/v2" "k8s.io/utils/clock" @@ -40,8 +41,10 @@ import ( mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" 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" mcopclientset "github.com/openshift/client-go/operator/clientset/versioned" mcopinformersv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" @@ -110,6 +113,8 @@ type Operator struct { nodeClusterLister configlistersv1.NodeLister moscLister mcfglistersv1.MachineOSConfigLister apiserverLister configlistersv1.APIServerLister + clusterVersionLister configlistersv1.ClusterVersionLister + osImageStreamLister mcfglistersv1alpha1.OSImageStreamLister crdListerSynced cache.InformerSynced deployListerSynced cache.InformerSynced @@ -142,6 +147,7 @@ type Operator struct { nodeClusterListerSynced cache.InformerSynced moscListerSynced cache.InformerSynced apiserverListerSynced cache.InformerSynced + osImageStreamListerSynced cache.InformerSynced // queue only ever has one item, but it has nice error handling backoff/retry semantics queue workqueue.TypedRateLimitingInterface[string] @@ -195,6 +201,8 @@ func New( nodeClusterInformer configinformersv1.NodeInformer, apiserverInformer configinformersv1.APIServerInformer, moscInformer mcfginformersv1.MachineOSConfigInformer, + clusterVersionInformer configinformersv1.ClusterVersionInformer, + osImageStreamInformer mcfginformersv1alpha1.OSImageStreamInformer, ctrlctx *ctrlcommon.ControllerContext, ) *Operator { eventBroadcaster := record.NewBroadcaster() @@ -332,6 +340,11 @@ func New( optr.apiserverListerSynced = apiserverInformer.Informer().HasSynced optr.moscLister = moscInformer.Lister() optr.moscListerSynced = moscInformer.Informer().HasSynced + optr.clusterVersionLister = clusterVersionInformer.Lister() + if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) { + optr.osImageStreamLister = osImageStreamInformer.Lister() + optr.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced + } optr.vStore.Set("operator", version.ReleaseVersion) optr.vStore.Set("operator-image", version.OperatorImage) @@ -386,7 +399,9 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) { optr.nodeClusterListerSynced, optr.moscListerSynced, } - + if optr.osImageStreamListerSynced != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) { + cacheSynced = append(cacheSynced, optr.osImageStreamListerSynced) + } if !cache.WaitForCacheSync(stopCh, cacheSynced...) { klog.Error("failed to sync caches") @@ -510,8 +525,11 @@ func (optr *Operator) sync(key string) error { // syncFuncs is the list of sync functions that are executed in order. // any error marks sync as failure. syncFuncs := []syncFunc{ - // "RenderConfig" must always run first as it sets the renderConfig in the operator - // for the sync funcs below + // OSImageStream must run FIRST to provide OS image information as RenderConfig will read + // images references from OSImageStream + {"OSImageStream", optr.syncOSImageStream}, + // "RenderConfig" should be the first one to run (except OSImageStream) as it sets the renderConfig in + // the operator for the sync funcs below {"RenderConfig", optr.syncRenderConfig}, {"MachineConfiguration", optr.syncMachineConfiguration}, {"MachineConfigNode", optr.syncMachineConfigNodes}, diff --git a/pkg/operator/osimagestream_ocp.go b/pkg/operator/osimagestream_ocp.go new file mode 100644 index 0000000000..e86852d80a --- /dev/null +++ b/pkg/operator/osimagestream_ocp.go @@ -0,0 +1,169 @@ +//go:build !fcos && !scos + +package operator + +import ( + "context" + "fmt" + "time" + + configv1 "github.com/openshift/api/config/v1" + mcfgv1 "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/osimagestream" + "github.com/openshift/machine-config-operator/pkg/version" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" +) + +func (optr *Operator) syncOSImageStream(_ *renderConfig, _ *configv1.ClusterOperator) error { + klog.V(4).Info("OSImageStream sync started") + defer func() { + klog.V(4).Info("OSImageStream sync complete") + }() + + // This sync runs once per version. Before performing the streams fetching + // process, that takes time as it requires inspecting images, ensure this function + // needs to build the stream. + existingOSImageStream, updateRequired, err := optr.isOSImageStreamBuildRequired() + if !updateRequired || err != nil { + return err + } + + // If the code reaches this point the OSImageStream CR is not + // present (new cluster) or it's out-dated (cluster update). + // Build the new OSImageStream and push it. + return optr.buildOSImageStream(existingOSImageStream) + +} + +func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImageStream) error { + klog.Info("Starting building of the OSImageStream instance") + + // Get the release payload image from ClusterVersion + image, err := osimagestream.GetReleasePayloadImage(optr.clusterVersionLister) + if err != nil { + return fmt.Errorf("error getting the Release Image digest from the ClusterVersion for OSImageStream sync: %w", err) + } + + // Get the cluster pull secret from well-known location + clusterPullSecret, err := optr.kubeClient.CoreV1().Secrets("openshift-config").Get(context.TODO(), "pull-secret", metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("could not get the cluster PullSecret for OSImageStream sync: %w", err) + } + + // Build a minimal ControllerConfig with image registry certs + // We can't use renderConfig (it runs after us) so we build the cert data directly + minimalCC, err := optr.buildMinimalControllerConfigForOSImageStream() + if err != nil { + return fmt.Errorf("could not build minimal ControllerConfig for OSImageStream: %w", err) + } + + // Build the OSImageStream using the default factory + // Use a longer timeout to account for DNS/network delays during cluster bootstrap + buildCtx, buildCancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer buildCancel() + + imageStreamFactory := osimagestream.NewDefaultStreamSourceFactory(optr.mcoCmLister, &osimagestream.DefaultImagesInspectorFactory{}) + osImageStream, err := osimagestream.BuildOsImageStreamRuntime(buildCtx, clusterPullSecret, minimalCC, image, imageStreamFactory) + if err != nil { + return fmt.Errorf("error building the OSImageStream: %w", err) + } + + // Create or update the OSImageStream resource + var updateOSImageStream *v1alpha1.OSImageStream + if existingOSImageStream == nil { + klog.V(4).Info("Creating OSImageStream singleton instance") + updateOSImageStream, err = optr.client.MachineconfigurationV1alpha1().OSImageStreams().Create(context.TODO(), osImageStream, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("error creating the OSImageStream: %w", err) + } + klog.Infof("Created OSImageStream with %d available streams, default stream: %s", + len(osImageStream.Status.AvailableStreams), osImageStream.Status.DefaultStream) + } else { + oldVersion := existingOSImageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey] + klog.V(4).Infof("Updating OSImageStream (previous version: %s, new version: %s)", oldVersion, version.Hash) + // Update metadata/spec first (mainly for annotations) + existingOSImageStream.ObjectMeta.Annotations = osImageStream.ObjectMeta.Annotations + updateOSImageStream, err = optr.client.MachineconfigurationV1alpha1().OSImageStreams().Update(context.TODO(), existingOSImageStream, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("error updating the OSImageStream: %w", err) + } + } + + // Update the status subresource (both for newly created and updated resources) + updateOSImageStream.Status = osImageStream.Status + if _, err = optr.client. + MachineconfigurationV1alpha1(). + OSImageStreams(). + UpdateStatus(context.TODO(), updateOSImageStream, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("error updating the OSImageStream status: %w", err) + } + + klog.Infof("OSImageStream synced successfully. Available streams: %s. Default stream: %s", + osimagestream.GetStreamSetsNames(updateOSImageStream.Status.AvailableStreams), + updateOSImageStream.Status.DefaultStream) + return nil +} + +func (optr *Operator) isOSImageStreamBuildRequired() (*v1alpha1.OSImageStream, bool, error) { + // Check if the feature is enabled + if !osimagestream.IsFeatureEnabled(optr.fgHandler) { + klog.V(4).Info("OSImageStream feature is not enabled, skipping sync") + return nil, false, nil + } + + // Get the existing OSImageStream if it exists + existingOSImageStream, err := optr.getExistingOSImageStream() + if err != nil { + return nil, true, err + } + + // Check if an update is needed + if !osImageStreamRequiresUpdate(existingOSImageStream) { + klog.V(4).Info("OSImageStream is already up-to-date, skipping sync") + return nil, false, nil + } + return existingOSImageStream, true, nil +} + +// buildMinimalControllerConfigForOSImageStream builds a minimal ControllerConfig with just the image registry certs +// needed for OSImageStream to inspect images. This is necessary because OSImageStream must run before RenderConfig. +func (optr *Operator) buildMinimalControllerConfigForOSImageStream() (*mcfgv1.ControllerConfig, error) { + imgRegistryData, imgRegistryUsrData, err := optr.getImageRegistryBundles() + if err != nil { + return nil, fmt.Errorf("could not get image registry bundles: %w", err) + } + + return &mcfgv1.ControllerConfig{ + Spec: mcfgv1.ControllerConfigSpec{ + ImageRegistryBundleData: imgRegistryData, + ImageRegistryBundleUserData: imgRegistryUsrData, + }, + }, nil +} + +// getExistingOSImageStream retrieves the existing OSImageStream from the lister. +// Returns nil if the OSImageStream does not exist. +func (optr *Operator) getExistingOSImageStream() (*v1alpha1.OSImageStream, error) { + osImageStream, err := optr.osImageStreamLister.Get(ctrlcommon.ClusterInstanceNameOSImageStream) + if err != nil { + if !apierrors.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 +} diff --git a/pkg/operator/osimagestream_okd.go b/pkg/operator/osimagestream_okd.go new file mode 100644 index 0000000000..af41f6dda6 --- /dev/null +++ b/pkg/operator/osimagestream_okd.go @@ -0,0 +1,13 @@ +//go:build fcos || scos + +package operator + +import ( + configv1 "github.com/openshift/api/config/v1" + "k8s.io/klog/v2" +) + +func (optr *Operator) syncOSImageStream(_ *renderConfig, _ *configv1.ClusterOperator) error { + klog.V(4).Info("OSImageStream sync skipped") + return nil +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 7fb9dac3c4..162531db73 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -18,6 +18,7 @@ import ( "time" configclientscheme "github.com/openshift/client-go/config/clientset/versioned/scheme" + "github.com/openshift/machine-config-operator/pkg/osimagestream" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -256,6 +257,75 @@ func isCloudConfRequired(infra *configv1.Infrastructure) bool { return platformsRequiringCloudConf.Has(string(infra.Status.PlatformStatus.Type)) } +// getImageRegistryBundles retrieves and returns image registry certificate bundles. +// It fetches both user-provided additional trusted CAs and managed registry CAs. +func (optr *Operator) getImageRegistryBundles() ([]mcfgv1.ImageRegistryBundle, []mcfgv1.ImageRegistryBundle, error) { + cfg, err := optr.imgLister.Get("cluster") + if err != nil { + return nil, nil, err + } + + imgRegistryUsrData := []mcfgv1.ImageRegistryBundle{} + if cfg.Spec.AdditionalTrustedCA.Name != "" { + cm, err := optr.ocCmLister.ConfigMaps(ctrlcommon.OpenshiftConfigNamespace).Get(cfg.Spec.AdditionalTrustedCA.Name) + if err != nil { + klog.Warningf("could not find configmap specified in image.config.openshift.io/cluster with the name %s", cfg.Spec.AdditionalTrustedCA.Name) + } else { + newKeys := sets.StringKeySet(cm.Data).List() + newBinaryKeys := sets.StringKeySet(cm.BinaryData).List() + for _, key := range newKeys { + raw, err := base64.StdEncoding.DecodeString(cm.Data[key]) + if err != nil { + imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: []byte(cm.Data[key]), + }) + } else { + imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: raw, + }) + } + } + for _, key := range newBinaryKeys { + imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: cm.BinaryData[key], + }) + } + } + } + + imgRegistryData := []mcfgv1.ImageRegistryBundle{} + cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("image-registry-ca") + if err == nil { + newKeys := sets.StringKeySet(cm.Data).List() + newBinaryKeys := sets.StringKeySet(cm.BinaryData).List() + for _, key := range newKeys { + raw, err := base64.StdEncoding.DecodeString(cm.Data[key]) + if err != nil { + imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: []byte(cm.Data[key]), + }) + } else { + imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: raw, + }) + } + } + for _, key := range newBinaryKeys { + imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ + File: key, + Data: cm.BinaryData[key], + }) + } + } + + return imgRegistryData, imgRegistryUsrData, nil +} + // Sync cloud config on supported platform from cloud.conf available in openshift-config-managed/kube-cloud-config ConfigMap. func (optr *Operator) syncCloudConfig(spec *mcfgv1.ControllerConfigSpec, infra *configv1.Infrastructure) error { cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("kube-cloud-config") @@ -332,67 +402,10 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig, _ *configv1.ClusterOpera // handle image registry certificates. // parse these, add them to ctrlcfgspec and then handle these in the daemon write to disk function - cfg, err := optr.imgLister.Get("cluster") + imgRegistryData, imgRegistryUsrData, err := optr.getImageRegistryBundles() if err != nil { return err } - imgRegistryUsrData := []mcfgv1.ImageRegistryBundle{} - if cfg.Spec.AdditionalTrustedCA.Name != "" { - cm, err := optr.ocCmLister.ConfigMaps(ctrlcommon.OpenshiftConfigNamespace).Get(cfg.Spec.AdditionalTrustedCA.Name) - if err != nil { - klog.Warningf("could not find configmap specified in image.config.openshift.io/cluster with the name %s", cfg.Spec.AdditionalTrustedCA.Name) - } else { - newKeys := sets.StringKeySet(cm.Data).List() - newBinaryKeys := sets.StringKeySet(cm.BinaryData).List() - for _, key := range newKeys { - raw, err := base64.StdEncoding.DecodeString(cm.Data[key]) - if err != nil { - imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: []byte(cm.Data[key]), - }) - } else { - imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: raw, - }) - } - } - for _, key := range newBinaryKeys { - imgRegistryUsrData = append(imgRegistryUsrData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: cm.BinaryData[key], - }) - } - } - } - - imgRegistryData := []mcfgv1.ImageRegistryBundle{} - cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("image-registry-ca") - if err == nil { - newKeys := sets.StringKeySet(cm.Data).List() - newBinaryKeys := sets.StringKeySet(cm.BinaryData).List() - for _, key := range newKeys { - raw, err := base64.StdEncoding.DecodeString(cm.Data[key]) - if err != nil { - imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: []byte(cm.Data[key]), - }) - } else { - imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: raw, - }) - } - } - for _, key := range newBinaryKeys { - imgRegistryData = append(imgRegistryData, mcfgv1.ImageRegistryBundle{ - File: key, - Data: cm.BinaryData[key], - }) - } - } mergedData := append([]mcfgv1.ImageRegistryBundle{}, append(imgRegistryData, imgRegistryUsrData...)...) caData := make(map[string]string, len(mergedData)) @@ -400,7 +413,7 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig, _ *configv1.ClusterOpera caData[CA.File] = string(CA.Data) } - cm, err = optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("merged-trusted-image-registry-ca") + cm, err := optr.clusterCmLister.ConfigMaps("openshift-config-managed").Get("merged-trusted-image-registry-ca") if err != nil && !apierrors.IsNotFound(err) { return err } @@ -556,15 +569,6 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig, _ *configv1.ClusterOpera internalRegistryPullSecret = nil } - // sync up os image url - // TODO: this should probably be part of the imgs - oscontainer, osextensionscontainer, err := optr.getOsImageURLs(optr.namespace) - if err != nil { - return err - } - imgs.BaseOSContainerImage = oscontainer - imgs.BaseOSExtensionsContainerImage = osextensionscontainer - // sync up the ControllerConfigSpec infra, network, proxy, dns, apiServer, err := optr.getGlobalConfig() if err != nil { @@ -608,6 +612,14 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig, _ *configv1.ClusterOpera return err } + oscontainer, osextensionscontainer, err := optr.getOsImageURLs(optr.namespace, "") + if err != nil { + return fmt.Errorf("could not get OS images: %w", err) + + } + imgs.BaseOSContainerImage = oscontainer + imgs.BaseOSExtensionsContainerImage = osextensionscontainer + spec.KubeAPIServerServingCAData = kubeAPIServerServingCABytes spec.RootCAData = machineConfigServerCABundle spec.ImageRegistryBundleData = imgRegistryData @@ -1728,9 +1740,9 @@ func (optr *Operator) syncRequiredMachineConfigPools(config *renderConfig, co *c _, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey] if hasRequiredPoolLabel { - opURL, _, err := optr.getOsImageURLs(optr.namespace) + opURL, _, err := optr.getOsImageURLs(optr.namespace, pool.Spec.OSImageStream.Name) if err != nil { - klog.Errorf("Error getting configmap osImageURL: %q", err) + klog.Errorf("Error getting OS images: %q", err) return false, nil } releaseVersion, _ := optr.vStore.Get("operator") @@ -1893,8 +1905,23 @@ func (optr *Operator) waitForControllerConfigToBeCompleted(resource *mcfgv1.Cont return nil } -// getOsImageURLs returns (new type, new extensions, old type) for operating system update images. -func (optr *Operator) getOsImageURLs(namespace string) (string, string, error) { +// getOsImageURLs retrieves the base OS and OS extensions container image URLs. +// It first checks OSImageStream (if enabled), then falls back to the ConfigMap. +func (optr *Operator) getOsImageURLs(namespace, osImageStreamName string) (string, string, error) { + // If OSImageStream is enabled fetch the URLs from there + if optr.osImageStreamLister != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) { + osImageStream, err := optr.osImageStreamLister.Get(ctrlcommon.ClusterInstanceNameOSImageStream) + if err != nil { + return "", "", fmt.Errorf("could not get OSImageStream: %w", err) + } + + stream, err := osimagestream.GetOSImageStreamSetByName(osImageStream, osImageStreamName) + if err != nil { + return "", "", fmt.Errorf("could not get OSImageStream: %w", err) + } + return string(stream.OSImage), string(stream.OSExtensionsImage), nil + } + cm, err := optr.mcoCmLister.ConfigMaps(namespace).Get(ctrlcommon.MachineConfigOSImageURLConfigMapName) if err != nil { return "", "", err diff --git a/pkg/controller/osimagestream/clusterversion.go b/pkg/osimagestream/clusterversion.go similarity index 100% rename from pkg/controller/osimagestream/clusterversion.go rename to pkg/osimagestream/clusterversion.go diff --git a/pkg/controller/osimagestream/clusterversion_test.go b/pkg/osimagestream/clusterversion_test.go similarity index 100% rename from pkg/controller/osimagestream/clusterversion_test.go rename to pkg/osimagestream/clusterversion_test.go diff --git a/pkg/controller/osimagestream/helpers.go b/pkg/osimagestream/helpers.go similarity index 50% rename from pkg/controller/osimagestream/helpers.go rename to pkg/osimagestream/helpers.go index 0cd7b552ac..107a462138 100644 --- a/pkg/controller/osimagestream/helpers.go +++ b/pkg/osimagestream/helpers.go @@ -3,10 +3,10 @@ package osimagestream import ( "fmt" - v1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/api/features" "github.com/openshift/api/machineconfiguration/v1alpha1" "github.com/openshift/machine-config-operator/pkg/controller/common" - "github.com/openshift/machine-config-operator/pkg/helpers" + "github.com/openshift/machine-config-operator/pkg/version" k8serrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -38,23 +38,8 @@ func GetOSImageStreamSetByName(osImageStream *v1alpha1.OSImageStream, name strin return nil, k8serrors.NewNotFound(v1alpha1.GroupVersion.WithResource("osimagestreams").GroupResource(), name) } -// 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) +// IsFeatureEnabled checks if the OSImageStream feature is enabled. +// Returns true only if the FeatureGateOSStreams is enabled and the cluster is not running SCOS or FCOS. +func IsFeatureEnabled(fgHandler common.FeatureGatesHandler) bool { + return fgHandler.Enabled(features.FeatureGateOSStreams) && !version.IsSCOS() && !version.IsFCOS() } diff --git a/pkg/osimagestream/helpers_test.go b/pkg/osimagestream/helpers_test.go new file mode 100644 index 0000000000..673500d664 --- /dev/null +++ b/pkg/osimagestream/helpers_test.go @@ -0,0 +1,127 @@ +// Assisted-by: Claude +package osimagestream + +import ( + "testing" + + "github.com/openshift/api/machineconfiguration/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" +) + +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 getStubOSImageStream() *v1alpha1.OSImageStream { + return &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"}, + }, + }, + } +} + +func TestGetOSImageStreamSetByName(t *testing.T) { + tests := []struct { + name string + osImageStreamFactory func() *v1alpha1.OSImageStream + streamName string + expected *v1alpha1.OSImageStreamSet + errorContains string + errorCheckFn func(*testing.T, error) + }{ + { + name: "find existing stream", + osImageStreamFactory: getStubOSImageStream, + streamName: "rhel-9", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "find another existing stream", + osImageStreamFactory: getStubOSImageStream, + streamName: "rhel-10", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-10", OSImage: "image2", OSExtensionsImage: "ext2"}, + }, + { + name: "empty name returns default stream", + osImageStreamFactory: getStubOSImageStream, + streamName: "", + expected: &v1alpha1.OSImageStreamSet{Name: "rhel-9", OSImage: "image1", OSExtensionsImage: "ext1"}, + }, + { + name: "non-existent stream", + osImageStreamFactory: getStubOSImageStream, + streamName: "non-existent", + errorContains: "not found", + errorCheckFn: func(t *testing.T, err error) { + assert.True(t, apierrors.IsNotFound(err)) + }, + }, + { + name: "nil osImageStream", + osImageStreamFactory: nil, + streamName: "rhel-9", + errorContains: "cannot be nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var osImageStream *v1alpha1.OSImageStream + if tt.osImageStreamFactory != nil { + osImageStream = tt.osImageStreamFactory() + } + + result, err := GetOSImageStreamSetByName(osImageStream, tt.streamName) + if tt.errorContains != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errorContains) + assert.Nil(t, result) + if tt.errorCheckFn != nil { + tt.errorCheckFn(t, err) + } + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} diff --git a/pkg/controller/osimagestream/image_data.go b/pkg/osimagestream/image_data.go similarity index 100% rename from pkg/controller/osimagestream/image_data.go rename to pkg/osimagestream/image_data.go diff --git a/pkg/controller/osimagestream/image_data_test.go b/pkg/osimagestream/image_data_test.go similarity index 100% rename from pkg/controller/osimagestream/image_data_test.go rename to pkg/osimagestream/image_data_test.go diff --git a/pkg/controller/osimagestream/imagestream_provider.go b/pkg/osimagestream/imagestream_provider.go similarity index 100% rename from pkg/controller/osimagestream/imagestream_provider.go rename to pkg/osimagestream/imagestream_provider.go diff --git a/pkg/controller/osimagestream/imagestream_provider_test.go b/pkg/osimagestream/imagestream_provider_test.go similarity index 100% rename from pkg/controller/osimagestream/imagestream_provider_test.go rename to pkg/osimagestream/imagestream_provider_test.go diff --git a/pkg/controller/osimagestream/imagestream_source.go b/pkg/osimagestream/imagestream_source.go similarity index 100% rename from pkg/controller/osimagestream/imagestream_source.go rename to pkg/osimagestream/imagestream_source.go diff --git a/pkg/controller/osimagestream/imagestream_source_test.go b/pkg/osimagestream/imagestream_source_test.go similarity index 100% rename from pkg/controller/osimagestream/imagestream_source_test.go rename to pkg/osimagestream/imagestream_source_test.go diff --git a/pkg/controller/osimagestream/inspector.go b/pkg/osimagestream/inspector.go similarity index 65% rename from pkg/controller/osimagestream/inspector.go rename to pkg/osimagestream/inspector.go index 8838f7487d..27ea843b21 100644 --- a/pkg/controller/osimagestream/inspector.go +++ b/pkg/osimagestream/inspector.go @@ -3,6 +3,8 @@ package osimagestream import ( "archive/tar" "context" + "errors" + "net" "strings" "github.com/containers/common/pkg/retry" @@ -10,6 +12,44 @@ import ( "github.com/openshift/machine-config-operator/pkg/imageutils" ) +// isNetworkErrorRetryable checks if an error is a network-related error that should be retried. +// This handles DNS and timeout errors that may occur during cluster bootstrap. +func isNetworkErrorRetryable(err error) bool { + if err == nil { + return false + } + + // Check for net.DNSError (DNS lookup failures) + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return true + } + + // Check for timeout errors (net.Error with Timeout() == true) + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return true + } + return false +} + +// newImageInspectionRetryOptions creates retry options suitable for image inspection operations. +// It includes special handling for DNS and network errors that may occur during cluster bootstrap. +func newImageInspectionRetryOptions() *retry.Options { + return &retry.Options{ + MaxRetry: 50, + IsErrorRetryable: func(err error) bool { + // Use the default retry logic first + if retry.IsErrorRetryable(err) { + return true + } + // Additionally check for network errors that may be wrapped in ways + // that don't match the default retry logic's type assertions + return isNetworkErrorRetryable(err) + }, + } +} + // ImagesInspector provides methods for inspecting container images and extracting their contents. type ImagesInspector interface { // Inspect retrieves metadata for one or more container images. @@ -29,9 +69,7 @@ func NewImagesInspector(sysCtx *types.SystemContext) *ImagesInspectorImpl { return &ImagesInspectorImpl{ sysCtx: sysCtx, bulkInspector: imageutils.NewBulkInspector(&imageutils.BulkInspectorOptions{ - RetryOpts: &retry.RetryOptions{ - MaxRetry: 2, - }, + RetryOpts: newImageInspectionRetryOptions(), Count: 5, FailOnErr: false, }), @@ -48,7 +86,7 @@ func (i *ImagesInspectorImpl) FetchImageFile(ctx context.Context, image, path st targetHeaderPath := strings.TrimLeft(path, "./") return imageutils.ReadImageFileContent(ctx, i.sysCtx, image, func(header *tar.Header) bool { return targetHeaderPath == strings.TrimLeft(header.Name, "./") - }) + }, newImageInspectionRetryOptions()) } // ImagesInspectorFactory creates ImagesInspector instances for different system contexts. diff --git a/pkg/controller/osimagestream/mocks_test.go b/pkg/osimagestream/mocks_test.go similarity index 100% rename from pkg/controller/osimagestream/mocks_test.go rename to pkg/osimagestream/mocks_test.go diff --git a/pkg/controller/osimagestream/osimagestream.go b/pkg/osimagestream/osimagestream.go similarity index 98% rename from pkg/controller/osimagestream/osimagestream.go rename to pkg/osimagestream/osimagestream.go index a5052cb990..a98ecb4ec3 100644 --- a/pkg/controller/osimagestream/osimagestream.go +++ b/pkg/osimagestream/osimagestream.go @@ -132,8 +132,7 @@ func BuildOSImageStreamFromSources(ctx context.Context, sources []StreamSource) ObjectMeta: metav1.ObjectMeta{ Name: ctrlcommon.ClusterInstanceNameOSImageStream, Annotations: map[string]string{ - ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, - ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, + ctrlcommon.ReleaseImageVersionAnnotationKey: version.Hash, }, }, Spec: &v1alpha1.OSImageStreamSpec{}, diff --git a/pkg/controller/osimagestream/osimagestream_test.go b/pkg/osimagestream/osimagestream_test.go similarity index 99% rename from pkg/controller/osimagestream/osimagestream_test.go rename to pkg/osimagestream/osimagestream_test.go index 139acafe40..f5c8cc3f17 100644 --- a/pkg/controller/osimagestream/osimagestream_test.go +++ b/pkg/osimagestream/osimagestream_test.go @@ -179,7 +179,6 @@ func TestBuildOSImageStreamFromSources(t *testing.T) { 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/controller/osimagestream/urls_source.go b/pkg/osimagestream/urls_source.go similarity index 100% rename from pkg/controller/osimagestream/urls_source.go rename to pkg/osimagestream/urls_source.go diff --git a/pkg/controller/osimagestream/urls_source_test.go b/pkg/osimagestream/urls_source_test.go similarity index 100% rename from pkg/controller/osimagestream/urls_source_test.go rename to pkg/osimagestream/urls_source_test.go diff --git a/test/e2e-2of2/imageutils_test.go b/test/e2e-2of2/imageutils_test.go index 45b9fd4b72..de1b6fa44c 100644 --- a/test/e2e-2of2/imageutils_test.go +++ b/test/e2e-2of2/imageutils_test.go @@ -106,7 +106,7 @@ func TestReadImageFileContent(t *testing.T) { timedCtx, timedCtxCancelFn := context.WithTimeout(context.Background(), time.Minute) defer timedCtxCancelFn() - content, err := imageutils.ReadImageFileContent(timedCtx, sysContext.SysContext, clusterVersion.Status.Desired.Image, releaseManifestsMatcher) + content, err := imageutils.ReadImageFileContent(timedCtx, sysContext.SysContext, clusterVersion.Status.Desired.Image, releaseManifestsMatcher, nil) require.NoError(t, err) // Note: The test file is a file used in the MCO to fetch OSImageStreams, and it's diff --git a/test/e2e-2of2/osimagestream_test.go b/test/e2e-2of2/osimagestream_test.go index bdcd4558d2..0b46538251 100644 --- a/test/e2e-2of2/osimagestream_test.go +++ b/test/e2e-2of2/osimagestream_test.go @@ -6,7 +6,7 @@ import ( "github.com/openshift/machine-config-operator/internal/clients" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" - "github.com/openshift/machine-config-operator/pkg/controller/osimagestream" + "github.com/openshift/machine-config-operator/pkg/osimagestream" "github.com/openshift/machine-config-operator/test/framework" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/test/e2e-bootstrap/bootstrap_test.go b/test/e2e-bootstrap/bootstrap_test.go index d0fd08fb97..000489c65d 100644 --- a/test/e2e-bootstrap/bootstrap_test.go +++ b/test/e2e-bootstrap/bootstrap_test.go @@ -511,6 +511,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.InformerFactory.Machineconfiguration().V1().ContainerRuntimeConfigs(), ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(), ctx.OperatorInformerFactory.Operator().V1().MachineConfigurations(), + ctx.InformerFactory.Machineconfiguration().V1alpha1().OSImageStreams(), ctx.ClientBuilder.KubeClientOrDie("render-controller"), ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"), ctx.FeatureGatesHandler,