Add ProjectResolver to RemoteClusterService constructor#133006
Merged
JeremyDahlgren merged 13 commits intoelastic:mainfrom Aug 28, 2025
Merged
Add ProjectResolver to RemoteClusterService constructor#133006JeremyDahlgren merged 13 commits intoelastic:mainfrom
ProjectResolver to RemoteClusterService constructor#133006JeremyDahlgren merged 13 commits intoelastic:mainfrom
Conversation
Follow up to work done in elastic#131894 to make RemoteClusterService multi-project aware. This task plumbs the ProjectResolver down into the RemoteClusterService from NodeConstruction. Resolves: ES-12572
Contributor
Author
|
There are elasticsearch multi-project tests that are not using the default Example test: Test config: |
ywangd
reviewed
Aug 26, 2025
Member
ywangd
left a comment
There was a problem hiding this comment.
For the YAML test failures, the short answer is that we can mute them in the associated build.gradle file. Please also add a comment to explain why the tests need to be muted.
The issue is that settings update consumers are invoked in a context without project. How to notify components for per-project setting changes is still an open issue (ES-11463). We might be able to fix the tests once the issue is resolved. Technically it may be out of scope for CPS if it only supports config via ProjectCustom. In that case, these tests will simply be incompatible with multi-project. If the same test coverage is critical, we (the ES team not just distrib-coord) will need to add a version that uses the ProjectCustom so that it can be tested in MP setup on the serverless side.
Comment on lines
258
to
259
| // NOTE: Only for use in tests | ||
| public TransportService( |
Member
There was a problem hiding this comment.
Nit: Convention for this kind of comment is something like "public for testing"
Contributor
Author
I was running into another test failure (see below). Is the assertion too strict given the current state of the codebase in terms of multi-project support? In 69a704b I replaced the assertion with a |
Collaborator
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
Member
|
OK that's unfortunate but seems to be the best course of action right now. The reason is that ML code is larget not MP ready. More specifically, it should a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow up to work done in #131894 to make
RemoteClusterServicemulti-project aware. This task plumbs theProjectResolverdown into theRemoteClusterServicefromNodeConstruction.Resolves: ES-12572