Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Support delaying EIS authorization revocation until after the node has finished booting #122644

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

jonathan-buttner
Copy link
Contributor

This PR addresses an issue where attempting to revoke authorization was failing because the node's client was not initialized yet.

[2025-02-14T12:35:44,956][WARN ][o.e.x.i.s.e.ElasticInferenceService] [javaRestTest-0] Failed to revoke access to default inference endpoint IDs: [rainbow-sprinkles], error: java.lang.IllegalStateException: NodeClient has not been initialized

I tried moving the revocation to when we get the onNodeStarted() call but then I ran into issues where the index wasn't initialized yet at least locally.

So the solution I went with was to kick off the revoke call in the onNodeStarted() method but have it wait 10 minutes to hopefully give the node enough time to initialize everything.

I also changed some logging so it wasn't so noisy.

@jonathan-buttner jonathan-buttner marked this pull request as ready for review February 19, 2025 13:25
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@jonathan-buttner jonathan-buttner merged commit 4de8244 into elastic:main Feb 20, 2025
17 checks passed
jonathan-buttner added a commit to jonathan-buttner/elasticsearch that referenced this pull request Feb 20, 2025
…de has finished booting (elastic#122644)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java
jonathan-buttner added a commit to jonathan-buttner/elasticsearch that referenced this pull request Feb 20, 2025
…de has finished booting (elastic#122644)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java
@jonathan-buttner
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x
9.0
8.18

Questions ?

Please refer to the Backport tool documentation

jonathan-buttner added a commit to jonathan-buttner/elasticsearch that referenced this pull request Feb 20, 2025
…de has finished booting (elastic#122644)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java
elasticsearchmachine pushed a commit that referenced this pull request Feb 20, 2025
…de has finished booting (#122644) (#123029)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java
elasticsearchmachine pushed a commit that referenced this pull request Feb 20, 2025
… the node has finished booting (#122644) (#123030)

* [ML] Support delaying EIS authorization revocation until after the node has finished booting (#122644)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java

* Fixing getFirst call
elasticsearchmachine pushed a commit that referenced this pull request Feb 20, 2025
…the node has finished booting (#122644) (#123027)

* [ML] Support delaying EIS authorization revocation until after the node has finished booting (#122644)

* Refactoring authorization to happen after the node starts

* Adding delay for model registry call

* Fixing test

(cherry picked from commit 4de8244)

# Conflicts:
#	x-pack/plugin/inference/src/internalClusterTest/java/org/elasticsearch/xpack/inference/integration/InferenceRevokeDefaultEndpointsIT.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferencePlugin.java
#	x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceTests.java

* Fixing getFirst call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:GenAI Features around GenAI :ml Machine learning >non-issue Team:ML Meta label for the ML team v8.18.1 v8.19.0 v9.0.0 v9.0.1 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants