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

[Endpoint][EPM] Endpoint depending on ingest manager to initialize #62871

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Apr 7, 2020

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:

xpack.ingestManager.enabled: true
xpack.ingestManager.epm.enabled: true
xpack.ingestManager.fleet.enabled: true

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

  • The Ingest Manager is doing its setup in the start function of it's public application
  • The Ingest Manager defines a contract that indicates whether setup was successful and an error message that occurred
  • The Endpoint package is installed by default

Endpoint

  • The endpoint functional tests were moved to their own directory x-pack/test/functional_endpoint because the xpack.ingestManager.* flags are causing error when enabled on the base functional tests
  • Added a toast if the Ingest Manager fails to initialize
  • Added two functional tests
    • One to test the error case and looks for a toast, this test must be done in its own directory because I have to sent a command line flag to an invalid package registry location
    • One to test the success case and looks for the absence of the toast

Testing

To test these changes add these settings to your kibana.dev.yaml file

xpack.ingestManager.enabled: true
# This will cause the ingest manager to look for the registry at a location that doesn't exist on your setup
xpack.ingestManager.epm.registryUrl: 'http://127.0.0.1:8080'
xpack.ingestManager.epm.enabled: true
xpack.ingestManager.fleet.enabled: true
Failure GIF

ingest_failure_small

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project Feature:Endpoint Elastic Endpoint feature v7.8.0 labels Apr 7, 2020
@jonathan-buttner jonathan-buttner requested a review from a team as a code owner April 7, 2020 22:19
@jonathan-buttner jonathan-buttner requested a review from a team April 7, 2020 22:19
@jonathan-buttner jonathan-buttner self-assigned this Apr 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Feature:EPM)

@@ -252,6 +252,7 @@ export enum IngestAssetType {
export enum DefaultPackages {
base = 'base',
system = 'system',
endpoint = 'endpoint',
Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -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'),
Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor

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

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"
, so yes. If you add a new FTR configuration it needs to be added to this list to be executed in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah thanks @spalger !

Copy link
Contributor

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,
Copy link
Contributor Author

@jonathan-buttner jonathan-buttner Apr 7, 2020

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',
Copy link
Contributor Author

@jonathan-buttner jonathan-buttner Apr 7, 2020

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.

@jonathan-buttner

This comment has been minimized.

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
Copy link
Contributor

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'

Copy link
Contributor Author

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);

Copy link
Contributor

@jfsiii jfsiii left a 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.

@jonathan-buttner
Copy link
Contributor Author

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

@jonathan-buttner jonathan-buttner merged commit 6b5cbd5 into elastic:master Apr 9, 2020
@jonathan-buttner jonathan-buttner deleted the endpoint-ingest-setup branch April 9, 2020 13:41
@mostlyjason
Copy link
Contributor

@jfsiii thanks yeah we talked about this on another thread but I don't remember which one. Seems fine to me.

jonathan-buttner added a commit to jonathan-buttner/kibana that referenced this pull request Apr 9, 2020
…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
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 9, 2020
…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
jonathan-buttner added a commit that referenced this pull request Apr 9, 2020
…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
@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Endpoint Elastic Endpoint feature Feature:EPM Fleet team's Elastic Package Manager (aka Integrations) project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants