From 85a3545ebcb9166a6fa8986747dfd6e878860bb3 Mon Sep 17 00:00:00 2001 From: Rafael Fonseca Date: Fri, 19 Jul 2024 21:31:08 +0200 Subject: [PATCH] CORS-3608: aws: deprecate platform.aws.amiID field This field was introduced [1] before the Installer had support for custom AMIs in machine pools [2]. Now that it does, the same functionality is achieved via the defaultMachinePlatform field `platform.aws.defaultMachinePlatform.amiID` [1] fdf94e39ee0226b152b6739a0dad848a005a5c26 [2] bc4722257643dd0d643564c686ecd5ea421f947b --- .../install.openshift.io_installconfigs.yaml | 6 ++--- docs/user/aws/customization.md | 2 +- pkg/asset/rhcos/image.go | 3 --- pkg/explain/printer_test.go | 2 +- pkg/types/aws/platform.go | 5 ++-- pkg/types/conversion/installconfig.go | 9 +++++++ pkg/types/conversion/installconfig_test.go | 27 +++++++++++++++++++ 7 files changed, 44 insertions(+), 10 deletions(-) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 0045b4fce3..210dff1abf 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -2155,9 +2155,9 @@ spec: description: AWS is the configuration used when installing on AWS. properties: amiID: - description: AMIID is the AMI that should be used to boot machines - for the cluster. If set, the AMI should belong to the same region - as the cluster. + description: The field is deprecated. AMIID is the AMI that should + be used to boot machines for the cluster. If set, the AMI should + belong to the same region as the cluster. type: string bestEffortDeleteIgnition: description: BestEffortDeleteIgnition is an optional field that diff --git a/docs/user/aws/customization.md b/docs/user/aws/customization.md index 727bd96ebc..2bf3fd6603 100644 --- a/docs/user/aws/customization.md +++ b/docs/user/aws/customization.md @@ -5,7 +5,7 @@ Beyond the [platform-agnostic `install-config.yaml` properties](../customization ## Cluster-scoped properties * `amiID` (optional string): The AMI that should be used to boot machines for the cluster. - If set, the AMI should belong to the same region as the cluster. + If set, the AMI should belong to the same region as the cluster. This field is now deprecated and `defaultMachinePlatform` should be used instead. * `region` (required string): The AWS region where the cluster will be created. * `subnets` (optional array of strings): Existing subnets (by ID) where cluster resources will be created. Leave unset to have the installer create subnets in a new VPC on your behalf. diff --git a/pkg/asset/rhcos/image.go b/pkg/asset/rhcos/image.go index ff49894a4c..d995543b70 100644 --- a/pkg/asset/rhcos/image.go +++ b/pkg/asset/rhcos/image.go @@ -84,9 +84,6 @@ func osImage(ctx context.Context, config *types.InstallConfig) (string, error) { } switch config.Platform.Name() { case aws.Name: - if len(config.Platform.AWS.AMIID) > 0 { - return config.Platform.AWS.AMIID, nil - } region := config.Platform.AWS.Region if !rhcos.AMIRegions(config.ControlPlane.Architecture).Has(region) { const globalResourceRegion = "us-east-1" diff --git a/pkg/explain/printer_test.go b/pkg/explain/printer_test.go index 043976af47..87fb88381a 100644 --- a/pkg/explain/printer_test.go +++ b/pkg/explain/printer_test.go @@ -148,7 +148,7 @@ func Test_PrintFields(t *testing.T) { path: []string{"platform", "aws"}, desc: `FIELDS: amiID - AMIID is the AMI that should be used to boot machines for the cluster. If set, the AMI should belong to the same region as the cluster. + The field is deprecated. AMIID is the AMI that should be used to boot machines for the cluster. If set, the AMI should belong to the same region as the cluster. bestEffortDeleteIgnition BestEffortDeleteIgnition is an optional field that can be used to ignore errors from S3 deletion of ignition objects during cluster bootstrap. The default behavior is to fail the installation if ignition objects cannot be deleted. Enable this functionality when there are known reasons disallowing their deletion. diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 1c41dde2fb..bbab237dd4 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -16,8 +16,9 @@ const ( // Platform stores all the global configuration that all machinesets // use. type Platform struct { - // AMIID is the AMI that should be used to boot machines for the cluster. - // If set, the AMI should belong to the same region as the cluster. + // The field is deprecated. AMIID is the AMI that should be used to boot + // machines for the cluster. If set, the AMI should belong to the same + // region as the cluster. // // +optional AMIID string `json:"amiID,omitempty"` diff --git a/pkg/types/conversion/installconfig.go b/pkg/types/conversion/installconfig.go index 6d50cf855b..2596a3df9f 100644 --- a/pkg/types/conversion/installconfig.go +++ b/pkg/types/conversion/installconfig.go @@ -289,5 +289,14 @@ func convertAWS(config *types.InstallConfig) error { if !config.AWS.BestEffortDeleteIgnition { config.AWS.BestEffortDeleteIgnition = config.AWS.PreserveBootstrapIgnition } + if ami := config.AWS.AMIID; len(ami) > 0 { + if config.AWS.DefaultMachinePlatform == nil { + config.AWS.DefaultMachinePlatform = &aws.MachinePool{} + } + // DefaultMachinePlatform.AMIID takes precedence in the machine manifest code anyway + if len(config.AWS.DefaultMachinePlatform.AMIID) == 0 { + config.AWS.DefaultMachinePlatform.AMIID = ami + } + } return nil } diff --git a/pkg/types/conversion/installconfig_test.go b/pkg/types/conversion/installconfig_test.go index 91562a6bcd..4a5d29c5c3 100644 --- a/pkg/types/conversion/installconfig_test.go +++ b/pkg/types/conversion/installconfig_test.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/baremetal" "github.com/openshift/installer/pkg/types/nutanix" "github.com/openshift/installer/pkg/types/openstack" @@ -706,6 +707,32 @@ func TestConvertInstallConfig(t *testing.T) { }, }, }, + { + name: "aws deprecated platform amiID", + config: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + AMIID: "deprec-id", + }, + }, + }, + expected: &types.InstallConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: types.InstallConfigVersion, + }, + Platform: types.Platform{ + AWS: &aws.Platform{ + AMIID: "deprec-id", + DefaultMachinePlatform: &aws.MachinePool{ + AMIID: "deprec-id", + }, + }, + }, + }, + }, } for _, tc := range cases {