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

Reindexing prepends to index name instead of appending #30114

Merged
merged 5 commits into from Feb 6, 2019

Conversation

Projects
None yet
4 participants
@tylersmalley
Copy link
Member

tylersmalley commented Feb 5, 2019

Similar to what is done in the Index Lifecycle Management, we prepend as to
avoid possible issues with conflicts in index patterns, templates, etc.

Reindexing prepends to index name instead of appending
Similar to what is done in the Index Lifecycle Management, we prepend as to
avoid possible issues with conflicts in index patterns, templates, etc.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine

This comment has been minimized.

Copy link

elasticmachine commented Feb 5, 2019

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 5, 2019

@tylersmalley

This comment has been minimized.

Copy link
Member Author

tylersmalley commented Feb 5, 2019

retest

@joshdover

This comment has been minimized.

Copy link
Member

joshdover commented Feb 5, 2019

I believe the hasRequiredPrivileges will need to be updated. It currently only requires these index permissions, (indexName is the original index name):

          {
            names: [`${indexName}*`],
            privileges: ['all'],
          },
@tylersmalley

This comment has been minimized.

Copy link
Member Author

tylersmalley commented Feb 5, 2019

I see - I thought we were passing it the target during the re-index and I missed the asterisk. Updating.

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 5, 2019

Preserve period for internal indices
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley

This comment has been minimized.

Copy link
Member Author

tylersmalley commented Feb 5, 2019

Based on a conversation in stack-upgrade, we're now preserving the leading period for internal indices.

@@ -64,11 +64,43 @@ describe('ReindexActions', () => {
});
});

it(`replaces -reindexed-v${PREV_MAJOR_VERSION} with -reindexed-v${CURRENT_MAJOR_VERSION}`, async () => {
await actions.createReindexOp(`myIndex-reindexed-v${PREV_MAJOR_VERSION}`);
it(`prepends reindexed-v${CURRENT_MAJOR_VERSION} to new name, preserving leading period`, async () => {

This comment has been minimized.

@tylersmalley

tylersmalley Feb 5, 2019

Author Member

Technically these tests are already covered in parseIndexName, but figure it won't hurt to leave in.

@@ -86,7 +87,7 @@ describe('reindexService', () => {
cluster: ['manage'],
index: [
{
names: [`anIndex*`],
names: ['anIndex', `reindexed-v${CURRENT_MAJOR_VERSION}-anIndex`],

This comment has been minimized.

@tylersmalley

tylersmalley Feb 5, 2019

Author Member

We're explicitly testing for privileges to the index and new index

Check privs on possible base index
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@joshdover
Copy link
Member

joshdover left a comment

LGTM, just a couple small things.

Addressed feedback
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine

This comment has been minimized.

Copy link

elasticmachine commented Feb 5, 2019

Typing is hard
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@elasticmachine

This comment has been minimized.

Copy link

elasticmachine commented Feb 6, 2019

@tylersmalley tylersmalley merged commit d9301dd into elastic:6.x Feb 6, 2019

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
kibana-ci Build finished.
Details

tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Feb 6, 2019

Reindexing prepends to index name instead of appending (elastic#30114)
Similar to what is done in the Index Lifecycle Management, we prepend as to
avoid possible issues with conflicts in index patterns, templates, etc.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley added a commit that referenced this pull request Feb 6, 2019

Reindexing prepends to index name instead of appending (#30114) (#30166)
Similar to what is done in the Index Lifecycle Management, we prepend as to
avoid possible issues with conflicts in index patterns, templates, etc.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment