Prune stale HTTPRoutes when tags are removed from Ingress rules #894

kahirokunn · 2025-11-09T03:13:00Z

Add pruning in reconcileIngress to delete owned HTTPRoutes not referenced
Add unit test to verify stale route deletion behavior

/kind bug

Fixes #895

Release Note:

Fix a bug where HTTPRoutes could remain after tags were removed. Stale routes are now deleted during reconciliation.

knative-prow · 2025-11-09T03:13:02Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2025-11-09T03:26:12Z

Codecov Report

❌ Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.38%. Comparing base (510df45) to head (e51dd42).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/reconciler/ingress/ingress.go	66.66%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #894      +/-   ##
==========================================
- Coverage   76.45%   76.38%   -0.08%     
==========================================
  Files          17       17              
  Lines        1253     1266      +13     
==========================================
+ Hits          958      967       +9     
- Misses        261      263       +2     
- Partials       34       36       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kahirokunn · 2025-11-09T09:37:37Z

@dprotaso Hello! I'd be grateful if you could leave a review when you have a moment! 🙏

kahirokunn · 2025-11-23T04:58:12Z

@dprotaso Hello 🤚

dprotaso

Change looks great - thanks for finding the bug.

Just looking for a minor tweak

pkg/reconciler/ingress/ingress.go

dprotaso · 2025-11-24T22:05:52Z

We'll want this fix in earlier releases
/cherry-pick release-1.20
/cherry-pick release-1.19

knative-prow-robot · 2025-11-24T22:05:54Z

@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.19, release-1.20 in new PRs and assign them to you.

Details

In response to this:

We'll want this fix in earlier releases
/cherry-pick release-1.20
/cherry-pick release-1.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kahirokunn · 2025-11-25T00:00:24Z

/test integration-tests-envoy-gateway

kahirokunn · 2025-11-25T00:02:21Z

@dprotaso I've addressed all the points you raised. When you have a moment, I would appreciate it if you could take another look 🙏

dprotaso

just some minor stuff

also note the unit test is failing

pkg/reconciler/ingress/resources/httproute.go

dprotaso · 2025-11-25T01:30:19Z

pkg/reconciler/ingress/ingress_test.go


+// --- helpers for forcing lister errors in specific tests ---
+
+type httpRouteListErrorKey struct{}


You can drop the list error case - I don't believe it'll happen with client-go

Thx. I fixed it.

It was a test we added because Codecov was too strict. The Codecov check is failing, but can we merge it?
https://github.com/knative-extensions/net-gateway-api/pull/894/checks?check_run_id=56299777195

Signed-off-by: kahirokunn <[email protected]>

kahirokunn · 2025-11-25T04:31:21Z

/test integration-tests-envoy-gateway

kahirokunn · 2025-11-25T04:48:05Z

@dprotaso I've addressed all the points you raised. All tests have been passed. When you have a moment, I would appreciate it if you could take another look 🙏

dprotaso · 2025-11-25T13:10:34Z

/lgtm
/approve
/override "codecov/patch"

knative-prow · 2025-11-25T13:10:38Z

@dprotaso: Overrode contexts on behalf of dprotaso: codecov/patch

Details

In response to this:

/lgtm
/approve
/override "codecov/patch"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

knative-prow · 2025-11-25T13:10:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, kahirokunn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dprotaso]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dprotaso · 2025-11-25T13:11:42Z

thanks @kahirokunn 🎉

knative-prow-robot · 2025-11-25T13:12:14Z

@dprotaso: new pull request created: #901

Details

In response to this:

We'll want this fix in earlier releases
/cherry-pick release-1.20
/cherry-pick release-1.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

knative-prow-robot · 2025-11-25T13:12:51Z

@dprotaso: new pull request created: #902

Details

In response to this:

We'll want this fix in earlier releases
/cherry-pick release-1.20
/cherry-pick release-1.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

knative-prow bot added kind/bug Categorizes issue or PR as related to a bug. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Nov 9, 2025

knative-prow bot requested review from dprotaso and skonto November 9, 2025 03:13

knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2025

kahirokunn force-pushed the gc-tag-route branch from 1b9440c to 5bb793f Compare November 9, 2025 03:23

kahirokunn marked this pull request as ready for review November 9, 2025 03:23

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2025

kahirokunn force-pushed the gc-tag-route branch 2 times, most recently from 907365e to 0b76360 Compare November 9, 2025 03:49

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 9, 2025

kahirokunn force-pushed the gc-tag-route branch 2 times, most recently from 2c942ba to 31521b5 Compare November 9, 2025 04:45

dprotaso reviewed Nov 24, 2025

View reviewed changes

pkg/reconciler/ingress/ingress.go Outdated Show resolved Hide resolved

pkg/reconciler/ingress/ingress.go Outdated Show resolved Hide resolved

kahirokunn force-pushed the gc-tag-route branch from 31521b5 to 7e34adf Compare November 24, 2025 23:59

dprotaso reviewed Nov 25, 2025

View reviewed changes

🐛 prune stale HTTPRoutes when tags are removed

e51dd42

Signed-off-by: kahirokunn <[email protected]>

kahirokunn force-pushed the gc-tag-route branch from 7e34adf to e51dd42 Compare November 25, 2025 04:31

knative-prow bot assigned dprotaso Nov 25, 2025

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 25, 2025

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 25, 2025

knative-prow bot merged commit 1836d3f into knative-extensions:main Nov 25, 2025
32 of 33 checks passed

knative-prow-robot mentioned this pull request Nov 25, 2025

[release-1.20] Prune stale HTTPRoutes when tags are removed from Ingress rules #901

Merged

knative-prow-robot mentioned this pull request Nov 25, 2025

[release-1.19] Prune stale HTTPRoutes when tags are removed from Ingress rules #902

Merged


		// --- helpers for forcing lister errors in specific tests ---

		type httpRouteListErrorKey struct{}

Prune stale HTTPRoutes when tags are removed from Ingress rules #894

Prune stale HTTPRoutes when tags are removed from Ingress rules #894

Conversation

kahirokunn commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knative-prow bot commented Nov 9, 2025

Uh oh!

codecov bot commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kahirokunn commented Nov 9, 2025

Uh oh!

kahirokunn commented Nov 23, 2025

Uh oh!

dprotaso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dprotaso commented Nov 24, 2025

Uh oh!

knative-prow-robot commented Nov 24, 2025

Uh oh!

kahirokunn commented Nov 25, 2025

Uh oh!

kahirokunn commented Nov 25, 2025

Uh oh!

dprotaso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dprotaso Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kahirokunn Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kahirokunn Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kahirokunn commented Nov 25, 2025

Uh oh!

kahirokunn commented Nov 25, 2025

Uh oh!

dprotaso commented Nov 25, 2025

Uh oh!

knative-prow bot commented Nov 25, 2025

Uh oh!

knative-prow bot commented Nov 25, 2025

Uh oh!

Uh oh!

dprotaso commented Nov 25, 2025

Uh oh!

knative-prow-robot commented Nov 25, 2025

Uh oh!

knative-prow-robot commented Nov 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kahirokunn commented Nov 9, 2025 •

edited

Loading

codecov bot commented Nov 9, 2025 •

edited

Loading