Skip to content

Conversation

@nielsbauman
Copy link
Contributor

This does not make the health indicator project-aware, it merely avoids exceptions in case there are multiple projects in the cluster. The health indicator would require significant refactoring to be made project-aware, which is not worth it since ILM will not be running in a multi-project context (i.e. serverless).

This does _not_ make the health indicator project-aware, it merely
avoids exceptions in case there are multiple projects in the cluster.
The health indicator would require significant refactoring to be made
project-aware, which is not worth it since ILM will not be running in a
multi-project context (i.e. serverless).
@nielsbauman nielsbauman added >non-issue :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Jun 24, 2025
@elasticsearchmachine elasticsearchmachine added v9.1.0 Team:Data Management Meta label for data/management team labels Jun 24, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Contributor

@samxbr samxbr left a comment

Choose a reason for hiding this comment

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

Should we add a NotMultiProjectCapable annotation to this class once that is merged?

Copy link
Member

@PeteGillinElastic PeteGillinElastic left a comment

Choose a reason for hiding this comment

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

Just one comment. Thanks.

* This method solely exists because we are not making ILM properly project-aware and it's not worth the investment of altering this
* health indicator to be project-aware.
*/
private static ProjectMetadata getDefaultILMProject(ClusterState state) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to get your new 'not MP compatible' annotation?

I assume that adding an assertion runs into the same problems that we have in the other cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just added the annotation.

I assume that adding an assertion runs into the same problems that we have in the other cases?

Adding the assertion would lead to similar problems. Since this is not a cluster state listener, the impact is far less, but we still call the health report here. Even though it's just one test/test suite, I'm still not very comfortable with the idea of trying to modify the big XPackRestIT or this test (we could retrieve a specific indicator rather than the full health report). The former is a larger effort and the latter reduces test coverage IMO.

@nielsbauman nielsbauman enabled auto-merge (squash) June 25, 2025 13:36
@nielsbauman nielsbauman merged commit dbc8d74 into elastic:main Jun 27, 2025
32 checks passed
@nielsbauman nielsbauman deleted the ilm-health-mp branch June 27, 2025 14:20
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
…ic#129961)

This does _not_ make the health indicator project-aware, it merely
avoids exceptions in case there are multiple projects in the cluster.
The health indicator would require significant refactoring to be made
project-aware, which is not worth it since ILM will not be running in a
multi-project context (i.e. serverless).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM Index and Snapshot lifecycle management >non-issue Team:Data Management Meta label for data/management team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants