srds: fix init_manager to use listener scope instead of server scope#43392
Conversation
When creating SRDS subscriptions, the SrdsFactory::createConfigProvider() was incorrectly using the server-level init_manager (via ServerFactoryContext::initManager()) instead of the listener-level init_manager. This caused SRDS subscriptions to never start when a listener with SRDS configuration was created after the server init_manager had already transitioned to the Initialized state (e.g., when using Delta xDS and the listener arrives in a subsequent LDS update after the initial empty response). The fix adds an Init::Manager& parameter to SrdsFactory::createConfigProvider() and passes the listener's init_manager from the HCM config, matching the behavior of RDS route config provider creation. Risk Level: Low Testing: Unit tests (config_test, scoped_rds_test), integration test Signed-off-by: jingze <jingze1115@gmail.com> Change-Id: I66484542678ef4b8b96a349f2769311a8aca4951 Co-developed-by: Cursor <noreply@cursor.com> Signed-off-by: jingze <daijingze.djz@alibaba-inc.com>
|
Hi @Jing-ze, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
Change-Id: Ibe9052bc6b41b4db8fd7f4eee0590435e3f1efdf Co-developed-by: Cursor <noreply@cursor.com> Signed-off-by: jingze <daijingze.djz@alibaba-inc.com>
|
/retest |
Change-Id: Id9004d29201c075028dfc3c65e745a577d2e5013 Co-developed-by: Cursor <noreply@cursor.com> Signed-off-by: jingze <daijingze.djz@alibaba-inc.com>
|
@alyssawilk @wbpcode Could you please help with the review when you have time? |
adisuissa
left a comment
There was a problem hiding this comment.
Thanks for fixing this bug! LGTM!
Please add a release note.
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @yanavlasov |
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @RyanTheOptimist |
Change-Id: I79b06cfe30b844a1ab3c8ce2ea7df6f12cb12eac Co-developed-by: Cursor <noreply@cursor.com> Signed-off-by: jingze <daijingze.djz@alibaba-inc.com>
Signed-off-by: Jingze <52855280+Jing-ze@users.noreply.github.com>
Release note has been added. Please take a look. |
|
/retest |
1 similar comment
|
/retest |
|
@RyanTheOptimist Please take a look. |
…nvoyproxy#43392) ### Description: PR envoyproxy#33192 refactored SRDS to use a `SrdsFactory` interface for decoupling, but inadvertently changed the `init_manager` passed to `ScopedRoutesConfigProviderUtil::create()` from the listener-level init_manager to the server-level init_manager. When a listener with SRDS arrives via LDS after the server init_manager has already reached the `Initialized` state, the SRDS init target is silently discarded in release builds, and the SRDS subscription never starts. The fix adds an `Init::Manager&` parameter to `SrdsFactory::createConfigProvider()` and passes the listener's init_manager from the HCM config constructor, restoring the correct behavior that existed before PR envoyproxy#33192 and matching how RDS handles init_manager propagation. AI-assisted: Claude opus-4.6 was used to help write the unit test and PR description. The fix itself and root cause analysis were done manually. ### Risk Level: Low ### Testing: - `//test/common/router:scoped_rds_test` PASSED - `//test/extensions/filters/network/http_connection_manager:config_test` PASSED - `//test/integration:scoped_rds_integration_test` PASSED - Higress kind cluster integration test with Delta xDS + SRDS PASSED ### Docs Changes: N/A ### Release Notes: N/A ### Platform Specific Features: N/A --------- Signed-off-by: jingze <daijingze.djz@alibaba-inc.com> Signed-off-by: Jingze <52855280+Jing-ze@users.noreply.github.com> Signed-off-by: bjmask <11672696+bjmask@users.noreply.github.com>
…nvoyproxy#43392) ### Description: PR envoyproxy#33192 refactored SRDS to use a `SrdsFactory` interface for decoupling, but inadvertently changed the `init_manager` passed to `ScopedRoutesConfigProviderUtil::create()` from the listener-level init_manager to the server-level init_manager. When a listener with SRDS arrives via LDS after the server init_manager has already reached the `Initialized` state, the SRDS init target is silently discarded in release builds, and the SRDS subscription never starts. The fix adds an `Init::Manager&` parameter to `SrdsFactory::createConfigProvider()` and passes the listener's init_manager from the HCM config constructor, restoring the correct behavior that existed before PR envoyproxy#33192 and matching how RDS handles init_manager propagation. AI-assisted: Claude opus-4.6 was used to help write the unit test and PR description. The fix itself and root cause analysis were done manually. ### Risk Level: Low ### Testing: - `//test/common/router:scoped_rds_test` PASSED - `//test/extensions/filters/network/http_connection_manager:config_test` PASSED - `//test/integration:scoped_rds_integration_test` PASSED - Higress kind cluster integration test with Delta xDS + SRDS PASSED ### Docs Changes: N/A ### Release Notes: N/A ### Platform Specific Features: N/A --------- Signed-off-by: jingze <daijingze.djz@alibaba-inc.com> Signed-off-by: Jingze <52855280+Jing-ze@users.noreply.github.com>
…nvoyproxy#43392) ### Description: PR envoyproxy#33192 refactored SRDS to use a `SrdsFactory` interface for decoupling, but inadvertently changed the `init_manager` passed to `ScopedRoutesConfigProviderUtil::create()` from the listener-level init_manager to the server-level init_manager. When a listener with SRDS arrives via LDS after the server init_manager has already reached the `Initialized` state, the SRDS init target is silently discarded in release builds, and the SRDS subscription never starts. The fix adds an `Init::Manager&` parameter to `SrdsFactory::createConfigProvider()` and passes the listener's init_manager from the HCM config constructor, restoring the correct behavior that existed before PR envoyproxy#33192 and matching how RDS handles init_manager propagation. AI-assisted: Claude opus-4.6 was used to help write the unit test and PR description. The fix itself and root cause analysis were done manually. ### Risk Level: Low ### Testing: - `//test/common/router:scoped_rds_test` PASSED - `//test/extensions/filters/network/http_connection_manager:config_test` PASSED - `//test/integration:scoped_rds_integration_test` PASSED - Higress kind cluster integration test with Delta xDS + SRDS PASSED ### Docs Changes: N/A ### Release Notes: N/A ### Platform Specific Features: N/A --------- Signed-off-by: jingze <daijingze.djz@alibaba-inc.com> Signed-off-by: Jingze <52855280+Jing-ze@users.noreply.github.com>
…nvoyproxy#43392) ### Description: PR envoyproxy#33192 refactored SRDS to use a `SrdsFactory` interface for decoupling, but inadvertently changed the `init_manager` passed to `ScopedRoutesConfigProviderUtil::create()` from the listener-level init_manager to the server-level init_manager. When a listener with SRDS arrives via LDS after the server init_manager has already reached the `Initialized` state, the SRDS init target is silently discarded in release builds, and the SRDS subscription never starts. The fix adds an `Init::Manager&` parameter to `SrdsFactory::createConfigProvider()` and passes the listener's init_manager from the HCM config constructor, restoring the correct behavior that existed before PR envoyproxy#33192 and matching how RDS handles init_manager propagation. AI-assisted: Claude opus-4.6 was used to help write the unit test and PR description. The fix itself and root cause analysis were done manually. ### Risk Level: Low ### Testing: - `//test/common/router:scoped_rds_test` PASSED - `//test/extensions/filters/network/http_connection_manager:config_test` PASSED - `//test/integration:scoped_rds_integration_test` PASSED - Higress kind cluster integration test with Delta xDS + SRDS PASSED ### Docs Changes: N/A ### Release Notes: N/A ### Platform Specific Features: N/A --------- Signed-off-by: jingze <daijingze.djz@alibaba-inc.com> Signed-off-by: Jingze <52855280+Jing-ze@users.noreply.github.com>
Description:
PR #33192 refactored SRDS to use a
SrdsFactoryinterface for decoupling, but inadvertently changed theinit_managerpassed toScopedRoutesConfigProviderUtil::create()from the listener-level init_manager to the server-level init_manager. When a listener with SRDS arrives via LDS after the server init_manager has already reached theInitializedstate, the SRDS init target is silently discarded in release builds, and the SRDS subscription never starts.The fix adds an
Init::Manager¶meter toSrdsFactory::createConfigProvider()and passes the listener's init_manager from the HCM config constructor, restoring the correct behavior that existed before PR #33192 and matching how RDS handles init_manager propagation.AI-assisted: Claude opus-4.6 was used to help write the unit test and PR description. The fix itself and root cause analysis were done manually.
Risk Level: Low
Testing:
//test/common/router:scoped_rds_testPASSED//test/extensions/filters/network/http_connection_manager:config_testPASSED//test/integration:scoped_rds_integration_testPASSEDDocs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A