From 0891c8c009b8b17b2621fe8a547f8d07047f45f5 Mon Sep 17 00:00:00 2001 From: Robin Schneider Date: Thu, 14 Mar 2024 09:43:33 +0100 Subject: [PATCH] Support arm64 (#46) * Support arm64 (#690) * Extend API for arm64 support Co-Authored-By: Konstantinos Angelopoulos Co-Authored-By: Robin Schneider <45321827+robinschneider@users.noreply.github.com> * Adapt worker controller to use architecture mapping Co-Authored-By: Konstantinos Angelopoulos Co-Authored-By: Robin Schneider <45321827+robinschneider@users.noreply.github.com> * Validate region and architecture mappings Co-Authored-By: Konstantinos Angelopoulos Co-Authored-By: Robin Schneider <45321827+robinschneider@users.noreply.github.com> * Update docs for `CloudProfileConfig` Co-Authored-By: Konstantinos Angelopoulos Co-Authored-By: Robin Schneider <45321827+robinschneider@users.noreply.github.com> --------- Co-authored-by: Robin Schneider Co-authored-by: Konstantinos Angelopoulos Co-authored-by: Robin Schneider <45321827+robinschneider@users.noreply.github.com> (cherry picked from commit 5387c16ad095dca0add0012cd431f23cd2cb5a0f) * Update go --------- Co-authored-by: Tim Ebert --- Dockerfile | 2 +- docs/operations/operations.md | 37 +++ hack/api-reference/api.md | 24 ++ pkg/apis/openstack/helper/helper.go | 36 ++- pkg/apis/openstack/helper/helper_test.go | 267 +++++++++++++----- pkg/apis/openstack/types_cloudprofile.go | 3 + pkg/apis/openstack/types_worker.go | 3 + .../openstack/v1alpha1/types_cloudprofile.go | 3 + pkg/apis/openstack/v1alpha1/types_worker.go | 3 + .../v1alpha1/zz_generated.conversion.go | 4 + .../v1alpha1/zz_generated.deepcopy.go | 18 +- pkg/apis/openstack/validation/cloudprofile.go | 17 ++ .../openstack/validation/cloudprofile_test.go | 79 ++++++ pkg/apis/openstack/zz_generated.deepcopy.go | 18 +- pkg/controller/worker/machine_images.go | 17 +- pkg/controller/worker/machines.go | 4 +- pkg/controller/worker/machines_test.go | 14 +- 17 files changed, 451 insertions(+), 98 deletions(-) diff --git a/Dockerfile b/Dockerfile index 582b10717..93e7efdf7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ ############# builder -FROM golang:1.20.4 AS builder +FROM golang:1.21.4 AS builder WORKDIR /go/src/github.com/gardener/gardener-extension-provider-openstack COPY . . diff --git a/docs/operations/operations.md b/docs/operations/operations.md index df98c2f34..59f66d0aa 100644 --- a/docs/operations/operations.md +++ b/docs/operations/operations.md @@ -49,7 +49,19 @@ machineImages: - name: coreos versions: - version: 2135.6.0 + # Fallback to image name if no region mapping is found + # Only works for amd64 and is strongly discouraged. Prefer image IDs! image: coreos-2135.6.0 + regions: + - name: europe + id: "1234-amd64" + architecture: amd64 # optional, defaults to amd64 + - name: europe + id: "1234-arm64" + architecture: arm64 + - name: asia + id: "5678-amd64" + architecture: amd64 # keystoneURL: https://url-to-keystone/v3/ # keystoneURLs: # - region: europe @@ -158,11 +170,24 @@ spec: - name: coreos versions: - version: 2135.6.0 + architectures: # optional, defaults to [amd64] + - amd64 + - arm64 machineTypes: - name: medium_4_8 cpu: "4" gpu: "0" memory: 8Gi + architecture: amd64 # optional, defaults to amd64 + storage: + class: standard + type: default + size: 40Gi + - name: medium_4_8_arm + cpu: "4" + gpu: "0" + memory: 8Gi + architecture: arm64 storage: class: standard type: default @@ -180,7 +205,19 @@ spec: - name: coreos versions: - version: 2135.6.0 + # Fallback to image name if no region mapping is found + # Only works for amd64 and is strongly discouraged. Prefer image IDs! image: coreos-2135.6.0 + regions: + - name: europe + id: "1234-amd64" + architecture: amd64 # optional, defaults to amd64 + - name: europe + id: "1234-arm64" + architecture: arm64 + - name: asia + id: "5678-amd64" + architecture: amd64 keystoneURL: https://url-to-keystone/v3/ constraints: floatingPools: diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md index 2143f267a..46f261704 100644 --- a/hack/api-reference/api.md +++ b/hack/api-reference/api.md @@ -1050,6 +1050,18 @@ string

ID is the id of the image. (one of Image or ID must be set)

+ + +architecture
+ +string + + + +(Optional) +

Architecture is the CPU architecture of the machine image

+ +

MachineImageVersion @@ -1480,6 +1492,18 @@ string

ID is the ID for the machine image in the given region.

+ + +architecture
+ +string + + + +(Optional) +

Architecture is the CPU architecture of the machine image

+ +

Router diff --git a/pkg/apis/openstack/helper/helper.go b/pkg/apis/openstack/helper/helper.go index fd02d4bb4..25cec75ca 100644 --- a/pkg/apis/openstack/helper/helper.go +++ b/pkg/apis/openstack/helper/helper.go @@ -17,6 +17,9 @@ package helper import ( "fmt" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + "k8s.io/utils/pointer" + api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/utils" ) @@ -48,9 +51,11 @@ func FindSecurityGroupByPurpose(securityGroups []api.SecurityGroup, purpose api. // FindMachineImage takes a list of machine images and tries to find the first entry // whose name, version, and zone matches with the given name, version, and cloud profile. If no such // entry is found then an error will be returned. -func FindMachineImage(machineImages []api.MachineImage, name, version string) (*api.MachineImage, error) { +func FindMachineImage(machineImages []api.MachineImage, name, version, architecture string) (*api.MachineImage, error) { for _, machineImage := range machineImages { - if machineImage.Name == name && machineImage.Version == version { + // If the architecture field is not present, ignore it for backwards-compatibility. + if machineImage.Name == name && machineImage.Version == version && + (machineImage.Architecture == nil || *machineImage.Architecture == architecture) { return &machineImage, nil } } @@ -60,7 +65,7 @@ func FindMachineImage(machineImages []api.MachineImage, name, version string) (* // FindImageFromCloudProfile takes a list of machine images, and the desired image name and version. It tries // to find the image with the given name and version in the desired cloud profile. If it cannot be found then an error // is returned. -func FindImageFromCloudProfile(cloudProfileConfig *api.CloudProfileConfig, imageName, imageVersion, regionName string) (*api.MachineImage, error) { +func FindImageFromCloudProfile(cloudProfileConfig *api.CloudProfileConfig, imageName, imageVersion, regionName, architecture string) (*api.MachineImage, error) { if cloudProfileConfig != nil { for _, machineImage := range cloudProfileConfig.MachineImages { if machineImage.Name != imageName { @@ -71,19 +76,28 @@ func FindImageFromCloudProfile(cloudProfileConfig *api.CloudProfileConfig, image continue } for _, region := range version.Regions { - if regionName == region.Name { + if regionName == region.Name && architecture == pointer.StringDeref(region.Architecture, v1beta1constants.ArchitectureAMD64) { return &api.MachineImage{ - Name: imageName, - Version: imageVersion, - ID: region.ID, + Name: imageName, + Version: imageVersion, + Architecture: &architecture, + ID: region.ID, }, nil } } - if version.Image != "" { + + // if we haven't found a region mapping, fallback to the image name + if version.Image != "" && architecture == v1beta1constants.ArchitectureAMD64 { + // The fallback image name doesn't specify an architecture, but we assume it is amd64 as arm was not supported + // previously. + // Referencing images by name is error-prone and is highly discouraged anyways. + // If people want to use arm images in their CloudProfile, they need to specify a region mapping and can't + // use the fallback MachineImage by name. return &api.MachineImage{ - Name: imageName, - Version: imageVersion, - Image: version.Image, + Name: imageName, + Version: imageVersion, + Architecture: pointer.String(v1beta1constants.ArchitectureAMD64), + Image: version.Image, }, nil } } diff --git a/pkg/apis/openstack/helper/helper_test.go b/pkg/apis/openstack/helper/helper_test.go index f1abf7dce..42f4253c3 100644 --- a/pkg/apis/openstack/helper/helper_test.go +++ b/pkg/apis/openstack/helper/helper_test.go @@ -54,49 +54,213 @@ var _ = Describe("Helper", func() { ) DescribeTable("#FindMachineImage", - func(machineImages []api.MachineImage, name, version string, expectedMachineImage *api.MachineImage, expectErr bool) { - machineImage, err := FindMachineImage(machineImages, name, version) + func(machineImages []api.MachineImage, name, version, architecture string, expectedMachineImage *api.MachineImage, expectErr bool) { + machineImage, err := FindMachineImage(machineImages, name, version, architecture) expectResults(machineImage, expectedMachineImage, err, expectErr) }, - Entry("list is nil", nil, "foo", "1.2.3", nil, true), - Entry("empty list", []api.MachineImage{}, "foo", "1.2.3", nil, true), - Entry("entry not found (no name)", []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, "foo", "1.2.3", nil, true), - Entry("entry not found (no version)", []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, "foo", "1.2.3", nil, true), - Entry("entry exists", []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, "bar", "1.2.3", &api.MachineImage{Name: "bar", Version: "1.2.3"}, false), + Entry("list is nil", + nil, + "foo", "1.2.3", "", + nil, true, + ), + Entry("empty list", + []api.MachineImage{}, + "foo", "1.2.3", "", + nil, true, + ), + Entry("entry not found (name mismatch)", + []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, + "foo", "1.2.3", "", + nil, true, + ), + Entry("entry not found (version mismatch)", + []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, + "foo", "1.2.3", "", + nil, true, + ), + Entry("entry not found (architecture mismatch)", + []api.MachineImage{{Name: "bar", Version: "1.2.3", Architecture: pointer.String("amd64")}}, + "bar", "1.2.3", "arm64", + nil, true, + ), + Entry("entry exists (architecture is ignored, amd64)", + []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, + "bar", "1.2.3", "amd64", + &api.MachineImage{Name: "bar", Version: "1.2.3"}, false, + ), + Entry("entry exists (architecture is ignored, arm64)", + []api.MachineImage{{Name: "bar", Version: "1.2.3"}}, + "bar", "1.2.3", "arm64", + &api.MachineImage{Name: "bar", Version: "1.2.3"}, false, + ), + Entry("entry exists (architecture amd64)", + []api.MachineImage{{Name: "bar", Version: "1.2.3", Architecture: pointer.String("amd64")}}, + "bar", "1.2.3", "amd64", + &api.MachineImage{Name: "bar", Version: "1.2.3", Architecture: pointer.String("amd64")}, false, + ), + Entry("entry exists (architecture arm64)", + []api.MachineImage{{Name: "bar", Version: "1.2.3", Architecture: pointer.String("arm64")}}, + "bar", "1.2.3", "arm64", + &api.MachineImage{Name: "bar", Version: "1.2.3", Architecture: pointer.String("arm64")}, false, + ), + Entry("entry exists (multiple architectures)", + []api.MachineImage{ + {Name: "bar", Version: "1.2.3", ID: "amd", Architecture: pointer.String("amd64")}, + {Name: "bar", Version: "1.2.3", ID: "arm", Architecture: pointer.String("arm64")}, + }, + "bar", "1.2.3", "amd64", + &api.MachineImage{Name: "bar", Version: "1.2.3", ID: "amd", Architecture: pointer.String("amd64")}, false, + ), ) regionName := "eu-de-1" - DescribeTable("#FindImageForCloudProfile", - func(profileImages []api.MachineImages, imageName, version, region, expectedImage string) { - cfg := &api.CloudProfileConfig{} - cfg.MachineImages = profileImages - image, err := FindImageFromCloudProfile(cfg, imageName, version, region) + Describe("#FindImageForCloudProfile", func() { + var ( + cfg *api.CloudProfileConfig + ) - if expectedImage == "" { - Expect(err).To(HaveOccurred()) - Expect(image).To(BeNil()) - return - } - Expect(err).NotTo(HaveOccurred()) - Expect(image).NotTo(BeNil()) - if image.ID != "" { - Expect(image.ID).To(Equal(expectedImage)) - } else { - Expect(image.Image).To(Equal(expectedImage)) + BeforeEach(func() { + cfg = &api.CloudProfileConfig{ + MachineImages: []api.MachineImages{ + { + Name: "flatcar", + Versions: []api.MachineImageVersion{ + { + Version: "1.0", + Image: "flatcar_1.0", + }, + { + Version: "2.0", + Image: "flatcar_2.0", + Regions: []api.RegionIDMapping{ + { + Name: "eu01", + ID: "flatcar_eu01_2.0", + }, + }, + }, + { + Version: "3.0", + Regions: []api.RegionIDMapping{ + { + Name: "eu01", + ID: "flatcar_eu01_3.0_amd64", + Architecture: pointer.String("amd64"), + }, + { + Name: "eu01", + ID: "flatcar_eu01_3.0_arm64", + Architecture: pointer.String("arm64"), + }, + }, + }, + }, + }, + }, } - }, + }) - Entry("list is nil", nil, "ubuntu", "1", regionName, ""), + Context("no image found", func() { + It("should not find image in nil list", func() { + cfg.MachineImages = nil - Entry("profile empty list", []api.MachineImages{}, "ubuntu", "1", regionName, ""), - Entry("profile entry not found (image does not exist)", makeProfileMachineImages("debian", "1", "0"), "ubuntu", "1", regionName, ""), - Entry("profile entry not found (version does not exist)", makeProfileMachineImages("ubuntu", "2", "0"), "ubuntu", "1", regionName, ""), - Entry("profile entry", makeProfileMachineImages("ubuntu", "1", "image-1234"), "ubuntu", "1", regionName, "image-1234"), - Entry("profile region entry", makeProfileRegionMachineImages("ubuntu", "1", "image-1234", regionName), "ubuntu", "1", regionName, "image-1234"), - Entry("profile region not found", makeProfileRegionMachineImages("ubuntu", "1", "image-1234", regionName+"x"), "ubuntu", "1", regionName, ""), - ) + image, err := FindImageFromCloudProfile(cfg, "flatcar", "1.0", "eu01", "amd64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + + It("should not find image in empty list", func() { + cfg.MachineImages = []api.MachineImages{} + + image, err := FindImageFromCloudProfile(cfg, "flatcar", "1.0", "eu01", "amd64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + + It("should not find image for wrong image name", func() { + image, err := FindImageFromCloudProfile(cfg, "gardenlinux", "1.0", "eu01", "amd64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + + It("should not find image for wrong version", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "1.1", "eu01", "amd64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + + }) + + Context("without region mapping", func() { + It("should fallback to image name (amd64)", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "1.0", "eu01", "amd64") + Expect(err).NotTo(HaveOccurred()) + Expect(image).To(Equal(&api.MachineImage{ + Name: "flatcar", + Version: "1.0", + Image: "flatcar_1.0", + Architecture: pointer.String("amd64"), + })) + }) + + It("should not fallback to image name (not amd64)", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "1.0", "eu01", "arm64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + }) + + Context("with region mapping, without architectures", func() { + It("should fallback to image name if region is not mapped", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "2.0", "eu02", "amd64") + Expect(err).NotTo(HaveOccurred()) + Expect(image).To(Equal(&api.MachineImage{ + Name: "flatcar", + Version: "2.0", + Image: "flatcar_2.0", + Architecture: pointer.String("amd64"), + })) + }) + + It("should use the correct mapping (without architecture)", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "2.0", "eu01", "amd64") + Expect(err).NotTo(HaveOccurred()) + Expect(image).To(Equal(&api.MachineImage{ + Name: "flatcar", + Version: "2.0", + ID: "flatcar_eu01_2.0", + Architecture: pointer.String("amd64"), + })) + }) + + It("should not find image because of non-amd64 architecture", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "2.0", "eu01", "arm64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + }) + + Context("with region mapping and architectures", func() { + It("should not find image if architecture is not mapped", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "3.0", "eu01", "ppc64") + Expect(image).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("could not find an image"))) + }) + + It("should pick the correctly mapped architecture", func() { + image, err := FindImageFromCloudProfile(cfg, "flatcar", "3.0", "eu01", "arm64") + Expect(err).NotTo(HaveOccurred()) + Expect(image).To(Equal(&api.MachineImage{ + Name: "flatcar", + Version: "3.0", + ID: "flatcar_eu01_3.0_arm64", + Architecture: pointer.String("arm64"), + })) + }) + }) + }) DescribeTable("#FindKeyStoneURL", func(keyStoneURLs []api.KeyStoneURL, keystoneURL, region, expectedKeyStoneURL string, expectErr bool) { @@ -138,45 +302,6 @@ var _ = Describe("Helper", func() { ) }) -func makeProfileMachineImages(name, version, image string) []api.MachineImages { - var versions []api.MachineImageVersion - if len(image) != 0 { - versions = append(versions, api.MachineImageVersion{ - Version: version, - Image: image, - }) - } - - return []api.MachineImages{ - { - Name: name, - Versions: versions, - }, - } -} - -func makeProfileRegionMachineImages(name, version, image, region string) []api.MachineImages { - var versions []api.MachineImageVersion - if len(image) != 0 { - versions = append(versions, api.MachineImageVersion{ - Version: version, - Regions: []api.RegionIDMapping{ - { - Name: region, - ID: image, - }, - }, - }) - } - - return []api.MachineImages{ - { - Name: name, - Versions: versions, - }, - } -} - func expectResults(result, expected interface{}, err error, expectErr bool) { if !expectErr { Expect(result).To(Equal(expected)) diff --git a/pkg/apis/openstack/types_cloudprofile.go b/pkg/apis/openstack/types_cloudprofile.go index c0666271e..fffbe8415 100644 --- a/pkg/apis/openstack/types_cloudprofile.go +++ b/pkg/apis/openstack/types_cloudprofile.go @@ -198,6 +198,9 @@ type RegionIDMapping struct { Name string // ID is the ID for the machine image in the given region. ID string + // Architecture is the CPU architecture of the machine image + // +optional + Architecture *string } // StorageClassDefinition is a definition of a storageClass diff --git a/pkg/apis/openstack/types_worker.go b/pkg/apis/openstack/types_worker.go index 9097113f7..da9df7b7f 100644 --- a/pkg/apis/openstack/types_worker.go +++ b/pkg/apis/openstack/types_worker.go @@ -46,6 +46,9 @@ type MachineImage struct { Image string // ID is the id of the image. (one of Image or ID must be set) ID string + // Architecture is the CPU architecture of the machine image + // +optional + Architecture *string } // ServerGroupDependency is a reference to an external machine dependency of openstack server groups. diff --git a/pkg/apis/openstack/v1alpha1/types_cloudprofile.go b/pkg/apis/openstack/v1alpha1/types_cloudprofile.go index a829de326..e0f7d5721 100644 --- a/pkg/apis/openstack/v1alpha1/types_cloudprofile.go +++ b/pkg/apis/openstack/v1alpha1/types_cloudprofile.go @@ -177,6 +177,9 @@ type RegionIDMapping struct { Name string `json:"name"` // ID is the ID for the machine image in the given region. ID string `json:"id"` + // Architecture is the CPU architecture of the machine image + // +optional + Architecture *string `json:"architecture,omitempty"` } // StorageClassDefinition is a definition of a storageClass diff --git a/pkg/apis/openstack/v1alpha1/types_worker.go b/pkg/apis/openstack/v1alpha1/types_worker.go index ebe306fa3..c53b8975d 100644 --- a/pkg/apis/openstack/v1alpha1/types_worker.go +++ b/pkg/apis/openstack/v1alpha1/types_worker.go @@ -49,6 +49,9 @@ type MachineImage struct { Image string `json:"image,omitempty"` // ID is the id of the image. (one of Image or ID must be set) ID string `json:"id,omitempty"` + // Architecture is the CPU architecture of the machine image + // +optional + Architecture *string `json:"architecture,omitempty"` } // ServerGroupDependency is a reference to an external machine dependency of OpenStack server groups. diff --git a/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go b/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go index b976783dd..fbfca5778 100644 --- a/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go @@ -702,6 +702,7 @@ func autoConvert_v1alpha1_MachineImage_To_openstack_MachineImage(in *MachineImag out.Version = in.Version out.Image = in.Image out.ID = in.ID + out.Architecture = (*string)(unsafe.Pointer(in.Architecture)) return nil } @@ -715,6 +716,7 @@ func autoConvert_openstack_MachineImage_To_v1alpha1_MachineImage(in *openstack.M out.Version = in.Version out.Image = in.Image out.ID = in.ID + out.Architecture = (*string)(unsafe.Pointer(in.Architecture)) return nil } @@ -886,6 +888,7 @@ func Convert_openstack_NodeStatus_To_v1alpha1_NodeStatus(in *openstack.NodeStatu func autoConvert_v1alpha1_RegionIDMapping_To_openstack_RegionIDMapping(in *RegionIDMapping, out *openstack.RegionIDMapping, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID + out.Architecture = (*string)(unsafe.Pointer(in.Architecture)) return nil } @@ -897,6 +900,7 @@ func Convert_v1alpha1_RegionIDMapping_To_openstack_RegionIDMapping(in *RegionIDM func autoConvert_openstack_RegionIDMapping_To_v1alpha1_RegionIDMapping(in *openstack.RegionIDMapping, out *RegionIDMapping, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID + out.Architecture = (*string)(unsafe.Pointer(in.Architecture)) return nil } diff --git a/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go index 17f48e4fa..b9a1040de 100644 --- a/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go @@ -458,6 +458,11 @@ func (in *LoadBalancerProvider) DeepCopy() *LoadBalancerProvider { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineImage) DeepCopyInto(out *MachineImage) { *out = *in + if in.Architecture != nil { + in, out := &in.Architecture, &out.Architecture + *out = new(string) + **out = **in + } return } @@ -477,7 +482,9 @@ func (in *MachineImageVersion) DeepCopyInto(out *MachineImageVersion) { if in.Regions != nil { in, out := &in.Regions, &out.Regions *out = make([]RegionIDMapping, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -623,6 +630,11 @@ func (in *NodeStatus) DeepCopy() *NodeStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RegionIDMapping) DeepCopyInto(out *RegionIDMapping) { *out = *in + if in.Architecture != nil { + in, out := &in.Architecture, &out.Architecture + *out = new(string) + **out = **in + } return } @@ -889,7 +901,9 @@ func (in *WorkerStatus) DeepCopyInto(out *WorkerStatus) { if in.MachineImages != nil { in, out := &in.MachineImages, &out.MachineImages *out = make([]MachineImage, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.ServerGroupDependencies != nil { in, out := &in.ServerGroupDependencies, &out.ServerGroupDependencies diff --git a/pkg/apis/openstack/validation/cloudprofile.go b/pkg/apis/openstack/validation/cloudprofile.go index 500f570bd..70ea52704 100644 --- a/pkg/apis/openstack/validation/cloudprofile.go +++ b/pkg/apis/openstack/validation/cloudprofile.go @@ -17,10 +17,13 @@ package validation import ( "fmt" "net" + "slices" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" "github.com/gardener/gardener/pkg/utils" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/pointer" api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack" ) @@ -113,6 +116,20 @@ func ValidateCloudProfileConfig(cloudProfile *api.CloudProfileConfig, fldPath *f if len(version.Version) == 0 { allErrs = append(allErrs, field.Required(jdxPath.Child("version"), "must provide a version")) } + + for k, region := range version.Regions { + kdxPath := jdxPath.Child("regions").Index(k) + + if len(region.Name) == 0 { + allErrs = append(allErrs, field.Required(kdxPath.Child("name"), "must provide a name")) + } + if len(region.ID) == 0 { + allErrs = append(allErrs, field.Required(kdxPath.Child("id"), "must provide an image ID")) + } + if !slices.Contains(v1beta1constants.ValidArchitectures, pointer.StringDeref(region.Architecture, v1beta1constants.ArchitectureAMD64)) { + allErrs = append(allErrs, field.NotSupported(kdxPath.Child("architecture"), *region.Architecture, v1beta1constants.ValidArchitectures)) + } + } } } diff --git a/pkg/apis/openstack/validation/cloudprofile_test.go b/pkg/apis/openstack/validation/cloudprofile_test.go index f4060a940..fde028fc0 100644 --- a/pkg/apis/openstack/validation/cloudprofile_test.go +++ b/pkg/apis/openstack/validation/cloudprofile_test.go @@ -328,6 +328,85 @@ var _ = Describe("CloudProfileConfig validation", func() { "Field": Equal("root.machineImages[0].versions[0].version"), })))) }) + + Context("region mapping validation", func() { + It("should forbid empty region name", func() { + cloudProfileConfig.MachineImages = []api.MachineImages{ + { + Name: "abc", + Versions: []api.MachineImageVersion{{ + Version: "foo", + Regions: []api.RegionIDMapping{{ + ID: "abc_foo", + }}, + }}, + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig, fldPath) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("root.machineImages[0].versions[0].regions[0].name"), + })))) + }) + + It("should forbid empty image ID", func() { + cloudProfileConfig.MachineImages = []api.MachineImages{ + { + Name: "abc", + Versions: []api.MachineImageVersion{{ + Version: "foo", + Regions: []api.RegionIDMapping{{ + Name: "eu01", + }}, + }}, + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig, fldPath) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("root.machineImages[0].versions[0].regions[0].id"), + })))) + }) + + It("should forbid unknown architectures", func() { + cloudProfileConfig.MachineImages = []api.MachineImages{ + { + Name: "abc", + Versions: []api.MachineImageVersion{{ + Version: "foo", + Regions: []api.RegionIDMapping{ + { + Name: "eu01", + ID: "abc_foo_amd64", + Architecture: pointer.String("amd64"), + }, + { + Name: "eu01", + ID: "abc_foo_arm64", + Architecture: pointer.String("arm64"), + }, + { + Name: "eu01", + ID: "abc_foo_ppc64", + Architecture: pointer.String("ppc64"), + }, + }, + }}, + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig, fldPath) + + Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeNotSupported), + "Field": Equal("root.machineImages[0].versions[0].regions[2].architecture"), + })))) + }) + }) }) Context("server group policy validation", func() { diff --git a/pkg/apis/openstack/zz_generated.deepcopy.go b/pkg/apis/openstack/zz_generated.deepcopy.go index 4934a2481..82973a1c9 100644 --- a/pkg/apis/openstack/zz_generated.deepcopy.go +++ b/pkg/apis/openstack/zz_generated.deepcopy.go @@ -458,6 +458,11 @@ func (in *LoadBalancerProvider) DeepCopy() *LoadBalancerProvider { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineImage) DeepCopyInto(out *MachineImage) { *out = *in + if in.Architecture != nil { + in, out := &in.Architecture, &out.Architecture + *out = new(string) + **out = **in + } return } @@ -477,7 +482,9 @@ func (in *MachineImageVersion) DeepCopyInto(out *MachineImageVersion) { if in.Regions != nil { in, out := &in.Regions, &out.Regions *out = make([]RegionIDMapping, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -623,6 +630,11 @@ func (in *NodeStatus) DeepCopy() *NodeStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RegionIDMapping) DeepCopyInto(out *RegionIDMapping) { *out = *in + if in.Architecture != nil { + in, out := &in.Architecture, &out.Architecture + *out = new(string) + **out = **in + } return } @@ -889,7 +901,9 @@ func (in *WorkerStatus) DeepCopyInto(out *WorkerStatus) { if in.MachineImages != nil { in, out := &in.MachineImages, &out.MachineImages *out = make([]MachineImage, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.ServerGroupDependencies != nil { in, out := &in.ServerGroupDependencies, &out.ServerGroupDependencies diff --git a/pkg/controller/worker/machine_images.go b/pkg/controller/worker/machine_images.go index e68afd2e2..24bddf509 100644 --- a/pkg/controller/worker/machine_images.go +++ b/pkg/controller/worker/machine_images.go @@ -19,7 +19,9 @@ import ( "fmt" "github.com/gardener/gardener/extensions/pkg/controller/worker" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" kutil "github.com/gardener/gardener/pkg/utils/kubernetes" + "k8s.io/utils/pointer" api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack" "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper" @@ -46,8 +48,8 @@ func (w *workerDelegate) UpdateMachineImagesStatus(ctx context.Context) error { return nil } -func (w *workerDelegate) findMachineImage(name, version string) (*api.MachineImage, error) { - image, err := helper.FindImageFromCloudProfile(w.cloudProfileConfig, name, version, w.cluster.Shoot.Spec.Region) +func (w *workerDelegate) findMachineImage(name, version, architecture string) (*api.MachineImage, error) { + image, err := helper.FindImageFromCloudProfile(w.cloudProfileConfig, name, version, w.cluster.Shoot.Spec.Region, architecture) if err == nil { return image, nil } @@ -59,11 +61,18 @@ func (w *workerDelegate) findMachineImage(name, version string) (*api.MachineIma return nil, fmt.Errorf("could not decode worker status of worker '%s': %w", kutil.ObjectName(w.worker), err) } - machineImage, err := helper.FindMachineImage(workerStatus.MachineImages, name, version) + machineImage, err := helper.FindMachineImage(workerStatus.MachineImages, name, version, architecture) if err != nil { return nil, worker.ErrorMachineImageNotFound(name, version) } + // The architecture field might not be present in the WorkerStatus if the Shoot has been created before introduction + // of the field. Hence, initialize it if it's empty. + machineImage = machineImage.DeepCopy() + if machineImage.Architecture == nil { + machineImage.Architecture = &architecture + } + return machineImage, nil } @@ -71,7 +80,7 @@ func (w *workerDelegate) findMachineImage(name, version string) (*api.MachineIma } func appendMachineImage(machineImages []api.MachineImage, machineImage api.MachineImage) []api.MachineImage { - if _, err := helper.FindMachineImage(machineImages, machineImage.Name, machineImage.Version); err != nil { + if _, err := helper.FindMachineImage(machineImages, machineImage.Name, machineImage.Version, pointer.StringDeref(machineImage.Architecture, v1beta1constants.ArchitectureAMD64)); err != nil { return append(machineImages, machineImage) } return machineImages diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 45fe3dae6..d8d1a88bf 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -30,6 +30,7 @@ import ( "github.com/gardener/gardener/pkg/client/kubernetes" "github.com/gardener/gardener/pkg/utils" machinev1alpha1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/gardener/gardener-extension-provider-openstack/charts" @@ -106,7 +107,8 @@ func (w *workerDelegate) generateMachineConfig() error { for _, pool := range w.worker.Spec.Pools { zoneLen := int32(len(pool.Zones)) - machineImage, err := w.findMachineImage(pool.MachineImage.Name, pool.MachineImage.Version) + architecture := pointer.StringDeref(pool.Architecture, v1beta1constants.ArchitectureAMD64) + machineImage, err := w.findMachineImage(pool.MachineImage.Name, pool.MachineImage.Version, architecture) if err != nil { return err } diff --git a/pkg/controller/worker/machines_test.go b/pkg/controller/worker/machines_test.go index 49dd331ba..85d60cf8c 100644 --- a/pkg/controller/worker/machines_test.go +++ b/pkg/controller/worker/machines_test.go @@ -543,9 +543,10 @@ var _ = Describe("Machines", func() { }, MachineImages: []apiv1alpha1.MachineImage{ { - Name: machineImageName, - Version: machineImageVersion, - Image: machineImage, + Name: machineImageName, + Version: machineImageVersion, + Image: machineImage, + Architecture: pointer.String(v1beta1constants.ArchitectureAMD64), }, }, } @@ -599,9 +600,10 @@ var _ = Describe("Machines", func() { }, MachineImages: []apiv1alpha1.MachineImage{ { - Name: machineImageName, - Version: machineImageVersion, - ID: machineImageID, + Name: machineImageName, + Version: machineImageVersion, + ID: machineImageID, + Architecture: pointer.String(v1beta1constants.ArchitectureAMD64), }, }, }