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

Failing ES Promotion: logstash pipeline list route add button links to the empty pipeline editor #86038

Closed
tylersmalley opened this issue Dec 15, 2020 · 17 comments · Fixed by #87056

Comments

@tylersmalley
Copy link
Contributor

tylersmalley commented Dec 15, 2020

This failure is preventing the promotion of the current Elasticsearch nightly snapshot.

For more information on the Elasticsearch snapshot promotion process: https://www.elastic.co/guide/en/kibana/master/development-es-snapshots.html

This is being called from https://github.com/elastic/kibana/blob/master/x-pack/plugins/logstash/server/routes/upgrade/upgrade.ts#L28-L40 every time a user visits the edit pipeline page.

12:22:47               └-> links to the empty pipeline editor
12:22:47                 └-> "before each" hook: global before each
12:22:47                 └-> "before each" hook
12:22:48                   │ERROR browser[SEVERE] http://localhost:6121/38971/bundles/core/core.entry.js 12:194165 TypeError: Failed to fetch
12:22:48                   │          at _callee3$ (http://localhost:6121/38971/bundles/core/core.entry.js:6:43987)
12:22:48                   │          at l (http://localhost:6121/38971/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js:321:968491)
12:22:48                   │          at Generator._invoke (http://localhost:6121/38971/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js:321:968244)
12:22:48                   │          at Generator.forEach.e.<computed> [as throw] (http://localhost:6121/38971/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js:321:968848)
12:22:48                   │          at fetch_asyncGeneratorStep (http://localhost:6121/38971/bundles/core/core.entry.js:6:39045)
12:22:48                   │          at _throw (http://localhost:6121/38971/bundles/core/core.entry.js:6:39453)
12:22:57                 │ proc [kibana]   log   [20:22:56.191] [error][http] StatusCodeError: [illegal_argument_exception] Cannot update mappings in [.logstash]: system indices can only use mappings from their descriptors, but the mappings in the request did not match those in the descriptors(s)
12:22:57                 │ proc [kibana]     at respond (/dev/shm/workspace/kibana-build-xpack-2/node_modules/elasticsearch/src/lib/transport.js:349:15)
12:22:57                 │ proc [kibana]     at checkRespForFailure (/dev/shm/workspace/kibana-build-xpack-2/node_modules/elasticsearch/src/lib/transport.js:306:7)
12:22:57                 │ proc [kibana]     at HttpConnector.<anonymous> (/dev/shm/workspace/kibana-build-xpack-2/node_modules/elasticsearch/src/lib/connectors/http.js:173:7)
12:22:57                 │ proc [kibana]     at IncomingMessage.wrapper (/dev/shm/workspace/kibana-build-xpack-2/node_modules/lodash/lodash.js:4949:19)
12:22:57                 │ proc [kibana]     at IncomingMessage.emit (events.js:327:22)
12:22:57                 │ proc [kibana]     at endReadableNT (_stream_readable.js:1327:12)
12:22:57                 │ proc [kibana]     at processTicksAndRejections (internal/process/task_queues.js:80:21) {
12:22:57                 │ proc [kibana]   status: 400,
12:22:57                 │ proc [kibana]   displayName: 'BadRequest',
12:22:57                 │ proc [kibana]   path: '/.logstash/_mapping',
12:22:57                 │ proc [kibana]   query: {},
12:22:57                 │ proc [kibana]   body: {
12:22:57                 │ proc [kibana]     error: {
12:22:57                 │ proc [kibana]       root_cause: [Array],
12:22:57                 │ proc [kibana]       type: 'illegal_argument_exception',
12:22:57                 │ proc [kibana]       reason: 'Cannot update mappings in [.logstash]: system indices can only use mappings from their descriptors, but the mappings in the request did not match those in the descriptors(s)'
12:22:57                 │ proc [kibana]     },
12:22:57                 │ proc [kibana]     status: 400
12:22:57                 │ proc [kibana]   },
12:22:57                 │ proc [kibana]   statusCode: 400,
12:22:57                 │ proc [kibana]   response: '{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"Cannot update mappings in [.logstash]: system indices can only use mappings from their descriptors, but the mappings in the request did not match those in the descriptors(s)"}],"type":"illegal_argument_exception","reason":"Cannot update mappings in [.logstash]: system indices can only use mappings from their descriptors, but the mappings in the request did not match those in the descriptors(s)"},"status":400}',
12:22:57                 │ proc [kibana]   toString: [Function (anonymous)],
12:22:57                 │ proc [kibana]   toJSON: [Function (anonymous)]
12:22:57                 │ proc [kibana] }
12:22:57                 │ proc [kibana]  error  [20:22:56.165]  Error: Internal Server Error
12:22:57                 │ proc [kibana]     at HapiResponseAdapter.toInternalError (/dev/shm/workspace/kibana-build-xpack-2/src/core/server/http/router/response_adapter.js:69:19)
12:22:57                 │ proc [kibana]     at Router.handle (/dev/shm/workspace/kibana-build-xpack-2/src/core/server/http/router/router.js:177:34)
12:22:57                 │ proc [kibana]     at runMicrotasks (<anonymous>)
12:22:57                 │ proc [kibana]     at processTicksAndRejections (internal/process/task_queues.js:93:5)
12:22:57                 │ proc [kibana]     at handler (/dev/shm/workspace/kibana-build-xpack-2/src/core/server/http/router/router.js:124:50)
12:22:57                 │ proc [kibana]     at module.exports.internals.Manager.execute (/dev/shm/workspace/kibana-build-xpack-2/node_modules/@hapi/hapi/lib/toolkit.js:45:28)
12:22:57                 │ proc [kibana]     at Object.internals.handler (/dev/shm/workspace/kibana-build-xpack-2/node_modules/@hapi/hapi/lib/handler.js:46:20)
12:22:57                 │ proc [kibana]     at exports.execute (/dev/shm/workspace/kibana-build-xpack-2/node_modules/@hapi/hapi/lib/handler.js:31:20)
12:22:57                 │ proc [kibana]     at Request._lifecycle (/dev/shm/workspace/kibana-build-xpack-2/node_modules/@hapi/hapi/lib/request.js:312:32)
12:22:57                 │ proc [kibana]     at Request._execute (/dev/shm/workspace/kibana-build-xpack-2/node_modules/@hapi/hapi/lib/request.js:221:9)
12:22:57                 │ proc [kibana] Sending error to Elastic APM { id: '959350d32b72378e8b68823c9d9a256c' }
12:22:59                 │ERROR browser[SEVERE] http://localhost:6121/api/logstash/upgrade - Failed to load resource: the server responded with a status of 500 (Internal Server Error)
12:22:59                 │ERROR browser[SEVERE] http://localhost:6121/app/management/ingest/pipelines/pipeline/new-pipeline 0:0 Uncaught (in promise)
12:29:05                 └- ✖ fail: logstash pipeline list route add button links to the empty pipeline editor
12:29:05                 │      Error: Timeout of 360000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/dev/shm/workspace/kibana/x-pack/test/functional/apps/logstash/pipeline_list.js)
12:29:05                 │       at listOnTimeout (internal/timers.js:554:17)
12:29:05                 │       at processTimers (internal/timers.js:497:7)
12:29:05                 │ 

https://kibana-ci.elastic.co/job/elasticsearch+snapshots+verify/1911/execution/node/389/log/

@tylersmalley
Copy link
Contributor Author

@jaymode, I see you took a pass at this here: #80405

The current issue we're running into is that a mapping update is being performed every time someone accesses the edit pipeline view. This update call is also being done by the current user.

It doesn't seem like this is the desired behavior, or if it's still valid for 8.0, but I am having a hard time tracking someone down that can confirm. Do you have any thoughts?

I am considering moving forward with #86045 to disable the mapping update call until someone from the Ingest team can address.

@jaymode
Copy link
Member

jaymode commented Dec 16, 2020

@tylersmalley sorry for this, it slipped my mind when we made a ES change; I think this is due to elastic/elasticsearch#66190

@pugnascotia can you help here? I think there is a conflict between making the logstash system index managed by ES and the code in Kibana. I think we need to confirm everything is right in the ES template and then proceed with removing the mapping/template update code in Kibana since we are handling it in ES now. If you need anything from the Logstash side please reach out to @kaisecheng.

cc @gwbrown @williamrandolph

@kobelb
Copy link
Contributor

kobelb commented Dec 16, 2020

The logstash plugin in Kibana is only adding the following mapping, when the .logstash index already exists per https://github.com/kobelb/kibana/blob/d015c24509d0ed8a2bce5802fea036ae7c910a38/x-pack/plugins/logstash/server/routes/upgrade/upgrade.ts#L32-L37

    properties: {
      pipeline_settings: {
        dynamic: false,
        type: 'object',
      },
    },

This property appears to be included in @pugnascotia's definition of the .logstash index in https://github.com/elastic/elasticsearch/pull/66190/files#diff-152d6ce430e540f86b4643fb19025325dc4d76329d41ffad828d82740af8cbb6R151-R154

      builder.startObject("pipeline_settings");
      builder.field("dynamic", false);
      builder.field("type", "object");
      builder.endObject();

So, when ES creates the new .logstash indices, this seems fine. However, the logstash plugin in Kibana is mutating existing .logstash indices to include this property. It feels potentially unsafe to remove this logic from the logstash plugin in Kibana. @ycombinator it looks like this logic was originally added by you in https://github.com/elastic/x-pack-kibana/pull/4573, am I understanding this correctly?

@pugnascotia
Copy link
Contributor

I do think that, going forwards, if Elasticsearch is managing a system index then it needs to be the only component managing them (and indeed ES now enforces this).

@kobelb Can you elaborate on what you mean by "potentially unsafe"? If there's a gap between what ES is now doing and what needs to be done, I'd like to get that closed ASAP. It so happens that I hadn't backported elastic/elasticsearch#66190 to 7.x before the 7.11 branch was cut, so we have a little breathing room.

@kobelb
Copy link
Contributor

kobelb commented Dec 16, 2020

@kobelb Can you elaborate on what you mean by "potentially unsafe"?

For sure. My fear is that there are existing .logstash indices that already exist without the "pipeline_settings" property. So if we remove this code from Kibana, people will hit a bug when trying to use Kibana's Logstash Pipeline editor.

@gwbrown
Copy link

gwbrown commented Dec 16, 2020

From the linked implementation of the update in Kibana, it looks like there's already a call made to ensure the index exists before updating the mapping - could we predicate doing the mapping update on one of [the mapping doesn't already have pipeline_settings; the Elasticsearch version is on <7.12.0; the index isn't flagged as a system index]? With the intent being that if Elasticsearch is managing the index, Kibana won't try to update the mapping, but otherwise can go ahead.

@kobelb
Copy link
Contributor

kobelb commented Dec 16, 2020

@gwbrown we can do that. However, won't we still have an issue if the index was created a long time ago before system-indices?

@gwbrown
Copy link

gwbrown commented Dec 16, 2020

Hm, I suppose there's still the case of a cluster upgrading directly via FCR from 6.0.0-6.3.0 to 7.12.0, in which case the mapping update may still need to be made but the index would be registered as a system index. We haven't built the final infrastructure for automated mapping updates to system indices yet, but we might be able to use some existing infrastructure temporarily to pull that check/upgrade into Elasticsearch - what do you think, @pugnascotia?

@pugnascotia
Copy link
Contributor

@gwbrown when you say "the final infrastructure for automated mapping updates", do you mean something other than what the SystemIndexManager class does? Because it will automatically update mappings. I haven't tried it with an index that originated in v6, mind you.

@gwbrown
Copy link

gwbrown commented Dec 17, 2020

Oh, I was thinking of the reindex infrastructure (i.e. for mapping changes that can't be applied as updates to an existing index). Whoops. If the infra you built will automatically update mappings, if we make sure that works, can we just rely on that and prevent Kibana from updating the mappings if ES version >=7.12.0? Having an index that originated in v6 shouldn't be a problem for updating the mapping.

@pugnascotia
Copy link
Contributor

Sounds like what I've built is exactly what we need. The SystemIndexManager will look at _meta.logstash-version in the mappings, and if that version is lower than Version.CURRENT (i.e. 7.12.0 or higher), then the mappings will be updated.

@tylersmalley
Copy link
Contributor Author

@gwbrown @pugnascotia I am not familiar with SystemIndexManager. To confirm, ES will handle the mapping update allowing the mapping update call to be removed from Kibana?

@ycombinator
Copy link
Contributor

I was on PTO last week so late to this conversation.

Indeed, I wrote the Kibana code for the .logstash mapping upgrade. As @kobelb surmised in #86038 (comment), the code exists to handle cases where a user has an existing but older .logstash mapping and needs the new additions to it before certain features in the UI can be correctly used. If this is upgrade responsibility can now be handled in ES itself then it would be safe to remove this piece of code from the logstash plugin in Kibana.

@spalger
Copy link
Contributor

spalger commented Dec 31, 2020

Closed by #87056

@spalger spalger closed this as completed Dec 31, 2020
@pugnascotia
Copy link
Contributor

Thanks @spalger 🙏

@spalger
Copy link
Contributor

spalger commented Jan 6, 2021

I guess we're doing this in 7.12 too? I'm not super concerned about the removal of this API in a minor as it's really only intended for internal use (I assume) but just wanted to raise a minor alert that this means we're removing functionality in a minor Kibana version.

Backport from ES: elastic/elasticsearch#66510

@pugnascotia
Copy link
Contributor

Yes, I backported the ES change to 7.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants