[Fleet] fix for upgrade package with tsds removed #157395

juliaElastic · 2023-05-11T14:13:51Z

Summary

To test:

Install nginx-1.12.0-beta which has index.mode:time_series.

POST http://elastic:changeme@localhost:5601/api/fleet/epm/packages/nginx-1.12.0-beta
kbn-xsrf: kibana

{
   "force": true
 }

Upgrade to nginx-1.12.1-beta which has index.mode:time_series removed.

Upload this package built manually:
nginx-1.12.1-beta.zip

curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/api/fleet/epm/packages -u elastic:changeme --data-binary @nginx-1.12.1-beta.zip

The package should install successfully and time_series should be removed from the index template metrics-nginx.stubstatus

WIP: update tests

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2023-05-11T14:13:58Z

Pinging @elastic/fleet (Team:Fleet)

apmmachine · 2023-05-11T14:14:12Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

kpollich

LGTM - I think gating this logic to only index templates that define index_mode: time_series is the safest option.

Would it be possible to add a test case for this to install.test.ts?

hop-dev · 2023-05-11T14:35:34Z

x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts

@@ -219,6 +219,17 @@ export async function installComponentAndIndexTemplateForDataStream({
  componentTemplates: TemplateMap;
  indexTemplate: IndexTemplateEntry;
 }) {
+  // update index template first in case TSDS was removed, so that it does not become invalid
+  const existingIndexTemplate = await esClient.indices.getIndexTemplate({


Does this API throw an error if the template doesnt exist?

Installing the index template first isn't great but I do think it is pretty safe in the update scenario. Obviously in the install scenario we couldn't fo it in this order as the index template references the component templates which wouldn't exist.

This does mean in the future if the component templates ever changed on upgrade e.g a new one was added on package upgrade then this wouldn't work, but that is completely hypothetical so I think this is fine.

Does this API throw an error if the template doesnt exist?

I want to test that scenario, might have to catch a 404 error.

This does mean in the future if the component templates ever changed on upgrade e.g a new one was added on package upgrade then this wouldn't work, but that is completely hypothetical so I think this is fine.

Good point, I think it is best to catch any errors for the first index template creation, as the codes does it again after component template update.

Updated the code with error handling, and added integration test.

hop-dev

+1 to adding an integration test for this scenario as this is something that someone could unknowingly break in a refactor.

We could even use the nginx package zips we already have for ease so you dont have to create a seperate test package

hop-dev

+1 to adding an integration test for this scenario as this is something that someone could unknowingly break in a refactor.

We could even use the nginx package zips we already have for ease so you dont have to create a seperate test package

kibana-ci · 2023-05-11T16:39:04Z

💚 Build Succeeded

Buildkite Build
Commit: 4cc179a

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	400	404	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	480	484	+4
total			+6

History

💔 Build #127150 failed 1056a6d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @juliaElastic

## Summary Fixes elastic#157345 To test: Install `nginx-1.12.0-beta` which has `index.mode:time_series`. ``` POST http://elastic:changeme@localhost:5601/api/fleet/epm/packages/nginx-1.12.0-beta kbn-xsrf: kibana { "force": true } ``` Upgrade to `nginx-1.12.1-beta` which has `index.mode:time_series` removed. Upload this package built manually: [nginx-1.12.1-beta.zip](https://github.com/elastic/kibana/files/11452945/nginx-1.12.1-beta.zip) ``` curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/api/fleet/epm/packages -u elastic:changeme --data-binary @nginx-1.12.1-beta.zip ``` The package should install successfully and time_series should be removed from the index template `metrics-nginx.stubstatus` WIP: update tests ### Checklist - [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

kibanamachine · 2023-05-11T16:49:41Z

💚 All backports created successfully

Status	Branch	Result
✅	8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

## Summary Fixes elastic#157345 To test: Install `nginx-1.12.0-beta` which has `index.mode:time_series`. ``` POST http://elastic:changeme@localhost:5601/api/fleet/epm/packages/nginx-1.12.0-beta kbn-xsrf: kibana { "force": true } ``` Upgrade to `nginx-1.12.1-beta` which has `index.mode:time_series` removed. Upload this package built manually: [nginx-1.12.1-beta.zip](https://github.com/elastic/kibana/files/11452945/nginx-1.12.1-beta.zip) ``` curl -XPOST -H 'content-type: application/zip' -H 'kbn-xsrf: true' http://localhost:5601/api/fleet/epm/packages -u elastic:changeme --data-binary @nginx-1.12.1-beta.zip ``` The package should install successfully and time_series should be removed from the index template `metrics-nginx.stubstatus` WIP: update tests ### Checklist - [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 (cherry picked from commit a0ee7b6)

kibanamachine · 2023-05-11T16:52:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…7415) ## Summary Backport #157395

juliaElastic · 2023-05-12T06:58:07Z

Backport to 8.8 merged in #157415

fix for upgrade package with tsds removed

1056a6d

juliaElastic requested a review from a team as a code owner May 11, 2023 14:13

juliaElastic self-assigned this May 11, 2023

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label May 11, 2023

juliaElastic changed the title ~~fix for upgrade package with tsds removed~~ [Fleet] fix for upgrade package with tsds removed May 11, 2023

juliaElastic added release_note:skip Skip the PR/issue when compiling release notes v8.8.0 v8.9.0 labels May 11, 2023

juliaElastic requested a review from hop-dev May 11, 2023 14:14

kpollich approved these changes May 11, 2023

View reviewed changes

hop-dev reviewed May 11, 2023

View reviewed changes

hop-dev approved these changes May 11, 2023

View reviewed changes

juliaElastic and others added 3 commits May 11, 2023 17:06

added error handling

6236ecb

Merge branch 'main' into fix-tsds-upgrade

34c4499

added integration test

4cc179a

juliaElastic enabled auto-merge (squash) May 11, 2023 15:49

juliaElastic merged commit a0ee7b6 into elastic:main May 11, 2023

juliaElastic added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label May 11, 2023

juliaElastic mentioned this pull request May 11, 2023

[8.8][Fleet] fix for upgrade package with tsds removed (#157395) #157415

Merged

kibanamachine mentioned this pull request May 11, 2023

[8.8] [Fleet] fix for upgrade package with tsds removed (#157395) #157416

Closed

juliaElastic added a commit that referenced this pull request May 12, 2023

[8.8][Fleet] fix for upgrade package with tsds removed (#157395) (#15…

88622e1

…7415) ## Summary Backport #157395

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] fix for upgrade package with tsds removed #157395

[Fleet] fix for upgrade package with tsds removed #157395

juliaElastic commented May 11, 2023 •

edited by kibanamachine

Loading

elasticmachine commented May 11, 2023

apmmachine commented May 11, 2023

kpollich left a comment

hop-dev May 11, 2023

juliaElastic May 11, 2023

juliaElastic May 11, 2023

hop-dev left a comment

hop-dev left a comment

kibana-ci commented May 11, 2023

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented May 11, 2023

kibanamachine commented May 11, 2023

juliaElastic commented May 12, 2023

[Fleet] fix for upgrade package with tsds removed #157395

[Fleet] fix for upgrade package with tsds removed #157395

Conversation

juliaElastic commented May 11, 2023 • edited by kibanamachine Loading

Summary

Checklist

elasticmachine commented May 11, 2023

apmmachine commented May 11, 2023

🤖 GitHub comments

kpollich left a comment

Choose a reason for hiding this comment

hop-dev May 11, 2023

Choose a reason for hiding this comment

juliaElastic May 11, 2023

Choose a reason for hiding this comment

juliaElastic May 11, 2023

Choose a reason for hiding this comment

hop-dev left a comment

Choose a reason for hiding this comment

hop-dev left a comment

Choose a reason for hiding this comment

kibana-ci commented May 11, 2023

💚 Build Succeeded

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented May 11, 2023

💚 All backports created successfully

Questions ?

kibanamachine commented May 11, 2023

💚 All backports created successfully

Questions ?

juliaElastic commented May 12, 2023

juliaElastic commented May 11, 2023 •

edited by kibanamachine

Loading