Skip to content

[v1.13] backports 2023-10-25 #28776

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
Merged
merged 1 commit into from
Oct 25, 2023
Merged

[v1.13] backports 2023-10-25 #28776

merged 1 commit into from
Oct 25, 2023

Conversation

sayboras
Copy link
Member

Description

This is to fix the flaky unit tests, introduced by #28331

https://jenkins.cilium.io/job/Cilium-PR-Runtime-kernel-net-next/1180/testReport/junit/(root)/Suite-runtime/RuntimeDatapathPrivilegedUnitTests_Run_Tests/

Testing

Testing was done locally as per below:

$ go test -count=1000 -cover ./operator/pkg/gateway-api/...
?       github.com/cilium/cilium/operator/pkg/gateway-api/helpers       [no test files]
ok      github.com/cilium/cilium/operator/pkg/gateway-api       8.582s  coverage: 54.3% of statements

$ go test -count=1000 -cover ./operator/pkg/model/...                          
ok      github.com/cilium/cilium/operator/pkg/model     0.097s  coverage: 37.7% of statements
ok      github.com/cilium/cilium/operator/pkg/model/ingestion   0.576s  coverage: 88.8% of statements
ok      github.com/cilium/cilium/operator/pkg/model/translation 3.547s  coverage: 76.4% of statements
ok      github.com/cilium/cilium/operator/pkg/model/translation/gateway-api     0.145s  coverage: 84.2% of statements
ok      github.com/cilium/cilium/operator/pkg/model/translation/ingress 0.590s  coverage: 84.0% of statements

$ go test -count=1000 -cover ./operator/pkg/ingress/...
ok      github.com/cilium/cilium/operator/pkg/ingress   0.249s  coverage: 3.3% of statements
ok      github.com/cilium/cilium/operator/pkg/ingress/annotations       0.117s  coverage: 36.4% of statements

@sayboras
gateway: Make Header matching deterministic
ec9c833 
[upstream commit 152b439]

This commit is to make sure that header matching rules are sorted in a
deterministic way, so that the behavior is predictable in envoy. The
main changes are as per below:

- Process http routes based on order from spec instead of random order
  from map.
- Use sort.Stable to reverse the original order of equal elements.
- Make sure that less(i, j) and less(j, i) return false if i-th and j-th
  elements are equal.

Kindly note that envoy will just iteratively check rule one by one, if a
match is found, subsequent rule will not be considered.

Fixes: #23999
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.13 kind/backports This PR provides functionality previously merged into master. labels Oct 25, 2023
@sayboras sayboras changed the title gateway: Make Header matching deterministic [v1.13] backports 2023-10-25 Oct 25, 2023
@sayboras
Copy link
Member Author

/test-backport-1.13

@sayboras sayboras mentioned this pull request Oct 25, 2023
Merged
6 tasks
@sayboras sayboras marked this pull request as ready for review October 25, 2023 09:19
@sayboras sayboras requested review from a team as code owners October 25, 2023 09:19
@sayboras
Copy link
Member Author

Marking this ready to merge as all CI jobs are green, extra testing to make sure no flaky test was done as per PR description.

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 25, 2023
@dylandreimerink dylandreimerink merged commit ecb2250 into v1.13 Oct 25, 2023
@dylandreimerink dylandreimerink deleted the tam/fix-flaky-test branch October 25, 2023 14:04
@nathanjsweet nathanjsweet mentioned this pull request Nov 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylandreimerink dylandreimerink dylandreimerink approved these changes

@viktor-kurchenko viktor-kurchenko viktor-kurchenko approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
No open projects
cilium v1.13.9
Status: Released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sayboras @dylandreimerink @viktor-kurchenko