Upgrade Assistant to New Platform Ready by jloleysens · Pull Request #50163 · elastic/kibana

jloleysens · 2019-11-11T15:24:09Z

Summary

Part of the refactoring of ES settings plugins to NP ready, this contribution updates Upgrade Assistant by:

Public

Moving to NP folder
Refactoring all ui/... dependency away and replacing with React Context where needed

Server

Moving to NP folder
Refactoring of Server to ServerShim and Request to RequestShim.
Refactor to use NP router!

- Reviving app + tests wip

… (tests working again)

…erShim and RequestShim

Fixed use of legacy server components

elasticmachine · 2019-11-11T15:24:44Z

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

jloleysens · 2019-11-11T15:24:54Z

@elasticmachine merge upstream

cchaos

SCSS review: Mostly file moves and removing ./ from paths. I did find one inherited problem that would be nice to fix in this PR. But otherwise, LGTM

elasticmachine · 2019-11-11T16:48:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 35c3ad7

elasticmachine · 2019-11-11T17:21:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: c0d123a

…skip ci] * upstream/master: Move table-list-view to kibana-react (elastic#50046) [ML] Stats bar for data frame analytics (elastic#49464) [ML] Make navigation in tests more stable (elastic#50132) Migrate authorization subsystem to the new platform. (elastic#46145) Bugfix: Interpreter conversion of string to number should throw on NaN elastic#27788 (elastic#50063) Update dependency @elastic/charts to v14 (elastic#49947) [ML] Adding cloud specific ML node warning (elastic#50139) Fixing bugs in the Shareable Runtime (elastic#49965) Revert router base name for Uptime plugin to use hash in default path. (elastic#50095) Ability to have telemetry always opted in (elastic#49798) Add "Get Help" and "Kibana Feedback" links to the help popover (elastic#49797) Removes references to Elasticsearch mapping types (elastic#47610) [skip-ci] Replace coordinate map in Kibana getting started docs with Maps (elastic#50167) [ML] Indicate missing required privileges for import in File Data Viz (elastic#50147) [SIEM][Detection Engine] Removes technical debt and minor bug fixes (elastic#50111)

elasticmachine · 2019-11-12T15:04:11Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: faa2676

jloleysens · 2019-11-13T09:09:25Z

@elasticmachine merge upstream

elasticmachine · 2019-11-13T09:09:27Z

merge conflict between base and head

* upstream/master: Remove internal platform types exports (elastic#50427) [APM] Document `apm_oss.metricsIndices` and `apm_oss.sourcemap… (elastic#50312) [Telemetry] Server side fetcher (elastic#50015) [SIEM] Detection engine placeholders (elastic#50220) [Uptime] Donut chart loader position centered vertically (elastic#50219) update telemetry banner notice text (elastic#50403) Fix aborting when searching without batching (elastic#49966) [Telemetry] Remove telemetry splash page and add conditional messaging (elastic#50189) Revert chromedriver update (elastic#50324) Remove deprecated argument include_type_name from ES calls (elastic#50285) [Maps] add settings to maps telemetry (elastic#50161) remove visualize loader (elastic#46910) Fix misuse of react-router and react-router-dom (elastic#50120)

jloleysens · 2019-11-13T10:13:08Z

@elasticmachine merge upstream

elasticmachine · 2019-11-13T10:33:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: b99342b

elasticmachine · 2019-11-13T11:29:05Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 95fc23c

* 'master' of github.com:elastic/kibana: (27 commits) [Rollup] Fix for clone job workflow (elastic#50501) Empty message "No data available" for Labels and User metadata sections missing (elastic#49846) [APM] Duration by Country map doesn't take `transactionName` into account (elastic#50315) Remove react references from core `Notifications` apis (elastic#49573) Updated APM Indices endpoints to use the SavedObjectsClient from the legacy request context, and set the apm-indices schema object to be namspace-agnostic [Metrics UI] Calculate interval based on the dataset's period (elastic#50194) chore(NA): add new platform discovered plugins as entry points to check for dependencies on clean dll tasks (elastic#50610) [Telemetry] change of optin status telemetry (elastic#50158) [SIEM][Detection Engine] REST API Additions (elastic#50514) [DOCS] Removes dashboard-only mode doc (elastic#50441) [Filters] Fix operator overflowing out popover (elastic#50030) Change telemetry optIn to default to true (elastic#50490) [Maps] make grid rectangles the default symbolization for geo grid source (elastic#50169) Allow registered applications to hide Kibana chrome (elastic#49795) Upgrade EUI to v14.9.0 (elastic#49678) [Metrics UI] Convert layouts to use React components (elastic#49134) [Search service] Add support for ES request preference (elastic#49424) [Newsfeed/Lint] fix chained fn lint (elastic#50515) [Monitoring] Fix logstash pipelines page in multi-cluster environment (elastic#50166) [SIEM] Events viewer fixes (elastic#50175) ...

elasticmachine · 2019-11-14T10:51:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6982fac

rudolf

Left a couple of comments but overall looks good. Switching out the axios client and consuming more NP API's on the server-side could definitely be done as a separate PR.

jloleysens · 2019-11-20T10:00:42Z

@elasticmachine merge upstream

rudolf

Like we discussed, I think core should make it easier to test your controllers in an isolated way, but they way you've set it up here gets us really far in terms of testing the controller's glue logic.

rudolf · 2019-11-20T10:00:16Z

-    method: 'GET',
-    options: {
-      pre: [EsVersionPrecheck],
+  server.router.get(


Nit: Because this mixes NP and legacy dependencies and the parameter/type is called server it's non-obvious from reading the code if you're using legacy, a shimmed facade or NP API's here.

I think that was partly the objective here 😅 . Ultimately this code shouldn't have to change much down the line.

rudolf · 2019-11-20T10:09:36Z

-    it('returns a 426 if EsVersionCheck throws', async () => {
-      (EsVersionPrecheck.method as jest.Mock).mockRejectedValue(
-        new Boom(`blah`, { statusCode: 426 })
-      );
-
-      const resp = await server.inject({
-        method: 'GET',
-        url: '/api/upgrade_assistant/status',
-      });
+      const resp = await serverShim.router.getHandler({
+        method: 'get',
+        pathPattern: '/api/upgrade_assistant/status',
+      })(ctxMock, createRequestMock(), kibanaResponseFactory);

-      expect(resp.statusCode).toEqual(426);


Isn't it worth keeping this, otherwise you're not testing if your versionCheckHandlerWrapper was applied, seems like you should be able to keep this test with your "test harness".

So I'm specifically opting out of the wrapper using the jest mock above like:

jest.mock('../lib/es_version_precheck', () => ({ versionCheckHandlerWrapper: (a: any) => a, }));

The bulk of the wrapper logic should be covered by es_version_precheck.test.ts. But I hear what you're saying it would be nice to test them in concert but I think since the wrapper is so thin it didn't merit a full mock out.

elasticmachine · 2019-11-20T10:28:08Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 3046e54

rudolf · 2019-11-20T10:39:46Z

I've created #51150 for enabling testing of controller logic.

jloleysens · 2019-11-20T10:54:13Z

@elasticmachine merge upstream

elasticmachine · 2019-11-20T12:17:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 18a06d3

Bamieh · 2019-11-27T00:18:42Z

@jloleysens This PR should be backported to 7.x correct? My backport had merge conflicts in the upgrade_assistant directory so i've checked out my changes on top of master: #51787

Can you double check if those changes are correct please?

* - Public NP Shim done - Reviving app + tests wip * Update components to consume via context and props instead of imports (tests working again) * Moved server files to np_ready dir, updated paths and introduced ServerShim and RequestShim * Removed unused comments Fixed use of legacy server components * Fix typings and add app context where needed * getSavedObjectsClient may be null, specifically during tests * Move styles for _index.scss file -> more specific file * Fix typings issue [skip ci] * Migrate to NP router

jloleysens · 2019-11-27T12:58:43Z

@Bamieh and I discussed offline what approach we would take for the UA backport.

His PR for the backport is here: #51787
The backport for this PR is here: #51810

* Upgrade Assistant to New Platform Ready (#50163) * - Public NP Shim done - Reviving app + tests wip * Update components to consume via context and props instead of imports (tests working again) * Moved server files to np_ready dir, updated paths and introduced ServerShim and RequestShim * Removed unused comments Fixed use of legacy server components * Fix typings and add app context where needed * getSavedObjectsClient may be null, specifically during tests * Move styles for _index.scss file -> more specific file * Fix typings issue [skip ci] * Migrate to NP router * Removed legacy app.tsx * APM changes to NP * More defensive use of SemVer * Update copy and include apm_oss plugin in 7.x server shim * Update expected message in spec

* - Public NP Shim done - Reviving app + tests wip * Update components to consume via context and props instead of imports (tests working again) * Moved server files to np_ready dir, updated paths and introduced ServerShim and RequestShim * Removed unused comments Fixed use of legacy server components * Fix typings and add app context where needed * getSavedObjectsClient may be null, specifically during tests * Move styles for _index.scss file -> more specific file * Fix typings issue [skip ci] * Migrate to NP router

jloleysens added 4 commits November 1, 2019 16:34

- Public NP Shim done

235b593

- Reviving app + tests wip

Update components to consume via context and props instead of imports…

6dba715

… (tests working again)

Moved server files to np_ready dir, updated paths and introduced Serv…

88f3394

…erShim and RequestShim

Removed unused comments

9777874

Fixed use of legacy server components

jloleysens requested a review from rudolf November 11, 2019 15:24

jloleysens requested a review from a team as a code owner November 11, 2019 15:24

jloleysens added Feature:Upgrade Assistant release_note:roadmap roadmap Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.6.0 v8.0.0 labels Nov 11, 2019

elasticmachine and others added 2 commits November 11, 2019 07:24

Merge branch 'master' into np-migration/ua

35c3ad7

Fix typings and add app context where needed

c0d123a

cchaos approved these changes Nov 11, 2019

View reviewed changes

Comment thread ...grade_assistant/public/np_ready/application/components/tabs/checkup/deprecations/_index.scss Outdated

jloleysens added 3 commits November 12, 2019 14:05

getSavedObjectsClient may be null, specifically during tests

1b229f5

Move styles for _index.scss file -> more specific file

7270c45

jloleysens added 2 commits November 13, 2019 10:10

Fix typings issue [skip ci]

bbad155

Merge branch 'master' into np-migration/ua

95fc23c

rudolf approved these changes Nov 18, 2019

View reviewed changes

jloleysens mentioned this pull request Nov 19, 2019

[Upgrade Assistant] Remove Axios #51011

Closed

Migrate to NP router

7336aca

jloleysens requested a review from rudolf November 20, 2019 09:42

Merge branch 'master' into np-migration/ua

3046e54

rudolf approved these changes Nov 20, 2019

View reviewed changes

Merge branch 'master' into np-migration/ua

18a06d3

jloleysens merged commit d035539 into elastic:master Nov 20, 2019

jloleysens deleted the np-migration/ua branch November 20, 2019 12:17

jloleysens mentioned this pull request Nov 27, 2019

[7.x] Upgrade Assistant to New Platform Ready (#50163) #51810

Merged

jloleysens mentioned this pull request Jan 9, 2020

Upgrade Assistant checkup fails if _meta.version does not follow SemVer format #45036

Closed

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

Conversation

jloleysens commented Nov 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Public

Server

Uh oh!

elasticmachine commented Nov 11, 2019

Uh oh!

jloleysens commented Nov 11, 2019

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Nov 11, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Nov 11, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Nov 12, 2019

💔 Build Failed

Uh oh!

jloleysens commented Nov 13, 2019

Uh oh!

elasticmachine commented Nov 13, 2019

Uh oh!

jloleysens commented Nov 13, 2019

Uh oh!

elasticmachine commented Nov 13, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Nov 13, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Nov 14, 2019

💚 Build Succeeded

Uh oh!

rudolf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jloleysens commented Nov 20, 2019

Uh oh!

rudolf left a comment

Choose a reason for hiding this comment

Uh oh!

rudolf Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

jloleysens Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

rudolf Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

jloleysens Nov 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 20, 2019

💔 Build Failed

Uh oh!

rudolf commented Nov 20, 2019

Uh oh!

jloleysens commented Nov 20, 2019

Uh oh!

elasticmachine commented Nov 20, 2019

💚 Build Succeeded

Uh oh!

Bamieh commented Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jloleysens commented Nov 11, 2019 •

edited

Loading

jloleysens Nov 20, 2019 •

edited

Loading

Bamieh commented Nov 27, 2019 •

edited

Loading

jloleysens commented Nov 27, 2019 •

edited

Loading