Improve CI stability #1843

davidcassany · 2023-10-11T11:26:53Z

~~This is brute force attempt to check if adding this simple no_timer_check kernel parameter is solving the tests flakiness.~~

[EDIT]

Adds several improvements:

Check VM pid on 'EventuallyConnects'. So it does not wait for a
command to succeed if the underlaying VM crashed and it is not
running.
Does not use '-daemonize' flag of qemu, now it simply runs on the
background and the stdout and stderr are redirected to vmstdout file.
Does not install QEMU, the runner already has a recent QEMU version
installed. This saves several minutes on each macos job.
Fixes some of the stability issues on macOS by disabling hugepages on
the kernel. This is not supported on macOS.

kkaempf · 2023-10-11T12:25:12Z

What is the "timer sync issue" ? Can you add a reference to the original bug report please ?

kkaempf · 2023-10-11T12:27:12Z

Ah, #1834 has some background

davidcassany · 2023-10-11T20:27:59Z

Ah, #1834 has some background

Yes sorry, this was just an attempt to verify if this parameter was solving the flaky CI tests. The thing is that this seams to be an issue only appearing in MacOS runners, hence likely to be a problem with hvf and qemu. It is pretty annoying as pretty often we get a kernel panic a boot... 😞

We haven't ever seen it in any other env beyond MacOS github runners. So we actually need to make a PR to check if the issue is solved.

codecov-commenter · 2023-10-30T07:43:59Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (fd92278) 75.33% compared to head (f973c2b) 75.34%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1843      +/-   ##
==========================================
+ Coverage   75.33%   75.34%   +0.01%     
==========================================
  Files          67       67              
  Lines        6814     6818       +4     
==========================================
+ Hits         5133     5137       +4     
  Misses       1311     1311              
  Partials      370      370

Files	Coverage Δ
cmd/config/config.go	`73.77% <100.00%> (ø)`
pkg/config/config.go	`96.34% <100.00%> (ø)`
pkg/types/v1/config.go	`87.02% <100.00%> (ø)`
pkg/types/v1/grub.go	`84.48% <100.00%> (+3.63%)`	⬆️
cmd/build-disk.go	`28.78% <66.66%> (ø)`
pkg/action/build-disk.go	`68.91% <71.42%> (-0.38%)`	⬇️

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

davidcassany · 2023-10-31T01:19:40Z

Still a draft as it builds on top of rancher-sandbox/ele-testhelpers#38, hence this should be merged first and then readapt go.mod on this project again.

frelon · 2023-10-31T08:26:12Z

.github/workflows/build_and_test.yaml

@@ -157,7 +157,6 @@ jobs:
      - if: ${{ env.ARCH == 'x86_64' }} 
        name: Run VM script dependencies
        run: |
-          brew update; brew upgrade qemu


Yep, this was added since there was an update to qemu which was not picked up by our runner, which broke the build... Probably not needed now!

Yes I guess we were suffering MacOS runner upgrades. I remember there used to be a very old qemu version when we first started using it, now I see it comes with Qemu v8 by default.

frelon

This looks great! 🚀

davidcassany · 2023-10-31T08:38:52Z

.github/workflows/build_and_test.yaml

@@ -157,7 +157,6 @@ jobs:
      - if: ${{ env.ARCH == 'x86_64' }} 
        name: Run VM script dependencies
        run: |
-          brew update; brew upgrade qemu


Yes I guess we were suffering MacOS runner upgrades. I remember there used to be a very old qemu version when we first started using it, now I see it comes with Qemu v8 by default.

davidcassany · 2023-10-31T08:40:15Z

scripts/run_vm.sh

@@ -6,19 +6,20 @@ SCRIPT=$(realpath -s "${0}")
 SCRIPTS_PATH=$(dirname "${SCRIPT}")
 TESTS_PATH=$(realpath -s "${SCRIPTS_PATH}/../tests")

+: "${ELMNTL_PREFIX:=}" 


I added a prefix to the files so we could consider at some point running in parallel multiple tests on the same machine without overlapping files.

davidcassany · 2023-10-31T08:42:15Z

scripts/run_vm.sh

@@ -67,12 +68,12 @@ function start {
        ;;
  esac

-  [ "hvf" == "${ELMNTL_ACCEL}" ] && accel_arg="-accel ${ELMNTL_ACCEL}" && firmware_arg="-bios ${ELMNTL_FIRMWARE} ${firmware_arg}"
+  [ "hvf" == "${ELMNTL_ACCEL}" ] && accel_arg="-accel ${ELMNTL_ACCEL}" && firmware_arg="-bios ${ELMNTL_FIRMWARE} ${firmware_arg}" && cpu_arg="-cpu max,-pdpe1gb"


I found this -pdpe1gb flag surfing the net... Apparently this disables hugepages and for some reason this is not supported in MacOS 🤷🏽‍♂️ I do believe this is the real change that fixes flakytests.

davidcassany · 2023-10-31T08:44:17Z

make/Makefile.test

-	$(GINKGO) run $(GINKGO_ARGS) ./tests/installer
-	$(GINKGO) run $(GINKGO_ARGS) ./tests/smoke
+	VM_PID=$$(scripts/run_vm.sh vmpid) go run $(GINKGO) $(GINKGO_ARGS) ./tests/installer
+	VM_PID=$$(scripts/run_vm.sh vmpid) go run $(GINKGO) $(GINKGO_ARGS) ./tests/smoke


Probably there are more elegant ways pass the VM pid to the test code, but so far I opted for this simple solution. This is not strictly required and can be omitted on local runs.

Adds several improvements: * Check VM pid on 'EventuallyConnects'. So it does not wait for a command to succeed if the underlaying VM crashed and it is not running. * Does not use '-daemonize' flag of qemu, now it simply runs on the background and the stdout and stderr are redirected to vmstdout file. * Does not install QEMU, the runner already has a recent QEMU version installed. This saves several minutes on each macos job. * Fixes some of the stability issues on macOS by disabling hugepages on the kernel. This is not supported on macOS. Signed-off-by: David Cassany <dcassany@suse.com>

davidcassany requested a review from a team as a code owner October 11, 2023 11:26

davidcassany marked this pull request as draft October 11, 2023 11:26

davidcassany force-pushed the fix_apic_timer_errors branch 2 times, most recently from a49ba5b to 73b619d Compare October 30, 2023 07:39

davidcassany force-pushed the fix_apic_timer_errors branch 22 times, most recently from bb42655 to c0f8fc2 Compare October 31, 2023 00:25

davidcassany force-pushed the fix_apic_timer_errors branch 4 times, most recently from 24a8842 to 9b441ee Compare October 31, 2023 01:13

davidcassany changed the title ~~WIP, checking the included kernel parameter solves the timer sync issues~~ Improve CI stability Oct 31, 2023

davidcassany requested a review from frelon October 31, 2023 01:25

frelon reviewed Oct 31, 2023

View reviewed changes

frelon approved these changes Oct 31, 2023

View reviewed changes

davidcassany force-pushed the fix_apic_timer_errors branch from 9b441ee to aeffeec Compare October 31, 2023 08:35

davidcassany marked this pull request as ready for review October 31, 2023 08:36

davidcassany commented Oct 31, 2023

View reviewed changes

davidcassany force-pushed the fix_apic_timer_errors branch from aeffeec to f973c2b Compare October 31, 2023 08:46

davidcassany merged commit 6290bf3 into rancher:main Oct 31, 2023
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve CI stability #1843

Improve CI stability #1843

davidcassany commented Oct 11, 2023 •

edited

Loading

kkaempf commented Oct 11, 2023

kkaempf commented Oct 11, 2023

davidcassany commented Oct 11, 2023

codecov-commenter commented Oct 30, 2023 •

edited

Loading

davidcassany commented Oct 31, 2023

frelon Oct 31, 2023

davidcassany Oct 31, 2023

frelon left a comment

davidcassany Oct 31, 2023

davidcassany Oct 31, 2023

davidcassany Oct 31, 2023

davidcassany Oct 31, 2023

Improve CI stability #1843

Improve CI stability #1843

Conversation

davidcassany commented Oct 11, 2023 • edited Loading

kkaempf commented Oct 11, 2023

kkaempf commented Oct 11, 2023

davidcassany commented Oct 11, 2023

codecov-commenter commented Oct 30, 2023 • edited Loading

Codecov Report

davidcassany commented Oct 31, 2023

frelon Oct 31, 2023

Choose a reason for hiding this comment

davidcassany Oct 31, 2023

Choose a reason for hiding this comment

frelon left a comment

Choose a reason for hiding this comment

davidcassany Oct 31, 2023

Choose a reason for hiding this comment

davidcassany Oct 31, 2023

Choose a reason for hiding this comment

davidcassany Oct 31, 2023

Choose a reason for hiding this comment

davidcassany Oct 31, 2023

Choose a reason for hiding this comment

davidcassany commented Oct 11, 2023 •

edited

Loading

codecov-commenter commented Oct 30, 2023 •

edited

Loading