Skip to content

fix(tests): Adjust skeleton test timeouts to reduce flakyness #4417

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.

Sign up for GitHub

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

Jump to bottom
Open
dangome3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(tests): Adjust skeleton test timeouts to reduce flakyness #4417

dangome3 wants to merge 1 commit into from

Conversation

@dangome3
Copy link
Contributor

@dangome3 dangome3 commented Dec 9, 2025 •
edited by mtardy
Loading

Increase the curl event checker time limit from 30 to 60 seconds and change the curl container imagePullPolicy from Always to IfNotPresent.

Tested several times:

--- PASS: TestSkeletonBasic (15.05s)
    --- PASS: TestSkeletonBasic/Get_Minimum_Kernel_Version (0.00s)
        --- PASS: TestSkeletonBasic/Get_Minimum_Kernel_Version/Lookup_Kernel_Version (0.00s)
    --- PASS: TestSkeletonBasic/Run_Event_Checks (2.98s)
        --- PASS: TestSkeletonBasic/Run_Event_Checks/Run_Event_Checks (2.98s)
    --- PASS: TestSkeletonBasic/Run_Workload (15.05s)
        --- PASS: TestSkeletonBasic/Run_Workload/Wait_for_Checker (0.00s)
        --- PASS: TestSkeletonBasic/Run_Workload/Run_Workload (10.02s)
        --- PASS: TestSkeletonBasic/Run_Workload/Run_Metrics_Checks (0.02s)
        --- PASS: TestSkeletonBasic/Run_Workload/Uninstall_policy (5.01s)

Found several pipelines failing due to timeout issue so I increased them. The execution time should not be affected when the network is working properly, the current change just add additional changes to make the tests work

@dangome3 dangome3 force-pushed the daniel/pr/skeletonTestsFix branch from 7fa8944 to caffc1a Compare December 10, 2025 14:06
@FedeDP
Copy link
Contributor

FedeDP commented Dec 10, 2025

Tested several times:

Question: were you able to reproduce the issue before your changes?

@dangome3
Copy link
Contributor Author

dangome3 commented Dec 10, 2025

Tested several times:

Question: were you able to reproduce the issue before your changes?

There are some pipelines that failed

@dangome3 dangome3 force-pushed the daniel/pr/skeletonTestsFix branch from caffc1a to a13104b Compare December 10, 2025 15:43
dangome3
dangome3 commented Dec 10, 2025
View reviewed changes
tests/e2e/tests/skeleton/skeleton_test.go
// Create an curl event checker with a limit or 10 events or 30 seconds, whichever comes first
curlChecker := curlEventChecker(kversion).WithEventLimit(100).WithTimeLimit(30 * time.Second)
// Create an curl event checker with a limit or 200 events or 120 seconds, whichever comes first
curlChecker := curlEventChecker(kversion).WithEventLimit(200).WithTimeLimit(120 * time.Second)
Copy link
Contributor Author

@dangome3 dangome3 Dec 10, 2025

Choose a reason for hiding this comment

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

Adjusted the time to make it more stable. It will not affect the time when it works, only give more chances to success when the infraestructure/network is a bit slower

Copy link
Member

@mtardy mtardy Dec 22, 2025

Choose a reason for hiding this comment

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

should the time limit be the same as the wait limit nearby Wait(60*time.Second)? So 120 s everywhere or 200 s everywhere? I don't exactly knows what .Wait does on a checker out of the top of my head.

Copy link
Contributor Author

@dangome3 dangome3 Jan 5, 2026

Choose a reason for hiding this comment

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

Hey @mtardy!

Thanks for your comment. The .Wait method defines how long to wait for the checker to start. So it can be shorter as it should start almost instantly once the gRPC connection is established

We can also update it to 120s, that shouldn't affect the execution time either but I didn't see any timeout related to the checker starting phase, so it is unnecessary IMO

Please let me know if you have other opinions :)

AritraDey-Dev
AritraDey-Dev reviewed Dec 10, 2025
View reviewed changes
@dangome3
fix(tests): Adjust skeleton test timeouts to reduce flakyness
0f0d884 
Increase the curl event checker time limit from 30 to 120 seconds.

Signed-off-by: Daniel Gomez <[email protected]>
@dangome3 dangome3 force-pushed the daniel/pr/skeletonTestsFix branch from a13104b to 0f0d884 Compare December 10, 2025 16:43
@dangome3 dangome3 marked this pull request as ready for review December 10, 2025 16:45
@dangome3 dangome3 requested a review from a team as a code owner December 10, 2025 16:45
@dangome3 dangome3 requested a review from tpapagian December 10, 2025 16:45
@FedeDP FedeDP requested a review from AritraDey-Dev December 12, 2025 15:50
@FedeDP FedeDP added area/testing Related to testing release-note/misc This PR makes changes that have no direct user impact. labels Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mtardy mtardy mtardy left review comments

@tpapagian tpapagian Awaiting requested review from tpapagian tpapagian is a code owner automatically assigned from cilium/tetragon-reviewers

@AritraDey-Dev AritraDey-Dev Awaiting requested review from AritraDey-Dev

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area/testing Related to testing release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dangome3 @FedeDP @mtardy @AritraDey-Dev