xDS: adds a callback for resource unsubscription to the xDS config tracker by birenroy · Pull Request #45898 · envoyproxy/envoy

birenroy · 2026-06-30T14:17:28Z

This conveys information about resources for which the Envoy takes client-side action to unsubscribe, and there is no server-side removal transmitted. This closes a small gap in xDS visibility for config tracker implementations.

I know the implementation in notifyUnsubscribedResources() is potentially expensive, but it is similar to the existing removeResourcesFromCache(), and only affects cases where xds_config_tracker_ is present. I have another PR in progress to improve the efficiency of both cases.

This PR was generated with the assistance of the Gemini AI model. I have read and understand the code diffs, and have made some manual tweaks.

Commit Message: adds a callback for resource unsubscription to the xDS config tracker
Additional Description:
Risk Level: low
Testing: ran unit and integration tests locally
Docs Changes:
Release Notes:
Platform Specific Features:

repokitteh-read-only · 2026-06-30T14:17:34Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #45898 was opened by birenroy.

see: more, trace.

birenroy · 2026-06-30T14:35:10Z

/assign @adisuissa

adisuissa · 2026-06-30T15:38:24Z

            return resource_name;
          });
+      // Computes the removed resources.
+      std::set<std::string> removed_resources;


will it make sense to only compute the diffs if eds_resources_cache_ is used or there is an xds_config_tracker_ defined?

adisuissa · 2026-06-30T15:39:33Z

+        return;
+      }
+      std::vector<absl::string_view> truly_removed;
+      for (const auto& resource_name : resources_to_remove) {


please add a comment that this filtering is because some resources weren't subscribed to previously.

I'm not sure I understand the proposed comment. I thought this code is trying to determine the set of resources that are no longer present in any watch.

I believe the idea is that if our Envoy receives a SRDS removal, for example, then it will drop a corresponding RDS subscription. The RDS object was subscribed to, but as a "dependent" resource, removal happens on the client side, without the xDS server explicitly indicating a removal.

adisuissa · 2026-06-30T15:40:24Z

+          if (watch == this) {
+            continue;
+          }
+          if (watch->resources_.find(resource_name) != watch->resources_.end()) {


Does this work for wildcard resources (say CDS)? IS there a test for it?

I added tests showing that wildcard resources don't get quite the same behavior. I think this is fine for now, unless someone needs the wildcard-derived resources tracked.

adisuissa · 2026-06-30T15:42:16Z

+
+  // 6. Verify tracker callback was called.
+  test_server_->waitForCounter("test_xds_tracker.on_resource_unsubscribed", Eq(1));
+  EXPECT_EQ(1, test_server_->counter("test_xds_tracker.on_resource_unsubscribed")->value());


Shouldn't this be equal to 2 (one for the cluster and one for the endpoint)?

The cluster is explicitly removed (as per the comment for step 5). Explicit server-side removals are already visible to an xDS config tracker, and do not need to be communicated through onResourceUnsubscribed().

birenroy

Thanks for the review, please take another look.

birenroy · 2026-06-30T18:15:56Z

            return resource_name;
          });
+      // Computes the removed resources.
+      std::set<std::string> removed_resources;


birenroy · 2026-06-30T19:08:47Z

+        return;
+      }
+      std::vector<absl::string_view> truly_removed;
+      for (const auto& resource_name : resources_to_remove) {


I'm not sure I understand the proposed comment. I thought this code is trying to determine the set of resources that are no longer present in any watch.

I believe the idea is that if our Envoy receives a SRDS removal, for example, then it will drop a corresponding RDS subscription. The RDS object was subscribed to, but as a "dependent" resource, removal happens on the client side, without the xDS server explicitly indicating a removal.

birenroy · 2026-06-30T19:21:10Z

+
+  // 6. Verify tracker callback was called.
+  test_server_->waitForCounter("test_xds_tracker.on_resource_unsubscribed", Eq(1));
+  EXPECT_EQ(1, test_server_->counter("test_xds_tracker.on_resource_unsubscribed")->value());


The cluster is explicitly removed (as per the comment for step 5). Explicit server-side removals are already visible to an xDS config tracker, and do not need to be communicated through onResourceUnsubscribed().

birenroy · 2026-07-01T00:09:37Z

+          if (watch == this) {
+            continue;
+          }
+          if (watch->resources_.find(resource_name) != watch->resources_.end()) {


I added tests showing that wildcard resources don't get quite the same behavior. I think this is fine for now, unless someone needs the wildcard-derived resources tracked.

…legate Add integration tests for XdsConfigTracker and XdsResourcesDelegate in both SotW (GrpcMuxImpl) and Delta (NewGrpcMuxImpl) implementations. Verify that: - onConfigAccepted/onConfigRejected are called on XdsConfigTracker. - getResources/onConfigUpdated/onResourceLoadFailed are called on XdsResourcesDelegate (SotW only). TAG=agy CONV=b942214e-2f01-4571-9478-b3eda0877a4c Signed-off-by: Biren Roy <birenroy@google.com>

…d mocks Move MockXdsConfigTracker and MockXdsResourcesDelegate from local test files to test/mocks/config/mocks.h to allow reuse and avoid duplication. Update BUILD dependencies accordingly. TAG=agy CONV=b942214e-2f01-4571-9478-b3eda0877a4c Signed-off-by: Biren Roy <birenroy@google.com>

Signed-off-by: Biren Roy <birenroy@google.com>

…tests Signed-off-by: Biren Roy <birenroy@google.com>

…ption Signed-off-by: Biren Roy <birenroy@google.com>

birenroy · 2026-07-02T01:10:41Z

Please review #45930 first; it adds some unit test coverage for existing code.

birenroy marked this pull request as ready for review June 30, 2026 14:35

repokitteh-read-only Bot assigned adisuissa Jun 30, 2026

adisuissa reviewed Jun 30, 2026

View reviewed changes

birenroy commented Jul 1, 2026

View reviewed changes

birenroy added 14 commits July 1, 2026 20:14

Adds a callback for resource unsubscription to the xDS config tracker.

7d46895

Signed-off-by: Biren Roy <birenroy@google.com>

Fixed up the integration test.

aecfd2a

Signed-off-by: Biren Roy <birenroy@google.com>

Added an integration test, fixed format.

d1210d8

Signed-off-by: Biren Roy <birenroy@google.com>

Minor simplifications.

c5425b6

Signed-off-by: Biren Roy <birenroy@google.com>

Makes a comment more concise.

372bc33

Signed-off-by: Biren Roy <birenroy@google.com>

Small cleanup.

8eaa07a

Signed-off-by: Biren Roy <birenroy@google.com>

Adds some comments.

ffeb4ae

Signed-off-by: Biren Roy <birenroy@google.com>

Addressed feedback and gcc failure.

1908814

Signed-off-by: Biren Roy <birenroy@google.com>

Added more tests.

72cf61f

Signed-off-by: Biren Roy <birenroy@google.com>

Fixed ASAN test failures.

2855f7e

Signed-off-by: Biren Roy <birenroy@google.com>

Merge remote-tracking branch 'upstream/main' into xds-config-tracker-…

428ee25

…tests Signed-off-by: Biren Roy <birenroy@google.com>

Merge branch 'xds-config-tracker-tests' into watch-resource-unsubscri…

7fd4841

…ption Signed-off-by: Biren Roy <birenroy@google.com>

birenroy force-pushed the watch-resource-unsubscription branch from 4f89e64 to 7fd4841 Compare July 2, 2026 01:00

birenroy mentioned this pull request Jul 2, 2026

config: adds GrpcMux tests with xds config tracker and resources delegate #45930

Open

Uh oh!

Conversation

birenroy commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only Bot commented Jun 30, 2026

Uh oh!

birenroy commented Jun 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birenroy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birenroy commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

birenroy commented Jun 30, 2026 •

edited

Loading