Skip to content

Commit 0f6dcbc

Browse files
committed
refactor: improve code quality in mixed format implementation
1 parent 25e0b10 commit 0f6dcbc

4 files changed

Lines changed: 29 additions & 45 deletions

File tree

pkg/admission/mutator/cloudprofile.go

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@
55
package mutator
66

77
import (
8+
"cmp"
89
"context"
910
"fmt"
1011
"slices"
1112

1213
extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook"
1314
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
15+
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
1416
"k8s.io/apimachinery/pkg/runtime"
1517
"k8s.io/apimachinery/pkg/runtime/serializer"
18+
"k8s.io/utils/ptr"
1619
"sigs.k8s.io/controller-runtime/pkg/client"
1720
"sigs.k8s.io/controller-runtime/pkg/manager"
1821

@@ -97,15 +100,14 @@ func convertCapabilityFlavors(providerFlavors []v1alpha1.MachineImageFlavor) []g
97100
return capabilityFlavors
98101
}
99102

100-
// convertRegionsToCapabilityFlavors converts old format (regions with architecture) to capability flavors
103+
// convertRegionsToCapabilityFlavors converts old format (regions with architecture) to capability flavors.
104+
// Note: A similar function exists in helper.go for internal API types that also preserves region mappings.
105+
// This version only extracts unique architectures for CloudProfile spec mutation.
101106
func convertRegionsToCapabilityFlavors(regions []v1alpha1.RegionAMIMapping) []gardencorev1beta1.MachineImageFlavor {
102-
// Group regions by architecture to create capability flavors
107+
// Collect unique architectures from regions
103108
architectureSet := make(map[string]struct{})
104109
for _, region := range regions {
105-
arch := "amd64" // default architecture
106-
if region.Architecture != nil {
107-
arch = *region.Architecture
108-
}
110+
arch := ptr.Deref(region.Architecture, v1beta1constants.ArchitectureAMD64)
109111
architectureSet[arch] = struct{}{}
110112
}
111113

@@ -114,28 +116,20 @@ func convertRegionsToCapabilityFlavors(regions []v1alpha1.RegionAMIMapping) []ga
114116
for arch := range architectureSet {
115117
capabilityFlavors = append(capabilityFlavors, gardencorev1beta1.MachineImageFlavor{
116118
Capabilities: gardencorev1beta1.Capabilities{
117-
"architecture": []string{arch},
119+
v1beta1constants.ArchitectureName: []string{arch},
118120
},
119121
})
120122
}
121123

122124
// Sort for deterministic output
123125
slices.SortFunc(capabilityFlavors, func(a, b gardencorev1beta1.MachineImageFlavor) int {
124-
aArch := ""
125-
bArch := ""
126-
if archList, ok := a.Capabilities["architecture"]; ok && len(archList) > 0 {
127-
aArch = archList[0]
128-
}
129-
if archList, ok := b.Capabilities["architecture"]; ok && len(archList) > 0 {
130-
bArch = archList[0]
131-
}
132-
if aArch < bArch {
133-
return -1
134-
}
135-
if aArch > bArch {
136-
return 1
126+
getArch := func(f gardencorev1beta1.MachineImageFlavor) string {
127+
if archList := f.Capabilities[v1beta1constants.ArchitectureName]; len(archList) > 0 {
128+
return archList[0]
129+
}
130+
return ""
137131
}
138-
return 0
132+
return cmp.Compare(getArch(a), getArch(b))
139133
})
140134

141135
return capabilityFlavors

pkg/admission/validator/namespacedcloudprofile.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,13 +251,6 @@ func validateRegionsFormatWithCapabilities(machineImage core.MachineImage, versi
251251
})
252252
availableArchitectures := slices.Collect(maps.Keys(architecturesMap))
253253

254-
// Collect regions per architecture
255-
regionsPerArchitecture := make(map[string][]string)
256-
for _, region := range providerImageVersion.Regions {
257-
arch := ptr.Deref(region.Architecture, v1beta1constants.ArchitectureAMD64)
258-
regionsPerArchitecture[arch] = append(regionsPerArchitecture[arch], region.Name)
259-
}
260-
261254
// 1. Check for excess architectures in provider that are not in spec
262255
for _, arch := range availableArchitectures {
263256
isFound := false

pkg/apis/aws/helper/helper.go

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,24 +167,21 @@ func findMachineImageFlavor(
167167
// When capabilities are defined, support both formats per version:
168168
// - New format: capabilityFlavors
169169
// - Old format: regions with architecture (converted to capability flavors)
170+
var capabilityFlavors []api.MachineImageFlavor
170171
if len(version.CapabilityFlavors) > 0 {
171-
// New format: use capabilityFlavors
172-
filteredCapabilityFlavors := filterCapabilityFlavorsByRegion(version.CapabilityFlavors, region)
173-
bestMatch, err := worker.FindBestImageFlavor(filteredCapabilityFlavors, machineCapabilities, capabilityDefinitions)
174-
if err != nil {
175-
return nil, fmt.Errorf("could not determine best flavor %w", err)
176-
}
177-
return bestMatch, nil
172+
capabilityFlavors = version.CapabilityFlavors
178173
} else if len(version.Regions) > 0 {
179-
// Old format: convert regions with architecture to capability flavors
180-
capabilityFlavors := convertRegionsToCapabilityFlavors(version.Regions)
181-
filteredCapabilityFlavors := filterCapabilityFlavorsByRegion(capabilityFlavors, region)
182-
bestMatch, err := worker.FindBestImageFlavor(filteredCapabilityFlavors, machineCapabilities, capabilityDefinitions)
183-
if err != nil {
184-
return nil, fmt.Errorf("could not determine best flavor %w", err)
185-
}
186-
return bestMatch, nil
174+
capabilityFlavors = convertRegionsToCapabilityFlavors(version.Regions)
175+
} else {
176+
continue
177+
}
178+
179+
filteredCapabilityFlavors := filterCapabilityFlavorsByRegion(capabilityFlavors, region)
180+
bestMatch, err := worker.FindBestImageFlavor(filteredCapabilityFlavors, machineCapabilities, capabilityDefinitions)
181+
if err != nil {
182+
return nil, fmt.Errorf("could not determine best flavor: %w", err)
187183
}
184+
return bestMatch, nil
188185
}
189186
}
190187
return nil, nil

pkg/apis/aws/helper/helper_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ var _ = Describe("Helper", func() {
186186
"ubuntu", "22.04", region, ptr.To("amd64"), "ami-ubuntu-amd64"),
187187

188188
Entry("finds arm64 image using old format with capabilities defined",
189-
makeProfileMachineImagesOldFormat("ubuntu", "22.04", region, "ami-ubuntu-arm64", ptr.To("arm64")),
190-
"ubuntu", "22.04", region, ptr.To("arm64"), "ami-ubuntu-arm64"),
189+
makeProfileMachineImagesOldFormat("gardenlinux", "1877.9.0", region, "ami-1877.9.0-arm64", ptr.To("arm64")),
190+
"gardenlinux", "1877.9.0", region, ptr.To("arm64"), "ami-1877.9.0-arm64"),
191191

192192
Entry("does not find image when architecture mismatch (old format)",
193193
makeProfileMachineImagesOldFormat("ubuntu", "22.04", region, "ami-ubuntu-amd64", ptr.To("amd64")),

0 commit comments

Comments
 (0)