-
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
[Endpoint][EPM] Endpoint depending on ingest manager to initialize #62871
[Endpoint][EPM] Endpoint depending on ingest manager to initialize #62871
Conversation
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
Pinging @elastic/ingest-management (Feature:EPM) |
@@ -252,6 +252,7 @@ export enum IngestAssetType { | |||
export enum DefaultPackages { | |||
base = 'base', | |||
system = 'system', | |||
endpoint = 'endpoint', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Installing the endpoint package by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @mostlyjason
@@ -43,6 +43,7 @@ const onlyNotInCoverageTests = [ | |||
require.resolve('../test/licensing_plugin/config.ts'), | |||
require.resolve('../test/licensing_plugin/config.public.ts'), | |||
require.resolve('../test/licensing_plugin/config.legacy.ts'), | |||
require.resolve('../test/functional_endpoint_ingest_failure/config.ts'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oatkiller @kqualters-elastic @jfsiii Any ideas if this needs to be here when we add new testing directories? @jfsiii I didn't see the epm_api_integration
tests in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathan-buttner I'm not sure. Perhaps someone from @elastic/kibana-platform or @elastic/kibana-operations can help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is currently run with the ciGroup
tags to determine which tests will run on CI, as you can see in
kibana/test/scripts/jenkins_xpack_ci_group.sh
Lines 8 to 12 in f84773f
checks-reporter-with-killswitch "X-Pack Chrome Functional tests / Group ${CI_GROUP}" \ | |
node scripts/functional_tests \ | |
--debug --bail \ | |
--kibana-install-dir "$KIBANA_INSTALL_DIR" \ | |
--include-tag "ciGroup$CI_GROUP" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah thanks @spalger !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was a little confusion here. I think we want the endpoint functional UI tests in these 2 paths;
require.resolve('../test/functional_endpoint_ingest_failure/config.ts'),
require.resolve('../test/functional_endpoint/config.ts'),
to be in the alwaysImportedTests
list, and not in the onlyNotInCoverageTests
list. If they are functional UI tests, we do want to run them in the Code Coverage job.
@wayneseymour is testing a change to move them to the other list...
@@ -389,7 +389,8 @@ | |||
"type": "nested" | |||
}, | |||
"file_extension": { | |||
"type": "long" | |||
"ignore_above": 1024, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was conflicting with the mapping installed by the Endpoint package. The file_extension should be a string, not a long.
serverArgs: [ | ||
...xpackFunctionalConfig.get('kbnTestServer.serverArgs'), | ||
// use a bogus port so the ingest manager setup will fail | ||
'--xpack.ingestManager.epm.registryUrl=http://127.0.0.1:12345', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an invalid URL to trigger the toast for the functional test.
This comment has been minimized.
This comment has been minimized.
…enabling ingest in the base tests
const errorText = new Error(defaultText); | ||
// we're leveraging the notification's error toast which is usually used for displaying stack traces of an | ||
// actually Error. Instead of displaying a stack trace we'll display the more detailed error text when the | ||
// user clicks `See the full error` button to see the modal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ Where is the "see the full error" message? The two I see here are:
- 'Ingest Manager failed during its setup.'
- 'App failed to initialize'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full error message is returned by the ingest manager app, it's just below these lines
ingestManager.error.message
if (!ingestManager.success) {
if (ingestManager.error) {
displayToastWithModal(ingestManager.error.message);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with the changes to Ingest Manager. The API call during lifecycle matches #62709
Installing Endpoint for all users is a call for Product / @mostlyjason, IMO but I think it's fine for us to revert/update that part later if we change our mind.
Thanks @jfsiii , yeah that seems reasonable to me |
@jfsiii thanks yeah we talked about this on another thread but I don't remember which one. Seems fine to me. |
…lastic#62871) * Endpoint successfully depending on ingest manager to initialize * Moving the endpoint functional tests to their own directory to avoid enabling ingest in the base tests * Removing page objects and other endpoint fields from base functional * Updating code owners with new functional location * Pointing resolver tests at endpoint functional tests * Pointing space tests at the endpoint functional directory * Adding jest test names
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…chore/put-all-xjson-together * 'master' of github.com:elastic/kibana: [EPM] Update UI copy to use `integration` (elastic#63077) [NP] Inline buildPointSeriesData and buildHierarchicalData dependencies (elastic#61575) [Maps] create NOT EXISTS filter for tooltip property with no value (elastic#62849) [Endpoint] Add link to Logs UI to the Host Details view (elastic#62852) [UI COPY] Fixes typo in max_shingle_size for search_as_you_type (elastic#63071) [APM] docs: add alerting examples for APM (elastic#62864) [EPM] Change PACKAGES_SAVED_OBJECT_TYPE id (elastic#62818) docs: fix rendering of bulleted list (elastic#62855) Exposed AddMessageVariables as separate component (elastic#63007) Add Data - Adding cloud reset password link to cloud instructions (elastic#62835) [ML] DF Analytics: update memory estimate after adding exclude fields (elastic#62850) [Table Vis] Fix visualization overflow (elastic#62630) [Endpoint][EPM] Endpoint depending on ingest manager to initialize (elastic#62871) [Remote clusters] Fix flaky jest tests (elastic#58768) [Discover] Hide time picker when an indexpattern without timefield is selected (elastic#62134) Move search source parsing and serializing to data (elastic#59919) [ML] Functional tests - stabilize typing in mml input (elastic#63091) [data.search.aggs]: Clean up TimeBuckets implementation (elastic#62123) [ML] Functional transform tests - stabilize source selection (elastic#63087) add embed flag to saved object url as well (elastic#62926) # Conflicts: # x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/es_index.tsx
…62871) (#63148) * Endpoint successfully depending on ingest manager to initialize * Moving the endpoint functional tests to their own directory to avoid enabling ingest in the base tests * Removing page objects and other endpoint fields from base functional * Updating code owners with new functional location * Pointing resolver tests at endpoint functional tests * Pointing space tests at the endpoint functional directory * Adding jest test names
This PR makes the Endpoint app depend on the Ingest Manager.
The endpoint functional tests were moved to their own directory
x-pack/test/functional_endpoint
. Adding the ingest manager as a dependency for endpoint required enabling the ingest manager via a couple of flags:When I added these flags to the base functional tests in
functional/config.js
it caused the ingest manager's initialization to be run for many of the plugins' tests and also caused test failures unrelated to the ingest manager and endpoint because templates, index patterns, and other bootstrapping is done by the ingest manager. To avoid these issues a different test configuration file needed to be added that was specific to the endpoint functional tests that would only enable the ingest manager when the endpoint's tests were being run. To accomplish this the flags were added here: https://github.com/elastic/kibana/blob/master/x-pack/test/functional_endpoint/config.ts#L32 and the new directory was added here: https://github.com/elastic/kibana/blob/master/x-pack/scripts/functional_tests.js#L47 to allow the tests to be run in CI. These changes isolated the ingest manager to only being enabled when the endpoint functional tests are being run and not when the other plugins' tests are run.Notable changes
Ingest Manager
start
function of it's public applicationEndpoint
x-pack/test/functional_endpoint
because thexpack.ingestManager.*
flags are causing error when enabled on the base functional testsTesting
To test these changes add these settings to your
kibana.dev.yaml
fileFailure GIF