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

Add docker Integration #632

Merged
merged 13 commits into from Feb 18, 2021

Conversation

fearful-symmetry
Copy link
Contributor

@fearful-symmetry fearful-symmetry commented Feb 2, 2021

What does this PR do?

This adds the Docker integration to Fleet, with all the metrics from the metricbeat docker module.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.

How to test this PR locally

  • Pull down PR
  • On a host with docker, run the elastic agent and enable the module
  • If you're using elastic-package, you'll need to modify the docker-compose to mount the docker socket and run as root:
   elastic-agent:
     image: docker.elastic.co/beats/elastic-agent:${STACK_VERSION}
     depends_on:
       elasticsearch:
         condition: service_healthy
       kibana:
         condition: service_healthy
     healthcheck:
       test: "sh -c 'grep \"Agent is starting\" /usr/share/elastic-agent/elastic-agent.log*'"
       retries: 30
       interval: 1s
     user: root
     environment:
     - "FLEET_ENROLL=1"
     - "FLEET_ENROLL_INSECURE=1"
     - "FLEET_SETUP=1"
     - "KIBANA_HOST=http://kibana:5601"
     volumes:
     - type: bind
       source: ../tmp/service_logs/
       target: /tmp/service_logs/
     - type: bind
       source: /var/run/docker.sock
       target: /var/run/docker.sock
     ports:
       - "0.0.0.0:5066:5066"

Screenshots

Screen Shot 2021-02-02 at 1 46 08 PM

@elasticmachine
Copy link

elasticmachine commented Feb 2, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #632 updated

  • Start Time: 2021-02-18T00:07:53.591+0000

  • Duration: 13 min 58 sec

  • Commit: 999750e

Test stats 🧪

Test Results
Failed 0
Passed 27
Skipped 0
Total 27

Trends 🧪

Image of Build Times

Image of Tests

@fearful-symmetry
Copy link
Contributor Author

The CI issue is I think unrelated?

@andresrc andresrc added the Team:Integrations Label for the Integrations team label Feb 3, 2021
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@mtojek mtojek requested a review from a team February 3, 2021 09:51
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

Left some comments.

@fearful-symmetry how about adding system tests for this one? It will help with catching cases of missing fields etc. I'm not sure if this can happen with connecting to an actual docker socket ( @mtojek wdyt?) but in any case a mock API could be used so as to mock the response and being able to validate the package. There is a similar case in k8s package from which you can reuse the mock json api.

packages/docker/_dev/build/docs/README.md Show resolved Hide resolved
packages/docker/_dev/build/docs/README.md Show resolved Hide resolved
packages/docker/manifest.yml Show resolved Hide resolved
@mtojek
Copy link
Contributor

mtojek commented Feb 3, 2021

@fearful-symmetry how about adding system tests for this one? It will help with catching cases of missing fields etc. I'm not sure if this can happen with connecting to an actual docker socket ( @mtojek wdyt?) but in any case a mock API could be used so as to mock the response and being able to validate the package. There is a similar case in k8s package from which you can reuse the mock json api.

I wonder if we can permanently mount the docker.sock in the Elastic-Agent image:

     - type: bind
       source: /var/run/docker.sock
       target: /var/run/docker.sock

If we can locate the sock path in every OS (Windows/Mac/Linux), then we can permanently set it there.

@fearful-symmetry
Copy link
Contributor Author

So, I know the path is the same on Darwin. Not sure about windows. Not sure if we'd want to? On Linux at least, we also need to run with --user=root to actually connect to the socket.

Also, do we have a guide somewhere for system tests and using whatever mock apis we have?

@fearful-symmetry
Copy link
Contributor Author

Yah @ChrsMark could you explain the mock stuff for k8s?

@fearful-symmetry
Copy link
Contributor Author

Okay, think I found the system test docs. Is there a reason they're all in elastic-package ?

@fearful-symmetry
Copy link
Contributor Author

CI failures look unrelated again.

So, Not sure how testing for this would work. Since this runs in CI, can our CI environment even access a docker daemon socket?

@ChrsMark
Copy link
Member

ChrsMark commented Feb 4, 2021

Yah @ChrsMark could you explain the mock stuff for k8s?

PR adding system tests for k8s using the mock APIs: #569
Using elastic-package and its system testing support will help validate the package not only through the CI but also locally on development. I've no strong opinion about using an actual docker socket or just using a mock API. Adding system tests for a package is a prerequisite in migration list (see #515).

Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

If you rebase this branch against the master branch, the CI will build only changes in this package (diff).

Answering your question - why is the documentation in elastic-package. Test runners are part of this tool, so they're included in tool's documentation. Besides this we have also updated the CONTRIBUTING guide for Integrations repo (incl. table of contents). I believe they should be easier to find.

Regarding the --priviledged and --user=root option, it would be great if you can perform short research on the CI side (experimental PR?).

@fearful-symmetry
Copy link
Contributor Author

Alright, I'm gonna take a look at the moked K8s tests and see what I can figure out.

@fearful-symmetry
Copy link
Contributor Author

Alright, so it looks like the way to do this is to actually spin up a server that can mock whatever the API is serving up. In which case, I'd need to mock the unix socket or TCP endpoint.

@fearful-symmetry
Copy link
Contributor Author

So, experimenting more with mocking the docker tests, and I want some opinions here. I'd need something semi-interactive, since the docker client will try to grab data on different containers, and hit a few APIs. It looks like the k8s tests rely on another container to mock a Prometheus endpoint. I'd need to do something similar, and write some custom python or a custom container to actually mock the API behavior itself.

@ChrsMark any opinions here?

@ChrsMark
Copy link
Member

ChrsMark commented Feb 5, 2021

@fearful-symmetry yeap, I think what you explain should be ok. In my case with k8s there were only static endpoints that I had to mock, return Prometheus and json responses. I think that if you are able to completely mock the full set of endpoints that docker client is gonna hit you should be fine. There are plenty of ready to use json mock servers out there so maybe you avoid writing your own python server. Also have a look into the nginx reverse proxy (in docker-compose.yml) which can actually redirect any specific request to any backend endpoint giving you some more flexibility. However, I'm not sure how easy it would be if requests are not deterministic. Happy to chat about it more if needed.

Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

Can we release as beta instead of experimental?

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Please add units and metric_types to fields definitions when possible (as in #402).

@fearful-symmetry
Copy link
Contributor Author

@ChrsMark / @mtojek can someone help me out with the system tests? I have a mock system setup, but I can't seem to run a test locally.

Running the test command in the docker directory just results in this:

elastic-package test system -v -m
2021/02/09 12:41:07 DEBUG Enable verbose logging
Run system tests for the package
--- Test results for package: docker - START ---
No test results
--- Test results for package: docker - END   ---
Done

The docker-compose.yml and associated test files in docker/_dev, and docker/data_stream/memory/_dev/test/system/config.yml are all in place. No idea what I'm missing.

@ycombinator
Copy link
Contributor

The docker-compose.yml and associated test files in docker/_dev, and docker/data_stream/memory/_dev/test/system/config.yml are all in place.

Would you mind pushing up these files to the PR so we can pull down the PR and try to repro what you're seeing with the system tests locally? Thanks!

@mtojek
Copy link
Contributor

mtojek commented Feb 10, 2021

The docker-compose.yml and associated test files in docker/_dev, and docker/data_stream/memory/_dev/test/system/config.yml are all in place. No idea what I'm missing.

Try this - jump out of the _dev directory, probably it misunderstood _dev/build directory as the project build dir. You can open an issue for this in elastic-package repo.

@fearful-symmetry
Copy link
Contributor Author

@ycombinator / @mtojek pushed a single system test here. We might want to improve the docs for these, since a lot of it is fairly abstract and I had to guess what should go in the docker-compose file.

@fearful-symmetry
Copy link
Contributor Author

@mtojek So, the only doc bug I've found is the issue with naming config.yml I've mentioned above, but I'm not sure if that's an actual mis-naming or if there's some more complicated naming scheme involved.

@mtojek
Copy link
Contributor

mtojek commented Feb 15, 2021

@mtojek So, the only doc bug I've found is the issue with naming config.yml I've mentioned above, but I'm not sure if that's an actual mis-naming or if there's some more complicated naming scheme involved.

I see, it seems like the documentation is bit outdated in this area. I will update it today. Thanks!

Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

LGTM (only one nit-pick)

packages/docker/manifest.yml Outdated Show resolved Hide resolved
@fearful-symmetry
Copy link
Contributor Author

@jsoriano still getting test failures with de-dot off and the mapping changed from flattened to object.

@jsoriano
Copy link
Member

@jsoriano still getting test failures with de-dot off and the mapping changed from flattened to object.

Yes, if we continue using object, then de-dotting needs to be enabled. Only if we change to flattened we should stop de-dotting.

@fearful-symmetry
Copy link
Contributor Author

Okay, now I'm running into issues with de-dotting and the object mapping:

[13] field "docker.image.labels.org_label-schema_schema-version" is undefined
[14] field "docker.image.labels.org_label-schema_url" is undefined
[15] field "docker.image.labels.org_label-schema_usage" is undefined
[16] field "docker.image.labels.org_label-schema_vcs-ref" is undefined
[17] field "docker.image.labels.org_label-schema_vcs-url" is undefined
[18] field "docker.image.labels.org_label-schema_vendor" is undefined
[19] field "docker.image.labels.org_label-schema_version" is undefined
[20] field "docker.image.labels.org_opencontainers_image_created" is undefined

Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

LGTM

@fearful-symmetry fearful-symmetry merged commit 2ad0fd3 into elastic:master Feb 18, 2021
@masci masci mentioned this pull request Mar 5, 2021
19 tasks
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* add docker Integration

* update ecs, add fields and screenshot

* update fields, release as beta

* run format

* try out systems tests

* run format

* setup system tests

* version as a beta release

* disable de-dot

* update object types, add de-dot

* add changelog

* format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants