From b241c4e11500bc2e33db05a7daf45f5ff295065e Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Thu, 19 Jun 2025 11:09:54 -0400 Subject: [PATCH] OCPBUGS-57348: add MCO operator manifest for boot image management (#9783) * pkg/asset/manifests: add MCO operator manifest Adds manifest generation for MCO configuration. Currently the manifest is only generated when custom boot images are specified, in order to disable MCO management of those boot images. The manifest generation uses a golang template as testing revealed that API server validation would not permit the manifests generated from serializing the golang structs, which would be more consistent with how we generate manifests for other openshift operators. As golang will populate the zero value for any non-pointer struct this triggered validation, where the API server expected certain required fields for these zero-value structs. Using a template allows us to bypass this problem. Fixes OCPBUGS-57348 * fixup! pkg/asset/manifests: add MCO operator manifest * fixup! pkg/asset/manifests: add MCO operator manifest --- .../90_cluster-mco-02-config.yaml.template | 15 ++ pkg/asset/manifests/mco.go | 74 +++++++++ pkg/asset/manifests/mco_test.go | 145 ++++++++++++++++++ pkg/asset/manifests/operators.go | 8 +- pkg/asset/manifests/template.go | 4 + pkg/asset/templates/content/helper.go | 14 +- .../content/manifests/cluster-mco-config.go | 69 +++++++++ 7 files changed, 324 insertions(+), 5 deletions(-) create mode 100644 data/data/manifests/manifests/90_cluster-mco-02-config.yaml.template create mode 100644 pkg/asset/manifests/mco.go create mode 100644 pkg/asset/manifests/mco_test.go create mode 100644 pkg/asset/templates/content/manifests/cluster-mco-config.go diff --git a/data/data/manifests/manifests/90_cluster-mco-02-config.yaml.template b/data/data/manifests/manifests/90_cluster-mco-02-config.yaml.template new file mode 100644 index 0000000000..b9b29a019d --- /dev/null +++ b/data/data/manifests/manifests/90_cluster-mco-02-config.yaml.template @@ -0,0 +1,15 @@ +apiVersion: operator.openshift.io/v1 +kind: MachineConfiguration +metadata: + name: cluster +spec: + logLevel: Normal + operatorLogLevel: Normal +{{- if .DisableMachinesetBootMgmt }} + managedBootImages: + machineManagers: + - resource: machinesets + apiGroup: machine.openshift.io + selection: + mode: None +{{- end }} diff --git a/pkg/asset/manifests/mco.go b/pkg/asset/manifests/mco.go new file mode 100644 index 0000000000..33884b851f --- /dev/null +++ b/pkg/asset/manifests/mco.go @@ -0,0 +1,74 @@ +package manifests + +import ( + "path/filepath" + "strings" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/gcp" +) + +func generateMCOManifest(installConfig *types.InstallConfig, template []*asset.File) []*asset.File { + _, customWImg := customBootImages(installConfig) + + // If there are no custom images, skip creating the manifest + // to defer to the MCO's default behavior. + if !customWImg { + return nil + } + + tmplData := mcoTemplateData{DisableMachinesetBootMgmt: customWImg} + + mcoCfg := applyTemplateData(template[0].Data, tmplData) + return []*asset.File{ + { + Filename: filepath.Join(manifestDir, strings.TrimSuffix(filepath.Base(template[0].Filename), ".template")), + Data: mcoCfg, + }, + } +} + +func customBootImages(ic *types.InstallConfig) (customCPImg, customWImg bool) { + switch ic.Platform.Name() { + case aws.Name: + customCPImg, customWImg = awsBootImages(ic) + case gcp.Name: + customCPImg, customWImg = gcpBootImages(ic) + default: + // We do not need to consider other platforms, because default boot image management has not been enabled yet. + return + } + return +} + +func awsBootImages(ic *types.InstallConfig) (cpImg bool, wImg bool) { + if dmp := ic.AWS.DefaultMachinePlatform; dmp != nil && dmp.AMIID != "" { + return true, true + } + + if cp := ic.ControlPlane; cp != nil && cp.Platform.AWS != nil && cp.Platform.AWS.AMIID != "" { + cpImg = true + } + + if w := ic.Compute; len(w) > 0 && w[0].Platform.AWS != nil && w[0].Platform.AWS.AMIID != "" { + wImg = true + } + return +} + +func gcpBootImages(ic *types.InstallConfig) (cpImg bool, wImg bool) { + if dmp := ic.GCP.DefaultMachinePlatform; dmp != nil && dmp.OSImage != nil { + return true, true + } + + if cp := ic.ControlPlane; cp != nil && cp.Platform.GCP != nil && cp.Platform.GCP.OSImage != nil { + cpImg = true + } + + if w := ic.Compute; len(w) > 0 && w[0].Platform.GCP != nil && w[0].Platform.GCP.OSImage != nil { + wImg = true + } + return +} diff --git a/pkg/asset/manifests/mco_test.go b/pkg/asset/manifests/mco_test.go new file mode 100644 index 0000000000..92d8ea720f --- /dev/null +++ b/pkg/asset/manifests/mco_test.go @@ -0,0 +1,145 @@ +package manifests + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/gcp" +) + +func TestGenerateMCO(t *testing.T) { + cases := []struct { + name string + installConfig *types.InstallConfig + expectedMCO *operatorv1.MachineConfiguration + }{ + { + name: "minimal install config doesn't panic", + installConfig: func() *types.InstallConfig { + ic := icBuild.build() + ic.ControlPlane = nil + return ic + }(), + expectedMCO: nil, + }, + { + name: "vanilla aws produces no mco cfg", + installConfig: icBuild.build(icBuild.forAWS()), + expectedMCO: nil, + }, + { + name: "aws with a custom compute image disables mco management", + installConfig: icBuild.build(icBuild.withAWSComputeAMI()), + expectedMCO: mcoBuild.build(mcoBuild.withComputeBootImageMgmtDisabled()), + }, + { + name: "gcp with a custom compute image disables mco management", + installConfig: icBuild.build(icBuild.withGCPComputeAMI()), + expectedMCO: mcoBuild.build(mcoBuild.withComputeBootImageMgmtDisabled()), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fileData, err := os.ReadFile("../../../data/data/manifests/manifests/90_cluster-mco-02-config.yaml.template") + if err != nil { + t.Errorf("failed reading mco template: %v", err) + } + + mcoData := generateMCOManifest(tc.installConfig, []*asset.File{{Data: fileData}}) + var actualMCO *operatorv1.MachineConfiguration + if mcoData != nil { + if err = yaml.Unmarshal(mcoData[0].Data, &actualMCO); err != nil { + t.Errorf("failed to serialize mco operator configuration: %v", err) + } + } + assert.Equal(t, tc.expectedMCO, actualMCO) + }) + } +} + +type mcoOption func(*operatorv1.MachineConfiguration) + +type mcoBuildNamespace struct{} + +var mcoBuild mcoBuildNamespace + +func (b mcoBuildNamespace) build(opts ...mcoOption) *operatorv1.MachineConfiguration { + mco := &operatorv1.MachineConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: operatorv1.SchemeGroupVersion.String(), + Kind: "MachineConfiguration", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: operatorv1.MachineConfigurationSpec{ + StaticPodOperatorSpec: operatorv1.StaticPodOperatorSpec{ + OperatorSpec: operatorv1.OperatorSpec{ + LogLevel: operatorv1.Normal, + OperatorLogLevel: operatorv1.Normal, + }, + }, + }, + } + for _, opt := range opts { + opt(mco) + } + return mco +} + +func (b mcoBuildNamespace) withComputeBootImageMgmtDisabled() mcoOption { + return func(mco *operatorv1.MachineConfiguration) { + mco.Spec.ManagedBootImages = operatorv1.ManagedBootImages{ + MachineManagers: []operatorv1.MachineManager{ + { + Resource: operatorv1.MachineSets, + APIGroup: operatorv1.MachineAPI, + Selection: operatorv1.MachineManagerSelector{ + Mode: operatorv1.None, + }, + }, + }, + } + } +} + +func (b icBuildNamespace) withAWSComputeAMI() icOption { + return func(ic *types.InstallConfig) { + b.forAWS()(ic) + ic.Compute = []types.MachinePool{ + { + Platform: types.MachinePoolPlatform{ + AWS: &aws.MachinePool{ + AMIID: "ami-xxxxxxxxxxxxx", + }, + }, + }, + } + } +} + +func (b icBuildNamespace) withGCPComputeAMI() icOption { + return func(ic *types.InstallConfig) { + b.forGCP()(ic) + ic.Compute = []types.MachinePool{ + { + Platform: types.MachinePoolPlatform{ + GCP: &gcp.MachinePool{ + OSImage: &gcp.OSImage{ + Name: "myMostFavoriteOSImage", + Project: "myMostFavoriteProject", + }, + }, + }, + }, + } + } +} diff --git a/pkg/asset/manifests/operators.go b/pkg/asset/manifests/operators.go index 01a676bb6f..a5fdd4c51d 100644 --- a/pkg/asset/manifests/operators.go +++ b/pkg/asset/manifests/operators.go @@ -17,7 +17,9 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/asset/rhcos" "github.com/openshift/installer/pkg/asset/templates/content/bootkube" + "github.com/openshift/installer/pkg/asset/templates/content/manifests" "github.com/openshift/installer/pkg/asset/tls" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/vsphere" @@ -60,6 +62,7 @@ func (m *Manifests) Dependencies() []asset.Asset { return []asset.Asset{ &installconfig.ClusterID{}, &installconfig.InstallConfig{}, + &manifests.MCO{}, &Ingress{}, &DNS{}, &Infrastructure{}, @@ -71,6 +74,7 @@ func (m *Manifests) Dependencies() []asset.Asset { &ImageDigestMirrorSet{}, &tls.RootCA{}, &tls.MCSCertKey{}, + new(rhcos.Image), &bootkube.CVOOverrides{}, &bootkube.KubeCloudConfig{}, @@ -94,8 +98,9 @@ func (m *Manifests) Generate(_ context.Context, dependencies asset.Parents) erro imageContentSourcePolicy := &ImageContentSourcePolicy{} clusterCSIDriverConfig := &ClusterCSIDriverConfig{} imageDigestMirrorSet := &ImageDigestMirrorSet{} + mcoCfgTemplate := &manifests.MCO{} - dependencies.Get(installConfig, ingress, dns, network, infra, proxy, scheduler, imageContentSourcePolicy, imageDigestMirrorSet, clusterCSIDriverConfig) + dependencies.Get(installConfig, ingress, dns, network, infra, proxy, scheduler, imageContentSourcePolicy, imageDigestMirrorSet, clusterCSIDriverConfig, mcoCfgTemplate) redactedConfig, err := redactedInstallConfig(*installConfig.Config) if err != nil { @@ -122,6 +127,7 @@ func (m *Manifests) Generate(_ context.Context, dependencies asset.Parents) erro }, } m.FileList = append(m.FileList, m.generateBootKubeManifests(dependencies)...) + m.FileList = append(m.FileList, generateMCOManifest(installConfig.Config, mcoCfgTemplate.Files())...) m.FileList = append(m.FileList, ingress.Files()...) m.FileList = append(m.FileList, dns.Files()...) diff --git a/pkg/asset/manifests/template.go b/pkg/asset/manifests/template.go index f405c9d2ef..16dfaebd41 100644 --- a/pkg/asset/manifests/template.go +++ b/pkg/asset/manifests/template.go @@ -105,3 +105,7 @@ type openshiftTemplateData struct { CloudCreds cloudCredsSecretData Base64EncodedKubeadminPwHash string } + +type mcoTemplateData struct { + DisableMachinesetBootMgmt bool +} diff --git a/pkg/asset/templates/content/helper.go b/pkg/asset/templates/content/helper.go index 6c6f51591f..6ce89a4877 100644 --- a/pkg/asset/templates/content/helper.go +++ b/pkg/asset/templates/content/helper.go @@ -8,23 +8,29 @@ import ( ) const ( - // TemplateDir is the target directory for all template assets' files + // TemplateDir is the target directory for all template assets' files. TemplateDir = "templates" bootkubeDataDir = "manifests/bootkube/" + manifestDataDir = "manifests/manifests" openshiftDataDir = "manifests/openshift/" ) -// GetBootkubeTemplate returns the contents of the file in bootkube data dir +// GetBootkubeTemplate returns the contents of the file in bootkube data dir. func GetBootkubeTemplate(uri string) ([]byte, error) { return getFileContents(path.Join(bootkubeDataDir, uri)) } -// GetOpenshiftTemplate returns the contents of the file in openshift data dir +// GetManifestTemplate returns the contents of the file in openshift data dir. +func GetManifestTemplate(uri string) ([]byte, error) { + return getFileContents(path.Join(manifestDataDir, uri)) +} + +// GetOpenshiftTemplate returns the contents of the file in openshift data dir. func GetOpenshiftTemplate(uri string) ([]byte, error) { return getFileContents(path.Join(openshiftDataDir, uri)) } -// getFileContents the content of the given URI, assuming that it's a file +// getFileContents returns the content of the given URI, assuming that it's a file. func getFileContents(uri string) ([]byte, error) { file, err := data.Assets.Open(uri) if err != nil { diff --git a/pkg/asset/templates/content/manifests/cluster-mco-config.go b/pkg/asset/templates/content/manifests/cluster-mco-config.go new file mode 100644 index 0000000000..1e8621aefa --- /dev/null +++ b/pkg/asset/templates/content/manifests/cluster-mco-config.go @@ -0,0 +1,69 @@ +package manifests + +import ( + "context" + "os" + "path/filepath" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/templates/content" +) + +const ( + // mcoConfigTemplateFileName is the filename for the template, and ultimately + // for the manifest (with 'template' trimmed). The filename is prefixed with + // 90_ to sort before the 99_-prefixed machinesets, so that the machineconfiguration + // is applied before the machinesets, which the MCO manages. + mcoConfigTemplateFileName = "90_cluster-mco-02-config.yaml.template" +) + +var _ asset.WritableAsset = (*MCO)(nil) + +// MCO is the template for the machineconfiguration operator manifest. +type MCO struct { + FileList []*asset.File +} + +// Dependencies returns all of the dependencies directly needed by the asset. +func (t *MCO) Dependencies() []asset.Asset { + return []asset.Asset{} +} + +// Name returns the human-friendly name of the asset. +func (t *MCO) Name() string { + return "MCO Config Template" +} + +// Generate creates the asset by loading it from the data dir. +func (t *MCO) Generate(_ context.Context, parents asset.Parents) error { + fileName := mcoConfigTemplateFileName + data, err := content.GetManifestTemplate(fileName) + if err != nil { + return err + } + t.FileList = []*asset.File{ + { + Filename: filepath.Join(content.TemplateDir, fileName), + Data: data, + }, + } + return nil +} + +// Files returns the files generated by the asset. +func (t *MCO) Files() []*asset.File { + return t.FileList +} + +// Load returns the asset from disk. +func (t *MCO) Load(f asset.FileFetcher) (bool, error) { + file, err := f.FetchByName(filepath.Join(content.TemplateDir, mcoConfigTemplateFileName)) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + t.FileList = []*asset.File{file} + return true, nil +}