fix: cleanup dangling parents in route and policy statuses #7307
Conversation
y-rabie
changed the title
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (25.46%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7307 +/- ##
==========================================
- Coverage 72.39% 72.10% -0.30%
==========================================
Files 231 231
Lines 34043 34234 +191
==========================================
+ Hits 24647 24684 +37
- Misses 7625 7770 +145
- Partials 1771 1780 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@y-rabie Looks like e2e tests are broken - can you fix it? |
|
Yeah sure, I'll work on your comments then fix e2e tests |
y-rabie
force-pushed
the
fix-statuses-with-multiple-parents
branch
4 times, most recently
from
d79bd54 to
0cdbd4b
Compare
|
/retest |
y-rabie
force-pushed
the
fix-statuses-with-multiple-parents
branch
from
0cdbd4b to
e0fdedb
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@zhaohuabing Appreciate you taking a look again when you can! |
zhaohuabing
force-pushed
the
fix-statuses-with-multiple-parents
branch
from
e0fdedb to
4b27182
Compare
|
Hi @y-rabie I haven't forgotten about this PR :-) I've been working on v1.6 and some other tasks, but now I'll have some time to review it. In the meantime, could you please resolve the conflicts? |
y-rabie
force-pushed
the
fix-statuses-with-multiple-parents
branch
3 times, most recently
from
caf36ac to
3de6775
Compare
y-rabie
force-pushed
the
fix-statuses-with-multiple-parents
branch
2 times, most recently
from
a47a53c to
4f23d53
Compare
Signed-off-by: y-rabie <[email protected]>
…troller Signed-off-by: y-rabie <[email protected]>
Signed-off-by: y-rabie <[email protected]>
Signed-off-by: y-rabie <[email protected]>
zhaohuabing
force-pushed
the
fix-statuses-with-multiple-parents
branch
from
4f23d53 to
4e4e846
Compare
| Namespace: namespace, | ||
| Name: string(p.Name), | ||
| } | ||
| if _, ok := gatewayMap[gwNN]; ok && (p.Kind == nil || *p.Kind == resource.KindGateway) { |
| }, | ||
| }, | ||
| { | ||
| ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", |
There was a problem hiding this comment.
@arkodg This status has been removed because this PR merge the status across multiple translation and update it only once at the end with the combined result.
2 participants
What type of PR is this?
fix: cleanup dangling parents in route and policies statuses
What this PR does / why we need it:
This is a re-iteration on my previous pr #6812. Changes in that pr was reverted later in #7068 because I didn't account for routes that have multiple parents of different
gatewayclasses.The code currently sends an incremental route status update for every
gatewayclassit processes, not a full one of all gateways (of all gatewayclasses) the route is attached to.This is resolved here for all kinds of routes and policies. The code now processes and accumulates the translation status results, then at the very end sends an atomic status update with all parents/ancestors status conditions.
Changes in #7068 are reverted back.
E2e tests have been added for same-gatewayclass parents and multi-gatewayclasses parents.
There's one more nit to go as I mentioned before, which is when a route reference one of our gateways, and then no longer reference us at all. It'll have its parent status dangling forever. I have the fix for that in progress but it's more involved, so for a later PR.
Which issue(s) this PR fixes:
Fixes #6933 #774 #656 #6125 #6175
Release Notes: Yes/No