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

Refactor run_tests.sh script #6280

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Conversation

manuelbuil
Copy link
Contributor

@manuelbuil manuelbuil commented Jul 5, 2024

Proposed Changes

This PR adds some extra logging in the latest_commit.sh script. If there was an error because of the API exceeding the limits, it was impossible to notice

Moreover, it refactors run_tests.sh script trying to make it simpler. Changes:

1 - Removes all the parameters that can be passed except for nodeOS, which is important for the results generation. Some of them depend on the test itself and should not be changed (servercount, agentcount). Some are not read by any test (db or hardened)

2 - Removes rke2_version and rke2_channel variables and references as the former was always empty and the latter was always commit, which is anyway the default: https://github.com/rancher/rke2/blob/master/tests/e2e/vagrantdefaults.rb#L19-L22

3 - Creates a loop that goes over an array with the different e2e tests available instead of calling each one in a different line

4 - Uses a 2h timeout for all e2e tests. In the end, the important timeout is the one happening in each Test Case (in the Eventually instruction)

5 - Slightly changes the "VM gargabe collection". It was detected that sometimes, VMs are not really cleaned up. That should always happen as we run the tests with the -ci. We suspect it's a problem related to the timeout, that's why we increased it to 2h as previously explained. Just in case, before starting the e2e test, we destroy the VMs related to that test. At the beginning of the script, we also destroy any VM which might be in an invalid state with vagrant global-status --prune

6 - The log file includes now the date in the name instead of the OS

7 - Upgrades all images to Ubuntu2310

Before running each test, it makes sure that VMs are destroyed.

Types of Changes

Code refactor

Verification

Testing

Linked Issues

User-Facing Change


Further Comments

@manuelbuil manuelbuil requested a review from a team as a code owner July 5, 2024 13:15
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.82%. Comparing base (61cebda) to head (fdc40dd).

❗ There is a different number of reports uploaded between BASE (61cebda) and HEAD (fdc40dd). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (61cebda) HEAD (fdc40dd)
unittests 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #6280       +/-   ##
==========================================
- Coverage   26.09%   9.82%   -16.28%     
==========================================
  Files          32      32               
  Lines        2698    2698               
==========================================
- Hits          704     265      -439     
- Misses       1948    2411      +463     
+ Partials       46      22       -24     
Flag Coverage Δ
inttests 9.82% <ø> (ø)
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

}

ls createreport/rke2_"$OS".log 2>/dev/null && rm createreport/rke2_"$OS".log
ls createreport/rke2_${date}.log 2>/dev/null && rm createreport/rke2_${date}.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave it as OS to avoid cleanup of s3 bucket and preferably one sharable link to results for different OS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, done! I moved all tests to Ubuntu2310 (same like in k3s)


echo 'RUNNING MIXEDOS TEST'
/usr/local/go/bin/go test -v mixedos/mixedos_test.go -nodeOS="$nodeOS" -serverCount=$((servercount)) -timeout=1h -json -ci |tee -a createreport/rke2_"$OS".log
for i in ${!tests[@]}; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Some tests have just server nodes and no agents. Some have HA and some single server single agent. Can we default then in Vagrantfile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, vagrantfile defaults the number of servers and agents per test. Same for the OS

Signed-off-by: Manuel Buil <mbuil@suse.com>
Signed-off-by: Manuel Buil <mbuil@suse.com>
@manuelbuil manuelbuil merged commit 43760e9 into rancher:master Jul 10, 2024
6 checks passed
@manuelbuil manuelbuil deleted the improveruntestssh branch July 10, 2024 06:25
iamsarthakk pushed a commit to iamsarthakk/rke2 that referenced this pull request Aug 19, 2024
* Improve run_tests script

Signed-off-by: Manuel Buil <mbuil@suse.com>

* Upgrade tests to ubuntu2310 and use nodeOS var

Signed-off-by: Manuel Buil <mbuil@suse.com>

---------

Signed-off-by: Manuel Buil <mbuil@suse.com>
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.

5 participants