Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate CCR to new ES JS client. #100131

Merged
merged 7 commits into from
Jun 1, 2021

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented May 14, 2021

Summary

Partially addresses #73973 and #83910.

Changes

Behavior to test

Actions

  • Create follower index
  • Update follower index
  • Pause follower index
  • Resume follower index
  • View details of follower index in flyout
  • View details of follower indices in table
  • Batch pause follower indices
  • Batch start follower indices
  • Batch delete follower indices
  • Create autofollow pattern
  • Collision of creating autofollow patterns with the same name
  • Update autofollow pattern
  • Pause autofollow pattern
  • Resume autofollow pattern
  • View details of autofollow pattern in flyout
  • View details of autofollow patterns in table
  • Batch pause autofollow patterns
  • Batch start autofollow patterns
  • Batch delete autofollow patterns

Errors

All errors were produced by editing the ES requests inside the API route handlers.

Error checking permissions.

image

Error creating auto-follow pattern.

image

Error creating follower index.

image

Can't load auto-follow pattern (404).

image

Can't load follower index (404).

image

@cjcenizal cjcenizal added chore Feature:CCR and Remote Clusters 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.14.0 labels May 14, 2021
@cjcenizal cjcenizal force-pushed the ccr/migrate-legacy-es-client branch from 78d822b to e0b22af Compare May 14, 2021 22:33
@@ -74,17 +74,17 @@ export default function ({ getService }) {

describe('get()', () => {
it('should return a 404 when the follower index does not exist', async () => {
const name = getRandomString();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sebelga I removed this helper in favor of explicit hard-coded values for a couple reasons:

  • When debugging a test gone wrong (for example, clean up code that deletes these primitives), explicit hard-coded values can provide debugging information
  • I generally try to simplify systems by removing any code that isn't necessary, and it didn't seem necessary to me to randomize these values

Do I have your blessing to make this change? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the reasoning. You do have my blessing! 👍 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Seb!

2. Start your "remote" cluster by running `yarn es snapshot --license=trial -E cluster.name=europe -E transport.port=9400` in a separate terminal tab.
3. Index a document into your remote cluster by running `curl -X PUT http://elastic:changeme@localhost:9201/my-leader-index --data '{"settings":{"number_of_shards":1,"soft_deletes.enabled":true}}' --header "Content-Type: application/json"`.
Note that these settings are required for testing auto-follow pattern conflicts errors (see below).
1. Ensure Kibana isn't running so it doesn't load up any data into your cluster. Run `yarn es snapshot --license=trial` to install a fresh snapshot. Wait for ES to finish setting up.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuliacech I believe you wrote these originally, but I wasn't able to get the original instructions to work. Can you check this and see if I was just doing it wrong?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating those instructions, @cjcenizal! I can confirm that the previous instructions didn't work for me either, tested your steps and it worked like a charm 👍

@@ -11,21 +11,19 @@ import { EuiCallOut, EuiSpacer } from '@elastic/eui';
export function SectionError(props) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll migrate this to use our shared SectionError as part of #84801

@cjcenizal cjcenizal marked this pull request as ready for review May 15, 2021 01:23
@cjcenizal cjcenizal requested a review from a team as a code owner May 15, 2021 01:23
@elasticmachine
Copy link
Contributor

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

@cjcenizal
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -25,24 +24,9 @@ export interface StartDependencies {
}

export interface RouteDependencies {
router: CcrPluginRouter;
router: IRouter<RequestHandlerContext>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: RequestHandlerContext is the default value of the generics in the IRouter interface

Suggested change
router: IRouter<RequestHandlerContext>;
router: IRouter;

@@ -5,13 +5,12 @@
* 2.0.
*/

import { IRouter, ILegacyScopedClusterClient, RequestHandlerContext } from 'src/core/server';
import { IRouter, RequestHandlerContext } from 'src/core/server';
Copy link
Contributor

@yuliacech yuliacech Jun 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: RequestHandlerContext is not needed, see comment below

Suggested change
import { IRouter, RequestHandlerContext } from 'src/core/server';
import { IRouter } from 'src/core/server';

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @cjcenizal, thanks a lot for migrating CCR to the new js client.
I tested locally and everything seems to work as before, I haven't noticed any regressions. Code changes LGTM too 👍 Just left a couple of nits in the comments.

@cjcenizal cjcenizal force-pushed the ccr/migrate-legacy-es-client branch from 1eab4f7 to f2f583d Compare June 1, 2021 14:58
@kibanamachine
Copy link
Contributor

💚 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
crossClusterReplication 293.3KB 293.5KB +163.0B
Unknown metric groups

References to deprecated APIs

id before after diff
crossClusterReplication 6 2 -4

History

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

@cjcenizal cjcenizal merged commit ba4e0d2 into elastic:master Jun 1, 2021
@cjcenizal cjcenizal deleted the ccr/migrate-legacy-es-client branch June 1, 2021 23:26
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Jun 1, 2021
* Update SectionError component to render error root causes correctly.
* Fix 404 error rendering.
* Add test for follower index update API route.
cjcenizal added a commit that referenced this pull request Jun 2, 2021
* Update SectionError component to render error root causes correctly.
* Fix 404 error rendering.
* Add test for follower index update API route.
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 2, 2021
…sens/kibana into reporting/new-png-pdf-report-type

* 'reporting/new-png-pdf-report-type' of github.com:jloleysens/kibana: (46 commits)
  [Security Solution] Add Ransomware canary advanced policy option (elastic#101068)
  [Exploratory view] Core web vitals (elastic#100320)
  [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034)
  [Lens] Use a setter function for the dimension panel (elastic#101123)
  [Index Patterns] Fix return saved index pattern object (elastic#101051)
  [CI] For PRs, build TS refs before public api docs check (elastic#100791)
  [Maps] fix line and polygon label regression (elastic#101085)
  Migrate CCR to new ES JS client. (elastic#100131)
  [Canvas] Switch Canvas to use React Router (elastic#100579)
  [Expressions] Use table column ID instead of name when set (elastic#99724)
  [DOCS] Updates docs landing page (elastic#100749)
  [DOCS] Corrects typo in step 3 (elastic#101079)
  [DOCS] Updates runtime example in Discover (elastic#100926)
  Migrate kibana.autocomplete config to data plugin (elastic#100586)
  [Uptime] New width/delay definition for waterfall sidebar item tooltip (elastic#100147)
  [FTR] Use importExport for saved_object/basic archive (elastic#100244)
  [Fleet] Better input for multi text input in agent policy builder (elastic#101020)
  [CI] Buildkite support with Baseline pipeline (elastic#100492)
  [Reporting/Telemetry] Do not send telemetry if we are in screenshot mode (elastic#100388)
  Create API keys with metadata (elastic#100682)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:CCR and Remote Clusters release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants