Skip to content

Conversation

@mselim00
Copy link
Contributor

@mselim00 mselim00 commented Jul 2, 2025

Issue #, if available:

Description of changes:
Enables a pass-through flag for nodeadm feature gates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mselim00 mselim00 marked this pull request as ready for review July 3, 2025 01:21
@mselim00 mselim00 requested a review from ndbaker1 July 3, 2025 01:21
@HusainZafar HusainZafar self-requested a review July 3, 2025 21:03
Copy link
Contributor

@ndbaker1 ndbaker1 left a comment

Choose a reason for hiding this comment

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

sgtm, just want a small change

stackName := m.getUnmanagedNodegroupStackName()
klog.Infof("creating unmanaged nodegroup stack %s...", stackName)
userData, userDataIsMimePart, err := generateUserData(opts.UserDataFormat, cluster, opts)
userData, userDataIsMimePart, err := generateUserData(cluster, opts)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

passing opts.UserDataFormat was redundant, opts is already passed

Copy link
Contributor

@ndbaker1 ndbaker1 left a comment

Choose a reason for hiding this comment

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

lgtm

return buf.String(), userDataIsMimePart, nil
}

func extractFeatureGates(featureGatePairs []string) (map[string]bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming the CLI framework doesn't support a map[string]string/map[string]bool, this looks fine

@HusainZafar HusainZafar merged commit 61ae556 into aws:main Jul 12, 2025
10 checks passed
@mselim00 mselim00 deleted the nodeadm-gates branch July 26, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants