-
Notifications
You must be signed in to change notification settings - Fork 287
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
Add e2e test for upgrading management-components #7251
Add e2e test for upgrading management-components #7251
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7251 +/- ##
=======================================
Coverage 71.63% 71.64%
=======================================
Files 556 556
Lines 43199 43199
=======================================
+ Hits 30945 30948 +3
+ Misses 10543 10541 -2
+ Partials 1711 1710 -1 ☔ View full report in Codecov by Sentry. |
a8fccd1
to
7453ba6
Compare
7453ba6
to
40b4bc0
Compare
// create cluster with old eksa | ||
test.CreateCluster(framework.ExecuteWithEksaRelease(release)) | ||
// upgrade management-components with new eksa | ||
test.RunEKSA([]string{"upgrade", "management-components", "-f", test.ClusterConfigLocation, "-v", "99"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think that we should we create a test.UpgradeManagmentComponents
function and hide these details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need this to test completing the rest of the upgrade after running upgrade management components?
Because the followup procedure of upgrading the cluster itself will fail due to the bug: https://github.com/aws/eks-anywhere-internal/issues/2126. And I think right now we need a test case to cover the most essential flow of the new subcommand more than we need a comprehensive one that verify the flow of eksa controller reconciliation process. Having said that, we can do this in another PR when the bug is fixed |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: d8660091 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue #, if available:
Description of changes:
Testing (if applicable): Manually invoked the new e2e test, and it passed.
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.