worker node selection with capabilities

* added function for finding best machine image (based on AWS GEP-33 implementation)
* added machineImage.Image logic to ported GEP-33 functions, if machineImage.Image is defined and no imageIDs are specified for each region, the machineImage.Image must be used for selection.

refactor machine_test.go:
* use describeTableSubTree to test capabilities and legacy clpf
* use describeTableSubTree to test if imageName is used instead of imageID and vice versa
* replace setup function with BeforeEach

Author: Vincinator

pkg/apis/openstack/helper/helper.go

@@ -118,13 +118,14 @@ func FindImageInCloudProfile(
 
 	capabilitySet, err := findMachineImageFlavor(machineImages, name, version, region, arch, machineCapabilities, capabilityDefinitions)
 	if err != nil {
-		return nil, fmt.Errorf("could not find an AMI for region %q, image %q, version %q that supports %v: %w", region, name, version, machineCapabilities, err)
+		return nil, fmt.Errorf("could not find an Image ID for region %q, image %q, version %q that supports %v: %w", region, name, version, machineCapabilities, err)
 	}
 
-	if capabilitySet != nil && len(capabilitySet.Regions) > 0 && capabilitySet.Regions[0].ID != "" {
+	// TODO: what if imageName is used?
+	if capabilitySet != nil && len(capabilitySet.Regions) > 0 && (capabilitySet.Regions[0].ID != "" || capabilitySet.Image != "") {
 		return capabilitySet, nil
 	}
-	return nil, fmt.Errorf("could not find an AMI for region %q, image %q, version %q that supports %v", region, name, version, machineCapabilities)
+	return nil, fmt.Errorf("could not find an Image ID for region %q, image %q, version %q that supports %v", region, name, version, machineCapabilities)
 }
 
 // FindImageInWorkerStatus takes a list of machine images from the worker status and tries to find the first entry
@@ -173,10 +174,12 @@ func findMachineImageFlavor(
 				continue
 			}
 
+			// No capability definitions means legacy format, so we match Architecture field of region
 			if len(capabilityDefinitions) == 0 {
 				for _, mapping := range version.Regions {
 					if region == mapping.Name && ptr.Equal(arch, mapping.Architecture) {
 						return &api.MachineImageFlavor{
+							Image:        version.Image,
 							Regions:      []api.RegionIDMapping{mapping},
 							Capabilities: gardencorev1beta1.Capabilities{},
 						}, nil
@@ -185,6 +188,7 @@ func findMachineImageFlavor(
 				continue
 			}
 
+			// TODO VR: filteredCapabilityFlavors.Image is empty. We need to set it here if it exists
 			filteredCapabilityFlavors := filterCapabilityFlavorsByRegion(version.CapabilityFlavors, region)
 			bestMatch, err := worker.FindBestImageFlavor(filteredCapabilityFlavors, machineCapabilities, capabilityDefinitions)
 			if err != nil {
@@ -212,6 +216,7 @@ func filterCapabilityFlavorsByRegion(capabilityFlavors []api.MachineImageFlavor,
 		if regionIDMapping != nil {
 			compatibleFlavors = append(compatibleFlavors, &api.MachineImageFlavor{
 				Regions:      []api.RegionIDMapping{*regionIDMapping},
+				Image:        capabilityFlavor.Image,
 				Capabilities: capabilityFlavor.Capabilities,
 			})
 		}

pkg/apis/openstack/helper/helper_test.go

@@ -5,6 +5,7 @@
 package helper_test
 
 import (
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/utils/ptr"

pkg/controller/worker/machine_images.go

@@ -9,8 +9,9 @@ import (
 	"fmt"
 
 	"github.com/gardener/gardener/extensions/pkg/controller/worker"
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
-	"k8s.io/utils/ptr"
+	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
@@ -38,10 +39,33 @@ func (w *workerDelegate) UpdateMachineImagesStatus(ctx context.Context) error {
 	return nil
 }
 
-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
+func (w *workerDelegate) selectMachineImageForWorkerPool(name, version string, region string, arch *string, machineCapabilities gardencorev1beta1.Capabilities) (*api.MachineImage, error) {
+	selectedMachineImage := &api.MachineImage{
+		Name:    name,
+		Version: version,
+	}
+
+	if capabilitySet, err := helper.FindImageInCloudProfile(w.cloudProfileConfig, name, version, region, arch, machineCapabilities, w.cluster.CloudProfile.Spec.MachineCapabilities); err == nil {
+		selectedMachineImage.Capabilities = capabilitySet.Capabilities
+		// ID takes precedence over Image attribute. Reminder: Image is global name while ID is region specific.
+		if capabilitySet.Regions[0].ID == "" {
+			selectedMachineImage.Image = capabilitySet.Image
+		} else {
+			selectedMachineImage.ID = capabilitySet.Regions[0].ID
+		}
+
+		if len(selectedMachineImage.Capabilities[v1beta1constants.ArchitectureName]) > 0 {
+			var selectedArch *string
+			selectedArch = &selectedMachineImage.Capabilities[v1beta1constants.ArchitectureName][0]
+			// Verify that selectedMachineImage has correct architecture
+			if selectedArch == nil || arch != nil && *selectedArch != *arch {
+				return nil, fmt.Errorf("architecture does not match for machine image")
+			}
+		} else {
+			selectedMachineImage.Architecture = capabilitySet.Regions[0].Architecture
+		}
+
+		return selectedMachineImage, nil
 	}
 
 	// Try to look up machine image in worker provider status as it was not found in componentconfig.
@@ -51,27 +75,46 @@ func (w *workerDelegate) findMachineImage(name, version, architecture string) (*
 			return nil, fmt.Errorf("could not decode worker status of worker '%s': %w", k8sclient.ObjectKeyFromObject(w.worker), err)
 		}
 
-		machineImage, err := helper.FindMachineImage(workerStatus.MachineImages, name, version, architecture)
-		if err != nil {
-			return nil, worker.ErrorMachineImageNotFound(name, version)
-		}
+		return helper.FindImageInWorkerStatus(workerStatus.MachineImages, name, version, arch, machineCapabilities, w.cluster.CloudProfile.Spec.MachineCapabilities)
+	}
 
-		// 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 nil, worker.ErrorMachineImageNotFound(name, version, *arch, region)
+}
 
-		return machineImage, nil
+func appendMachineImage(machineImages []api.MachineImage, machineImage api.MachineImage, capabilityDefinitions []gardencorev1beta1.CapabilityDefinition) []api.MachineImage {
+	// support for cloudprofile machine images without capabilities
+	if len(capabilityDefinitions) == 0 {
+		for _, image := range machineImages {
+			if image.Name == machineImage.Name && image.Version == machineImage.Version && machineImage.Architecture == image.Architecture {
+				// If the image already exists without capabilities, we can just return the existing list.
+				return machineImages
+			}
+		}
+		return append(machineImages, api.MachineImage{
+			Name:         machineImage.Name,
+			Version:      machineImage.Version,
+			ID:           machineImage.ID,
+			Architecture: machineImage.Architecture,
+		})
 	}
 
-	return nil, worker.ErrorMachineImageNotFound(name, version)
-}
+	defaultedCapabilities := gardencorev1beta1.GetCapabilitiesWithAppliedDefaults(machineImage.Capabilities, capabilityDefinitions)
 
-func appendMachineImage(machineImages []api.MachineImage, machineImage api.MachineImage) []api.MachineImage {
-	if _, err := helper.FindMachineImage(machineImages, machineImage.Name, machineImage.Version, ptr.Deref(machineImage.Architecture, v1beta1constants.ArchitectureAMD64)); err != nil {
-		return append(machineImages, machineImage)
+	for _, existingMachineImage := range machineImages {
+		existingDefaultedCapabilities := gardencorev1beta1.GetCapabilitiesWithAppliedDefaults(existingMachineImage.Capabilities, capabilityDefinitions)
+		if existingMachineImage.Name == machineImage.Name && existingMachineImage.Version == machineImage.Version && gardencorev1beta1helper.AreCapabilitiesEqual(defaultedCapabilities, existingDefaultedCapabilities) {
+			// If the image already exists with the same capabilities return the existing list.
+			return machineImages
+		}
 	}
+
+	// If the image does not exist, we create a new machine image entry with the capabilities.
+	machineImages = append(machineImages, api.MachineImage{
+		Name:         machineImage.Name,
+		Version:      machineImage.Version,
+		ID:           machineImage.ID,
+		Capabilities: machineImage.Capabilities,
+	})
+
 	return machineImages
 }

pkg/controller/worker/machines.go

@@ -15,6 +15,7 @@ import (
 	extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
 	"github.com/gardener/gardener/extensions/pkg/controller/worker"
 	genericworkeractuator "github.com/gardener/gardener/extensions/pkg/controller/worker/genericactuator"
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
 	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
@@ -27,6 +28,7 @@ import (
 
 	"github.com/gardener/gardener-extension-provider-openstack/charts"
 	api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
+	openstackapi "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
 	"github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper"
 	"github.com/gardener/gardener-extension-provider-openstack/pkg/openstack"
 )
@@ -99,12 +101,20 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error {
 	for _, pool := range w.worker.Spec.Pools {
 		zoneLen := int32(len(pool.Zones)) // #nosec: G115 - We validate if num pool zones exceeds max_int32.
 
+		//TODO: this returns a capability with arch amd64, but worker pool defined architecture arm64.
+		machineTypeFromCloudProfile := gardencorev1beta1helper.FindMachineTypeByName(w.cluster.CloudProfile.Spec.MachineTypes, pool.MachineType)
+		if machineTypeFromCloudProfile == nil {
+			return fmt.Errorf("machine type %q not found in cloud profile %q", pool.MachineType, w.cluster.CloudProfile.Name)
+		}
+
 		architecture := ptr.Deref(pool.Architecture, v1beta1constants.ArchitectureAMD64)
-		machineImage, err := w.findMachineImage(pool.MachineImage.Name, pool.MachineImage.Version, architecture)
+		machineImage, err := w.selectMachineImageForWorkerPool(pool.MachineImage.Name, pool.MachineImage.Version, w.worker.Spec.Region, &architecture, machineTypeFromCloudProfile.Capabilities)
 		if err != nil {
 			return err
 		}
-		machineImages = appendMachineImage(machineImages, *machineImage)
+
+		machineImages = EnsureUniformMachineImages(machineImages, w.cluster.CloudProfile.Spec.MachineCapabilities)
+		machineImages = appendMachineImage(machineImages, *machineImage, w.cluster.CloudProfile.Spec.MachineCapabilities)
 
 		var volumeSize int
 		if pool.Volume != nil {
@@ -324,3 +334,50 @@ func addTopologyLabel(labels map[string]string, zone string) map[string]string {
 		openstack.CSIManilaDriverTopologyKey: zone,
 	})
 }
+
+// EnsureUniformMachineImages ensures that all machine images are in the same format, either with or without Capabilities.
+func EnsureUniformMachineImages(images []openstackapi.MachineImage, definitions []gardencorev1beta1.CapabilityDefinition) []openstackapi.MachineImage {
+	var uniformMachineImages []openstackapi.MachineImage
+
+	if len(definitions) == 0 {
+		// transform images that were added with Capabilities to the legacy format without Capabilities
+		for _, img := range images {
+			if len(img.Capabilities) == 0 {
+				// image is already legacy format
+				uniformMachineImages = appendMachineImage(uniformMachineImages, img, definitions)
+				continue
+			}
+			// transform to legacy format by using the Architecture capability if it exists
+			var architecture *string
+			if len(img.Capabilities[v1beta1constants.ArchitectureName]) > 0 {
+				architecture = &img.Capabilities[v1beta1constants.ArchitectureName][0]
+			}
+			// TODO: make uniform for imageID is not set BUT the global image name is set
+			uniformMachineImages = appendMachineImage(uniformMachineImages, openstackapi.MachineImage{
+				Name:         img.Name,
+				Version:      img.Version,
+				ID:           img.ID,
+				Architecture: architecture,
+			}, definitions)
+		}
+		return uniformMachineImages
+	}
+
+	// transform images that were added without Capabilities to contain a MachineImageFlavor with defaulted Architecture
+	for _, img := range images {
+		if len(img.Capabilities) > 0 {
+			// image is already in the new format with Capabilities
+			uniformMachineImages = appendMachineImage(uniformMachineImages, img, definitions)
+		} else {
+			// add image as a capability set with defaulted Architecture
+			architecture := ptr.Deref(img.Architecture, v1beta1constants.ArchitectureAMD64)
+			uniformMachineImages = appendMachineImage(uniformMachineImages, openstackapi.MachineImage{
+				Name:         img.Name,
+				Version:      img.Version,
+				ID:           img.ID,
+				Capabilities: gardencorev1beta1.Capabilities{v1beta1constants.ArchitectureName: []string{architecture}},
+			}, definitions)
+		}
+	}
+	return uniformMachineImages
+}