[NP migration] Shim ILM #56068

cjcenizal · 2020-01-27T21:05:31Z

Test plan

Testing the Index Management extensions

Create an index lifecycle policy with a Hot Phase that has enabled rollover and max age of a few nanoseconds.
Create an index in Console and add the policy to it in Index Management. You should get a modal and then once you've confirmed, a toast.
View the index's details to see
Add a document to the index and wait a few seconds
Refresh Index Management and you should see a warning callout about errors in the policy
Retry the index lifecycle step for the index
Remove the index lifecycle policy from the index. You should get a modal and then once you've confirmed, a toast.

Verification that dependencies still function

Toasts

Fatal error

Fetching index templates

Fetching nodes

Fetching node details

Index Management extension

elasticmachine · 2020-01-27T21:05:33Z

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

joshdover

Good first step! Just a couple suggestions / tips.

joshdover · 2020-01-27T23:11:32Z

x-pack/legacy/plugins/index_lifecycle_management/shim.ts

+export interface CoreSetup {
+  server: Legacy.Server;
+}


You may want to use the LegacySetup pattern that was recently added to the Migration Guide.

The gist for this is, add a 3rd argument to your plugin lifecycle method that represents all the legacy dependencies, and use the real CoreSetup interface as the first argument. This makes it easier to identify which parts still need to be migrated away from Legacy APIs and which parts are using the real CoreSetup APIs.

Thanks, Josh. I attempted to implement this pattern in f35f1b5, but I'm not entirely sure if this is correct.

joshdover · 2020-01-27T23:12:28Z

...x_lifecycle_management/server/lib/license_pre_routing_factory/license_pre_routing_factory.js

 import { PLUGIN_ID } from '../../../common/constants';
+import { wrapCustomError } from '../error_wrappers';

 export const licensePreRoutingFactory = once(server => {


Just an FYI, I just opened a PR for adding examples of how to migrate these pre-route handlers: #56080. Not relevant for this PR, but should be helpful for completing the server-side.

cjcenizal · 2020-02-01T00:26:15Z

@joshdover Would you mind reviewing my latest commit, which shims the public directory?

joshdover · 2020-02-03T17:37:21Z

x-pack/legacy/plugins/index_lifecycle_management/public/legacy.ts

+              // NOTE: We depend upon Angular's $http service because it's decorated with interceptors,
+              // e.g. to check license status per request.


The core.http.* APIs have the same interceptors installed as well.

joshdover · 2020-02-03T17:40:21Z

x-pack/legacy/plugins/index_lifecycle_management/public/np_ready/application/index.tsx

+  element: HTMLElement;
+}
+
+export const boot = (appDependencies: AppDependencies) => {


By convention this should be named renderApp.

joshdover · 2020-02-03T17:41:42Z

x-pack/legacy/plugins/index_lifecycle_management/public/legacy.ts

+  const esSection = management.getSection('elasticsearch');
+  esSection.register('index_lifecycle_policies', {
+    visible: true,
+    display: PLUGIN.TITLE,
+    order: 2,
+    url: `#${BASE_PATH}policies`,
+  });


FYI once you're app is de-coupled from Angular, you can leverage the new Management plugin in the NP. Can do this in the next PR though.

joshdover · 2020-02-03T17:43:57Z

x-pack/legacy/plugins/index_lifecycle_management/public/legacy.ts

+            application: {
+              ...npSetup.core.application,
+              async register(app: App) {
+                const unmountApp = await app.mount({ ...npStart } as any, {
+                  element,
+                  appBasePath: '',
+                  onAppLeave: () => undefined,
+                });
+                manageAngularLifecycle($scope, $route, unmountApp as any);
+              },
+            },


In the case of a management section app, you'll be using the management plugin's API instead of the Application service since this is not a top-level app. You can see the management API here: https://github.com/elastic/kibana/blob/master/src/plugins/management/public/types.ts#L25

joshdover · 2020-02-03T17:44:55Z

x-pack/legacy/plugins/index_lifecycle_management/public/legacy.ts

+                  kbnUrl.redirect(path);
+                });
+              },
+              fatalError,


There is an NP API on CoreSetup.fatalErrors

- Move UI metric constants into public directory.

…ameter.

…ming.

- Refactor DI pattern of Index Mangement extensions to require various NP service dependencies.

cjcenizal · 2020-02-03T20:40:00Z

...public/app/sections/home/index_list/index_actions_context_menu/index_actions_context_menu.js

@@ -211,7 +216,20 @@ export class IndexActionsContextMenu extends Component {
      },
    });
    getActionExtensions().forEach(actionExtension => {
-      const actionExtensionDefinition = actionExtension(indices, reloadIndices);
+      const actionExtensionDefinition = actionExtension({


@sebelga Just a head's up that a few files are changing in Index Management, which we'll need to reconcile.

If I merge this PR in before you finish migrating Index Management then you'll just need to reconcile the conflicts by accepting these changes but passing in the NP http service in lieu of getNewPlatformCompatibleHttpClient.

If you merge in the Index Management migration before I merge this PR then I'll be able to reconcile the conflicts myself, but I don't think that's the best way to proceed. It would create less work for you to wait for this PR to get merged first, because then you won't need to preserve the dependency upon Angular's $http service.

OK no problem, thanks for the heads up @cjcenizal !

- Update Index Management to provide a NP-compatible adapater around Angular's $http service. - Remove unused sendPut method from API service. - Update http error handling to consume NP http service errors.

cjcenizal · 2020-02-03T22:31:59Z

...public/app/sections/home/index_list/index_actions_context_menu/index_actions_context_menu.js

+
+// We will be able to remove this after the NP migration is complete and we can just inject the
+// NP fatalErrors service..
+const getNewPlatformCompatibleFatalErrorService = () => ({


@sebelga Here's another example of a service adapter that will become unnecessary as part of your migration.

cjcenizal · 2020-02-03T22:33:05Z

@joshdover Thanks for the review! I've made the changes you suggested. Could you please re-review?

joshdover

Shims LGTM

…dency upon Angular's $http.

jloleysens

@cjcenizal great work on this! 🎉

Code

Left a few comments and a question about when we should introduce IRouter. No other blockers.

UI/UX

UI looks good, didn't see any visual regressions!
I was not able to get step 5 of the Testing the Index Management extensions section in the PR description, everything else seemed to work!

jloleysens · 2020-02-04T10:43:43Z

x-pack/legacy/plugins/index_lifecycle_management/plugin.ts

+    registerLicenseChecker(server);
+
+    // Register routes.
+    registerIndexRoutes(server);


As part of this shimming, is it worth migrating the http routes to use the new platform IRouter? It should be available on CoreSetup which I see we are passing in from x-pack/legacy/plugins/index_lifecycle_management/index.ts. It does affect all of the routes so happy if this done later.

I think it is most useful that when we migrate the routes we don't pass the legacy server object in directly but rather create a RouteDependencies object that contains a __LEGACY entry. That way all of the legacy interfaces are clearly marked here and downstream.

+1 Agreed. Let's discuss later at the sync those conventions as I am also afraid of other places we might do things differently.

I'll migrate to the NP router in a subsequent PR. I agree that it would be great if we could align on a convention.

jloleysens · 2020-02-04T10:53:07Z

x-pack/legacy/plugins/index_lifecycle_management/public/np_ready/application/services/api.js

+// The extend_index_management module that we support an injected httpClient here.
+
+export async function loadNodes(httpClient) {
+  const response = await sendGet(`nodes/list`, httpClient);


Nit: unnecessary creation of response constant. Can just return send.... Same for other instances below. In some cases the await can also be omitted if just a one-liner.

jloleysens · 2020-02-04T10:58:57Z

x-pack/legacy/plugins/index_lifecycle_management/server/lib/check_license/check_license.ts

 import { i18n } from '@kbn/i18n';
-export function checkLicense(xpackLicenseInfo) {
+
+export function checkLicense(xpackLicenseInfo: any): any {


I think there are typings available for this here: x-pack/legacy/plugins/xpack_main/server/lib/xpack_info.ts

Thanks, I'll use them in the subsequent PR when I make more significant changes to the server.

…np-shim

cjcenizal · 2020-02-04T21:34:18Z

Jenkins test this

mistic · 2020-02-05T01:07:50Z

@elasticmachine merge upstream

brianseeders · 2020-02-05T18:07:49Z

@elasticmachine merge upstream

brianseeders · 2020-02-05T18:12:34Z

@elasticmachine merge upstream

…np-shim

mistic · 2020-02-05T19:34:51Z

@elasticmachine merge upstream

cjcenizal · 2020-02-05T19:36:02Z

Jenkins test this

cjcenizal · 2020-02-05T20:20:27Z

@elasticmachine merge upstream

kibanamachine · 2020-02-05T23:06:53Z

💚 Build Succeeded

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

History

💔 Build #24717 failed c729fa7
💔 Build #24702 failed c06c10f
💔 Build #24695 failed 2801485
💔 Build #24675 failed 87e813754c61ce30ab744ccf599e60fbd1d501cf
💔 Build #24603 failed dd320e4e2732d1e3b71a1bb699b7356933647042

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

- Move UI metric constants into public directory. - Convert server-side code to TS, largely with use of any. - Remove unused sendPut method from API service. - Update http error handling to consume NP http service errors. - Refactor DI pattern of Index Mangement extensions to require various NP service dependencies. - Inject NP http service into Index Management extensions. Remove dependency upon Angular's $http.

cjcenizal added chore Feature:ILM 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 v7.7.0 labels Jan 27, 2020

joshdover approved these changes Jan 27, 2020

View reviewed changes

cjcenizal force-pushed the chore/ilm-to-np-shim branch 3 times, most recently from c21fa42 to 9eafd5e Compare February 1, 2020 15:37

joshdover reviewed Feb 3, 2020

View reviewed changes

cjcenizal added 5 commits February 3, 2020 11:28

Shim ILM server.

67066a1

- Move UI metric constants into public directory.

Convert server-side code to TS, largely with use of any.

a406f30

Use real CoreSetup and extract shimmed dependencies into __LEGACY par…

b4e6902

…ameter.

Reorder client-side imports to isolate dependencies that require shim…

cc1e3a8

…ming.

Shim public directory with a np_ready subdirectory.

e9fa60a

- Refactor DI pattern of Index Mangement extensions to require various NP service dependencies.

cjcenizal force-pushed the chore/ilm-to-np-shim branch from 9eafd5e to af15a71 Compare February 3, 2020 20:34

cjcenizal commented Feb 3, 2020

View reviewed changes

cjcenizal added 2 commits February 3, 2020 13:57

Consume NP http service.

adc84b2

- Update Index Management to provide a NP-compatible adapater around Angular's $http service. - Remove unused sendPut method from API service. - Update http error handling to consume NP http service errors.

Rename boot function to renderApp.

0b9e0ab

cjcenizal force-pushed the chore/ilm-to-np-shim branch from af15a71 to 0b9e0ab Compare February 3, 2020 21:58

cjcenizal added 2 commits February 3, 2020 14:24

Migrate to NP fatalErrors service.

154db30

Add some comments about future migrations.

e46dad1

cjcenizal commented Feb 3, 2020

View reviewed changes

cjcenizal marked this pull request as ready for review February 3, 2020 22:33

cjcenizal requested a review from jloleysens February 3, 2020 22:33

joshdover approved these changes Feb 4, 2020

View reviewed changes

Inject NP http service into Index Management extensions. Remove depen…

225bf51

…dency upon Angular's $http.

cjcenizal force-pushed the chore/ilm-to-np-shim branch from 9acd7ef to 225bf51 Compare February 4, 2020 06:09

jloleysens approved these changes Feb 4, 2020

View reviewed changes

cjcenizal added 2 commits February 4, 2020 11:51

Address PR feedback and fix TS errors.

1fb3a78

Merge branch 'master' of github.com:elastic/kibana into chore/ilm-to-…

7725b5b

…np-shim

Merge branch 'master' into chore/ilm-to-np-shim

a66b2a5

cjcenizal force-pushed the chore/ilm-to-np-shim branch from 628e162 to dd320e4 Compare February 5, 2020 15:06

cjcenizal added 2 commits February 5, 2020 11:22

Fix extend_index_lifecycle_management tests.

ec6c117

Merge branch 'master' of github.com:elastic/kibana into chore/ilm-to-…

2801485

…np-shim

cjcenizal force-pushed the chore/ilm-to-np-shim branch from 87e8137 to 2801485 Compare February 5, 2020 19:23

Merge branch 'master' into chore/ilm-to-np-shim

c06c10f

elasticmachine and others added 2 commits February 5, 2020 15:20

Merge branch 'master' into chore/ilm-to-np-shim

c729fa7

Update snapshots.

bb7706f

cjcenizal merged commit 51854c7 into elastic:master Feb 5, 2020

cjcenizal deleted the chore/ilm-to-np-shim branch February 5, 2020 23:14

cjcenizal mentioned this pull request Feb 6, 2020

[7.x] [NP migration] Shim ILM (#56068) #56938

Merged

cjcenizal added the Feature:NP Migration label Feb 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NP migration] Shim ILM #56068

[NP migration] Shim ILM #56068

cjcenizal commented Jan 27, 2020 •

edited

Loading

elasticmachine commented Jan 27, 2020

joshdover left a comment

joshdover Jan 27, 2020

cjcenizal Jan 28, 2020

joshdover Jan 27, 2020

cjcenizal commented Feb 1, 2020

joshdover Feb 3, 2020

joshdover Feb 3, 2020

joshdover Feb 3, 2020

joshdover Feb 3, 2020

joshdover Feb 3, 2020

cjcenizal Feb 3, 2020

sebelga Feb 4, 2020

cjcenizal Feb 3, 2020

cjcenizal commented Feb 3, 2020

joshdover left a comment

jloleysens left a comment

jloleysens Feb 4, 2020

sebelga Feb 4, 2020

cjcenizal Feb 4, 2020

jloleysens Feb 4, 2020

jloleysens Feb 4, 2020

cjcenizal Feb 4, 2020

cjcenizal commented Feb 4, 2020

mistic commented Feb 5, 2020

brianseeders commented Feb 5, 2020

brianseeders commented Feb 5, 2020

mistic commented Feb 5, 2020

cjcenizal commented Feb 5, 2020

cjcenizal commented Feb 5, 2020

kibanamachine commented Feb 5, 2020

		// NOTE: We depend upon Angular's $http service because it's decorated with interceptors,
		// e.g. to check license status per request.

[NP migration] Shim ILM #56068

[NP migration] Shim ILM #56068

Conversation

cjcenizal commented Jan 27, 2020 • edited Loading

Test plan

Testing the Index Management extensions

Verification that dependencies still function

Toasts

Fatal error

Fetching index templates

Fetching nodes

Fetching node details

Index Management extension

elasticmachine commented Jan 27, 2020

joshdover 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

cjcenizal commented Feb 1, 2020

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

cjcenizal commented Feb 3, 2020

joshdover left a comment

Choose a reason for hiding this comment

jloleysens left a comment

Choose a reason for hiding this comment

Code

UI/UX

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

cjcenizal commented Feb 4, 2020

mistic commented Feb 5, 2020

brianseeders commented Feb 5, 2020

brianseeders commented Feb 5, 2020

mistic commented Feb 5, 2020

cjcenizal commented Feb 5, 2020

cjcenizal commented Feb 5, 2020

kibanamachine commented Feb 5, 2020

💚 Build Succeeded

History

cjcenizal commented Jan 27, 2020 •

edited

Loading