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

tests: fix flaky gateway conformance HTTPRouteListenerHostnameMatching test #3732

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Mar 15, 2023

What this PR does / why we need it:

This fixes setting the incorrect status for Gateway API routes.

  • It sets the section name in ParentReference when it was specified
  • It adds unit tests for cases where section names are specified and where more than 1 gateway is matching the route's ParentRefs
  • It prevents adding multiple parentRefs status for the same parent which was found in local debugging

The relevant conformance test was run multiple times in a loop locally to ensure the fix is in place.

Which issue this PR fixes:

Fixes: #3721

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@pmalek pmalek added area/tests area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API test-flake labels Mar 15, 2023
@pmalek pmalek self-assigned this Mar 15, 2023
@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Patch coverage: 95.0% and no project coverage change.

Comparison is base (e87a4b7) 73.5% compared to head (1688039) 73.5%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3732   +/-   ##
=====================================
  Coverage   73.5%   73.5%           
=====================================
  Files        133     133           
  Lines      15843   15828   -15     
=====================================
- Hits       11651   11646    -5     
+ Misses      3431    3423    -8     
+ Partials     761     759    -2     
Impacted Files Coverage Δ
internal/controllers/gateway/route_utils.go 81.2% <95.0%> (+<0.1%) ⬆️

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pmalek pmalek marked this pull request as ready for review March 15, 2023 14:36
@pmalek pmalek requested a review from a team as a code owner March 15, 2023 14:36
@pmalek pmalek force-pushed the fix-flaky-gateway-conformance-HTTPRouteListenerHostnameMatching-test branch from d035e24 to 1688039 Compare March 15, 2023 14:37
@pmalek pmalek enabled auto-merge (squash) March 15, 2023 18:28
@pmalek pmalek added this to the KIC v2.9.0 milestone Mar 15, 2023
@pmalek pmalek merged commit 923cdf3 into main Mar 15, 2023
@pmalek pmalek deleted the fix-flaky-gateway-conformance-HTTPRouteListenerHostnameMatching-test branch March 15, 2023 19:53
Copy link
Member

@mlavacca mlavacca left a comment

Choose a reason for hiding this comment

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

I know this has already been merged and I'm late, but I wonder how this missing field can produce failures in an inconsistent way in our conformance tests 🤔

@pmalek
Copy link
Member Author

pmalek commented Mar 16, 2023

I know this has already been merged and I'm late, but I wonder how this missing field can produce failures in an inconsistent way in our conformance tests 🤔

Missing section name was one of the things fixed in this PR. The problem apart from this one was most likely due to inconsistent status processing and at times, creating 2 nearly equal parent statuses for a route, which caused the "expect 1 parent, got 2" error in test logs.

@mlavacca
Copy link
Member

I know this has already been merged and I'm late, but I wonder how this missing field can produce failures in an inconsistent way in our conformance tests thinking

Missing section name was one of the things fixed in this PR. The problem apart from this one was most likely due to inconsistent status processing and at times, creating 2 nearly equal parent statuses for a route, which caused the "expect 1 parent, got 2" error in test logs.

Gotcha, thanks for the explanation 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API area/tests size/L test-flake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky Gateway API conformance test: TestGatewayConformance/HTTPRouteListenerHostnameMatching
3 participants