From a4be7f88ea7e037c9d5bd1f869feb6462579f63f Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Wed, 31 Jan 2024 14:59:01 -0500 Subject: [PATCH 1/4] populate metadata for bootstrap --- cloud/scope/machine.go | 37 ++++++++++++++++++++++++++ config/rbac/role.yaml | 11 ++++++++ controller/linodemachine_controller.go | 35 +++++++++++++++++------- util/reconciler/defaults.go | 2 ++ 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index ef2f9f8fe..81cb31239 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -1,9 +1,14 @@ package scope import ( + "context" + b64 "encoding/base64" "errors" "fmt" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" "github.com/linode/linodego" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -69,3 +74,35 @@ func NewMachineScope(apiKey string, params MachineScopeParams) (*MachineScope, e LinodeMachine: params.LinodeMachine, }, nil } + +// GetBootstrapData returns the bootstrap data from the secret in the Machine's bootstrap.dataSecretName. +func (m *MachineScope) GetBootstrapData(ctx context.Context) (string, error) { + if m.Machine.Spec.Bootstrap.DataSecretName == nil { + return "", fmt.Errorf( + "bootstrap data secret is nil for LinodeMachine %s/%s", + m.LinodeMachine.Namespace, + m.LinodeMachine.Name, + ) + } + + secret := &corev1.Secret{} + key := types.NamespacedName{Namespace: m.LinodeMachine.Namespace, Name: *m.Machine.Spec.Bootstrap.DataSecretName} + if err := m.client.Get(ctx, key, secret); err != nil { + return "", fmt.Errorf( + "failed to retrieve bootstrap data secret for LinodeMachine %s/%s", + m.LinodeMachine.Namespace, + m.LinodeMachine.Name, + ) + } + + value, ok := secret.Data["value"] + if !ok { + return "", fmt.Errorf( + "bootstrap data secret value key is missing for LinodeMachine %s/%s", + m.LinodeMachine.Namespace, + m.LinodeMachine.Name, + ) + } + + return b64.StdEncoding.EncodeToString(value), nil +} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 5f7cf5498..e55764d7d 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -10,8 +10,19 @@ rules: - events verbs: - create + - get + - list - patch - update + - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - cluster.x-k8s.io resources: diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 5a05f0b5b..9e75f37ba 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -81,13 +81,14 @@ type LinodeMachineReconciler struct { ReconcileTimeout time.Duration } -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines/finalizers,verbs=update +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodemachines/finalizers,verbs=update -//+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;watch;list -//+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;watch;list -//+kubebuilder:rbac:groups="",resources=events,verbs=create;update;patch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;watch;list +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;watch;list +// +kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -118,7 +119,6 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil case skippedMachinePhases[machine.Status.Phase]: - log.Info("Machine phase is not the one we are looking for, skipping reconciliation", "phase", machine.Status.Phase) return ctrl.Result{}, nil default: @@ -209,6 +209,9 @@ func (r *LinodeMachineReconciler) reconcile( } }() + // Add the finalizer if not already there + controllerutil.AddFinalizer(machineScope.LinodeMachine, infrav1.GroupVersion.String()) + // Delete if !machineScope.LinodeMachine.ObjectMeta.DeletionTimestamp.IsZero() { failureReason = cerrs.DeleteMachineError @@ -218,8 +221,6 @@ func (r *LinodeMachineReconciler) reconcile( return } - controllerutil.AddFinalizer(machineScope.LinodeMachine, infrav1.GroupVersion.String()) - var linodeInstance *linodego.Instance defer func() { machineScope.LinodeMachine.Status.InstanceState = util.Pointer(linodego.InstanceOffline) @@ -241,7 +242,12 @@ func (r *LinodeMachineReconciler) reconcile( // Create failureReason = cerrs.CreateMachineError + // Make sure bootstrap data is available and populated. + if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil { + logger.Info("Bootstrap data secret is not yet available") + return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForBootstrapDelay}, nil + } linodeInstance, err = r.reconcileCreate(ctx, machineScope, logger) return @@ -281,6 +287,17 @@ func (*LinodeMachineReconciler) reconcileCreate(ctx context.Context, machineScop } createConfig.Tags = tags + // get the bootstrap data for the Linode instance and set it for create config + bootstrapData, err := machineScope.GetBootstrapData(ctx) + if err != nil { + logger.Info("Failed to get bootstrap data", "error", err.Error()) + + return nil, err + } + createConfig.Metadata = &linodego.InstanceMetadataOptions{ + UserData: bootstrapData, + } + if linodeInstance, err = machineScope.LinodeClient.CreateInstance(ctx, *createConfig); err != nil { logger.Info("Failed to create Linode machine instance", "error", err.Error()) diff --git a/util/reconciler/defaults.go b/util/reconciler/defaults.go index 0c789e20a..dd8d3a7a5 100644 --- a/util/reconciler/defaults.go +++ b/util/reconciler/defaults.go @@ -26,6 +26,8 @@ const ( // DefaultMappingTimeout is the default timeout for a controller request mapping func. DefaultMappingTimeout = 60 * time.Second + // DefaultMachineControllerWaitForBootstrapDelay is the default requeue delay if bootstrap data is not ready. + DefaultMachineControllerWaitForBootstrapDelay = 5 * time.Second // DefaultMachineControllerWaitForRunningDelay is the default requeue delay if instance is not running. DefaultMachineControllerWaitForRunningDelay = 5 * time.Second // DefaultMachineControllerWaitForRunningTimeout is the default timeout if instance is not running. From 9bb835ede3c759dc822bbc9cbce8b24d61facad5 Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Wed, 31 Jan 2024 15:20:27 -0500 Subject: [PATCH 2/4] fix recorder event to not spam controller logs for load balancer ready --- controller/linodecluster_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index cf73c8e4a..fe5bf2323 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -143,13 +143,12 @@ func (r *LinodeClusterReconciler) reconcile( if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil { return res, err } + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") } clusterScope.LinodeCluster.Status.Ready = true conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) - r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") - return res, nil } From f7057ff7a7c4c7774fb1f9f835cd80a0563c4613 Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Thu, 1 Feb 2024 10:06:19 -0500 Subject: [PATCH 3/4] address review comments --- cloud/scope/machine.go | 43 ++++++++++++++++++-------- controller/linodemachine_controller.go | 10 ++++-- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 81cb31239..1857400d0 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -2,26 +2,26 @@ package scope import ( "context" - b64 "encoding/base64" "errors" "fmt" + "github.com/linode/linodego" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - - infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" - "github.com/linode/linodego" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" ) type MachineScopeParams struct { Client client.Client Cluster *clusterv1.Cluster Machine *clusterv1.Machine - LinodeCluster *infrav1.LinodeCluster - LinodeMachine *infrav1.LinodeMachine + LinodeCluster *infrav1alpha1.LinodeCluster + LinodeMachine *infrav1alpha1.LinodeMachine } type MachineScope struct { @@ -31,8 +31,8 @@ type MachineScope struct { Cluster *clusterv1.Cluster Machine *clusterv1.Machine LinodeClient *linodego.Client - LinodeCluster *infrav1.LinodeCluster - LinodeMachine *infrav1.LinodeMachine + LinodeCluster *infrav1alpha1.LinodeCluster + LinodeMachine *infrav1alpha1.LinodeMachine } func validateMachineScopeParams(params MachineScopeParams) error { @@ -75,10 +75,27 @@ func NewMachineScope(apiKey string, params MachineScopeParams) (*MachineScope, e }, nil } +// PatchObject persists the machine configuration and status. +func (s *MachineScope) PatchObject(ctx context.Context) error { + return s.PatchHelper.Patch(ctx, s.LinodeMachine) +} + +// Close closes the current scope persisting the machine configuration and status. +func (s *MachineScope) Close(ctx context.Context) error { + return s.PatchObject(ctx) +} + +// AddFinalizer adds a finalizer and immediately patches the object to avoid any race conditions +func (s *MachineScope) AddFinalizer(ctx context.Context) error { + controllerutil.AddFinalizer(s.LinodeMachine, infrav1alpha1.GroupVersion.String()) + + return s.Close(ctx) +} + // GetBootstrapData returns the bootstrap data from the secret in the Machine's bootstrap.dataSecretName. -func (m *MachineScope) GetBootstrapData(ctx context.Context) (string, error) { +func (m *MachineScope) GetBootstrapData(ctx context.Context) ([]byte, error) { if m.Machine.Spec.Bootstrap.DataSecretName == nil { - return "", fmt.Errorf( + return []byte{}, fmt.Errorf( "bootstrap data secret is nil for LinodeMachine %s/%s", m.LinodeMachine.Namespace, m.LinodeMachine.Name, @@ -88,7 +105,7 @@ func (m *MachineScope) GetBootstrapData(ctx context.Context) (string, error) { secret := &corev1.Secret{} key := types.NamespacedName{Namespace: m.LinodeMachine.Namespace, Name: *m.Machine.Spec.Bootstrap.DataSecretName} if err := m.client.Get(ctx, key, secret); err != nil { - return "", fmt.Errorf( + return []byte{}, fmt.Errorf( "failed to retrieve bootstrap data secret for LinodeMachine %s/%s", m.LinodeMachine.Namespace, m.LinodeMachine.Name, @@ -97,12 +114,12 @@ func (m *MachineScope) GetBootstrapData(ctx context.Context) (string, error) { value, ok := secret.Data["value"] if !ok { - return "", fmt.Errorf( + return []byte{}, fmt.Errorf( "bootstrap data secret value key is missing for LinodeMachine %s/%s", m.LinodeMachine.Namespace, m.LinodeMachine.Name, ) } - return b64.StdEncoding.EncodeToString(value), nil + return value, nil } diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 9e75f37ba..7e6fff34e 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + b64 "encoding/base64" "encoding/json" "errors" "fmt" @@ -202,7 +203,8 @@ func (r *LinodeMachineReconciler) reconcile( r.Recorder.Event(machineScope.LinodeMachine, corev1.EventTypeWarning, string(failureReason), err.Error()) } - if patchErr := machineScope.PatchHelper.Patch(ctx, machineScope.LinodeMachine); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { + // Always close the scope when exiting this function so we can persist any LinodeMachine changes. + if patchErr := machineScope.Close(ctx); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { logger.Error(patchErr, "failed to patch LinodeMachine") err = errors.Join(err, patchErr) @@ -210,7 +212,9 @@ func (r *LinodeMachineReconciler) reconcile( }() // Add the finalizer if not already there - controllerutil.AddFinalizer(machineScope.LinodeMachine, infrav1.GroupVersion.String()) + if err := machineScope.AddFinalizer(ctx); err != nil { + return res, err + } // Delete if !machineScope.LinodeMachine.ObjectMeta.DeletionTimestamp.IsZero() { @@ -295,7 +299,7 @@ func (*LinodeMachineReconciler) reconcileCreate(ctx context.Context, machineScop return nil, err } createConfig.Metadata = &linodego.InstanceMetadataOptions{ - UserData: bootstrapData, + UserData: b64.StdEncoding.EncodeToString(bootstrapData), } if linodeInstance, err = machineScope.LinodeClient.CreateInstance(ctx, *createConfig); err != nil { From e00f4c126b89d0fa8bd6ccc94e8285a8dbb44daf Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Thu, 1 Feb 2024 14:06:46 -0500 Subject: [PATCH 4/4] need to have no swap for kubeadm to pass preflight checks --- controller/linodemachine_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 7e6fff34e..d9de87c1b 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -290,6 +290,7 @@ func (*LinodeMachineReconciler) reconcileCreate(ctx context.Context, machineScop return nil, err } createConfig.Tags = tags + createConfig.SwapSize = util.Pointer(0) // get the bootstrap data for the Linode instance and set it for create config bootstrapData, err := machineScope.GetBootstrapData(ctx)