-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[license check] await green elasticsearch plugin #33584
Conversation
Pinging @elastic/kibana-operations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Not sure if the failures were due to this PR, but could use a merge with upstream |
@@ -32,12 +32,6 @@ export function setupXPackMain(server) { | |||
} | |||
}; | |||
|
|||
// trigger an xpack info refresh whenever the elasticsearch plugin status changes | |||
server.plugins.elasticsearch.status.on('change', async () => { | |||
await info.refreshNow(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure one of the reasons this being done is in case you change you license and then restart your cluster, not that it's a good way to cover that scenario. License info comes back in some sort of signature with each request now, right? Is that why we don't need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add it back in if we want, I removed it because it overlaps with https://github.com/elastic/kibana/pull/33584/files#diff-577f99d7103576efa3d57d5e26ec3fb5R138 now. It may fire a little quicker if we leave it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not picky either way, just want to make sure that this wasn't the only mechanism we had to refresh our cache when the license is updated in es.
This comment has been minimized.
This comment has been minimized.
💔 Build Failed |
The license check currently polls every 30 seconds regardless of elasticsearch status. This awaits a green elasticsearch plugin before sending a new request.