Skip to content

Conversation

@momesgin
Copy link
Member

@momesgin momesgin commented Oct 21, 2025

Summary

Fixes #11629

Occurred changes and/or fixed issues

I have updated the issue's description with UI Requirements, please check them.

  • Previously you were not able to get the correct results by searching for ARM or x86_64 , but now you can
  • Previously we were displaying both "Instance Types" and "Spot Instance Types" dropdowns, but now we toggle between them
  • A new dropdown is added to filter the options by "Architecture", it has three filter options, by default "All" is selected and you can choose "ARM" or "x86_64" as well
  • Some form fields have moved in order to keep the instance type related fields in one row(@kwwii has already approved this)
  • When the architecture is changed, the selected "Instance Type" is automatically reset to a suitable default for the new architecture (e.g., t3.medium for x86_64, t4g.medium for ARM, but If spot instances are selected, the selection is cleared)

Technical notes summary

  • A new method, filterByArchitecture(options), was implemented. It processes a flat list of instance type options (including group headers), filters them based on whether their supportedArchitectures array contains the selected architecture, and returns a new filtered flat list.
  • The logic for fetching AWS instance types has been updated to include architecture information
  • Unit tests

Areas or cases that should be tested

  • Try different architecture filters
  • Test searching by typing
  • Toggle "Request Spot Instances" should work properly

Areas which could experience regressions

Both Edit and Create modes should be tested:

  • Make sure you can provision an EKS cluster successfully
  • The payload of the network request has NOT been changed
  • Try different options of instance type with and without selecting a Launch Template, pre-filled templates will make some instance types fields disabled, these behaviors should be the same as before

Screenshot/Video

eks-pr.mov

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes
  • The PR has been reviewed in terms of Accessibility
  • The PR has considered, and if applicable tested with, the three Global Roles Admin, Standard User and User Base

@momesgin momesgin added this to the v2.13.0 milestone Oct 21, 2025
@momesgin momesgin self-assigned this Oct 21, 2025
@momesgin momesgin added the QA/manual-test Indicates issue requires manually testing label Oct 21, 2025
@momesgin momesgin marked this pull request as ready for review October 21, 2025 02:28
Copy link
Member

@mantis-toboggan-md mantis-toboggan-md left a comment

Choose a reason for hiding this comment

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

The PR looks good overall and represents a nice enhancement to EKS provisioning. I had only one minor complaint: it would be nice if the instance type selection wasn't cleared when switching back to 'all' in the architecture filter.

@momesgin
Copy link
Member Author

momesgin commented Oct 21, 2025

The PR looks good overall and represents a nice enhancement to EKS provisioning. I had only one minor complaint: it would be nice if the instance type selection wasn't cleared when switching back to 'all' in the architecture filter.

Before my changes enabling and disabling Request Spot... checkbox clears the previously selected instance type, I thought I should follow the same pattern, but I can work on it

UPDATE: done

Copy link
Member

@mantis-toboggan-md mantis-toboggan-md left a comment

Choose a reason for hiding this comment

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

LGTM

@momesgin momesgin merged commit 29f93e6 into rancher:master Oct 22, 2025
86 of 90 checks passed
@momesgin momesgin deleted the 11629-arm-node-group branch October 22, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA/manual-test Indicates issue requires manually testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for ARM node groups

2 participants