Add BootstrapContext to expose settings and recovered state to bootstrap checks #26628
Conversation
This exposes the persistent part of the cluster state to the bootstrap checks to allow plugins to enforce certain preconditions based on the recovered state.
s1monw
added
:Core/Infra/Core
| * @param metaData the on-disk metadata for this nodes. This allows bootstrap-checks based on the locally recovered cluster metadata | ||
| */ | ||
| public List<BootstrapCheck> getBootstrapChecks() { return Collections.emptyList(); } | ||
| public List<BootstrapCheck> getBootstrapChecks(MetaData metaData) { return Collections.emptyList(); } |
There was a problem hiding this comment.
I think that I would rather see this pushed through at check execution time (it's not fair this is only exposed to plugin checks). We also have some checks that rely on settings and we do this at construction time instead of check time so maybe the change here to add a BootstrapCheckContext that carries settings and this metadata, check pushes this through to each check and they can make decisions on that basis?
There was a problem hiding this comment.
so my biggest concern here is the way bootstrap checks work today. we have a method that returns a boolean and a method that returns a message. This nature makes it very hard to give a good message bases on the state while if I have all information at construction time its simple to prepare the message. The settings is another issue since it might access settings that are coming from the keystore that should be accessed ahead of time (before we execute them) I think there are many issues here that I think we should address. The internal bootstrap checks can also get this information if needed we can change the protected node method?
There was a problem hiding this comment.
The return type of BootstrapCheck#check can easily be changed away from being a boolean?
There was a problem hiding this comment.
yeah I wonder if that is a bit too much of a refactoring give this goes into 6.0?
There was a problem hiding this comment.
I hear that concern and while it will be a lot of lines of code changed, I think it can be done quickly and is not a de-stabilizing change.
|
@jasontedor I pushed a new commit |
| * the transport protocol is bound to a non-loopback interface. | ||
| * | ||
| * @param settings the current node settings | ||
| * @param context the current node boostrap context |
s1monw
changed the title
…rap checks (#26628) This exposes the node settings and the persistent part of the cluster state to the bootstrap checks to allow plugins to enforce certain preconditions based on the recovered state.
…rap checks (#26628) This exposes the node settings and the persistent part of the cluster state to the bootstrap checks to allow plugins to enforce certain preconditions based on the recovered state.
|
I don't think this is a good change to make. The cluster state can be arbitrarily stale here, which means that the bootstrap check could fail for the on-disk cluster state but would pass for the cluster state of the master that this node will join. |
* master: (39 commits) [Docs] Correct typo in removal_of_types.asciidoc (elastic#26646) [Docs] "The the" is a great band, but ... (elastic#26644) Move all repository-azure classes under one single package (elastic#26624) [Docs] Update link in removal_of_types.asciidoc (elastic#26614) Fix percolator highlight sub fetch phase to not highlight query twice (elastic#26622) Refactor bootstrap check results and error messages Add BootstrapContext to expose settings and recovered state to bootstrap checks (elastic#26628) [Tests] Removing skipping tests in search rest tests Initialize checkpoint tracker with allocation ID Move non-core mappers to a module. (elastic#26549) [Docs] Clarify size parameter in Completion Suggester doc (elastic#26617) Add soft limit on allowed number of script fields in request (elastic#26598) Remove MapperService#dynamic. (elastic#26603) Fix incomplete sentences in parent-join docs (elastic#26623) More efficient encoding of range fields. (elastic#26470) Ensure module is bundled before installing in tests Add boolean similarity to built in similarity types (elastic#26613) [Tests] Remove skip tests in search/30_limits.yml Let search phases override max concurrent requests Add a soft limit for the number of requested doc-value fields (elastic#26574) ...
this might be ok for certain checks. Certain situations are checkable even with a stale clusterstate |
6 participants
This exposes the node settings and the persistent part of the cluster state to the
bootstrap checks to allow plugins to enforce certain preconditions based on the
recovered state.