-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Adapt ES-Archiver to use savedObject types defined in the new platform #55860
Comments
Pinging @elastic/kibana-platform (Team:Platform) |
One solution is to get back from our programmatic API to a more static declaration (as it's done for the plugin's configuration for example). This way, we would have the archiver scan for the SO definitions as it's currently doing for legacy plugins. But I think we would really like to avoid this if possible. Other question that could be asked is why does the archiver needs to performs the migration after a load? could there be a workaround to avoid doing this? |
My guess is that because the functional test runner creates one kibana server for the entire test suite (not for each To solve this, the esArchiver could call an internal SavedObjects API to retrieve all registered types, or the esArchiver could call an internal SavedObjects |
Implementation depends on #55857, but as the esArchiver is not run from inside kibana, and as some SO-related internals are not serializable (thinking mostly of migrations), I don't think this is an option
Just to be sure to understand, as the esArchiver does not run from inside the kibana server during FTR, by |
Even though the esArchiver runs in a separate process from the Kibana FTR server, it could start it's own Kibana server just for migrations which would give it access to core's setup/start API's. Similar to how we start the server in kibana/src/test_utils/kbn_server.ts Lines 287 to 290 in 205fbce
This internal server would have to use the same configuration as the FTR server (but listen on a different http port). The disadvantage would be that we potentially have a slower startup for test suites but since these two processes can run in parallel it might be negligible... Another option would be to use IPC between the FTR parent process and the kibana server child process to send a "rerun migrations" message. I don't know the internals of the FTR very well so there might be some complications I didn't consider. But I think it's worth exploring the options so that we don't design the API around our testing infrastructure. |
If we go in this direction, I think it would be better to have a specific method on our root server (or somewhere else) that would performs only what is required for the migration (doing I agree that avoiding exposing test-purposes APIs should be avoided. I don't really like giving the capability to the es-archiver to boot a whole kibana/core ping @spalger @joshdover ^ your insight is welcome. |
TIL: we actually already have a SO util for FTR in However I have the feeling that adding an additional, internal API endpoint to perform a savedObject migration and using the test-util |
This was going to be my suggestion. If we go this route, it would be great if this endpoint is only available if some CLI flag is passed to the Kibana server. While it is the "quick-win" solution, I don't think it's necessarily a bad one. Another option would be to use the SavedObjects REST API to import the documents that EsArchiver needs to create. Since all documents that go through the import API are migrated by the Kibana server, this should solve the problem. This would only be necessary for data that is imported into the |
I know some where not big fan of 'test-only' flags, as it means different behavior during FTR (which are supposed to run in 'pure' production mode), and actual prod mode. But I agree that exposing that kind of endpoint in production should (really) be avoided.
That would be great. However it seems the es archiver is actually dumping whole indexes instead of only SO documents (see @elastic/kibana-operations @tylersmalley Can we get some insight / help here ? |
My feeling is the proper solution would be to remove the need for migrations to be ran before functional tests. I have long disliked that the es_archive's were gunziped for multiple reasons - if we kept these in plain text, provided tooling to update them when new migrations are added, could we do away with needing to run migrations at all in functional tests? |
@tylersmalley To be sure to understand: The only usage of executing a SO migration after |
This sounds right from my experience. The only other case I can think of is when maybe you want to test migrations themselves by loading old objects and then making sure features still work. I'm not even sure that is a current usecase but even if it is, those subset of tests could use the SO Import API instead. |
The SO import API does not migrate imported objects? |
I spent some time trying to see how we could adapt the savedObject's However, the data format for the archiver's documents is the 'raw' es format ( Our OptionsI think we only have two options here:
We would be creating an API that would only be used for tests. If we go in this direction, I see no upside on this approach comparing to add a
We will need proper tooling to convert all the test data. As the converter will need to be aware of registered types, the tool should be able to kickstart the server (at least to the end of the This solution should provides both a one-time migration of the existing data, and adds proper tooling for plugin to be able to add/convert any new test data they would like to add later on. Course of actionI think solution two is what we want long term. Having actual SO representation of our test data probably makes more sense than the actual low-level ES format, and we would be using proper SO api to load SOs instead of dumping them into ES. However, these are some deep, long and sensible changes. As this issue is a blocker to implements SO registration in NP, but migrating the data to the new format is not, I think the best course of action is to implement the first solution now (the If this is acceptable for everyone, I would start working in the |
Completely agree 👍 |
Also agree with the workaround solution for now. Seems like technical debt that is worth the tradeoff. Let's make sure to open an issue with enough details on how to resolve this in the future. |
Created #56727 for the long term solution. Will start working on the |
Currently, the es-archiver is creating a KibanaMigrator to migrate the kibana index after a load
kibana/src/es_archiver/actions/load.js
Lines 90 to 97 in 8e9a8a8
However to do that, it manually gathers the uiExports from the legacy plugins to get the SO schemas/mappings/migrations
kibana/src/es_archiver/lib/indices/kibana_index.js
Lines 37 to 49 in c65c28a
kibana/src/es_archiver/lib/indices/kibana_index.js
Lines 80 to 81 in c65c28a
kibana/src/es_archiver/lib/indices/kibana_index.js
Lines 106 to 113 in c65c28a
After #55825 and the other SO types related issues are done, the SO types will be created from the new platform, and the archiver will not be able to be aware of them for the migration.
We need to find a way to have the archiver be aware of the NP-declared types. However, as the types registration is now done programmatically from the plugins
setup
phase, I'm really not seeing anything obvious to address this.The text was updated successfully, but these errors were encountered: