[Upgrade Assistant] Move out of legacy folder #58034

jloleysens · 2020-02-19T17:38:17Z

Summary

Move Upgrade Assistant out of legacy folder. This required the following changes:

Updating dependencies in Reindex service and the Reindex worker (functionality should be completely unchanged)
Migrate to using Route handler context for getting the elasticsearch dependency scoped to a request
Replace the dependency on x-pack with a dependency on licensing
Fix all Jest tests

To do

Test kibana.yml config
Run through of reindex worker

Additional

Also note that the management section registered under kibana. Using the new platform version moved Upgrade Assistant to the Kibana section (see the screenshot). Is this change acceptable? Given that the original code had it as kibana but it appeared under the Elasticsearch section:

Screenshot of management section after migration

Backport

This work will need to be backported to 7.x, there is special handling of APM indexes there which will probably lead to merge conflicts that need to be resolved.

For maintainers

This was checked for breaking API changes and was labeled appropriately

Move the public folder of Upgrade Assistant and migrate public to use HttpSetup (remove axios)

Begin migration of Reindex worker to new platform Move imports around so that it satsifies new platform constraints like not importing server side code (even types) in client.

After changing function signatures for the reindex server factory (and others) all of the tests needed to be revisited and brought in line with the new APIs. Also used core/server mocks where appropriate

The security plugin is a potential future consumer of the Upgrade Assistant's deprecation feature and we would therefore not want to create a circular depedency here. We pull in the licensing plugin rather (as it is less likely that we will depend on that) and use it to determine whether security is available and enabled.

xpack.upgrade_assistant.enabled

elasticmachine · 2020-02-19T17:38:19Z

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

joshdover · 2020-02-21T00:26:57Z

Also note that the management section registered under kibana. Using the new platform version moved Upgrade Assistant to the Kibana section (see the screenshot). Is this change acceptable? Given that the original code had it as kibana but it appeared under the Elasticsearch section

I think it should be under Elasticsearch in the UI, since all the data it surfaces right now is related to Elasticsearch, at least for the time being.

Added elasticsearch ui team as upgrade assistant code owner Updated i18nrc.json path

…-out-of-legacy * 'master' of github.com:elastic/kibana: (109 commits) document difference between log record formats (elastic#57798) Expose elasticsearch config schema (elastic#57655) [ui/agg_response/tabify] update types for search/expressions/build_tabular_inspector_data.ts (elastic#58130) [SIEM] Cleans Cypress tests code (elastic#58134) fix: 🐛 make dev server Storybook builds work again (elastic#58188) Prevent core savedObjects plugin from being overridden (elastic#58193) Expose serverBasePath on client-side (elastic#58070) Fix legend sizing on area charts (elastic#58083) Drilldown plugin (elastic#58097) [skip-ci] Fix broken links to saved objects APIs in MIGRATION.md (elastic#58033) [ML] New Platform server shim: update datafeed routes (elastic#57739) Add flag for building static storybook site (elastic#58050) add monaco to kbn/ui-shared-deps and load required features for all uses (elastic#58075) [SIEM] Let us try out code owners for a little while and see what happens Add throttle param to Alerting readme (elastic#57609) [NP] Move ui/saved_objects to NP (elastic#57452) [Logs UI] Fix column reordering in settings page (elastic#58104) Fix browser date format (elastic#57714) Add filter for ILM phase to Index Management (revert elastic#45486) (elastic#57402) Clarify Precision function in Timelion Kibana (elastic#58031) ... # Conflicts: # x-pack/.i18nrc.json

joshdover

Code changes LGTM, have not tested manually

jloleysens · 2020-02-24T08:06:44Z

@elasticmachine merge upstream

elasticmachine · 2020-02-24T08:06:46Z

merge conflict between base and head

…-out-of-legacy * 'master' of github.com:elastic/kibana: [SIEM] [Case] Enable case by default. Snake to camel on UI (elastic#57936) [File upload] Update remaining File Upload dependencies for NP migration (elastic#58128) Use EuiTokens for ES field types (elastic#57911) Added UI support for the default action group for Alert Type Model (elastic#57603) force savedObject API consumers to define SO type explicitly (elastic#58022) Update dependency @elastic/charts to ^17.1.1 (elastic#57634) [Endpoint] Add a flyout to alert list. (elastic#57926) Make sure index pattern has fields before parsing (elastic#58242) Sanitize workpad before sending to API (elastic#57704) [ML] Transform: Support multi-line JSON notation in advanced editor (elastic#58015) [Endpoint] Refactor Management List Tests (elastic#58148) [kbn/optimizer] include bootstrap cache key in optimizer cache key (elastic#58176) Do not refresh color scale on each lookup (elastic#57792) Updating to @elastic/lodash@3.10.1-kibana4 (elastic#54662) Trigger context (elastic#57870) [ML] Transforms: Adds clone feature to transforms list. (elastic#57837) [ML] New Platform server shim: update fields service routes (elastic#58060) [Endpoint] EMT-184: change endpoints to metadata up and down the code base. (elastic#58038)

jloleysens · 2020-02-24T17:42:09Z

@elasticmachine merge upstream

jloleysens · 2020-02-24T22:35:51Z

@elasticmachine merge upstream

sebelga

Awesome work @jloleysens ! I only added small nit comments here and there but the code looks good to me 👍
If you need me to test locally ping me in slack to help me get started

sebelga · 2020-02-25T05:44:44Z

x-pack/.i18nrc.json

@@ -39,8 +39,8 @@
    "xpack.spaces": ["legacy/plugins/spaces", "plugins/spaces"],
    "xpack.taskManager": "legacy/plugins/task_manager",
    "xpack.transform": "legacy/plugins/transform",
+    "xpack.upgradeAssistant": "plugins/upgrade_assistant",


We might want to keep the alphabetical order here.

sebelga · 2020-02-25T06:11:14Z

x-pack/plugins/upgrade_assistant/public/application/components/tabs.test.tsx

-  create: jest.fn(),
-}));
-
+import { httpServiceMock } from '../../../../../../src/core/public/mocks';


If the @elastic/kibana-core-ui could provide us with an alias to src/core it would be awesome 😊

sebelga · 2020-02-25T06:15:50Z

...tant/public/application/components/tabs/checkup/deprecations/reindex/polling_service.test.ts

-import { mockClient } from './polling_service.test.mocks';
-
-import { ReindexStatus, ReindexStep } from '../../../../../../../../common/types';
+import { ReindexStatus, ReindexStep } from '../../../../../../../common/types';
 import { ReindexPollingService } from './polling_service';
 import { httpServiceMock } from 'src/core/public/http/http_service.mock';


Oh but we have an alias to src/core can we remove the relative path above?

sebelga · 2020-02-25T06:21:18Z

...upgrade_assistant/public/application/components/tabs/overview/deprecation_logging_toggle.tsx

 import React from 'react';

 import { EuiLoadingSpinner, EuiSwitch } from '@elastic/eui';
 import { injectI18n } from '@kbn/i18n/react';

-import { HttpSetup } from 'src/core/public';
+import { HttpSetup } from 'kibana/public';


I am confused with the difference between src/core and kibana/public. Do we have some info about it?

I don't think there is a difference, but for some reason when I import existing things (not types) from there it doesn't seem to work 🤷‍♂ . Also I believe some of these changes were introduced by my IDE.

I will change them all to src/core/public so that it is consistent here at least.

sebelga · 2020-02-25T06:23:35Z

x-pack/plugins/upgrade_assistant/public/plugin.ts

+import { i18n } from '@kbn/i18n';
+import { Plugin, CoreSetup, PluginInitializerContext } from 'src/core/public';
+
+import { renderApp } from './application/render_app';


nit: in the order of import, this one should come last (as a pattern we try to follow) as it is the closest to the current file.

sebelga · 2020-02-25T07:23:22Z

x-pack/plugins/upgrade_assistant/server/routes/reindex_indices.ts

+      ) => {
+        const { indexName } = request.params as any;
+        const { client } = savedObjects;
+        const callCluster = dataClient.callAsCurrentUser.bind(dataClient);


nit: why not directly const callAsCurrentUser = ?

sebelga · 2020-02-25T07:28:32Z

x-pack/plugins/upgrade_assistant/server/routes/reindex_indices.ts

+            },
+          });
+        } catch (e) {
+          if (!e.isBoom) {


Can it ever be a Boom error?

sebelga · 2020-02-25T07:29:08Z

x-pack/plugins/upgrade_assistant/server/routes/reindex_indices.ts

+
+          return response.ok({ body: { acknowledged: true } });
+        } catch (e) {
+          if (!e.isBoom) {


Same here about boom error

sebelga · 2020-02-25T07:37:28Z

x-pack/plugins/upgrade_assistant/server/routes/telemetry.ts

@@ -23,16 +22,23 @@ export function registerTelemetryRoutes(server: ServerShimWithRouter) {
      },
    },
    async (ctx, request, response) => {
-      const reqShim = createRequestShim(request);
+      const { cluster, indices, overview } = request.body as any;


I think it would be better to have the validation schema outside and then use

request.body as TypeOf<typeof bodySchema>;

sebelga · 2020-02-25T07:37:49Z

x-pack/plugins/upgrade_assistant/server/routes/telemetry.ts

@@ -45,9 +51,17 @@ export function registerTelemetryRoutes(server: ServerShimWithRouter) {
      },
    },
    async (ctx, request, response) => {
-      const reqShim = createRequestShim(request);
+      const { close, open, start, stop } = request.body as any;


Renamed callCluster -> callAsUser to be more consistent with platform naming. Added comment about why we are not using security plugin directly inside of Upgrade Assistant. Fixed long path imports and use 'src/core/..' throughout. Fixed import ordering. Renamed variables inside of telemetry lib.

In plugin.ts importing from 'kibana/server' or 'src/core/server' results in a module not found error.

kibanamachine · 2020-02-25T13:41:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5c32021

History

💔 Build #28730 failed e4dba40
💚 Build #28647 succeeded b7d17db
💔 Build #28545 failed c8b2da1
💔 Build #28374 failed 73c6267
💚 Build #28103 succeeded 8d944ba

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

ryankeairns

LGTM. Moving SCSS files, no style changes.

* Also migrated query default field endpoint with tests * Create x-pack/plugins skeleton for upgrade assistant * Move public folder contents Move the public folder of Upgrade Assistant and migrate public to use HttpSetup (remove axios) * Include stylesheets in public * Move server side out of legacy Begin migration of Reindex worker to new platform Move imports around so that it satsifies new platform constraints like not importing server side code (even types) in client. * Updated the routes with new dependencies and removed server shim * Fix router unit tests * Fix server lib tests After changing function signatures for the reindex server factory (and others) all of the tests needed to be revisited and brought in line with the new APIs. Also used core/server mocks where appropriate * Clean up types issues * Fix setting credentials on request header * Removed the security plugin from Upgrade Assistant The security plugin is a potential future consumer of the Upgrade Assistant's deprecation feature and we would therefore not want to create a circular depedency here. We pull in the licensing plugin rather (as it is less likely that we will depend on that) and use it to determine whether security is available and enabled. * Migrate to config to new platform config xpack.upgrade_assistant.enabled * Remove unused types * Fix import issue * Move upgrade assistant back to Elasticsearch management section * Update dotfiles Added elasticsearch ui team as upgrade assistant code owner Updated i18nrc.json path * Alphabetical ordering in xpack/i18nrc.json * Implemented PR feedback Renamed callCluster -> callAsUser to be more consistent with platform naming. Added comment about why we are not using security plugin directly inside of Upgrade Assistant. Fixed long path imports and use 'src/core/..' throughout. Fixed import ordering. Renamed variables inside of telemetry lib. * Revert to longer import path In plugin.ts importing from 'kibana/server' or 'src/core/server' results in a module not found error. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kibanamachine · 2020-02-26T14:38:45Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

* [Upgrade Assistant] Move out of legacy folder (#58034) * Also migrated query default field endpoint with tests * Create x-pack/plugins skeleton for upgrade assistant * Move public folder contents Move the public folder of Upgrade Assistant and migrate public to use HttpSetup (remove axios) * Include stylesheets in public * Move server side out of legacy Begin migration of Reindex worker to new platform Move imports around so that it satsifies new platform constraints like not importing server side code (even types) in client. * Updated the routes with new dependencies and removed server shim * Fix router unit tests * Fix server lib tests After changing function signatures for the reindex server factory (and others) all of the tests needed to be revisited and brought in line with the new APIs. Also used core/server mocks where appropriate * Clean up types issues * Fix setting credentials on request header * Removed the security plugin from Upgrade Assistant The security plugin is a potential future consumer of the Upgrade Assistant's deprecation feature and we would therefore not want to create a circular depedency here. We pull in the licensing plugin rather (as it is less likely that we will depend on that) and use it to determine whether security is available and enabled. * Migrate to config to new platform config xpack.upgrade_assistant.enabled * Remove unused types * Fix import issue * Move upgrade assistant back to Elasticsearch management section * Update dotfiles Added elasticsearch ui team as upgrade assistant code owner Updated i18nrc.json path * Alphabetical ordering in xpack/i18nrc.json * Implemented PR feedback Renamed callCluster -> callAsUser to be more consistent with platform naming. Added comment about why we are not using security plugin directly inside of Upgrade Assistant. Fixed long path imports and use 'src/core/..' throughout. Fixed import ordering. Renamed variables inside of telemetry lib. * Revert to longer import path In plugin.ts importing from 'kibana/server' or 'src/core/server' results in a module not found error. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Fixed plugin name and fixed linting apmOSS -> apm_oss * Fix import and type issue * Added query_default_field route back 🤦🏼‍♂️ * Improved boom error handling on POST reindex endpoint Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

jloleysens added 12 commits February 18, 2020 13:06

Create x-pack/plugins skeleton for upgrade assistant

b631a40

Move public folder contents

c1fe10a

Move the public folder of Upgrade Assistant and migrate public to use HttpSetup (remove axios)

Include stylesheets in public

0272a14

Move server side out of legacy

e93348e

Begin migration of Reindex worker to new platform Move imports around so that it satsifies new platform constraints like not importing server side code (even types) in client.

Updated the routes with new dependencies and removed server shim

c7691a9

Fix router unit tests

5938e91

Fix server lib tests

64ec947

After changing function signatures for the reindex server factory (and others) all of the tests needed to be revisited and brought in line with the new APIs. Also used core/server mocks where appropriate

Clean up types issues

e91d248

Fix setting credentials on request header

3362e94

Migrate to config to new platform config

b540a6e

xpack.upgrade_assistant.enabled

Remove unused types

ee141c0

jloleysens added 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.7.0 labels Feb 19, 2020

jloleysens requested a review from a team as a code owner February 19, 2020 17:38

Fix import issue

a0e1bd2

jloleysens added 3 commits February 21, 2020 11:11

Move upgrade assistant back to Elasticsearch management section

db64a18

Update dotfiles

9684204

Added elasticsearch ui team as upgrade assistant code owner Updated i18nrc.json path

jloleysens requested review from sebelga and joshdover February 21, 2020 12:45

joshdover approved these changes Feb 21, 2020

View reviewed changes

Merge branch 'master' into ua/chore/move-out-of-legacy

c8b2da1

Merge branch 'master' into ua/chore/move-out-of-legacy

b7d17db

sebelga approved these changes Feb 25, 2020

View reviewed changes

jloleysens added 3 commits February 25, 2020 09:55

Alphabetical ordering in xpack/i18nrc.json

1d793d7

Revert to longer import path

5c32021

In plugin.ts importing from 'kibana/server' or 'src/core/server' results in a module not found error.

ryankeairns approved these changes Feb 25, 2020

View reviewed changes

jloleysens merged commit 899270a into elastic:master Feb 25, 2020

jloleysens deleted the ua/chore/move-out-of-legacy branch February 25, 2020 14:31

jloleysens mentioned this pull request Feb 25, 2020

[7.x] [Upgrade Assistant] Move out of legacy folder (#58034) #58535

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 26, 2020

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 27, 2020

cjcenizal added the Feature:NP Migration label Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Upgrade Assistant] Move out of legacy folder #58034

[Upgrade Assistant] Move out of legacy folder #58034

jloleysens commented Feb 19, 2020 •

edited

Loading

elasticmachine commented Feb 19, 2020

joshdover commented Feb 21, 2020

joshdover left a comment

jloleysens commented Feb 24, 2020

elasticmachine commented Feb 24, 2020

jloleysens commented Feb 24, 2020

jloleysens commented Feb 24, 2020

sebelga left a comment

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

jloleysens Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

sebelga Feb 25, 2020

kibanamachine commented Feb 25, 2020

ryankeairns left a comment

kibanamachine commented Feb 26, 2020

[Upgrade Assistant] Move out of legacy folder #58034

[Upgrade Assistant] Move out of legacy folder #58034

Conversation

jloleysens commented Feb 19, 2020 • edited Loading

Summary

To do

Additional

Backport

For maintainers

elasticmachine commented Feb 19, 2020

joshdover commented Feb 21, 2020

joshdover left a comment

Choose a reason for hiding this comment

jloleysens commented Feb 24, 2020

elasticmachine commented Feb 24, 2020

jloleysens commented Feb 24, 2020

jloleysens commented Feb 24, 2020

sebelga 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

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 25, 2020

💚 Build Succeeded

History

ryankeairns left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 26, 2020

jloleysens commented Feb 19, 2020 •

edited

Loading