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

Increase ipset creation / deletion test timeouts to allow for slow CI #8989

Merged

Conversation

matthewdupre
Copy link
Member

The first of these tests failed here: #8957

CI machines don't provide good or consistent performance, so I don't believe the FVs are an appropriate place for tightly bounded performance tests.

@matthewdupre matthewdupre added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact labels Jul 8, 2024
@matthewdupre matthewdupre requested a review from a team as a code owner July 8, 2024 18:33
@marvin-tigera marvin-tigera added this to the Calico v3.29.0 milestone Jul 8, 2024
Comment on lines 123 to 124
// Before we rate limited deletions, this would take 80s+. Now it takes
// 10s so 20s should give some headroom.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: update comment?

Copy link
Member

@fasaxc fasaxc left a comment

Choose a reason for hiding this comment

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

Still useful for manual retests even if we bump the timeout 👍

@matthewdupre matthewdupre force-pushed the update-ipset-rendering-timeout branch 2 times, most recently from b291014 to bc71bf7 Compare July 10, 2024 23:52
@matthewdupre matthewdupre force-pushed the update-ipset-rendering-timeout branch from bc71bf7 to 8bec16a Compare July 15, 2024 18:26
@matthewdupre matthewdupre merged commit 9d99f44 into projectcalico:master Jul 15, 2024
2 checks passed
@matthewdupre matthewdupre deleted the update-ipset-rendering-timeout branch July 15, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required Docs not required for this change hold-merge release-note-not-required Change has no user-facing impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants