[GEP-33] Adoption of Capabilities Cloudprofile-API Change#1480
[GEP-33] Adoption of Capabilities Cloudprofile-API Change#1480hebelsan merged 13 commits intogardener:masterfrom
Conversation
hebelsan
left a comment
There was a problem hiding this comment.
looks good, only two small nits.
pkg/apis/aws/types_cloudprofile.go
Outdated
|
|
||
| // CapabilitySet groups all RegionAMIMappings for a specific et of capabilities. | ||
| type CapabilitySet struct { | ||
| // MachineImageFlavor groups all RegionAMIMappings for a specific et of capabilities. |
There was a problem hiding this comment.
set of capabilities instead of et of capabilities
pkg/apis/aws/helper/helper.go
Outdated
| func filterCapabilitySetsByRegion(capabilitySets []api.CapabilitySet, regionName string) []*api.CapabilitySet { | ||
| var compatibleSets []*api.CapabilitySet | ||
| func filterCapabilityFlavorsByRegion(capabilityFlavors []api.MachineImageFlavor, regionName string) []*api.MachineImageFlavor { | ||
| var compatibleSets []*api.MachineImageFlavor |
There was a problem hiding this comment.
var compatibleSets renaming the variable to compatibleFlavors improves readability.
…ofile and related structures
…ofile and related structures
5613c03 to
7328c6e
Compare
|
|
||
| func validateRegions(regions []apisaws.RegionAMIMapping, version, name string, hasCloudProfileCapabilities bool, jdxPath *field.Path) field.ErrorList { | ||
| // validateRegions validates the regions of a machine image version or capability flavor. | ||
| func validateRegions(regions []apisaws.RegionAMIMapping, version, name string, capabilityDefinitions []gardencorev1beta1.CapabilityDefinition, jdxPath *field.Path) field.ErrorList { |
There was a problem hiding this comment.
The signature of validateRegions is (regions []..., version, name string, ...), but all call sites pass (name, version, ...) this looks like an error.
There was a problem hiding this comment.
Reordered the parameter. And added some UT to check the output of the error details.
| if isCapabilitiesCloudProfile { | ||
| capabilityDefinitions = []gardencorev1beta1.CapabilityDefinition{ | ||
| {Name: "some-capability", Values: []string{"a", "b", "c"}}, | ||
| {Name: v1beta1constants.ArchitectureName, Values: []string{v1beta1constants.ArchitectureAMD64, v1beta1constants.ArchitectureARM64}}, |
There was a problem hiding this comment.
Why did you you remove the constants?
There was a problem hiding this comment.
See "Use constants in test code with care" in the test guide.
There was a problem hiding this comment.
From the test guide I read that one shouldn't use constants from the same package as the tested code.
But anyway I'm fine with the change.
| namePool2 = "pool-2" | ||
| minPool2 = 30 | ||
| maxPool2 = 45 | ||
| priorityPool2 = 100 |
There was a problem hiding this comment.
Why did you remove the priority field?
There was a problem hiding this comment.
Sorry, my bad. The field was changed from int32 to *int32 in the latest gardener. I correctly adopted it now.
f4524eb to
f5876f9
Compare
|
This change set involves significant updates to a project related to managing cloud profiles and machine images. Key updates focus on transitioning from "CapabilitySets" to "MachineImageFlavors," updating dependencies, improving validation and mutation logic, and code structuring. The updates are aimed at enhancing the functionality of managing machine images and their capabilities in a cloud environment, potentially leading to better compatibility and configuration validation. Walkthrough
Model: gpt-4o | Prompt Tokens: 54073 | Completion Tokens: 191 |
…ws version to get fix for breaking spec Bug: While strict decoding a providerConfig (defined in aws extension), we encounter a field name change (capabilitySets -> capabilityFlavours). Even though we don't use it, the strict decoder fails and reports an error. PR breaking the current functionality: gardener/gardener-extension-provider-aws#1480 Fix: bump to provider extension aws, so the expencted cloudprofile.spec.providerconfig has the same schema as the ones in the dev landscape.
…ws version to get fix for breaking spec Bug: While strict decoding a providerConfig (defined in aws extension), we encounter a field name change (capabilitySets -> capabilityFlavours). Even though we don't use it, the strict decoder fails and reports an error. PR breaking the current functionality: gardener/gardener-extension-provider-aws#1480 Fix: bump to provider extension aws, so the expencted cloudprofile.spec.providerconfig has the same schema as the ones in the dev landscape.
…et fix for breaking spec Bug: While strict decoding a providerConfig (defined in aws extension), we encounter a field name change (capabilitySets -> capabilityFlavours). Even though we don't use it, the strict decoder fails and reports an error. PR breaking the current functionality: gardener/gardener-extension-provider-aws#1480 Fix: bump to provider extension aws, so the expencted cloudprofile.spec.providerconfig has the same schema as the ones in the dev landscape.
…et fix for breaking spec Bug: While strict decoding a providerConfig (defined in aws extension), we encounter a field name change (capabilitySets -> capabilityFlavours). Even though we don't use it, the strict decoder fails and reports an error. PR breaking the current functionality: gardener/gardener-extension-provider-aws#1480 Fix: bump to provider extension aws, so the expencted cloudprofile.spec.providerconfig has the same schema as the ones in the dev landscape.
How to categorize this PR?
/area control-plane
/kind enhancement
/platform aws
What this PR does / why we need it:
This PR is part of GEP-33 Machine Image Capabilities
It adopts api changes to the alpha feature of
CloudProfileCapabilitiesmade in gardener/gardener#12751:The field
CapabilitySetsof MachineImageVersion is renamed toCapabilityFlavorsto improve readability and avoid confusion with by e.g. a "set of capability values for a given capability name". As a consequence the corresponding fieldspec.ProviderConfig.MachineImages[].Versions[].capabilitySetsis renamed toCapabilityFlavorsas well.The change to
spec.machineCapabilitiesis to stay in line withmachineTypesandmachineImages.Which issue(s) this PR fixes:
Part of: gardener/gardener#11301
Special notes for your reviewer:
Release note: