Skip to content

fix: support status reporting and stale status cleanup for GRPCRoute, TCPRoute, and TLSRoute#14294

Merged
davidjumani merged 6 commits into
kgateway-dev:mainfrom
ApurveKaranwal:fix/route-status-cleanup-grpc-tcp-tls
Jul 15, 2026
Merged

fix: support status reporting and stale status cleanup for GRPCRoute, TCPRoute, and TLSRoute#14294
davidjumani merged 6 commits into
kgateway-dev:mainfrom
ApurveKaranwal:fix/route-status-cleanup-grpc-tcp-tls

Conversation

@ApurveKaranwal

@ApurveKaranwal ApurveKaranwal commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

  • Motivation: kgateway was not cleaning up stale status reports for detached/orphaned GRPCRoute, TCPRoute, and TLSRoute resources when their parent references were removed, leaving outdated status entries. Additionally, version conversions for TLSRoute were dropping the Status field, preventing status markers from checking existing status.
  • What changed:
    1. Extended krt.StatusCollection tracking in RoutesIndex (policy.go) to GRPCRoute, TCPRoute, and TLSRoute.
    2. Implemented Process*StatusMarkers for GRPCRoute, TCPRoute, and TLSRoute to generate empty status reports to clear stale status entries for detached routes.
    3. Preserved the Status field during API version promotions (v1 -> v1alpha2 and v1alpha3 -> v1alpha2) inside tlsroute_version.go.
    4. Added thorough unit test coverage in policy_test.go and tlsroute_version_test.go to verify correct behavior.

Change Type

/kind fix

Changelog

Fix route status reporting and ensure stale status is cleared for GRPCRoute, TCPRoute, and TLSRoute resources.

Copilot AI review requested due to automatic review settings June 23, 2026 16:27
@gateway-bot gateway-bot added do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. kind/fix Categorizes issue or PR as related to a bug. labels Jun 23, 2026
… TCPRoute, and TLSRoute

Signed-off-by: ApurveKaranwal <[email protected]>
@ApurveKaranwal
ApurveKaranwal force-pushed the fix/route-status-cleanup-grpc-tcp-tls branch from 6c25db8 to 8171154 Compare June 23, 2026 16:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes stale/orphaned status handling for non-HTTP Gateway API routes by extending the status-marker mechanism to GRPCRoute, TCPRoute, and TLSRoute, and by ensuring TLSRoute version normalization preserves Status so marker-based cleanup can work reliably. This fits into kgateway’s status reporting pipeline by making the ProxySyncer’s merged report generation capable of explicitly clearing outdated Route status entries.

Changes:

  • Extended RoutesIndex to track status markers and IR collections for GRPCRoute/TCPRoute/TLSRoute, and to emit “empty” reports for marked routes that weren’t translated (stale cleanup).
  • Updated route transforms to produce status markers when existing Route status contains this controller’s ControllerName.
  • Preserved TLSRoute Status.RouteStatus during v1 -> v1alpha2 conversion and added/updated unit tests validating the behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/pluginsdk/collections/tlsroute_version.go Preserve TLSRoute Status.RouteStatus when converting gwv1.TLSRoute to gwv1a2.TLSRoute.
pkg/pluginsdk/collections/tlsroute_version_test.go Add assertions ensuring TLSRoute status survives version conversion.
pkg/krtcollections/policy.go Add GRPC/TCP/TLS status-marker collections, marker processing, and update route transforms to return markers.
pkg/krtcollections/policy_test.go Add unit tests covering empty-report generation, preservation of existing reports, and marker collection inclusion for stale routes.
pkg/krtcollections/grpc_route.go Update GRPCRoute transform to return a status marker when existing status matches controller.
pkg/kgateway/proxy_syncer/proxy_syncer.go Invoke GRPC/TCP/TLS marker processing during merged status report generation (stale cleanup path).

@gateway-bot gateway-bot added release-note and removed do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 23, 2026
@ApurveKaranwal

Copy link
Copy Markdown
Contributor Author

@davidjumani sir, please retrigger the CI checks.

Comment thread pkg/krtcollections/policy.go Outdated
}
}
return h.httpRoutes.HasSynced() && h.routes.HasSynced() && h.policies.HasSynced() && h.backends.HasSynced() && h.refgrants.HasSynced()
return h.httpRoutes.HasSynced() && h.grpcRoutes.HasSynced() && h.tcpRoutes.HasSynced() && h.tlsRoutes.HasSynced() && h.routes.HasSynced() && h.policies.HasSynced() && h.backends.HasSynced() && h.refgrants.HasSynced()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You dont need these hassynced and the collections, these are already added to hassync at L1162. HTTP is special because its standalone IR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alexliu541 sir! Reverted the redundant HasSynced checks since all these route collections are joined into h.routes (which is already checked in the return statement), making the individual checks unnecessary.

rp.Route(status.Obj)
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lots of repeated code here, can we reuse? something like

// ProcessRouteStatusMarkers adds empty status in the report map for any marked route (HTTP, TCP,
// TLS, GRPC) that has no status reported. Used for clearing stale status for orphaned routes.
func (r *RoutesIndex) ProcessRouteStatusMarkers(kctx krt.HandlerContext, reportMap reports.ReportMap) {
	rp := reports.NewReporter(&reportMap)
	processRouteStatusMarkers(kctx, r.httpRouteStatusMarkers, reportMap.HTTPRoutes, rp)
	processRouteStatusMarkers(kctx, r.tcpRouteStatusMarker, reportMap.TCPRoutes, rp)
	processRouteStatusMarkers(kctx, r.tlsRouteStatusMarker, reportMap.TLSRoutes, rp)
	processRouteStatusMarkers(kctx, r.grpcRouteStatusMarker, reportMap.GRPCRoutes, rp)
}

// processRouteStatusMarkers fetches the route status markers and adds empty status for routes not
// in the report map to clear stale status.
func processRouteStatusMarkers[T controllers.Object](
	kctx krt.HandlerContext,
	statusCol krt.StatusCollection[T, StatusMarker],
	reportMap map[types.NamespacedName]*reports.RouteReport,
	rp reporter.Reporter,
) {
	objStatus := krt.Fetch(kctx, statusCol)
	for _, status := range objStatus {
		routeKey := types.NamespacedName{
			Namespace: status.Obj.GetNamespace(),
			Name:      status.Obj.GetName(),
		}

		// Add empty status to clear stale status for routes with no valid ParentRefs
		if reportMap[routeKey] == nil {
			_ = rp.Route(status.Obj)
		}
	}
}

Then proxysyncer can just call ProcessRouteStatusMarkers, don't even need the getter for the statusmarkers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had these locally for quite sometime, got carried away by other things lol

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion @alexliu541 sir! I consolidated the status marker processing into a generic helper function (processRouteStatusMarkers), removed all the individual getters, and exposed a single ProcessRouteStatusMarkers(kctx, reportMap) method on RoutesIndex. The proxy syncer has been simplified to use this single call, and the unit tests have been updated and simplified to match.

@davidjumani
davidjumani enabled auto-merge July 13, 2026 19:07

@alexliu541 alexliu541 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick response, one more minor improvement

Comment thread pkg/krtcollections/policy.go Outdated
tcpRoutesCollection := krt.NewCollection(tcproutes, func(kctx krt.HandlerContext, i *gwv1a2.TCPRoute) *RouteWrapper {
t := h.transformTcpRoute(kctx, i)
return &RouteWrapper{Route: t}
h.grpcRouteStatusMarkers, h.grpcRoutes = krt.NewStatusCollection(grpcroutes, func(kctx krt.HandlerContext, i *gwv1.GRPCRoute) (*StatusMarker, *ir.HttpRouteIR) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

after the hassync is removed, I dont think you need the h.grpcRoutes, just local var for h.routes collection should be enough. something like

	var tcpRoutesCollection, tlsRoutesCollection, grpcRoutesCollection krt.Collection[RouteWrapper]
	h.tcpRouteStatusMarker, tcpRoutesCollection = krt.NewStatusCollection(tcproutes, func(kctx krt.HandlerContext, i *gwv1a2.TCPRoute) (*StatusMarker, *RouteWrapper) {
		status, route := h.transformTcpRoute(kctx, i, controllerName)
		return status, &RouteWrapper{Route: route}
	}, krtopts.ToOptions("routes-tcp-routes-with-policy")...)

	h.tlsRouteStatusMarker, tlsRoutesCollection = krt.NewStatusCollection(tlsroutes, func(kctx krt.HandlerContext, i *gwv1a2.TLSRoute) (*StatusMarker, *RouteWrapper) {
		status, route := h.transformTlsRoute(kctx, i, controllerName)
		return status, &RouteWrapper{Route: route}
	}, krtopts.ToOptions("routes-tls-routes-with-policy")...)

	h.grpcRouteStatusMarker, grpcRoutesCollection = krt.NewStatusCollection(grpcroutes, func(kctx krt.HandlerContext, i *gwv1.GRPCRoute) (*StatusMarker, *RouteWrapper) {
		status, route := h.transformGRPCRoute(kctx, i, controllerName)
		return status, &RouteWrapper{Route: route}
	}, krtopts.ToOptions("routes-grpc-routes-with-policy")...)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alexliu541 sir! I've simplified it exactly as suggested. I removed the redundant grpcRoutes, tcpRoutes, and tlsRoutes fields from RoutesIndex, combined the status and wrapper collection creation into a single NewStatusCollection call, and used local variables for the collections.

auto-merge was automatically disabled July 14, 2026 05:44

Head branch was pushed to by a user without write access

@alexliu541 alexliu541 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks for the PR!

@ApurveKaranwal

Copy link
Copy Markdown
Contributor Author

hey @davidjumani sir, please retrigger the CI checks.

@davidjumani
davidjumani enabled auto-merge July 14, 2026 14:23
@davidjumani
davidjumani added this pull request to the merge queue Jul 14, 2026
@davidjumani
davidjumani removed this pull request from the merge queue due to a manual request Jul 14, 2026
@davidjumani
davidjumani added this pull request to the merge queue Jul 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 14, 2026
@davidjumani
davidjumani added this pull request to the merge queue Jul 14, 2026
@davidjumani
davidjumani removed this pull request from the merge queue due to a manual request Jul 14, 2026
@davidjumani
davidjumani added this pull request to the merge queue Jul 15, 2026
Merged via the queue into kgateway-dev:main with commit 5e9f2b9 Jul 15, 2026
42 of 43 checks passed
sheidkamp pushed a commit to sheidkamp/kgateway that referenced this pull request Jul 16, 2026
… TCPRoute, and TLSRoute (kgateway-dev#14294)

Signed-off-by: ApurveKaranwal <[email protected]>
Co-authored-by: David Jumani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/fix Categorizes issue or PR as related to a bug. release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants