Skip to content

[2.12] Handle pagination in EKS AWS ListRoles request #13730

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

Merged

Conversation

mantis-toboggan-md
Copy link
Member

@mantis-toboggan-md mantis-toboggan-md commented Mar 12, 2025

Summary

Fixes #11554

Occurred changes and/or fixed issues

This PR updates aws api calls made in the EKS provisioning form to account for pagination. The instance type api call already handled pagination. describeKeyPairs and describeSecurityGroups are not paginated.

Technical notes summary

DescribeAddonVersions
ListRoles
DescribeLaunchTemplates
DescribeKeyPairs - not paginated
ListKeys
DescribeVpcs
DescribeSubnets
DescribeSecurityGroups - not paginated

Areas or cases that should be tested

We have too few of most types of resources to get paginated responses from aws by default. I found the easiest way to test pagination was to alter the dashboard code to lower the number of items returned in a response. Different API calls use a different parameter to do that: describeX calls accept MaxIResults while ListRoles accepts MaxItems and ListKeys accepts Limit eg

const addons = await this.$store.dispatch('aws/depaginateList', { client: eksClient, cmd: 'describeAddonVersions', opt:{MaxResults: 3} });

To verify pagination, you may set a limit on the number of items returned per request, then watch the network tab and verify that multiple requests are made to fetch all items

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

Sorry, something went wrong.

@@ -107,11 +106,11 @@ describe('eKS K8s configuration', () => {
expect(wrapper.exists()).toBe(true);
expect(spy).toHaveBeenCalledTimes(0);
await setCredential(wrapper);
expect(spy).toHaveBeenCalledTimes(4);
expect(spy).toHaveBeenCalledTimes(8);
Copy link
Member Author

Choose a reason for hiding this comment

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

these are doubled because for each paginated aws api call we call dispatch twice: once to get the relevant aws client and once more to use the client in the depaginate action

Copy link
Member

@momesgin momesgin left a comment

Choose a reason for hiding this comment

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

I tested this as explained in the pr's description, and was able to see multiple calls to aws instead of one, lgtm

@mantis-toboggan-md mantis-toboggan-md merged commit 966bb3d into rancher:master Mar 13, 2025
38 checks passed
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.

Unable to select custom cluster service role/node instance role for EKS clusters
2 participants