fix: retrigger reconciliation when backendRef of type ServiceImport is updated by nareddyt · Pull Request #5461 · envoyproxy/gateway

nareddyt · 2025-03-10T23:49:25Z

Problem

Currently, when a HTTPRoute references a ServiceImport, a new reconcile is NOT triggered when:

The ServiceImport is updated.
EndpointSlices that are part of the ServiceImport are updated.

Fix

This PR adds ServiceImport to the backendHTTPRouteIndexFunc, ensuring a reconcile is triggered when ServiceImport changes or the underlying EndpointSlices change.

Testing Done

Add new test case under TestValidateEndpointSliceForReconcile. It was failing before the code change, but now it passes.
Needed some minor refactoring of test setup code to support ServiceImport backendRef.

Additional Info

Fixes #5258

Release Notes: Yes

…updated Signed-off-by: Teju Nareddy <tejunareddy@gmail.com>

arkodg

LGTM thanks !

arkodg · 2025-03-11T21:29:44Z

seeing some test failures @nareddyt

# github.com/envoyproxy/gateway/internal/provider/kubernetes [github.com/envoyproxy/gateway/internal/provider/kubernetes.test]
Error: internal/provider/kubernetes/kubernetes_test.go:1435:75: too many arguments in call to test.GetHTTPRoute
	have ("k8s.io/apimachinery/pkg/types".NamespacedName, string, "k8s.io/apimachinery/pkg/types".NamespacedName, number, string)
	want ("k8s.io/apimachinery/pkg/types".NamespacedName, string, "sigs.k8s.io/gateway-api/apis/v1".BackendObjectReference, string)
Error: internal/provider/kubernetes/kubernetes_test.go:1445:83: too many arguments in call to test.GetHTTPRoute

muwaqar

left a nit comment for clarity in release notes.

Signed-off-by: Teju Nareddy <tejunareddy@gmail.com>

# Conflicts: # release-notes/current.yaml

nareddyt · 2025-03-12T22:03:56Z

seeing some test failures @nareddyt

# github.com/envoyproxy/gateway/internal/provider/kubernetes [github.com/envoyproxy/gateway/internal/provider/kubernetes.test]
Error: internal/provider/kubernetes/kubernetes_test.go:1435:75: too many arguments in call to test.GetHTTPRoute
	have ("k8s.io/apimachinery/pkg/types".NamespacedName, string, "k8s.io/apimachinery/pkg/types".NamespacedName, number, string)
	want ("k8s.io/apimachinery/pkg/types".NamespacedName, string, "sigs.k8s.io/gateway-api/apis/v1".BackendObjectReference, string)
Error: internal/provider/kubernetes/kubernetes_test.go:1445:83: too many arguments in call to test.GetHTTPRoute

Thanks, my IDE was misconfigured and did not compile these files locally because they are integration tests. Fixed

codecov · 2025-03-12T22:11:46Z

Codecov Report

Attention: Patch coverage is 4.34783% with 22 lines in your changes missing coverage. Please review.

Project coverage is 65.20%. Comparing base (bfd239b) to head (2cf700e).
Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/provider/kubernetes/test/utils.go	0.00%	22 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5461      +/-   ##
==========================================
- Coverage   65.31%   65.20%   -0.11%     
==========================================
  Files         213      213              
  Lines       33942    33957      +15     
==========================================
- Hits        22168    22141      -27     
- Misses      10447    10484      +37     
- Partials     1327     1332       +5

☔ 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.

nareddyt · 2025-03-12T23:02:45Z

/retest

nareddyt · 2025-03-12T23:03:04Z

Codecov Report

Attention: Patch coverage is 4.34783% with 22 lines in your changes missing coverage. Please review.

Project coverage is 65.20%. Comparing base (bfd239b) to head (2cf700e).

Files with missing lines Patch % Lines
internal/provider/kubernetes/test/utils.go 0.00% 22 Missing ⚠️
❌ Your patch check has failed because the patch coverage (4.34%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

Not sure how to fix this. These lines of code are definitely covered

arkodg · 2025-03-12T23:32:56Z

yeah the coverage failure is fine here @nareddyt, there's a test in this PR

…s updated (envoyproxy#5461) * fix: retrigger reconilation when backendRef of type ServiceImport is updated Signed-off-by: Teju Nareddy <tejunareddy@gmail.com> (cherry picked from commit e2f8978) Signed-off-by: Guy Daich <guy.daich@sap.com>

* load BackendTLSPolicy in standalone mode (#5431) Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 4d914ae) Signed-off-by: Guy Daich <guy.daich@sap.com> * Wasm: cache Wasm OCI image permission check results (#5358) * add TTL for wasm permission check Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * change Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * refresh the cache Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * purge the cache Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * refactor Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * on retry on retriable errors Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * add release note Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> (cherry picked from commit 672de8a) Signed-off-by: Guy Daich <guy.daich@sap.com> * Load EnvoyExtensionPolicy in standalone mode (#5460) * load EnvoyExtensionPolicy in standalone mode Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * more Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * release note Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * review: use a valid target name instead of myapp Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> * gen Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> (cherry picked from commit 4be098d) Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: check for mirror backendRef in httproute index (#5497) * check for mirror backendRef Signed-off-by: mark winter <mark.winter@thetradedesk.com> (cherry picked from commit 72b72c4) Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: dont return an err when gatewayclass is not accepted (#5524) * bug: dont return an err when gatewayclass is not accepted this is a user generated error, we shouldnt log it as a system error, and return with an error Signed-off-by: Arko Dasgupta <arko@tetrate.io> * release notes Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> (cherry picked from commit 51e87ca) Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: host header should not be allowed to modify (#5533) * host header is not allowed to be modified Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * address comment Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> (cherry picked from commit 54efa34) Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: retrigger reconciliation when backendRef of type ServiceImport is updated (#5461) * fix: retrigger reconilation when backendRef of type ServiceImport is updated Signed-off-by: Teju Nareddy <tejunareddy@gmail.com> (cherry picked from commit e2f8978) Signed-off-by: Guy Daich <guy.daich@sap.com> * pin envoy and ratelimit Signed-off-by: Guy Daich <guy.daich@sap.com> * fix: otel sink json access logging without text field (#5498) * fix otel sink json access logging without text field Signed-off-by: Tomi Juntunen <tomi.juntunen@iki.fi> * use json format as default when format or type is not set Signed-off-by: Tomi Juntunen <tomi.juntunen@iki.fi> * set formatters only if the slice of formatters is not empty Signed-off-by: Tomi Juntunen <tomi.juntunen@iki.fi> --------- Signed-off-by: Tomi Juntunen <tomi.juntunen@iki.fi> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> (cherry picked from commit cb3ffd2) Signed-off-by: Guy Daich <guy.daich@sap.com> * [release/v1.3] v1.3.2 release notes (#5584) v1.3.2 release notes Signed-off-by: Guy Daich <guy.daich@sap.com> --------- Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Guy Daich <guy.daich@sap.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com> Signed-off-by: mark winter <mark.winter@thetradedesk.com> Signed-off-by: Teju Nareddy <tejunareddy@gmail.com> Signed-off-by: Tomi Juntunen <tomi.juntunen@iki.fi> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Takeshi Yoneda <t.y.mathetake@gmail.com> Co-authored-by: Mark Winter <wintermarkedward@gmail.com> Co-authored-by: Teju Nareddy <tejunareddy@gmail.com> Co-authored-by: Tomi Juntunen <tomi.juntunen@iki.fi>

nareddyt requested a review from a team as a code owner March 10, 2025 23:49

fix: retrigger reconilation when backendRef of type ServiceImport is …

c4750e7

…updated Signed-off-by: Teju Nareddy <tejunareddy@gmail.com>

nareddyt force-pushed the fix-serviceimport-index branch from 8cc5b98 to c4750e7 Compare March 10, 2025 23:53

nareddyt changed the title ~~fix: retrigger reconilation when backendRef of type ServiceImport is updated~~ fix: retrigger reconciliation when backendRef of type ServiceImport is updated Mar 10, 2025

arkodg previously approved these changes Mar 11, 2025

View reviewed changes

muwaqar reviewed Mar 12, 2025

View reviewed changes

Comment thread release-notes/current.yaml Outdated

muwaqar approved these changes Mar 12, 2025

View reviewed changes

nareddyt dismissed arkodg’s stale review via 11921bf March 12, 2025 22:00

review comments, fix test

2e59c34

Signed-off-by: Teju Nareddy <tejunareddy@gmail.com>

nareddyt force-pushed the fix-serviceimport-index branch from 11921bf to 2e59c34 Compare March 12, 2025 22:01

Merge branch 'main' into fix-serviceimport-index

2cf700e

# Conflicts: # release-notes/current.yaml

nareddyt requested a review from arkodg March 12, 2025 22:03

wenfengp approved these changes Mar 12, 2025

View reviewed changes

arkodg requested review from a team March 12, 2025 23:32

zirain approved these changes Mar 13, 2025

View reviewed changes

arkodg merged commit e2f8978 into envoyproxy:main Mar 13, 2025

nareddyt deleted the fix-serviceimport-index branch March 13, 2025 03:35

guydc added the cherrypick/release-v1.3.2 label Mar 21, 2025

arkodg mentioned this pull request Nov 25, 2025

Nominating nareddyt as a Reviewer #7598

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: retrigger reconciliation when backendRef of type ServiceImport is updated#5461

fix: retrigger reconciliation when backendRef of type ServiceImport is updated#5461
arkodg merged 3 commits intoenvoyproxy:mainfrom
nareddyt:fix-serviceimport-index

nareddyt commented Mar 10, 2025 •

edited

Loading

Uh oh!

arkodg left a comment

Uh oh!

arkodg commented Mar 11, 2025

Uh oh!

Uh oh!

muwaqar left a comment

Uh oh!

nareddyt commented Mar 12, 2025

Uh oh!

codecov Bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

nareddyt commented Mar 12, 2025

Uh oh!

nareddyt commented Mar 12, 2025

Codecov Report

Uh oh!

arkodg commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

nareddyt commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Testing Done

Additional Info

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

arkodg commented Mar 11, 2025

Uh oh!

Uh oh!

muwaqar left a comment

Choose a reason for hiding this comment

Uh oh!

nareddyt commented Mar 12, 2025

Uh oh!

codecov Bot commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nareddyt commented Mar 12, 2025

Uh oh!

nareddyt commented Mar 12, 2025

Codecov Report

Uh oh!

arkodg commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nareddyt commented Mar 10, 2025 •

edited

Loading

codecov Bot commented Mar 12, 2025 •

edited

Loading