Skip to content

Commit

Permalink
✨ Add image resolution policy (#237)
Browse files Browse the repository at this point in the history
**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)
  • Loading branch information
avorima authored Jan 10, 2025
1 parent 8f9ffc6 commit a927ab8
Show file tree
Hide file tree
Showing 11 changed files with 547 additions and 6 deletions.
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`
//+optional
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}
}

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

0 comments on commit a927ab8

Please sign in to comment.