[Upgrade Assistant] Align code between branches #91862

alisonelizabeth · 2021-02-18T15:37:58Z

This PR attempts to align the Upgrade Assistant code between master and 7.x to improve DX and the backporting process.

Related to #78923.

How to review

It may be helpful to review the changes incrementally by commit.

The steps I took:

Merged all 7.x code into master. (2e4163d)
Removed code that is no longer applicable. These changes were largely concentrated around APM-related code that was specific to the 6-7 upgrade. The APM team confirmed this is no longer applicable, although there will likely need to be new APM-specific logic in UA for the 7-8 upgrade (TBD). I also removed index setting fixes to beats indices, which is also specific to the 6-7 upgrade. (2e2e92f)
Added conditional logic for any code remaining that is still pertinent to the 7-8 upgrade, but will be obsolete for the 8-9 upgrade. This includes conditional logic for the tests as well. (bc7ec3b)

There's not a great way to manually test this on master, as most of the new logic is behind the v7 flag. I plan to do more thorough testing on the backport PR. For the review, I'm mainly looking for feedback on the implementation and verification (to the best of your ability) that nothing was removed inadvertently that would still be applicable for the 8.0 upgrade.

alisonelizabeth · 2021-02-18T18:50:13Z

x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts

@@ -347,6 +347,11 @@ export const reindexServiceFactory = (
      await esClient.indices.open({ index: indexName });
    }

+    const flatSettings = await actions.getFlatSettings(indexName);


It's not clear to me why this was removed on master, but it seems like it is a valid check.

Hmm, good point! It may have been removed in error.

elasticmachine · 2021-02-18T18:57:41Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

alisonelizabeth · 2021-02-19T20:57:46Z

@elasticmachine merge upstream

alisonelizabeth · 2021-02-22T14:23:34Z

@elasticmachine merge upstream

…ign_branches

jloleysens

This is looking really solid @alisonelizabeth ! Thanks for bringing these in line, this will greatly improve DX on upgrade assistant moving forward 🍻

jloleysens · 2021-02-24T09:17:53Z

x-pack/plugins/upgrade_assistant/common/constants.ts

+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+import SemVer from 'semver/classes/semver';


Does this help with bundle size? Just curious ☺️

Good question! I think I copied this from elsewhere in Kibana 😅 . I'd have to go back and compare to see how it affects the bundle size.

jloleysens · 2021-02-24T09:21:21Z

...ant/public/application/components/tabs/checkup/deprecations/reindex/flyout/warnings_step.tsx

+                id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.warningsStep.customTypeNameWarningDetail"
+                defaultMessage="Mapping types are no longer supported in 8.0. This index mapping does not use the
+                    default type name, {defaultType}. Ensure no application code or scripts rely on a different type."
+                values={{
+                  defaultType: <EuiCode>_doc</EuiCode>,
+                }}


Is this new copy? If it is, it looks good to me, but it could be worth pulling in someone from esdocs@ too!

It's slightly modified from 7.x 😄. I'm going to hold off on a copy review for now, as I think this UI will change as part of the redesign.

jloleysens · 2021-02-24T09:23:25Z

x-pack/plugins/upgrade_assistant/server/lib/reindexing/index_settings.test.ts

@@ -177,4 +179,24 @@ describe('getReindexWarnings', () => {
      })
    ).toEqual([]);
  });
+
+  if (mockKibanaSemverVersion.major === 7) {


Not sure how the test harness is being set up, but it could be a better pattern to create a nested describe blocks for the 7 cases and 8 cases with their own test setups. WDYT?

jloleysens · 2021-02-24T09:24:26Z

x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts

@@ -347,6 +347,11 @@ export const reindexServiceFactory = (
      await esClient.indices.open({ index: indexName });
    }

+    const flatSettings = await actions.getFlatSettings(indexName);


Hmm, good point! It may have been removed in error.

kibanamachine · 2021-02-24T21:10:51Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`upgradeAssistant`	133.4KB	133.9KB	+498.0B

History

💚 Build #108739 succeeded eac1feb
💚 Build #108403 succeeded 04ce802
💚 Build #108171 succeeded ccecb36
💔 Build #108167 failed bde335f
💛 Build #107740 was flaky 4556e74

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

* master: (38 commits) Fixes Cypress flake by adding pipe, click, and should (elastic#92762) [Discover] Fix filtering selected sidebar fields (elastic#91828) [ML] Fixes positions of calendar arrow buttons in start datafeed modal (elastic#92625) [dev/build_ts_refs] check that commit in outDirs matches mergeBase (elastic#92513) add dep on `@kbn/config` so it is built first [Expressions] [Lens] Add id and copyMetaFrom arg to mapColumn fn + add configurable onError argument to math fn (elastic#90481) [Lens] Fix Workspace hidden when using Safari (elastic#92616) [Lens] Fixes vertical alignment validation messages (elastic#91878) forbid x-elastic-product-origin header in elasticsearch configuration (elastic#92359) [Security Solution][Detections] Set default indicator path to reduce friction with new filebeat modules (elastic#92081) [ILM][Accessibility] Added A11y test for ILM new policy form. (elastic#92570) [Security Solution][Exceptions] - Fixes exceptions builder UI where invalid values can cause overwrites of other values (elastic#90634) Automatically generated Api documentation (elastic#86232) Increase index pattern select limit to 1000 (elastic#92093) [core.logging] Add RewriteAppender for filtering LogMeta. (elastic#91492) [Security Solution][Detection Rules] Update prebuilt rule threats to match schema (elastic#92281) [Security Solutions][Detection Engine] Fixes bug with not being able to duplicate indicator matches (elastic#92565) [Dashboard] Export appropriate references from byValue panels (elastic#91567) [Upgrade Assistant] Align code between branches (elastic#91862) [Security Solution][Case] Fix alerts push (elastic#91638) ...

* [Upgrade Assistant] Align code between branches (#91862) # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json # x-pack/plugins/upgrade_assistant/common/types.ts # x-pack/plugins/upgrade_assistant/public/application/components/tabs.test.tsx # x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/checkup_tab.test.tsx # x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/deprecations/index_table.tsx # x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/deprecations/reindex/flyout/__snapshots__/warning_step.test.tsx.snap # x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/deprecations/reindex/flyout/warning_step.test.tsx # x-pack/plugins/upgrade_assistant/public/application/components/tabs/checkup/deprecations/reindex/flyout/warnings_step.tsx # x-pack/plugins/upgrade_assistant/server/lib/reindexing/index_settings.test.ts # x-pack/plugins/upgrade_assistant/server/lib/reindexing/index_settings.ts # x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts # x-pack/plugins/upgrade_assistant/server/lib/reindexing/types.ts * fix backport

alisonelizabeth added 2 commits February 17, 2021 14:32

merge 7.x code

2e4163d

remove irrelevant code

2e2e92f

alisonelizabeth added chore v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Upgrade Assistant v7.13.0 labels Feb 18, 2021

alisonelizabeth force-pushed the ua/align_branches branch from b23698b to 97a0737 Compare February 18, 2021 16:02

add version-specific logic

bc7ec3b

alisonelizabeth force-pushed the ua/align_branches branch from 97a0737 to bc7ec3b Compare February 18, 2021 16:21

update i18n

4556e74

alisonelizabeth commented Feb 18, 2021

View reviewed changes

alisonelizabeth marked this pull request as ready for review February 18, 2021 18:57

alisonelizabeth requested a review from a team as a code owner February 18, 2021 18:57

alisonelizabeth requested a review from jloleysens February 18, 2021 18:57

kibanamachine and others added 2 commits February 19, 2021 15:57

Merge branch 'master' into ua/align_branches

bde335f

update mapping type warning message

ccecb36

kibanamachine and others added 2 commits February 22, 2021 09:23

Merge branch 'master' into ua/align_branches

04ce802

Merge branch 'master' of https://github.com/elastic/kibana into ua/al…

eac1feb

…ign_branches

jloleysens approved these changes Feb 24, 2021

View reviewed changes

address review feedback

b948291

alisonelizabeth merged commit fe6bd2e into elastic:master Feb 24, 2021

alisonelizabeth deleted the ua/align_branches branch February 24, 2021 21:15

alisonelizabeth mentioned this pull request Feb 24, 2021

[7.x] [Upgrade Assistant] Align code between branches (#91862) #92718

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Upgrade Assistant] Align code between branches #91862

[Upgrade Assistant] Align code between branches #91862

alisonelizabeth commented Feb 18, 2021 •

edited

Loading

alisonelizabeth Feb 18, 2021

jloleysens Feb 24, 2021

elasticmachine commented Feb 18, 2021

alisonelizabeth commented Feb 19, 2021

alisonelizabeth commented Feb 22, 2021

jloleysens left a comment

jloleysens Feb 24, 2021

alisonelizabeth Feb 24, 2021

jloleysens Feb 24, 2021

alisonelizabeth Feb 24, 2021

jloleysens Feb 24, 2021

jloleysens Feb 24, 2021

kibanamachine commented Feb 24, 2021

[Upgrade Assistant] Align code between branches #91862

[Upgrade Assistant] Align code between branches #91862

Conversation

alisonelizabeth commented Feb 18, 2021 • edited Loading

How to review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Feb 18, 2021

alisonelizabeth commented Feb 19, 2021

alisonelizabeth commented Feb 22, 2021

jloleysens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Feb 24, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

alisonelizabeth commented Feb 18, 2021 •

edited

Loading