[Ingest Manager] Use DockerServers service in integration tests. #69822

skh · 2020-06-24T16:12:58Z

Summary

Implements #61699

Supersedes #68172

Documentation

(this is also part of this PR in x-pack/plugins/ingest_manager/dev_docs/api_integration_tests.md)

Many API integration tests for Ingest Manager trigger at some point a connection to the package registry, and retrieval of some packages. If these connections are made to a package registry deployment outside of Kibana CI, these tests can fail at any time for two reasons:

the deployed registry is temporarily unavailable
the packages served by the registry do not match the expectation of the code under test

For that reason, we run a dockerized version of the package registry in Kibana CI. For this to work, our tests must run against a custom test configuration and be kept in a custom directory, x-pack/test/ingest_manager_api_integration.

How to run the tests locally

Usually, having the test server and the test runner in two different shells is most efficient, as it is possible to keep the server running and only rerun the test runner as often as needed. To do so, in one shell in the main kibana directory, run:

$ export INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT=12345
$ yarn test:ftr:server --config x-pack/test/ingest_manager_api_integration/config.ts

In another shell in the same directory, run

$ export INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT=12345
$ yarn test:ftr:runner --config x-pack/test/ingest_manager_api_integration/config.ts

However, it is also possible to alternatively run everything in one go, again from the main kibana directory:

$ export INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT=12345
$ yarn test:ftr --config x-pack/test/ingest_manager_api_integration/config.ts

Port 12345 is used as an example here, it can be anything, but the environment variable has to be present for the tests to run at all.

DockerServers service setup

We use the DockerServers service provided by kbn-test. The documentation for this functionality can be found here:
https://github.com/elastic/kibana/blob/master/packages/kbn-test/src/functional_test_runner/lib/docker_servers/README.md

The main configuration for the DockerServers service for our tests can be found in x-pack/test/ingest_manager_api_integration/config.ts:

Specify the arguments to pass to `docker run`:

  const dockerArgs: string[] = [
    '-v',
    `${path.join(
      path.dirname(__filename),
      './apis/fixtures/package_registry_config.yml'
    )}:/registry/config.yml`,
    '-v',
    `${path.join(
      path.dirname(__filename),
      './apis/fixtures/test_packages'
    )}:/registry/packages/test-packages`,
  ];

-v mounts local paths into the docker image. The first one puts a custom configuration file into the correct place in the docker container, the second one mounts a directory containing additional packages.

Specify the docker image to use

image: 'docker.elastic.co/package-registry/package-registry:kibana-testing-1'

This image contains the content of docker.elastic.co/package-registry/package-registry:master on June 26 2020. The image used here should be stable, i.e. using master would defeat the purpose of having a stable set of packages to be used in Kibana CI.

Packages available for testing

The containerized package registry contains a set of packages which should be sufficient to run tests against all parts of Ingest Manager. The list of the packages are logged to the console when the docker container is initialized during testing, or when the container is started manually with

docker run -p 8080:8080 docker.elastic.co/package-registry/package-registry:kibana-testing-1

Additional packages for testing certain corner cases or error conditions can be put into x-pack/test/ingest_manager_api_integration/apis/fixtures/test_packages. A package filetest has been added there as an example.

Some DockerServers background

For the DockerServers servers to run correctly in CI, the INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT environment variable needs to be under control of the CI environment. The reason behind this: it is possible that several versions of our tests are run in parallel on the same worker in Jenkins, and if we used a hard-coded port number here, those tests would run into port conflicts. (This is also the case for a few other ports, and the setup happens in vars/kibanaPipeline.groovy).

Also, not every developer has docker installed on their workstation, so it must be possible to run the testsuite as a whole without docker, and preferably this should be the default behaviour. Therefore, our DockerServers service is only enabled when INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT is set. This needs to be checked in every test like this:

    it('fetches a .json search file', async function () {
      if (server.enabled) {
        await supertest
          .get('/api/ingest_manager/epm/packages/filetest/0.1.0/kibana/search/sample_search.json')
          .set('kbn-xsrf', 'xxx')
          .expect('Content-Type', 'application/json; charset=utf-8')
          .expect(200);
      } else {
        warnAndSkipTest(this, log);
      }
    });

If the tests are skipped in this way, they are marked in the test summary as pending and a warning is logged:

└-: EPM Endpoints
   └-> "before all" hook
   └-: list
     └-> "before all" hook
     └-> lists all packages from the registry
       └-> "before each" hook: global before each
       │ warn disabling tests because DockerServers service is not enabled, set INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT to run them
       └-> lists all packages from the registry
       └-> "after all" hook
[...]  
           │
           │1 passing (233ms)
           │6 pending
           │

Known issues

Not all tests in ingest_manager_api_integration have been re-enabled. Those that remain need to be changed to work with v2 index templates, which is beyond the scope of this PR.

jonathan-buttner · 2020-06-25T12:56:28Z

x-pack/scripts/functional_tests.js

@@ -52,6 +52,7 @@ const onlyNotInCoverageTests = [
 require.resolve('../test/endpoint_api_integration_no_ingest/config.ts'),
 require.resolve('../test/reporting_api_integration/config.js'),
 require.resolve('../test/functional_embedded/config.ts'),
+ require.resolve('../test/ingest_manager_api_integration'),


Thanks for working on this Sonja! The endpoint team will also hopefully be able to pick up the docker registry changes in our tests too.

How is the package storage repo brought in with the registry? Or do we have to commit the packages to kibana that we want the registry to serve for the tests?

Right now the docker container comes with a certain set of packages. I'm using a container from a specific PR (see here: https://github.com/elastic/kibana/pull/69822/files#diff-61a40c93b50ddc25b22ebb367b272aa9R21 ) but it's not very easy to find out what packages and versions are actually included in that build. I don't have a solution for that yet and am open to all suggestions.

In addition to that it is possible to mount a directory into the container and tell the package registry to serve packages from that in addition to those that are delivered with the container, so it will be possible to craft special packages to test specific corner cases. (Watch out for the next commits on this PR how this will work.)

The issue with that approach is that our production packages contain lots of small files, and I'm not comfortable with adding them to the test directory. If nothing else, it makes PRs unreadable (see spalger#34 for an example).

Right now the docker container comes with a certain set of packages. I'm using a container from a specific PR

Are we able to override that package registry version when running the tests locally? If I understand correctly we can just set INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT to the port of our locally running registry?

The issue with that approach is that our production packages contain lots of small files, and I'm not comfortable with adding them to the test directory. If nothing else, it makes PRs unreadable (see spalger#34 for an example).

Yeah I noticed that too. @ruflin I wonder if we could add functionality to the registry so it could decompress a tar.gz file to get extra packages. That way we could at least commit gzipped files here for tests. It still doesn't make it clear what packages are included until someone decompresses it but it makes it easier to review.

Another option is we could pull down a specific branch of the package storage repo but adding another external dependency might not be a good idea. Plus that might conflict with the proposed package release flow.

Also can we pin the registry to specific commits or is it just master and specific PRs?

Also can we pin the registry to specific commits or is it just master and specific PRs?

You can use anything that is available on https://container-library.elastic.co/r/package-registry/package-registry

@ruflin I wonder if we could add functionality to the registry so it could decompress a tar.gz file to get extra packages. That way we could at least commit gzipped files here for tests.

Seconded, that would make it easier to add extra packages for tests.

Are we able to override that package registry version when running the tests locally?

The docker container that is configured in the config.{ts,js} for your *_api_integration directory is used for all the tests. It is currently not possible to start up a docker container per test. That means that all packages you need in addition to those in the container will need to be in one central place for all your tests.

If I understand correctly we can just set INGEST_MANAGEMENT_PACKAGE_REGISTRY_PORT to the port of our locally running registry?

No. This is an environment variable that will be set by CI tooling. The tests are run in a distributed way, and it is possible that one jenkins worker runs two sets of tests using docker at the same time. Therefore, all the ports that docker containers are mounted to need to be unter control of the CI environment to ensure there are no port conflicts.

When the tests are run locally, the presence of this environment variable indicates that the tests should be run at all. Leaving it out makes it possible to run the tests on a machine without docker. This all will be documented.

Sounds good, thanks for the explanation!

I think adding files instead of tar.gz is a feature. If a package is changed for a test, the PR reviewer should also be able to review the package change. If it is in a .tar.gz, the user can't.

My expectation is that for the tests, we build very small packages dedicated to what we want to test. So the number of files will be low and once commited, from the on the diffs will only be on the changes. I don't expect us to push a new package version with all the files each time we update a package here.

If we want later to test across many packages I would rather have a testing distribution of the registry with these packages already inside instead of storing all the .tar.gz inside, but I think this is a different testing scenario.

skh · 2020-06-25T16:06:02Z

@paul-tavares This needs polishing but is probably worth a look now.

paul-tavares

This is AWSOME @skh 😃
Thanks for working on this. I left a minor comment that would assist with us being able to reuse the setup in our test.

paul-tavares · 2020-06-25T20:34:03Z

x-pack/test/ingest_manager_api_integration/config.ts

+ return {
+ testFiles: [require.resolve('./apis')],
+ servers: xPackAPITestsConfig.get('servers'),
+ dockerServers: defineDockerServersConfig({


So for the endpoint functional tests (those that execute from here x-pack/test/security_solution_endpoint/config.ts), we would need to add something similar to this (along with an endpoint package fixture), right?

Any chance this setup could be extracted into shared lib/module so that we can just re-use from our end, and thus ensure we're testing with the same setup?

I would prose adding a method to x-pack/test/common/services/ingest_manager.ts that would return some of of this setup - example:

return { async setup() { //.... }, getPackageRegistryDockerConfig() { return { registry: { enabled: !!registryPort, image: 'docker.elastic.co/package-registry/package-registry:PR-539', portInContainer: 8080, port: registryPort, args: [ '-v', `${path.join( path.dirname(__filename), './apis/fixtures/package_registry_config.yml' )}:/registry/config.yml`, '-v', `${path.join( path.dirname(__filename), './apis/fixtures/test_packages' )}:/registry/packages/test-packages`, ], waitForLogLine: 'package manifests loaded into memory', }, } } };

We could then use the same setup, add in our package in the args and enable this in our runs of Front-end functional tests (and maybe even our api integration tests as well).

//cc: @nnamdifrankie , @charlie-pichette , @jonathan-buttner FYI

I like the idea. Let's add this after this PR has been reviewed and merged?

sounds good @skh 👍

paul-tavares · 2020-06-25T20:38:09Z

x-pack/test/ingest_manager_api_integration/config.ts

+ dockerServers: defineDockerServersConfig({
+ registry: {
+ enabled: !!registryPort,
+ image: 'docker.elastic.co/package-registry/package-registry:PR-539',


This might just be my ignorance on the subject of docker images 😬 - what is the PR-539 at the end? is that the ID of the package (which maybe is derived from the PR that created it?)

It is the ID of the PR in elastic/package-registry that the image was built from. We need a stable docker image, but it is not decided how we will keep track of the images used for these tests that is both trackable and at least somewhat understandable for the developer. I'm adding some documentation that addresses this, but: good catch.

Oh, and we can use anything that is available on https://container-library.elastic.co/r/package-registry/package-registry , and the @elastic/observablt-robots team can help us put the containers there that we need. We "just" need to come up with a plan when to build a container to use for CI, from what branch and commit, and how to name it. (Cc: @ruflin )

It was too noisy to publish the PR Docker images on the package-registry namespace, so we have changed the PR images to the observability-ci namespace(https://container-library.elastic.co/r/observability-ci/package-registry) we use the PR-XXXX and the SHA1 as tags, every green build deploy a Docker image.
If you need anything you just have to ping us someone will manage to help you.

thanks for the explanation 😄

elasticmachine · 2020-06-26T15:17:18Z

Pinging @elastic/ingest-management (Team:Ingest Management)

skh · 2020-06-26T15:18:16Z

@tylersmalley I'm waiting for CI to voice its opinion, but other than that this is ready for review.

skh · 2020-06-26T17:27:45Z

@elasticmachine merge upstream

jfsiii · 2020-06-26T18:47:48Z

x-pack/plugins/ingest_manager/dev_docs/api_integration_tests.md

+
+## How to run the tests locally
+
+In the `x-pack` directory, run:


I recently learned about some yarn scripts that I think are preferable using node scripts/*

For the two terminal approach listed shown here, in the kibana root,

in one shell, run yarn test:ftr:server --config x-pack/test/ingest_manager_api_integration/config.ts

in another, run yarn test:ftr:runner --config x-pack/test/ingest_manager_api_integration/config.ts

If you want to run everything in one go, this seems to work

yarn test:ftr --config x-pack/test/ingest_manager_api_integration/config.ts

Thank you! I've updated the documentation accordingly.

That's cool. I wasn't aware of that either. 👍

jfsiii · 2020-06-26T18:49:27Z

x-pack/test/ingest_manager_api_integration/apis/file.ts

+ const supertest = getService('supertest');
+ const dockerServers = getService('dockerServers');
+
+ const server = dockerServers.get('registry');


jfsiii · 2020-06-26T18:52:20Z

x-pack/test/ingest_manager_api_integration/apis/list.ts

+ const dockerServers = getService('dockerServers');
+
+ const server = dockerServers.get('registry');
+ // use function () {} and not () => {} here


Thank you so much for this note. I've "discovered" it a few times in my career. Thanks from Future Me for the found time and mental health.

skh · 2020-06-29T07:06:14Z

@elasticmachine merge upstream

mistic

Changes on files under operations team code owners LGTM

kibanamachine · 2020-06-29T13:23:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 51dc949

Build metrics

✅ unchanged

History

💛 Build #57015 was flaky a8ecc01
💔 Build #56782 failed 31b29f1
💚 Build #56725 succeeded 4190651
💚 Build #56690 succeeded e4bddfd
💚 Build #56673 succeeded 6304146

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

ruflin · 2020-06-29T13:29:33Z

👏 Really great to have this in. This will change the way we can test.

…stic#69822) * Partially disable test files. * Use DockerServers in EPM tests. * Only run tests when DockerServers have been set up * Reenable ingest manager API integration tests * Pass new test_packages to registry container * Enable DockerServers tests in CI. * Correctly serve filetest package for file tests. * Add helper to skip test and log warning. * Reenable further file tests. * Add developer documentation about Docker in Kibana CI. * Document use of yarn test:ftr Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # x-pack/scripts/functional_tests.js

) (#70197) * Partially disable test files. * Use DockerServers in EPM tests. * Only run tests when DockerServers have been set up * Reenable ingest manager API integration tests * Pass new test_packages to registry container * Enable DockerServers tests in CI. * Correctly serve filetest package for file tests. * Add helper to skip test and log warning. * Reenable further file tests. * Add developer documentation about Docker in Kibana CI. * Document use of yarn test:ftr Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # x-pack/scripts/functional_tests.js

…stic#69822) * Partially disable test files. * Use DockerServers in EPM tests. * Only run tests when DockerServers have been set up * Reenable ingest manager API integration tests * Pass new test_packages to registry container * Enable DockerServers tests in CI. * Correctly serve filetest package for file tests. * Add helper to skip test and log warning. * Reenable further file tests. * Add developer documentation about Docker in Kibana CI. * Document use of yarn test:ftr Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

skh added 3 commits June 24, 2020 18:09

Partially disable test files.

3449e25

Use DockerServers in EPM tests.

f8250f7

Only run tests when DockerServers have been set up

9c20197

jonathan-buttner reviewed Jun 25, 2020

View reviewed changes

Reenable ingest manager API integration tests

536cc69

skh force-pushed the 61699-docker-registry-ci branch from d6a933e to 536cc69 Compare June 25, 2020 13:47

skh added 2 commits June 25, 2020 18:01

Pass new test_packages to registry container

8eadb6e

Enable DockerServers tests in CI.

2c4d1a6

paul-tavares reviewed Jun 25, 2020

View reviewed changes

skh added 4 commits June 26, 2020 15:21

Correctly serve filetest package for file tests.

ef693e8

Add helper to skip test and log warning.

6304146

Reenable further file tests.

e4bddfd

Add developer documentation about Docker in Kibana CI.

4190651

skh marked this pull request as ready for review June 26, 2020 15:17

skh requested a review from a team June 26, 2020 15:17

skh requested a review from a team as a code owner June 26, 2020 15:17

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 26, 2020

skh added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.9.0 labels Jun 26, 2020

ph assigned skh Jun 26, 2020

Merge branch 'master' into 61699-docker-registry-ci

31b29f1

jfsiii reviewed Jun 26, 2020

View reviewed changes

jfsiii approved these changes Jun 26, 2020

View reviewed changes

elasticmachine and others added 2 commits June 29, 2020 02:06

Merge branch 'master' into 61699-docker-registry-ci

a8ecc01

Document use of yarn test:ftr

51dc949

mistic approved these changes Jun 29, 2020

View reviewed changes

skh merged commit e91594a into elastic:master Jun 29, 2020

skh deleted the 61699-docker-registry-ci branch June 29, 2020 13:24

This was referenced Jun 29, 2020

[Fleet] Run locally the package registry during api integration tests #61699

Closed

[7.x] [Ingest Manager] Use DockerServers service in integration tests. (#69822) #70197

Merged

spalger mentioned this pull request Jun 29, 2020

[ftr/docker-services] Use docker services in epm_api_integration suite #68172

Closed

jen-huang mentioned this pull request Jul 16, 2020

[Ingest Manager] Migrate all API integration tests to use dockerized registry #72151

Closed

jen-huang mentioned this pull request Jul 28, 2020

Failing test: X-Pack API Integration Tests.x-pack/test/api_integration/apis/fleet/setup·ts - apis Fleet Endpoints fleet_setup should create a fleet_enroll user and role #68568

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Manager] Use DockerServers service in integration tests. #69822

[Ingest Manager] Use DockerServers service in integration tests. #69822

skh commented Jun 24, 2020 •

edited

Loading

jonathan-buttner Jun 25, 2020

skh Jun 25, 2020

jonathan-buttner Jun 25, 2020

skh Jun 25, 2020

skh Jun 25, 2020

jonathan-buttner Jun 25, 2020

ruflin Jun 29, 2020

skh commented Jun 25, 2020

paul-tavares left a comment

paul-tavares Jun 25, 2020

skh Jun 26, 2020

paul-tavares Jun 26, 2020

paul-tavares Jun 25, 2020

skh Jun 26, 2020

skh Jun 26, 2020

kuisathaverat Jun 26, 2020

paul-tavares Jun 26, 2020

elasticmachine commented Jun 26, 2020

skh commented Jun 26, 2020

skh commented Jun 26, 2020

jfsiii Jun 26, 2020

skh Jun 29, 2020

jonathan-buttner Jun 29, 2020

jfsiii Jun 26, 2020

jfsiii Jun 26, 2020

skh commented Jun 29, 2020

mistic left a comment

kibanamachine commented Jun 29, 2020

ruflin commented Jun 29, 2020


		## How to run the tests locally

		In the `x-pack` directory, run:

[Ingest Manager] Use DockerServers service in integration tests. #69822

[Ingest Manager] Use DockerServers service in integration tests. #69822

Conversation

skh commented Jun 24, 2020 • edited Loading

Summary

Documentation

How to run the tests locally

DockerServers service setup

Specify the arguments to pass to docker run:

Specify the docker image to use

Packages available for testing

Some DockerServers background

Known issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skh commented Jun 25, 2020

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jun 26, 2020

skh commented Jun 26, 2020

skh commented Jun 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skh commented Jun 29, 2020

mistic left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2020

💚 Build Succeeded

Build metrics

History

ruflin commented Jun 29, 2020

skh commented Jun 24, 2020 •

edited

Loading

Specify the arguments to pass to `docker run`: