From abca9045967be182557b0c4db7a70b261d16a3f0 Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 8 Dec 2021 16:20:41 -0500 Subject: [PATCH 1/2] rhcos: ami regions from rhcos stream at runtime Currently, the installer relies on a generated go file for determining the AWS region in which the RHCOS image is published. The `go generate` directive was inadvertently removed in https://github.com/openshift/installer/pull/4582. Rather than resurrecting the directive, this commit removes the generated code in favor of gathering the regions directly from the rhcos stream data when needed. https://issues.redhat.com/browse/CORS-1838 --- hack/verify-codegen.sh | 2 - pkg/asset/installconfig/aws/regions.go | 6 +- pkg/asset/installconfig/aws/validation.go | 14 +-- pkg/rhcos/ami_regions.go | 33 +++++++ pkg/rhcos/ami_regions_aarch64.go | 26 ------ pkg/rhcos/ami_regions_generate.go | 102 ---------------------- pkg/rhcos/ami_regions_x86_64.go | 28 ------ 7 files changed, 36 insertions(+), 175 deletions(-) create mode 100644 pkg/rhcos/ami_regions.go delete mode 100644 pkg/rhcos/ami_regions_aarch64.go delete mode 100644 pkg/rhcos/ami_regions_generate.go delete mode 100644 pkg/rhcos/ami_regions_x86_64.go diff --git a/hack/verify-codegen.sh b/hack/verify-codegen.sh index 8bcbe789c4c..14f4a4500fe 100755 --- a/hack/verify-codegen.sh +++ b/hack/verify-codegen.sh @@ -3,8 +3,6 @@ if [ "$IS_CONTAINER" != "" ]; then set -xe go generate ./pkg/types/installconfig.go - # See https://github.com/openshift/installer/pull/5447#discussion_r762340594 - # go generate ./pkg/rhcos/ami.go set +ex git diff --exit-code else diff --git a/pkg/asset/installconfig/aws/regions.go b/pkg/asset/installconfig/aws/regions.go index c00ad313a30..1453e753d8d 100644 --- a/pkg/asset/installconfig/aws/regions.go +++ b/pkg/asset/installconfig/aws/regions.go @@ -2,7 +2,6 @@ package aws import ( "github.com/aws/aws-sdk-go/aws/endpoints" - "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/installer/pkg/rhcos" "github.com/openshift/installer/pkg/types" @@ -12,10 +11,7 @@ import ( // This is subset of AWS regions and the regions where RHEL CoreOS images are published. // The result is a map of region identifier to region description func knownRegions(architecture types.Architecture) map[string]string { - required := sets.NewString(rhcos.AMIRegionsX86_64...) - if architecture == types.ArchitectureARM64 { - required = sets.NewString(rhcos.AMIRegionsAARCH64...) - } + required := rhcos.AMIRegions(architecture) regions := make(map[string]string) for _, partition := range endpoints.DefaultPartitions() { diff --git a/pkg/asset/installconfig/aws/validation.go b/pkg/asset/installconfig/aws/validation.go index 1d9a6d82bfd..2e31a7a83b9 100644 --- a/pkg/asset/installconfig/aws/validation.go +++ b/pkg/asset/installconfig/aws/validation.go @@ -20,7 +20,6 @@ import ( "github.com/openshift/installer/pkg/rhcos" "github.com/openshift/installer/pkg/types" awstypes "github.com/openshift/installer/pkg/types/aws" - awsvalidation "github.com/openshift/installer/pkg/types/aws/validation" ) type resourceRequirements struct { @@ -81,17 +80,8 @@ func validatePlatform(ctx context.Context, meta *Metadata, fldPath *field.Path, func validateAMI(ctx context.Context, config *types.InstallConfig) field.ErrorList { // accept AMI from the rhcos stream metadata - switch config.ControlPlane.Architecture { - case types.ArchitectureAMD64: - if sets.NewString(rhcos.AMIRegionsX86_64...).Has(config.Platform.AWS.Region) { - return nil - } - case types.ArchitectureARM64: - if sets.NewString(rhcos.AMIRegionsAARCH64...).Has(config.Platform.AWS.Region) { - return nil - } - default: - return field.ErrorList{field.NotSupported(field.NewPath("controlPlane", "architecture"), config.ControlPlane.Architecture, awsvalidation.ValidArchitectureValues)} + if rhcos.AMIRegions(config.ControlPlane.Architecture).Has(config.Platform.AWS.Region) { + return nil } // accept AMI specified at the platform level diff --git a/pkg/rhcos/ami_regions.go b/pkg/rhcos/ami_regions.go new file mode 100644 index 00000000000..5ca3faa1459 --- /dev/null +++ b/pkg/rhcos/ami_regions.go @@ -0,0 +1,33 @@ +package rhcos + +import ( + "context" + + "github.com/coreos/stream-metadata-go/arch" + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/installer/pkg/types" +) + +// AMIRegions returns the AWS regions in which an RHCOS AMI for the specified architecture is published. +func AMIRegions(architecture types.Architecture) sets.String { + stream, err := FetchCoreOSBuild(context.Background()) + if err != nil { + logrus.Error("could not fetch the rhcos stream data: %w", err) + return nil + } + rpmArch := arch.RpmArch(string(architecture)) + awsImages := stream.Architectures[rpmArch].Images.Aws + if awsImages == nil { + return nil + } + regions := make([]string, 0, len(awsImages.Regions)) + for name, r := range awsImages.Regions { + if r.Image == "" { + continue + } + regions = append(regions, name) + } + return sets.NewString(regions...) +} diff --git a/pkg/rhcos/ami_regions_aarch64.go b/pkg/rhcos/ami_regions_aarch64.go deleted file mode 100644 index b8eac69d251..00000000000 --- a/pkg/rhcos/ami_regions_aarch64.go +++ /dev/null @@ -1,26 +0,0 @@ -// Code generated by ami_regions_generate.go; DO NOT EDIT. - -package rhcos - -// AMIRegionsAARCH64 is a list of regions where the RHEL CoreOS is published for architecture AARCH64. -var AMIRegionsAARCH64 = []string{ - "ap-east-1", - "ap-northeast-1", - "ap-northeast-2", - "ap-south-1", - "ap-southeast-1", - "ap-southeast-2", - "ca-central-1", - "eu-central-1", - "eu-north-1", - "eu-south-1", - "eu-west-1", - "eu-west-2", - "eu-west-3", - "me-south-1", - "sa-east-1", - "us-east-1", - "us-east-2", - "us-west-1", - "us-west-2", -} diff --git a/pkg/rhcos/ami_regions_generate.go b/pkg/rhcos/ami_regions_generate.go deleted file mode 100644 index 4d6e4043105..00000000000 --- a/pkg/rhcos/ami_regions_generate.go +++ /dev/null @@ -1,102 +0,0 @@ -//go:build tools -// +build tools - -package main - -import ( - "bytes" - "encoding/json" - "fmt" - "io/ioutil" - "log" - "os" - "path/filepath" - "sort" - "strings" - "text/template" -) - -func main() { - if len(os.Args) != 5 { - log.Fatalln("exactly 5 arguments must be provided") - } - argsWithoutProg := os.Args[1:] - - pkg := argsWithoutProg[0] - srcPath, err := filepath.Abs(argsWithoutProg[1]) - log.Println("srcPath: ", srcPath) - if err != nil { - log.Fatalln("failed to load absolute path for the source") - } - dstPath, err := filepath.Abs(argsWithoutProg[2]) - log.Println("dstPath: ", dstPath) - if err != nil { - log.Fatalln("failed to load absolute path for the source") - } - - architecture := argsWithoutProg[3] - log.Println("architecture: ", architecture) - - srcData, err := ioutil.ReadFile(srcPath) - if err != nil { - log.Fatalln(err) - } - - var m metadata - if err := json.Unmarshal(srcData, &m); err != nil { - log.Fatalln(fmt.Errorf("failed to unmarshal source: %v", err)) - } - - var regions []string - for arch, images := range m.Architectures { - if arch == architecture { - regions = make([]string, 0, len(images.Images.AWS.Regions)) - for region := range images.Images.AWS.Regions { - regions = append(regions, region) - } - } - } - sort.Strings(regions) - - tinput := struct { - Architecture string - Pkg string - Regions []string - }{Architecture: strings.ToUpper(architecture), Pkg: pkg, Regions: regions} - - t := template.Must(template.New(fmt.Sprintf("ami_regions_%s", architecture)).Parse(tmpl)) - buf := &bytes.Buffer{} - if err := t.Execute(buf, tinput); err != nil { - log.Fatalln(fmt.Errorf("failed to execute the template: %v", err)) - } - - if err := ioutil.WriteFile(dstPath, buf.Bytes(), 0664); err != nil { - log.Fatalln(err) - } -} - -// use the rhco-stream.json to get AMIs per arch -type metadata struct { - Architectures map[string]struct { - Images struct { - AWS struct { - Regions map[string]struct { - Release string `json:"release"` - Image string `json:"image"` - } `json:"regions"` - } `json:"aws"` - } `json:"images"` - } `json:"architectures"` -} - -var tmpl = `// Code generated by ami_regions_generate.go; DO NOT EDIT. - -package {{ .Pkg }} - -// AMIRegions{{ .Architecture }} is a list of regions where the RHEL CoreOS is published for architecture {{ .Architecture }}. -var AMIRegions{{ .Architecture }} = []string{ -{{- range $region := .Regions}} - "{{ $region }}", -{{- end}} -} -` diff --git a/pkg/rhcos/ami_regions_x86_64.go b/pkg/rhcos/ami_regions_x86_64.go deleted file mode 100644 index ae88b0e6523..00000000000 --- a/pkg/rhcos/ami_regions_x86_64.go +++ /dev/null @@ -1,28 +0,0 @@ -// Code generated by ami_regions_generate.go; DO NOT EDIT. - -package rhcos - -// AMIRegionsX86_64 is a list of regions where the RHEL CoreOS is published for architecture X86_64. -var AMIRegionsX86_64 = []string{ - "af-south-1", - "ap-east-1", - "ap-northeast-1", - "ap-northeast-2", - "ap-northeast-3", - "ap-south-1", - "ap-southeast-1", - "ap-southeast-2", - "ca-central-1", - "eu-central-1", - "eu-north-1", - "eu-south-1", - "eu-west-1", - "eu-west-2", - "eu-west-3", - "me-south-1", - "sa-east-1", - "us-east-1", - "us-east-2", - "us-west-1", - "us-west-2", -} From 547ec65a03c87c2a2d265514cac0a1f114faa5dc Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 8 Dec 2021 16:30:25 -0500 Subject: [PATCH 2/2] aws: make validArchitectureValues var private Since the ValidArchitectureValues variable is no longer used outside of the pkg/types/aws/validation package, it can be made private. The use of the variable outside of the package was removed when validation of the architecture was removed from the validation of the AMI. --- pkg/types/aws/validation/machinepool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/types/aws/validation/machinepool.go b/pkg/types/aws/validation/machinepool.go index 6d989a954d4..f329579d7ce 100644 --- a/pkg/types/aws/validation/machinepool.go +++ b/pkg/types/aws/validation/machinepool.go @@ -17,8 +17,8 @@ var ( types.ArchitectureARM64: true, } - // ValidArchitectureValues lists the supported arches for AWS - ValidArchitectureValues = func() []string { + // validArchitectureValues lists the supported arches for AWS + validArchitectureValues = func() []string { v := make([]string, 0, len(validArchitectures)) for m := range validArchitectures { v = append(v, string(m)) @@ -66,7 +66,7 @@ func ValidateMachinePoolArchitecture(pool *types.MachinePool, fldPath *field.Pat allErrs := field.ErrorList{} if !validArchitectures[pool.Architecture] { - allErrs = append(allErrs, field.NotSupported(fldPath, pool.Architecture, ValidArchitectureValues)) + allErrs = append(allErrs, field.NotSupported(fldPath, pool.Architecture, validArchitectureValues)) } return allErrs }