-
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
[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations #211847
[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations #211847
Conversation
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.
Did not test locally, just did a code review. Left a q about testing on cloud and I think we should also add hidden
to expand wildcards for extra safety in pickup up indices.
@@ -120,6 +124,7 @@ export function getEnterpriseSearchNodeDeprecation( | |||
'Enterprise Search is not supported in versions >= 9.x.\n\n' + | |||
'Please note the following:\n' + | |||
'- You must remove any Enterprise Search nodes from your deployment to proceed with the upgrade.\n' + | |||
'- You must also remove any Enterprise Search configuration elements in your Kibana config.\n' + |
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.
Should we list the enterprise search configuration elements? Or perhaps the namespace xpack.enterpriseSearch.*
?
Have we verified that this config can be removed by cloud users?
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.
Have we verified that this config can be removed by cloud users?
We tried... and this config cannot be removed by cloud users - these instructions only appear in self-managed.
} | ||
const entSearchDatastreams = await esClient.indices.getDataStream({ | ||
name: ENT_SEARCH_DATASTREAM_PATTERN.join(','), | ||
expand_wildcards: ['all'], |
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.
expand_wildcards: ['all'], | |
expand_wildcards: ['all' ,'hidden'], |
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.
Doesn't all
include hidden? Or is that only for getting indices?
@@ -24,9 +30,10 @@ export async function getPreEightEnterpriseSearchIndices( | |||
expand_wildcards: ['all'], |
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.
expand_wildcards: ['all'], | |
expand_wildcards: ['all', 'hidden'], |
existingIndex.hasDatastream = true; | ||
existingIndex.datastreams.push(datastream); | ||
} else { | ||
returnIndices.push({ | ||
name: lastIndexName, | ||
hasDatastream: true, | ||
datastreams: [datastream], |
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 think this makes sense: if the index is already picked up we make sure it's marked as having a data stream. Otherwise it's not been picked up and we add to the list of indices to make read-only.
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.
Wait there was one other thing that stood out to me, from a previous PR actually:
indexData.settings?.index?.version?.created?.startsWith('7') &&
indexData.settings?.index?.blocks?.write !== 'true'
I think we should remove the indexData.settings?.index?.blocks?.write !== 'true'
. It's technically not the thing we should be checking for. There's a special verified_read_only
setting.
But besides that, I think it's safe to do this action multiple times against the same indices.
'logs-enterprise_search.*', | ||
'logs-app_search.*', | ||
'logs-workplace_search.*', | ||
'logs-crawler-*', |
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.
Sorry for the late thoughts... I am a bit concerned about this pattern... Is it conceivable that users may have data streams NOT created/owned by enterprise search may match logs-crawler-*
?
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.
All of these are managed by Enterprise Search... The logs-crawler-*
items are for the App Search crawler, that is from 7.x onwards... the managed crawler has the logs-elastic_crawler-*
prefix.. .;)
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.
Did not test locally, thanks for addressing my feedback!
Just an FYI - I've removed the |
With the latest commit, I've moved the pre 8.x index deprecation pieces into the Kibana Upgrade Assistant plugin - as it's possible that if the Enterprise Search plugin is disabled, these may not show up |
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.
Great work @markjhoy , I think we're finally there haha. Left a few non-blocker comments.
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.
Would you mind adding a CODEOWNERS
entry like:
x-pack/platform/plugins/private/upgrade_assistant/server/lib/enterprise_search @elastic/search-kibana
@@ -8,11 +8,16 @@ | |||
import { ElasticsearchClient } from '@kbn/core/server'; | |||
|
|||
const ENT_SEARCH_INDEX_PREFIX = '.ent-search-'; | |||
const ENT_SEARCH_DATASTREAM_PATTERN = [ |
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'll update my PR to use these values directly 👌🏻
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.
WDYT about moving these into your ../lib/enterprise_search
folder? Since we're already doing something a little weird, having routes
dir in ../lib/enterprise_search/routes
seems OK?
💚 Build Succeeded
Metrics [docs]
History
|
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ic#211847) ## Summary The [previous PR](elastic#210688) for deprecating Enterprise Search indices before 8.x did not handle all of the necessary datastreams to be rolled over. This adds those. Also, the configuration check for `enterpriseSearch.customHeaders` has now been rolled into the Enterprise Search node removal and configuration check. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit bf8cfca)
…#211847) (#212285) # Backport This will backport the following commits from `8.x` to `8.18`: - [[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations (#211847)](#211847) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Mark J. Hoy","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-02-24T16:32:44Z","message":"[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations (#211847)\n\n## Summary\n\nThe [previous PR](#210688) for\ndeprecating Enterprise Search indices before 8.x did not handle all of\nthe necessary datastreams to be rolled over. This adds those. Also, the\nconfiguration check for `enterpriseSearch.customHeaders` has now been\nrolled into the Enterprise Search node removal and configuration check.\n\n### Checklist\n\n- [x] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"bf8cfca07c76ac27aed41560bf4a6f6a9db1dadb","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Upgrade Assistant","Team:EnterpriseSearch","v8.18.0","v8.19.0","backport:8.18"],"title":"[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations","number":211847,"url":"https://github.com/elastic/kibana/pull/211847","mergeCommit":{"message":"[8.x] Ensure Enterprise Search pre 8.x Datastream Deprecations (#211847)\n\n## Summary\n\nThe [previous PR](#210688) for\ndeprecating Enterprise Search indices before 8.x did not handle all of\nthe necessary datastreams to be rolled over. This adds those. Also, the\nconfiguration check for `enterpriseSearch.customHeaders` has now been\nrolled into the Enterprise Search node removal and configuration check.\n\n### Checklist\n\n- [x] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [x] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"bf8cfca07c76ac27aed41560bf4a6f6a9db1dadb"}},"sourceBranch":"8.x","suggestedTargetBranches":["8.18"],"targetPullRequestStates":[{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
…ions (elastic#211881) ## Summary Close elastic#211712 Related elastic#211847 (cherry picked from commit 4c42363) # Conflicts: # x-pack/platform/plugins/private/upgrade_assistant/server/lib/es_deprecations_status/index.test.ts
Summary
The previous PR for deprecating Enterprise Search indices before 8.x did not handle all of the necessary datastreams to be rolled over. This adds those. Also, the configuration check for
enterpriseSearch.customHeaders
has now been rolled into the Enterprise Search node removal and configuration check.Checklist
release_note:breaking
label should be applied in these situations.