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

[improvement] : add new phase to check if we can use cloudinit or not to reduce API calls #522

Merged
merged 2 commits into from
Oct 2, 2024
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
5 changes: 5 additions & 0 deletions api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func Convert_v1alpha2_LinodeMachineSpec_To_v1alpha1_LinodeMachineSpec(in *infras
return autoConvert_v1alpha2_LinodeMachineSpec_To_v1alpha1_LinodeMachineSpec(in, out, s)
}

func Convert_v1alpha2_LinodeMachineStatus_To_v1alpha1_LinodeMachineStatus(in *infrastructurev1alpha2.LinodeMachineStatus, out *LinodeMachineStatus, s conversion.Scope) error {
// Ok to use the auto-generated conversion function
return autoConvert_v1alpha2_LinodeMachineStatus_To_v1alpha1_LinodeMachineStatus(in, out, s)
}

func Convert_v1alpha1_LinodeMachineSpec_To_v1alpha2_LinodeMachineSpec(in *LinodeMachineSpec, out *infrastructurev1alpha2.LinodeMachineSpec, s conversion.Scope) error {
return autoConvert_v1alpha1_LinodeMachineSpec_To_v1alpha2_LinodeMachineSpec(in, out, s)
}
Expand Down
16 changes: 6 additions & 10 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/v1alpha2/linodemachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ type LinodeMachineStatus struct {
// Addresses contains the Linode instance associated addresses.
Addresses []clusterv1.MachineAddress `json:"addresses,omitempty"`

// CloudinitMetadataSupport determines whether to use cloud-init or not.
// +optional
// +kubebuilder:default=true
CloudinitMetadataSupport bool `json:"cloudinitMetadataSupport,omitempty"`
rahulait marked this conversation as resolved.
Show resolved Hide resolved

// InstanceState is the state of the Linode instance for this machine.
// +optional
InstanceState *linodego.InstanceStatus `json:"instanceState,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,11 @@ spec:
- type
type: object
type: array
cloudinitMetadataSupport:
default: true
description: CloudinitMetadataSupport determines whether to use cloud-init
or not.
type: boolean
conditions:
description: Conditions defines current service state of the LinodeMachine.
items:
Expand Down
51 changes: 43 additions & 8 deletions controller/linodemachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"errors"
"fmt"
"net/http"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -58,13 +59,14 @@
defaultDiskFilesystem = string(linodego.FilesystemExt4)

// conditions for preflight instance creation
ConditionPreflightCreated clusterv1.ConditionType = "PreflightCreated"
ConditionPreflightRootDiskResizing clusterv1.ConditionType = "PreflightRootDiskResizing"
ConditionPreflightRootDiskResized clusterv1.ConditionType = "PreflightRootDiskResized"
ConditionPreflightAdditionalDisksCreated clusterv1.ConditionType = "PreflightAdditionalDisksCreated"
ConditionPreflightConfigured clusterv1.ConditionType = "PreflightConfigured"
ConditionPreflightBootTriggered clusterv1.ConditionType = "PreflightBootTriggered"
ConditionPreflightReady clusterv1.ConditionType = "PreflightReady"
ConditionPreflightMetadataSupportConfigured clusterv1.ConditionType = "PreflightMetadataSupportConfigured"
ConditionPreflightCreated clusterv1.ConditionType = "PreflightCreated"
ConditionPreflightRootDiskResizing clusterv1.ConditionType = "PreflightRootDiskResizing"
ConditionPreflightRootDiskResized clusterv1.ConditionType = "PreflightRootDiskResized"
ConditionPreflightAdditionalDisksCreated clusterv1.ConditionType = "PreflightAdditionalDisksCreated"
ConditionPreflightConfigured clusterv1.ConditionType = "PreflightConfigured"
ConditionPreflightBootTriggered clusterv1.ConditionType = "PreflightBootTriggered"
ConditionPreflightReady clusterv1.ConditionType = "PreflightReady"

// WaitingForBootstrapDataReason used when machine is waiting for bootstrap data to be ready before proceeding.
WaitingForBootstrapDataReason = "WaitingForBootstrapData"
Expand Down Expand Up @@ -212,7 +214,7 @@
// Make sure bootstrap data is available and populated.
if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {
logger.Info("Bootstrap data secret is not yet available")
conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightCreated, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "")
conditions.MarkFalse(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured, WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "")

Check warning on line 217 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L217

Added line #L217 was not covered by tests
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -240,6 +242,13 @@
return ctrl.Result{}, err
}

if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured) && machineScope.LinodeMachine.Spec.ProviderID == nil {
res, err := r.reconcilePreflightMetadataSupportConfigure(ctx, logger, machineScope)
if err != nil || !res.IsZero() {
return res, err
}
}

if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightCreated) && machineScope.LinodeMachine.Spec.ProviderID == nil {
res, err := r.reconcilePreflightCreate(ctx, logger, machineScope)
if err != nil || !res.IsZero() {
Expand Down Expand Up @@ -278,6 +287,32 @@
return ctrl.Result{}, nil
}

func (r *LinodeMachineReconciler) reconcilePreflightMetadataSupportConfigure(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
region, err := machineScope.LinodeClient.GetRegion(ctx, machineScope.LinodeMachine.Spec.Region)
if err != nil {
logger.Error(err, fmt.Sprintf("Failed to fetch region %s", machineScope.LinodeMachine.Spec.Region))
return retryIfTransient(err)

Check warning on line 294 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L293-L294

Added lines #L293 - L294 were not covered by tests
}
regionMetadataSupport := slices.Contains(region.Capabilities, linodego.CapabilityMetadata)
imageName := reconciler.DefaultMachineControllerLinodeImage
if machineScope.LinodeMachine.Spec.Image != "" {
imageName = machineScope.LinodeMachine.Spec.Image
}
image, err := machineScope.LinodeClient.GetImage(ctx, imageName)
if err != nil {
logger.Error(err, fmt.Sprintf("Failed to fetch image %s", imageName))
return retryIfTransient(err)
}
imageMetadataSupport := slices.Contains(image.Capabilities, "cloud-init")
machineScope.LinodeMachine.Status.CloudinitMetadataSupport = true
if !imageMetadataSupport || !regionMetadataSupport {
logger.Info("cloud-init metadata support not available", "imageMetadataSupport", imageMetadataSupport, "regionMetadataSupport", regionMetadataSupport)
machineScope.LinodeMachine.Status.CloudinitMetadataSupport = false

Check warning on line 310 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L309-L310

Added lines #L309 - L310 were not covered by tests
}
conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightMetadataSupportConfigured)
return ctrl.Result{}, nil
}

func (r *LinodeMachineReconciler) reconcilePreflightCreate(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
// get the bootstrap data for the Linode instance and set it for create config
createOpts, err := newCreateConfig(ctx, machineScope, logger)
Expand Down
21 changes: 2 additions & 19 deletions controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,29 +444,12 @@ func setUserData(ctx context.Context, machineScope *scope.MachineScope, createCo
return err
}

region, err := machineScope.LinodeClient.GetRegion(ctx, machineScope.LinodeMachine.Spec.Region)
if err != nil {
return fmt.Errorf("get region: %w", err)
}
regionMetadataSupport := slices.Contains(region.Capabilities, "Metadata")
imageName := reconciler.DefaultMachineControllerLinodeImage
if machineScope.LinodeMachine.Spec.Image != "" {
imageName = machineScope.LinodeMachine.Spec.Image
}
image, err := machineScope.LinodeClient.GetImage(ctx, imageName)
if err != nil {
return fmt.Errorf("get image: %w", err)
}
imageMetadataSupport := slices.Contains(image.Capabilities, "cloud-init")
if imageMetadataSupport && regionMetadataSupport {
if machineScope.LinodeMachine.Status.CloudinitMetadataSupport {
createConfig.Metadata = &linodego.InstanceMetadataOptions{
UserData: b64.StdEncoding.EncodeToString(bootstrapData),
}
} else {
logger.Info("using StackScripts for bootstrapping",
"imageMetadataSupport", imageMetadataSupport,
"regionMetadataSupport", regionMetadataSupport,
)
logger.Info("using StackScripts for bootstrapping")
capiStackScriptID, err := services.EnsureStackscript(ctx, machineScope)
if err != nil {
return fmt.Errorf("ensure stackscript: %w", err)
Expand Down
91 changes: 3 additions & 88 deletions controller/linodemachine_controller_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestSetUserData(t *testing.T) {
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"},
Status: infrav1alpha2.LinodeMachineStatus{},
Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: true},
}},
createConfig: &linodego.InstanceCreateOptions{},
wantConfig: &linodego.InstanceCreateOptions{Metadata: &linodego.InstanceMetadataOptions{
Expand All @@ -119,12 +119,6 @@ func TestSetUserData(t *testing.T) {
*obj = cred
return nil
})
mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(&linodego.Region{
Capabilities: []string{"Metadata"},
}, nil)
mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{
Capabilities: []string{"cloud-init"},
}, nil)
},
},
{
Expand All @@ -143,7 +137,7 @@ func TestSetUserData(t *testing.T) {
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-east", Image: "linode/ubuntu22.04", Type: "g6-standard-1"},
Status: infrav1alpha2.LinodeMachineStatus{},
Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: false},
}},
createConfig: &linodego.InstanceCreateOptions{},
wantConfig: &linodego.InstanceCreateOptions{StackScriptID: 1234, StackScriptData: map[string]string{
Expand All @@ -160,10 +154,6 @@ func TestSetUserData(t *testing.T) {
*obj = cred
return nil
})
mockClient.EXPECT().GetRegion(gomock.Any(), "us-east").Return(&linodego.Region{
Capabilities: []string{"Metadata"},
}, nil)
mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{}, nil)
mockClient.EXPECT().ListStackscripts(gomock.Any(), &linodego.ListOptions{Filter: "{\"label\":\"CAPL-dev\"}"}).Return([]linodego.Stackscript{{
Label: "CAPI Test 1",
ID: 1234,
Expand Down Expand Up @@ -228,77 +218,6 @@ func TestSetUserData(t *testing.T) {
},
expectedError: fmt.Errorf("bootstrap data secret is nil for LinodeMachine default/test-cluster"),
},
{
name: "Error - SetUserData failed to get regions",
machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{
Spec: v1beta1.MachineSpec{
ClusterName: "",
Bootstrap: v1beta1.Bootstrap{
DataSecretName: ptr.To("test-data"),
},
InfrastructureRef: corev1.ObjectReference{},
},
}, LinodeMachine: &infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"},
Status: infrav1alpha2.LinodeMachineStatus{},
}},
createConfig: &linodego.InstanceCreateOptions{},
wantConfig: &linodego.InstanceCreateOptions{},
expects: func(mockClient *mock.MockLinodeClient, kMock *mock.MockK8sClient) {
kMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
"value": []byte("hello"),
},
}
*obj = cred
return nil
})
mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(nil, fmt.Errorf("cannot find region"))
},
expectedError: fmt.Errorf("cannot find region"),
},
{
name: "Error - SetUserData failed to get images",
machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{
Spec: v1beta1.MachineSpec{
ClusterName: "",
Bootstrap: v1beta1.Bootstrap{
DataSecretName: ptr.To("test-data"),
},
InfrastructureRef: corev1.ObjectReference{},
},
}, LinodeMachine: &infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-ord", Image: "linode/ubuntu22.04"},
Status: infrav1alpha2.LinodeMachineStatus{},
}},
createConfig: &linodego.InstanceCreateOptions{},
wantConfig: &linodego.InstanceCreateOptions{},
expects: func(mockClient *mock.MockLinodeClient, kMock *mock.MockK8sClient) {
kMock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
"value": []byte("hello"),
},
}
*obj = cred
return nil
})
mockClient.EXPECT().GetRegion(gomock.Any(), "us-ord").Return(&linodego.Region{
Capabilities: []string{"Metadata"},
}, nil)
mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(nil, fmt.Errorf("cannot find image"))
},
expectedError: fmt.Errorf("cannot find image"),
},
{
name: "Error - SetUserData failed to get stackscripts",
machineScope: &scope.MachineScope{Machine: &v1beta1.Machine{
Expand All @@ -315,7 +234,7 @@ func TestSetUserData(t *testing.T) {
Namespace: "default",
},
Spec: infrav1alpha2.LinodeMachineSpec{Region: "us-east", Image: "linode/ubuntu22.04", Type: "g6-standard-1"},
Status: infrav1alpha2.LinodeMachineStatus{},
Status: infrav1alpha2.LinodeMachineStatus{CloudinitMetadataSupport: false},
}},
createConfig: &linodego.InstanceCreateOptions{},
wantConfig: &linodego.InstanceCreateOptions{StackScriptID: 1234, StackScriptData: map[string]string{
Expand All @@ -332,10 +251,6 @@ func TestSetUserData(t *testing.T) {
*obj = cred
return nil
})
mockClient.EXPECT().GetRegion(gomock.Any(), "us-east").Return(&linodego.Region{
Capabilities: []string{"Metadata"},
}, nil)
mockClient.EXPECT().GetImage(gomock.Any(), "linode/ubuntu22.04").Return(&linodego.Image{}, nil)
mockClient.EXPECT().ListStackscripts(gomock.Any(), &linodego.ListOptions{Filter: "{\"label\":\"CAPL-dev\"}"}).Return(nil, fmt.Errorf("failed to get stackscripts"))
},
expectedError: fmt.Errorf("ensure stackscript: failed to get stackscript with label CAPL-dev: failed to get stackscripts"),
Expand Down
Loading
Loading