Skip to content

Commit b8911f0

Browse files
AmitSahastraAmitSahastra
committed
code refactor
1 parent d12643c commit b8911f0

File tree

2 files changed

+190
-192
lines changed

2 files changed

+190
-192
lines changed

‎bootstrap/eks/controllers/eksconfig_controller.go‎

Lines changed: 95 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func (r *EKSConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
148148
}
149149
}()
150150

151-
return ctrl.Result{}, r.joinWorker(ctx, cluster, config, configOwner)
151+
return r.joinWorker(ctx, cluster, config, configOwner)
152152
}
153153

154154
func (r *EKSConfigReconciler) resolveFiles(ctx context.Context, cfg *eksbootstrapv1.EKSConfig) ([]eksbootstrapv1.File, error) {
@@ -186,7 +186,7 @@ func (r *EKSConfigReconciler) resolveSecretFileContent(ctx context.Context, ns s
186186
return data, nil
187187
}
188188

189-
func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig, configOwner *bsutil.ConfigOwner) error {
189+
func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1.Cluster, config *eksbootstrapv1.EKSConfig, configOwner *bsutil.ConfigOwner) (ctrl.Result, error) {
190190
log := logger.FromContext(ctx)
191191

192192
// only need to reconcile the secret for Machine kinds once, but MachinePools need updates for new launch templates
@@ -200,15 +200,15 @@ func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1
200200
err := r.Client.Get(ctx, secretKey, existingSecret)
201201
switch {
202202
case err == nil:
203-
return nil
203+
return ctrl.Result{}, nil
204204
case !apierrors.IsNotFound(err):
205205
log.Error(err, "unable to check for existing bootstrap secret")
206-
return err
206+
return ctrl.Result{}, err
207207
}
208208
}
209209

210210
if cluster.Spec.ControlPlaneRef == nil || cluster.Spec.ControlPlaneRef.Kind != "AWSManagedControlPlane" {
211-
return errors.New("Cluster's controlPlaneRef needs to be an AWSManagedControlPlane in order to use the EKS bootstrap provider")
211+
return ctrl.Result{}, errors.New("Cluster's controlPlaneRef needs to be an AWSManagedControlPlane in order to use the EKS bootstrap provider")
212212
}
213213

214214
if !cluster.Status.InfrastructureReady {
@@ -217,19 +217,19 @@ func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1
217217
eksbootstrapv1.DataSecretAvailableCondition,
218218
eksbootstrapv1.WaitingForClusterInfrastructureReason,
219219
clusterv1.ConditionSeverityInfo, "")
220-
return nil
220+
return ctrl.Result{}, nil
221221
}
222222

223223
if !conditions.IsTrue(cluster, clusterv1.ControlPlaneInitializedCondition) {
224224
log.Info("Control Plane has not yet been initialized")
225225
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition, eksbootstrapv1.WaitingForControlPlaneInitializationReason, clusterv1.ConditionSeverityInfo, "")
226-
return nil
226+
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
227227
}
228228

229229
// Get the AWSManagedControlPlane
230230
controlPlane := &ekscontrolplanev1.AWSManagedControlPlane{}
231231
if err := r.Get(ctx, client.ObjectKey{Name: cluster.Spec.ControlPlaneRef.Name, Namespace: cluster.Spec.ControlPlaneRef.Namespace}, controlPlane); err != nil {
232-
return errors.Wrap(err, "failed to get control plane")
232+
return ctrl.Result{}, errors.Wrap(err, "failed to get control plane")
233233
}
234234

235235
// Check if control plane is ready
@@ -238,37 +238,84 @@ func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1
238238
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition,
239239
eksbootstrapv1.DataSecretGenerationFailedReason,
240240
clusterv1.ConditionSeverityInfo, "Control plane is not ready yet")
241-
return nil
241+
return ctrl.Result{}, nil
242242
}
243243

244244
log.Info("Generating userdata")
245245
files, err := r.resolveFiles(ctx, config)
246246
if err != nil {
247247
log.Info("Failed to resolve files for user data")
248248
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition, eksbootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, "%s", err.Error())
249-
return err
249+
return ctrl.Result{}, err
250+
}
251+
252+
// Create unified NodeInput for both AL2 and AL2023
253+
nodeInput := &userdata.NodeInput{
254+
// Common fields
255+
ClusterName: controlPlane.Spec.EKSClusterName,
256+
KubeletExtraArgs: config.Spec.KubeletExtraArgs,
257+
ContainerRuntime: config.Spec.ContainerRuntime,
258+
DNSClusterIP: config.Spec.DNSClusterIP,
259+
DockerConfigJSON: config.Spec.DockerConfigJSON,
260+
APIRetryAttempts: config.Spec.APIRetryAttempts,
261+
UseMaxPods: config.Spec.UseMaxPods,
262+
PreBootstrapCommands: config.Spec.PreBootstrapCommands,
263+
PostBootstrapCommands: config.Spec.PostBootstrapCommands,
264+
BootstrapCommandOverride: config.Spec.BootstrapCommandOverride,
265+
NTP: config.Spec.NTP,
266+
Users: config.Spec.Users,
267+
DiskSetup: config.Spec.DiskSetup,
268+
Mounts: config.Spec.Mounts,
269+
Files: files,
270+
}
271+
272+
// Set default UseMaxPods if not specified
273+
if nodeInput.UseMaxPods == nil {
274+
defaultUseMaxPods := false
275+
nodeInput.UseMaxPods = &defaultUseMaxPods
276+
}
277+
278+
log.Info("NodeInput created",
279+
"dnsClusterIP", config.Spec.DNSClusterIP,
280+
"useMaxPods", config.Spec.UseMaxPods,
281+
"nodeType", config.Spec.NodeType)
282+
283+
if config.Spec.PauseContainer != nil {
284+
nodeInput.PauseContainerAccount = &config.Spec.PauseContainer.AccountNumber
285+
nodeInput.PauseContainerVersion = &config.Spec.PauseContainer.Version
286+
}
287+
288+
// Check if IPv6 was provided to the user configuration first
289+
// If not, we also check if the cluster is ipv6 based.
290+
if config.Spec.ServiceIPV6Cidr != nil && *config.Spec.ServiceIPV6Cidr != "" {
291+
nodeInput.ServiceIPV6Cidr = config.Spec.ServiceIPV6Cidr
292+
nodeInput.IPFamily = ptr.To[string]("ipv6")
250293
}
251294

252-
// Generate userdata based on node type
253-
var userDataScript []byte
295+
// we don't want to override any manually set configuration options.
296+
if config.Spec.ServiceIPV6Cidr == nil && controlPlane.Spec.NetworkSpec.VPC.IsIPv6Enabled() {
297+
log.Info("Adding ipv6 data to userdata....")
298+
nodeInput.ServiceIPV6Cidr = ptr.To[string](controlPlane.Spec.NetworkSpec.VPC.IPv6.CidrBlock)
299+
nodeInput.IPFamily = ptr.To[string]("ipv6")
300+
}
254301

302+
// Set AMI family type and AL2023-specific fields if needed
255303
if config.Spec.NodeType == "al2023" {
256-
// Use the ControlPlaneEndpoint from the AWSManagedControlPlane spec
257-
apiServerEndpoint := controlPlane.Spec.ControlPlaneEndpoint.Host
304+
nodeInput.AMIFamilyType = userdata.AMIFamilyAL2023
258305

259-
log.Info("Generating AL2023 userdata",
260-
"cluster", controlPlane.Spec.EKSClusterName,
261-
"endpoint", apiServerEndpoint)
306+
// Set AL2023-specific fields
307+
nodeInput.APIServerEndpoint = controlPlane.Spec.ControlPlaneEndpoint.Host
308+
nodeInput.NodeGroupName = config.Name
262309

263-
// Fetch CA cert directly from EKS API
310+
// Fetch CA cert from EKS API
264311
sess, err := session.NewSession(&aws.Config{Region: aws.String(controlPlane.Spec.Region)})
265312
if err != nil {
266313
log.Error(err, "Failed to create AWS session for EKS API")
267314
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition,
268315
eksbootstrapv1.DataSecretGenerationFailedReason,
269316
clusterv1.ConditionSeverityWarning,
270317
"Failed to create AWS session: %v", err)
271-
return err
318+
return ctrl.Result{}, err
272319
}
273320
eksClient := eks.New(sess)
274321
describeInput := &eks.DescribeClusterInput{Name: aws.String(controlPlane.Spec.EKSClusterName)}
@@ -279,142 +326,67 @@ func (r *EKSConfigReconciler) joinWorker(ctx context.Context, cluster *clusterv1
279326
eksbootstrapv1.DataSecretGenerationFailedReason,
280327
clusterv1.ConditionSeverityWarning,
281328
"Failed to describe EKS cluster: %v", err)
282-
return err
329+
return ctrl.Result{}, err
283330
}
284331

285-
caCert := ""
286332
if clusterOut.Cluster != nil && clusterOut.Cluster.CertificateAuthority != nil && clusterOut.Cluster.CertificateAuthority.Data != nil {
287-
caCert = *clusterOut.Cluster.CertificateAuthority.Data
333+
nodeInput.CACert = *clusterOut.Cluster.CertificateAuthority.Data
288334
} else {
289335
log.Error(nil, "CA certificate not found in EKS cluster response")
290336
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition,
291337
eksbootstrapv1.DataSecretGenerationFailedReason,
292338
clusterv1.ConditionSeverityWarning,
293339
"CA certificate not found in EKS cluster response")
294-
return fmt.Errorf("CA certificate not found in EKS cluster response")
340+
return ctrl.Result{}, fmt.Errorf("CA certificate not found in EKS cluster response")
295341
}
296342

297343
// Get AMI ID from AWSManagedMachinePool's launch template if specified
298-
var amiID string
299344
if configOwner.GetKind() == "MachinePool" {
300345
amp := &expinfrav1.AWSManagedMachinePool{}
301346
if err := r.Get(ctx, client.ObjectKey{Namespace: config.Namespace, Name: configOwner.GetName()}, amp); err == nil {
347+
log.Info("Found AWSManagedMachinePool", "name", amp.Name, "launchTemplate", amp.Spec.AWSLaunchTemplate != nil)
302348
if amp.Spec.AWSLaunchTemplate != nil && amp.Spec.AWSLaunchTemplate.AMI.ID != nil {
303-
amiID = *amp.Spec.AWSLaunchTemplate.AMI.ID
349+
nodeInput.AMIImageID = *amp.Spec.AWSLaunchTemplate.AMI.ID
350+
log.Info("Set AMI ID from launch template", "amiID", nodeInput.AMIImageID)
351+
} else {
352+
log.Info("No AMI ID found in launch template")
304353
}
354+
if amp.Spec.CapacityType != nil {
355+
nodeInput.CapacityType = amp.Spec.CapacityType
356+
log.Info("Set capacity type from AWSManagedMachinePool", "capacityType", *amp.Spec.CapacityType)
357+
} else {
358+
log.Info("No capacity type found in AWSManagedMachinePool")
359+
}
360+
} else {
361+
log.Info("Failed to get AWSManagedMachinePool", "error", err)
305362
}
306363
}
307364

308-
input := &userdata.AL2023UserDataInput{
309-
ClusterName: controlPlane.Spec.EKSClusterName,
310-
APIServerEndpoint: apiServerEndpoint,
311-
CACert: caCert,
312-
NodeGroupName: config.Name, // Use the config name as nodegroup name
313-
MaxPods: getMaxPods(config), // Get from config or use default
314-
ClusterDNS: getClusterDNS(config), // Get from config or use default
315-
AMIImageID: amiID, // Use launch template AMI if specified
316-
CapacityType: getCapacityType(config), // Get from config or use default
317-
}
318-
319-
// Try to generate userdata with retries
320-
var userDataErr error
321-
for i := 0; i < 3; i++ { // Retry up to 3 times
322-
userDataScript, userDataErr = userdata.GenerateAL2023UserData(input)
323-
if userDataErr == nil {
324-
break
325-
}
326-
log.Error(userDataErr, "Failed to generate AL2023 userdata, retrying",
327-
"attempt", i+1,
328-
"cluster", input.ClusterName)
329-
time.Sleep(time.Second * time.Duration(i+1)) // Exponential backoff
330-
}
331-
332-
if userDataErr != nil {
333-
log.Error(userDataErr, "Failed to generate AL2023 userdata after retries")
334-
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition,
335-
eksbootstrapv1.DataSecretGenerationFailedReason,
336-
clusterv1.ConditionSeverityWarning,
337-
"Failed to generate AL2023 userdata: %v", userDataErr)
338-
return userDataErr
339-
}
365+
log.Info("Generating AL2023 userdata",
366+
"cluster", controlPlane.Spec.EKSClusterName,
367+
"endpoint", nodeInput.APIServerEndpoint)
340368
} else {
369+
nodeInput.AMIFamilyType = userdata.AMIFamilyAL2
341370
log.Info("Generating standard userdata for node type", "type", config.Spec.NodeType)
342-
nodeInput := &userdata.NodeInput{
343-
// AWSManagedControlPlane webhooks default and validate EKSClusterName
344-
ClusterName: controlPlane.Spec.EKSClusterName,
345-
KubeletExtraArgs: config.Spec.KubeletExtraArgs,
346-
ContainerRuntime: config.Spec.ContainerRuntime,
347-
DNSClusterIP: config.Spec.DNSClusterIP,
348-
DockerConfigJSON: config.Spec.DockerConfigJSON,
349-
APIRetryAttempts: config.Spec.APIRetryAttempts,
350-
UseMaxPods: config.Spec.UseMaxPods,
351-
PreBootstrapCommands: config.Spec.PreBootstrapCommands,
352-
PostBootstrapCommands: config.Spec.PostBootstrapCommands,
353-
BootstrapCommandOverride: config.Spec.BootstrapCommandOverride,
354-
NTP: config.Spec.NTP,
355-
Users: config.Spec.Users,
356-
DiskSetup: config.Spec.DiskSetup,
357-
Mounts: config.Spec.Mounts,
358-
Files: files,
359-
}
360-
361-
if config.Spec.PauseContainer != nil {
362-
nodeInput.PauseContainerAccount = &config.Spec.PauseContainer.AccountNumber
363-
nodeInput.PauseContainerVersion = &config.Spec.PauseContainer.Version
364-
}
365-
366-
// Check if IPv6 was provided to the user configuration first
367-
// If not, we also check if the cluster is ipv6 based.
368-
if config.Spec.ServiceIPV6Cidr != nil && *config.Spec.ServiceIPV6Cidr != "" {
369-
nodeInput.ServiceIPV6Cidr = config.Spec.ServiceIPV6Cidr
370-
nodeInput.IPFamily = ptr.To[string]("ipv6")
371-
}
372-
373-
// we don't want to override any manually set configuration options.
374-
if config.Spec.ServiceIPV6Cidr == nil && controlPlane.Spec.NetworkSpec.VPC.IsIPv6Enabled() {
375-
log.Info("Adding ipv6 data to userdata....")
376-
nodeInput.ServiceIPV6Cidr = ptr.To[string](controlPlane.Spec.NetworkSpec.VPC.IPv6.CidrBlock)
377-
nodeInput.IPFamily = ptr.To[string]("ipv6")
378-
}
371+
}
379372

380-
userDataScript, err = userdata.NewNode(nodeInput)
381-
if err != nil {
382-
log.Error(err, "Failed to create a worker join configuration")
383-
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition, eksbootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, "")
384-
return err
385-
}
373+
// Generate userdata using unified approach
374+
userDataScript, err := userdata.NewNode(nodeInput)
375+
if err != nil {
376+
log.Error(err, "Failed to create a worker join configuration")
377+
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition, eksbootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, "")
378+
return ctrl.Result{}, err
386379
}
387380

388381
// Store the userdata in a secret
389382
if err := r.storeBootstrapData(ctx, cluster, config, userDataScript); err != nil {
390383
log.Error(err, "Failed to store bootstrap data")
391384
conditions.MarkFalse(config, eksbootstrapv1.DataSecretAvailableCondition, eksbootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, "")
392-
return err
385+
return ctrl.Result{}, err
393386
}
394387

395388
conditions.MarkTrue(config, eksbootstrapv1.DataSecretAvailableCondition)
396-
return nil
397-
}
398-
399-
// Helper functions to get dynamic values
400-
func getMaxPods(config *eksbootstrapv1.EKSConfig) int {
401-
if config.Spec.UseMaxPods != nil && *config.Spec.UseMaxPods {
402-
return 58 // Default value when UseMaxPods is true
403-
}
404-
return 110 // Default value when UseMaxPods is false
405-
}
406-
407-
func getClusterDNS(config *eksbootstrapv1.EKSConfig) string {
408-
if config.Spec.DNSClusterIP != nil && *config.Spec.DNSClusterIP != "" {
409-
return *config.Spec.DNSClusterIP
410-
}
411-
return "10.96.0.10" // Default value
412-
}
413-
414-
func getCapacityType(config *eksbootstrapv1.EKSConfig) string {
415-
// TODO: Get from AWSManagedMachinePool spec if available
416-
// For now, return default
417-
return "ON_DEMAND"
389+
return ctrl.Result{}, nil
418390
}
419391

420392
func (r *EKSConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, option controller.Options) error {

0 commit comments

Comments
 (0)