Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add image resolution policy #237

Merged
merged 6 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions api/v1alpha1/ionoscloudmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,32 @@ type ImageSpec struct {
Selector *ImageSelector `json:"selector,omitempty"`
}

// ImageSelectorResolutionPolicy defines what action to take when the image selector has ambiguous results.
// +kubebuilder:validation:Enum=Exact;Newest
type ImageSelectorResolutionPolicy string

const (
// ResolutionPolicyExact only succeeds if the image selector resolves to exactly 1 image.
ResolutionPolicyExact ImageSelectorResolutionPolicy = "Exact"
// ResolutionPolicyNewest uses the newest entry if the image selector resolves to more than 1 image.
ResolutionPolicyNewest ImageSelectorResolutionPolicy = "Newest"
)

// ImageSelector defines label selectors for looking up images.
type ImageSelector struct {
// MatchLabels is a map of key/value pairs.
//
//+kubebuilder:validation:MinProperties=1
MatchLabels map[string]string `json:"matchLabels"`

// ResolutionPolicy controls the lookup behavior.
// The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
// Use policy 'Newest' to select the newest image instead.
//
//+kubebuilder:default=`Exact`
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
//+optional
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
ResolutionPolicy ImageSelectorResolutionPolicy `json:"resolutionPolicy,omitempty"`

// UseMachineVersion indicates whether to use the parent Machine's version field to look up image names.
// Enabled by default.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,16 @@ spec:
description: MatchLabels is a map of key/value pairs.
minProperties: 1
type: object
resolutionPolicy:
default: Exact
description: |-
ResolutionPolicy controls the lookup behavior.
The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
Use policy 'Newest' to select the newest image instead.
enum:
- Exact
- Newest
type: string
useMachineVersion:
default: true
description: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ spec:
pairs.
minProperties: 1
type: object
resolutionPolicy:
default: Exact
description: |-
ResolutionPolicy controls the lookup behavior.
The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
Use policy 'Newest' to select the newest image instead.
enum:
- Exact
- Newest
type: string
useMachineVersion:
default: true
description: |-
Expand Down
17 changes: 15 additions & 2 deletions internal/service/cloud/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"slices"
"strings"

sdk "github.com/ionos-cloud/sdk-go/v6"
Expand Down Expand Up @@ -70,8 +71,20 @@ func (s *Service) lookupImageID(ctx context.Context, ms *scope.Machine) (string,
images = filterImagesByName(images, version)
}

if n := len(images); n != 1 {
return "", imageMatchError{imageIDs: getImageIDs(images), selector: imageSpec.Selector}
if len(images) == 0 {
return "", imageMatchError{selector: imageSpec.Selector}
mcbenjemaa marked this conversation as resolved.
Show resolved Hide resolved
}

switch imageSpec.Selector.ResolutionPolicy {
case infrav1.ResolutionPolicyExact:
if len(images) > 1 {
return "", imageMatchError{imageIDs: getImageIDs(images), selector: imageSpec.Selector}
}
case infrav1.ResolutionPolicyNewest:
slices.SortFunc(images, func(lhs, rhs *sdk.Image) int {
// swap lhs and rhs to produce reverse order
return rhs.Metadata.CreatedDate.Compare(lhs.Metadata.CreatedDate.Time)
})
}

return ptr.Deref(images[0].GetId(), ""), nil
Expand Down
34 changes: 32 additions & 2 deletions internal/service/cloud/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"slices"
"testing"
"time"

"github.com/go-logr/logr"
sdk "github.com/ionos-cloud/sdk-go/v6"
Expand All @@ -47,6 +48,7 @@ func (s *imageTestSuite) SetupTest() {
MatchLabels: map[string]string{
"test": "image",
},
ResolutionPolicy: infrav1.ResolutionPolicyExact,
}
}

Expand Down Expand Up @@ -121,7 +123,7 @@ func (s *imageTestSuite) TestLookupImageIgnoreMissingMachineVersion() {
s.Equal("image-1", imageID)
}

func (s *imageTestSuite) TestLookupImageOK() {
func (s *imageTestSuite) TestLookupImageExactOK() {
s.ionosClient.EXPECT().ListLabels(s.ctx).Return(
[]sdk.Label{
makeTestLabel("image", "image-1", "test", "image"),
Expand All @@ -135,10 +137,37 @@ func (s *imageTestSuite) TestLookupImageOK() {
s.Equal("image-1", imageID)
}

func (s *imageTestSuite) TestLookupImageNewestOK() {
s.ionosClient.EXPECT().ListLabels(s.ctx).Return(
[]sdk.Label{
makeTestLabel("image", "image-1", "test", "image"),
makeTestLabel("image", "image-2", "test", "image"),
makeTestLabel("image", "image-3", "test", "image"),
}, nil,
).Once()
s.ionosClient.EXPECT().GetDatacenterLocationByID(s.ctx, s.infraMachine.Spec.DatacenterID).Return("loc", nil).Once()
baseTime := time.Now().Round(time.Second)
s.ionosClient.EXPECT().GetImage(s.ctx, "image-1").Return(s.makeTestImageWithDate("image-1", "img-ver1-", "loc", baseTime), nil).Once()
s.ionosClient.EXPECT().GetImage(s.ctx, "image-2").Return(s.makeTestImageWithDate("image-2", "img-ver2-", "loc", baseTime.Add(2*time.Minute)), nil).Once()
s.ionosClient.EXPECT().GetImage(s.ctx, "image-3").Return(s.makeTestImageWithDate("image-3", "img-ver3-", "loc", baseTime.Add(1*time.Minute)), nil).Once()

s.infraMachine.Spec.Disk.Image.Selector.ResolutionPolicy = infrav1.ResolutionPolicyNewest

imageID, err := s.service.lookupImageID(s.ctx, s.machineScope)
s.NoError(err)
s.Equal("image-2", imageID)
}

func (s *imageTestSuite) makeTestImage(id, namePrefix, location string) *sdk.Image {
return makeTestImage(id, namePrefix+*s.capiMachine.Spec.Version, location)
}

func (s *imageTestSuite) makeTestImageWithDate(id, namePrefix, location string, createdDate time.Time) *sdk.Image {
img := s.makeTestImage(id, namePrefix, location)
img.Metadata.CreatedDate = &sdk.IonosTime{Time: createdDate}
return img
}

func TestFilterImagesByName(t *testing.T) {
images := []*sdk.Image{
makeTestImage("image-1", "img-foo-v1.1.qcow2", "test"),
Expand Down Expand Up @@ -194,7 +223,8 @@ func TestLookupImagesBySelector(t *testing.T) {

func makeTestImage(id, name, location string) *sdk.Image {
return &sdk.Image{
Id: &id,
Id: &id,
Metadata: &sdk.DatacenterElementMetadata{},
Properties: &sdk.ImageProperties{
Name: &name,
Location: &location,
Expand Down
2 changes: 2 additions & 0 deletions templates/cluster-template-auto-image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ spec:
disk:
image:
selector:
resolutionPolicy: ${IONOSCLOUD_IMAGE_RESOLUTION_POLICY:-Newest}
matchLabels:
${IONOSCLOUD_IMAGE_LABEL_KEY}: ${IONOSCLOUD_IMAGE_LABEL_VALUE}
---
Expand Down Expand Up @@ -336,6 +337,7 @@ spec:
disk:
image:
selector:
resolutionPolicy: ${IONOSCLOUD_IMAGE_RESOLUTION_POLICY:-Newest}
matchLabels:
${IONOSCLOUD_IMAGE_LABEL_KEY}: ${IONOSCLOUD_IMAGE_LABEL_VALUE}
---
Expand Down
1 change: 1 addition & 0 deletions test/e2e/capic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ var _ = Describe("Should be able to create a cluster with 1 control-plane and 1
E2EConfig: e2eConfig,
ClusterctlConfigPath: clusterctlConfigPath,
BootstrapClusterProxy: bootstrapClusterProxy,
Flavor: "image-selector",
ArtifactFolder: artifactFolder,
SkipCleanup: skipCleanup,
PostNamespaceCreated: cloudEnv.createCredentialsSecretPNC,
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/config/ionoscloud.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@ providers:
- sourcePath: "../../../metadata.yaml"
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template.yaml"
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template-ipam.yaml"
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template-image-selector.yaml"
variables:
# Default variables for the e2e test; those values could be overridden via env variables, thus
# allowing the same e2e config file to be re-used in different Prow jobs e.g. each one with a K8s version permutation.
# The following Kubernetes versions should be the latest versions with already published kindest/node images.
# This avoids building node images in the default case which improves the test duration significantly.
KUBERNETES_VERSION: "v1.29.2"
KUBERNETES_VERSION: "v1.30.6"
CNI: "./data/cni/calico.yaml"
KUBETEST_CONFIGURATION: "./data/kubetest/conformance.yaml"
CLUSTER_NAME: "e2e-cluster-${RANDOM}"
Expand Down
Loading
Loading