Skip to content

Commit a927ab8

Browse files
authored
✨ Add image resolution policy (#237)
**What is the purpose of this pull request/Why do we need it?** Users might want to update VM images without changing the Kubernetes version. When using an image selector this currently requires removing labels from the old image and adding them to the new one. I wanted to provide a better solution for this. **Issue #, if available:** **Description of changes:** Add a new field `resolutionPolicy` to the `imageSelector`. Currently only two values are allowed `Exact` and `Newest`. `Exact` maps to the current behavior and is the default. `Newest` will use the newest entry (determined by `createdDate`) from the set of matching images. **Special notes for your reviewer:** **Checklist:** - [ ] Documentation updated - [x] Unit Tests added - [x] E2E Tests added - [x] Includes [emojis](https://github.com/kubernetes-sigs/kubebuilder-release-tools?tab=readme-ov-file#kubebuilder-project-versioning)
1 parent 8f9ffc6 commit a927ab8

File tree

11 files changed

+547
-6
lines changed

11 files changed

+547
-6
lines changed

api/v1alpha1/ionoscloudmachine_types.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,13 +240,32 @@ type ImageSpec struct {
240240
Selector *ImageSelector `json:"selector,omitempty"`
241241
}
242242

243+
// ImageSelectorResolutionPolicy defines what action to take when the image selector has ambiguous results.
244+
// +kubebuilder:validation:Enum=Exact;Newest
245+
type ImageSelectorResolutionPolicy string
246+
247+
const (
248+
// ResolutionPolicyExact only succeeds if the image selector resolves to exactly 1 image.
249+
ResolutionPolicyExact ImageSelectorResolutionPolicy = "Exact"
250+
// ResolutionPolicyNewest uses the newest entry if the image selector resolves to more than 1 image.
251+
ResolutionPolicyNewest ImageSelectorResolutionPolicy = "Newest"
252+
)
253+
243254
// ImageSelector defines label selectors for looking up images.
244255
type ImageSelector struct {
245256
// MatchLabels is a map of key/value pairs.
246257
//
247258
//+kubebuilder:validation:MinProperties=1
248259
MatchLabels map[string]string `json:"matchLabels"`
249260

261+
// ResolutionPolicy controls the lookup behavior.
262+
// The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
263+
// Use policy 'Newest' to select the newest image instead.
264+
//
265+
//+kubebuilder:default=`Exact`
266+
//+optional
267+
ResolutionPolicy ImageSelectorResolutionPolicy `json:"resolutionPolicy,omitempty"`
268+
250269
// UseMachineVersion indicates whether to use the parent Machine's version field to look up image names.
251270
// Enabled by default.
252271
//

config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachines.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,16 @@ spec:
213213
description: MatchLabels is a map of key/value pairs.
214214
minProperties: 1
215215
type: object
216+
resolutionPolicy:
217+
default: Exact
218+
description: |-
219+
ResolutionPolicy controls the lookup behavior.
220+
The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
221+
Use policy 'Newest' to select the newest image instead.
222+
enum:
223+
- Exact
224+
- Newest
225+
type: string
216226
useMachineVersion:
217227
default: true
218228
description: |-

config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudmachinetemplates.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@ spec:
234234
pairs.
235235
minProperties: 1
236236
type: object
237+
resolutionPolicy:
238+
default: Exact
239+
description: |-
240+
ResolutionPolicy controls the lookup behavior.
241+
The default policy 'Exact' will raise an error if the selector resolves to more than 1 image.
242+
Use policy 'Newest' to select the newest image instead.
243+
enum:
244+
- Exact
245+
- Newest
246+
type: string
237247
useMachineVersion:
238248
default: true
239249
description: |-

internal/service/cloud/image.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"slices"
2324
"strings"
2425

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

73-
if n := len(images); n != 1 {
74-
return "", imageMatchError{imageIDs: getImageIDs(images), selector: imageSpec.Selector}
74+
if len(images) == 0 {
75+
return "", imageMatchError{selector: imageSpec.Selector}
76+
}
77+
78+
switch imageSpec.Selector.ResolutionPolicy {
79+
case infrav1.ResolutionPolicyExact:
80+
if len(images) > 1 {
81+
return "", imageMatchError{imageIDs: getImageIDs(images), selector: imageSpec.Selector}
82+
}
83+
case infrav1.ResolutionPolicyNewest:
84+
slices.SortFunc(images, func(lhs, rhs *sdk.Image) int {
85+
// swap lhs and rhs to produce reverse order
86+
return rhs.Metadata.CreatedDate.Compare(lhs.Metadata.CreatedDate.Time)
87+
})
7588
}
7689

7790
return ptr.Deref(images[0].GetId(), ""), nil

internal/service/cloud/image_test.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"slices"
2323
"testing"
24+
"time"
2425

2526
"github.com/go-logr/logr"
2627
sdk "github.com/ionos-cloud/sdk-go/v6"
@@ -47,6 +48,7 @@ func (s *imageTestSuite) SetupTest() {
4748
MatchLabels: map[string]string{
4849
"test": "image",
4950
},
51+
ResolutionPolicy: infrav1.ResolutionPolicyExact,
5052
}
5153
}
5254

@@ -121,7 +123,7 @@ func (s *imageTestSuite) TestLookupImageIgnoreMissingMachineVersion() {
121123
s.Equal("image-1", imageID)
122124
}
123125

124-
func (s *imageTestSuite) TestLookupImageOK() {
126+
func (s *imageTestSuite) TestLookupImageExactOK() {
125127
s.ionosClient.EXPECT().ListLabels(s.ctx).Return(
126128
[]sdk.Label{
127129
makeTestLabel("image", "image-1", "test", "image"),
@@ -135,10 +137,37 @@ func (s *imageTestSuite) TestLookupImageOK() {
135137
s.Equal("image-1", imageID)
136138
}
137139

140+
func (s *imageTestSuite) TestLookupImageNewestOK() {
141+
s.ionosClient.EXPECT().ListLabels(s.ctx).Return(
142+
[]sdk.Label{
143+
makeTestLabel("image", "image-1", "test", "image"),
144+
makeTestLabel("image", "image-2", "test", "image"),
145+
makeTestLabel("image", "image-3", "test", "image"),
146+
}, nil,
147+
).Once()
148+
s.ionosClient.EXPECT().GetDatacenterLocationByID(s.ctx, s.infraMachine.Spec.DatacenterID).Return("loc", nil).Once()
149+
baseTime := time.Now().Round(time.Second)
150+
s.ionosClient.EXPECT().GetImage(s.ctx, "image-1").Return(s.makeTestImageWithDate("image-1", "img-ver1-", "loc", baseTime), nil).Once()
151+
s.ionosClient.EXPECT().GetImage(s.ctx, "image-2").Return(s.makeTestImageWithDate("image-2", "img-ver2-", "loc", baseTime.Add(2*time.Minute)), nil).Once()
152+
s.ionosClient.EXPECT().GetImage(s.ctx, "image-3").Return(s.makeTestImageWithDate("image-3", "img-ver3-", "loc", baseTime.Add(1*time.Minute)), nil).Once()
153+
154+
s.infraMachine.Spec.Disk.Image.Selector.ResolutionPolicy = infrav1.ResolutionPolicyNewest
155+
156+
imageID, err := s.service.lookupImageID(s.ctx, s.machineScope)
157+
s.NoError(err)
158+
s.Equal("image-2", imageID)
159+
}
160+
138161
func (s *imageTestSuite) makeTestImage(id, namePrefix, location string) *sdk.Image {
139162
return makeTestImage(id, namePrefix+*s.capiMachine.Spec.Version, location)
140163
}
141164

165+
func (s *imageTestSuite) makeTestImageWithDate(id, namePrefix, location string, createdDate time.Time) *sdk.Image {
166+
img := s.makeTestImage(id, namePrefix, location)
167+
img.Metadata.CreatedDate = &sdk.IonosTime{Time: createdDate}
168+
return img
169+
}
170+
142171
func TestFilterImagesByName(t *testing.T) {
143172
images := []*sdk.Image{
144173
makeTestImage("image-1", "img-foo-v1.1.qcow2", "test"),
@@ -194,7 +223,8 @@ func TestLookupImagesBySelector(t *testing.T) {
194223

195224
func makeTestImage(id, name, location string) *sdk.Image {
196225
return &sdk.Image{
197-
Id: &id,
226+
Id: &id,
227+
Metadata: &sdk.DatacenterElementMetadata{},
198228
Properties: &sdk.ImageProperties{
199229
Name: &name,
200230
Location: &location,

templates/cluster-template-auto-image.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ spec:
291291
disk:
292292
image:
293293
selector:
294+
resolutionPolicy: ${IONOSCLOUD_IMAGE_RESOLUTION_POLICY:-Newest}
294295
matchLabels:
295296
${IONOSCLOUD_IMAGE_LABEL_KEY}: ${IONOSCLOUD_IMAGE_LABEL_VALUE}
296297
---
@@ -336,6 +337,7 @@ spec:
336337
disk:
337338
image:
338339
selector:
340+
resolutionPolicy: ${IONOSCLOUD_IMAGE_RESOLUTION_POLICY:-Newest}
339341
matchLabels:
340342
${IONOSCLOUD_IMAGE_LABEL_KEY}: ${IONOSCLOUD_IMAGE_LABEL_VALUE}
341343
---

test/e2e/capic_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ var _ = Describe("Should be able to create a cluster with 1 control-plane and 1
9292
E2EConfig: e2eConfig,
9393
ClusterctlConfigPath: clusterctlConfigPath,
9494
BootstrapClusterProxy: bootstrapClusterProxy,
95+
Flavor: "image-selector",
9596
ArtifactFolder: artifactFolder,
9697
SkipCleanup: skipCleanup,
9798
PostNamespaceCreated: cloudEnv.createCredentialsSecretPNC,

test/e2e/config/ionoscloud.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,13 @@ providers:
6565
- sourcePath: "../../../metadata.yaml"
6666
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template.yaml"
6767
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template-ipam.yaml"
68+
- sourcePath: "../data/infrastructure-ionoscloud/cluster-template-image-selector.yaml"
6869
variables:
6970
# Default variables for the e2e test; those values could be overridden via env variables, thus
7071
# allowing the same e2e config file to be re-used in different Prow jobs e.g. each one with a K8s version permutation.
7172
# The following Kubernetes versions should be the latest versions with already published kindest/node images.
7273
# This avoids building node images in the default case which improves the test duration significantly.
73-
KUBERNETES_VERSION: "v1.29.2"
74+
KUBERNETES_VERSION: "v1.30.6"
7475
CNI: "./data/cni/calico.yaml"
7576
KUBETEST_CONFIGURATION: "./data/kubetest/conformance.yaml"
7677
CLUSTER_NAME: "e2e-cluster-${RANDOM}"

0 commit comments

Comments
 (0)