Skip to content
Closed
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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ S5CMD ?= $(CACHE_BIN)/s5cmd

## Tool Versions
KUSTOMIZE_VERSION ?= v5.4.3
CTLPTL_VERSION ?= v0.8.29
CTLPTL_VERSION ?= v0.8.42
CLUSTERCTL_VERSION ?= v1.7.2
CRD_REF_DOCS_VERSION ?= v0.1.0
KUBECTL_VERSION ?= v1.28.0
Expand Down
8 changes: 8 additions & 0 deletions api/v1alpha2/linodemachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ const (

// LinodeMachineSpec defines the desired state of LinodeMachine
type LinodeMachineSpec struct {

// LabelPrefix is the prefix to use for the Linode instance label.
// If not specified, defaults are applied.
// If specified but a Machine doesn't have a owner reference, the prefix is added to the Machine name.
// If specified and a Machine has a owner reference, owner reference name is replaced with the prefix.
// +optional
LabelPrefix string `json:"labelPrefix,omitempty"`

// ProviderID is the unique identifier as specified by the cloud provider.
// +optional
ProviderID *string `json:"providerID,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,13 @@ spec:
x-kubernetes-validations:
- message: Value is immutable
rule: self == oldSelf
labelPrefix:
description: |-
LabelPrefix is the prefix to use for the Linode instance label.
If not specified, defaults are applied.
If specified but a Machine doesn't have a owner reference, the prefix is added to the Machine name.
If specified and a Machine has a owner reference, owner reference name is replaced with the prefix.
type: string
osDisk:
description: |-
OSDisk is configuration for the root disk that includes the OS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,13 @@ spec:
x-kubernetes-validations:
- message: Value is immutable
rule: self == oldSelf
labelPrefix:
description: |-
LabelPrefix is the prefix to use for the Linode instance label.
If not specified, defaults are applied.
If specified but a Machine doesn't have a owner reference, the prefix is added to the Machine name.
If specified and a Machine has a owner reference, owner reference name is replaced with the prefix.
type: string
osDisk:
description: |-
OSDisk is configuration for the root disk that includes the OS,
Expand Down
1 change: 1 addition & 0 deletions docs/src/reference/out.md
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ _Appears in:_

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `labelPrefix` _string_ | LabelPrefix is the prefix to use for the Linode instance label.<br />If not specified, defaults are applied.<br />If specified but a Machine doesn't have a owner reference, the prefix is added to the Machine name.<br />If specified and a Machine has a owner reference, owner reference name is replaced with the prefix. | | |
| `providerID` _string_ | ProviderID is the unique identifier as specified by the cloud provider. | | |
| `instanceID` _integer_ | InstanceID is the Linode instance ID for this machine. | | |
| `region` _string_ | | | Required: \{\} <br /> |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ metadata:
name: lmt-e2e
labels:
all:
linodemachine:
linodemachinetemplate:
spec:
bindings:
# A short identifier for the E2E test run
Expand Down Expand Up @@ -76,4 +76,4 @@ spec:
catch:
- describe:
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: LinodeMachine
kind: LinodeMachine
3 changes: 1 addition & 2 deletions internal/controller/linodemachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"net/http"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -747,7 +746,7 @@ func (r *LinodeMachineReconciler) reconcileUpdate(ctx context.Context, logger lo

// update the tags if needed
machineTags := getTags(machineScope, linodeInstance.Tags)
if !slices.Equal(machineTags, linodeInstance.Tags) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason slices.Equal won't work in this case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slices.Equal only works if the order is the same - https://cs.opensource.google/go/go/+/refs/tags/go1.24.5:src/slices/slices.go;l=20
we'd either have to sort both these lists and then do slices.Equal or take this approach

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

machineTags is created from linodeInstance.Tags. linodeInstance.Tags is initially converted to a map. After performing a few operations, it gets converted back to slice. Practically, the order is not changed but theoretically the map is not an orderedMap. So, there is no guarantee for the order.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would prefer we sort the lists rather than write our own equality logic

if !areSlicesEqual(machineTags, linodeInstance.Tags) {
_, err = machineScope.LinodeClient.UpdateInstance(ctx, instanceID, linodego.InstanceUpdateOptions{Tags: &machineTags})
if err != nil {
logger.Error(err, "Failed to update tags for Linode instance")
Expand Down
49 changes: 47 additions & 2 deletions internal/controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func fillCreateConfig(createConfig *linodego.InstanceCreateOptions, machineScope
func newCreateConfig(ctx context.Context, machineScope *scope.MachineScope, gzipCompressionEnabled bool, logger logr.Logger) (*linodego.InstanceCreateOptions, error) {
var err error

createConfig := linodeMachineSpecToInstanceCreateConfig(machineScope.LinodeMachine.Spec, getTags(machineScope, []string{}))
createConfig := linodeMachineSpecToInstanceCreateConfig(machineScope, getTags(machineScope, []string{}))
if createConfig == nil {
err = errors.New("failed to convert machine spec to create instance config")
logger.Error(err, "Panic! Struct of LinodeMachineSpec is different than InstanceCreateOptions")
Expand Down Expand Up @@ -552,7 +552,8 @@ func getVPCInterfaceConfigFromDirectID(ctx context.Context, machineScope *scope.
}, nil
}

func linodeMachineSpecToInstanceCreateConfig(machineSpec infrav1alpha2.LinodeMachineSpec, machineTags []string) *linodego.InstanceCreateOptions {
func linodeMachineSpecToInstanceCreateConfig(machineScope *scope.MachineScope, machineTags []string) *linodego.InstanceCreateOptions {
machineSpec := machineScope.LinodeMachine.Spec
interfaces := make([]linodego.InstanceConfigInterfaceCreateOptions, len(machineSpec.Interfaces))
for idx, iface := range machineSpec.Interfaces {
interfaces[idx] = linodego.InstanceConfigInterfaceCreateOptions{
Expand All @@ -569,6 +570,7 @@ func linodeMachineSpecToInstanceCreateConfig(machineSpec infrav1alpha2.LinodeMac
privateIP = *machineSpec.PrivateIP
}
return &linodego.InstanceCreateOptions{
Label: getDesiredLinodeInstanceLabel(machineScope),
Region: machineSpec.Region,
Type: machineSpec.Type,
AuthorizedKeys: machineSpec.AuthorizedKeys,
Expand Down Expand Up @@ -1021,3 +1023,46 @@ func getTags(machineScope *scope.MachineScope, instanceTags []string) []string {
machineScope.LinodeMachine.Status.Tags = slices.Clone(machineScope.LinodeMachine.Spec.Tags)
return outTags
}

func areSlicesEqual(a, b []string) bool {
if len(a) != len(b) {
return false
}
aSet := constructSet(a)
bSet := constructSet(b)
if len(aSet) != len(bSet) {
return false
}
for key := range aSet {
if _, ok := bSet[key]; !ok {
return false
}
}
return true
}

func getDesiredLinodeInstanceLabel(machineScope *scope.MachineScope) string {
// If no label prefix is specified, use the machine name as the label
if machineScope.LinodeMachine.Spec.LabelPrefix == "" {
return machineScope.LinodeMachine.Name
}

// if machine is created by a deployment / control-plane, it's name will be prefixed with the label of linode.
machineOwners := machineScope.Machine.GetOwnerReferences()

// get the longest prefix match from machine owner names.
longestPrefix := ""
for _, owner := range machineOwners {
if strings.HasPrefix(machineScope.LinodeMachine.Name, owner.Name) && len(owner.Name) > len(longestPrefix) {
longestPrefix = owner.Name
}
}

// If no owner name matches the prefix, use the machine name as the label
if longestPrefix == "" {
// If no owner name matches the prefix, use the label prefix
return machineScope.LinodeMachine.Spec.LabelPrefix + "-" + machineScope.LinodeMachine.Name
} else {
return strings.Replace(machineScope.LinodeMachine.Name, longestPrefix, machineScope.LinodeMachine.Spec.LabelPrefix, 1)
}
}
88 changes: 87 additions & 1 deletion internal/controller/linodemachine_controller_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestLinodeMachineSpecToCreateInstanceConfig(t *testing.T) {
RootPass: "rootPass",
AuthorizedKeys: []string{"key"},
AuthorizedUsers: []string{"user"},
LabelPrefix: "test-label-prefix",
BackupID: 1,
Image: "image",
Interfaces: []infrav1alpha2.InstanceConfigInterfaceCreateOptions{
Expand All @@ -64,7 +65,12 @@ func TestLinodeMachineSpecToCreateInstanceConfig(t *testing.T) {
PrivateIP: util.Pointer(true),
}

createConfig := linodeMachineSpecToInstanceCreateConfig(machineSpec, []string{"tag"})
createConfig := linodeMachineSpecToInstanceCreateConfig(&scope.MachineScope{
LinodeMachine: &infrav1alpha2.LinodeMachine{
Spec: machineSpec,
},
Machine: &v1beta1.Machine{},
}, []string{"tag"})
assert.NotNil(t, createConfig, "Failed to convert LinodeMachineSpec to InstanceCreateOptions")
}

Expand Down Expand Up @@ -1301,3 +1307,83 @@ func TestGetTags(t *testing.T) {
})
}
}

func TestGetDesiredLinodeInstanceLabel(t *testing.T) {
t.Parallel()

// Setup test cases
testCases := []struct {
name string
machineScope *scope.MachineScope
expectedLabel string
}{
{
name: "Success - Default label",
machineScope: &scope.MachineScope{
LinodeMachine: &infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "test-machine",
Namespace: "default",
},
},
Machine: &v1beta1.Machine{},
},
expectedLabel: "test-machine",
},
{
name: "Success - Custom label prefix",
machineScope: &scope.MachineScope{
LinodeMachine: &infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "test-machine",
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{
LabelPrefix: "custom-prefix",
},
},
Machine: &v1beta1.Machine{},
},
expectedLabel: "custom-prefix-test-machine",
},
{
name: "Success - Custom label prefix with owner reference",
machineScope: &scope.MachineScope{
LinodeMachine: &infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "test-machine",
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{
LabelPrefix: "custom-prefix",
},
},
Machine: &v1beta1.Machine{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
Kind: "MachineDeployment",
Name: "te",
},
{
Kind: "MachineSet",
Name: "test",
},
},
},
},
},
expectedLabel: "custom-prefix-machine",
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

label := getDesiredLinodeInstanceLabel(tc.machineScope)
require.Equal(t, tc.expectedLabel, label)
})
}
}
Loading