Skip to content
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

test: Added E2E tests for secondary region and windows nodes #95

Merged
merged 6 commits into from
Mar 22, 2023
Merged

test: Added E2E tests for secondary region and windows nodes #95

merged 6 commits into from
Mar 22, 2023

Conversation

quixoticmonk
Copy link
Contributor

@quixoticmonk quixoticmonk commented Mar 22, 2023

Summary

This PR extends the existing E2E workflow to consider a secondary region us-east-2 and a cluster with windows nodes.

Resolves: #94

Changes

  • Updated the test workflows to take region parameter from the workflow call than the originally hard coded us-east-1.
  • The region value is picked from a map including cluster versions( current, target) to invoke the e2e template.
  • Cluster deletion action updated to pick the map value of region than us-east-1
  • Note: A redundant aws-actions/configure-aws-credentials@v2 is added before cluster upgrade test to handle some of the additional time taken for the node groups in the windows usecase( Can possibly look to run --parallel)

User experience

No user experience changes for end user. This would support the maintainers in testing the utility across additional parameters.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (5f164f7) 27.55% compared to head (6df992d) 27.55%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #95   +/-   ##
=======================================
  Coverage   27.55%   27.55%           
=======================================
  Files          14       14           
  Lines        1938     1938           
=======================================
  Hits          534      534           
  Misses       1404     1404           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@quixoticmonk quixoticmonk marked this pull request as ready for review March 22, 2023 04:22
@quixoticmonk quixoticmonk requested a review from a team March 22, 2023 04:22
region: us-east-2
version: "1.22"

managedNodeGroups:
Copy link
Member

Choose a reason for hiding this comment

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

This is a great start but its also somewhat starting in the "ideal" scenario. We should look at purposely starting from a state that is far from ideal to ensure that the process will upgrade correctly across the known hurdles we throw its way. Here is an example "non-ideal" cluster in Terraform that I have used for executing checks on https://github.com/clowdhaus/eksup/blob/7980b93818f55b5c3d795dd1cc430832dafd09e8/examples/mixed/main.tf#L53-L155

  • Control plane does not match data plane, there are various levels of skew
  • Addons are intentionally out of date and even not supported on next Kubernetes version
  • Mix of managed, self-managed, and Fargate profiles

Copy link
Member

Choose a reason for hiding this comment

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

Note: Just to be clear, I am not proposing Terraform here - its more of the fact that we start with a cluster that would have several hurdles that it would need to overcome in the upgrade process. Make sure it upgrades the managed nodegorup first if it has a skew of +1 already since thats required by EKS, make the VPC CNI upgrade several times in sequence across one minor version at a time, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. Agreed on the required breadth on these tests. Will take a look at some of those examples and have those in our eksctl configs as well.

@mbeacom mbeacom merged commit 86bfc31 into aws-samples:main Mar 22, 2023
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.

Maintenance: Extend automated test cases to a secondary region
4 participants