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

[Elastic Agent] Add docker composable dynamic provider. #20842

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

blakerouse
Copy link
Contributor

@blakerouse blakerouse commented Aug 28, 2020

What does this PR do?

Adds a docker composable dynamic provider.

Why is it important?

Support generating inputs from discovered docker containers.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Using the following configuration results in the inputs results in a new input per container.

inputs:
  - type: logfile
     streams:
      - path: /var/lib/docker/containers/${docker.container.id}/${docker.container.id}-json.log

Related issues

@blakerouse blakerouse self-assigned this Aug 28, 2020
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Aug 28, 2020
@blakerouse blakerouse marked this pull request as ready for review August 28, 2020 01:22
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 28, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request elastic/beats#20842 updated]

  • Start Time: 2020-09-08T15:07:43.574+0000

  • Duration: 34 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 1691
Skipped 6
Total 1697

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Nice. How similar is this code from our Beats autodiscovery code?

I really like that this allows us to start playing around with it.


if err := watcher.Start(); err != nil {
// warn only; return nil (do nothing)
c.logger.Warnf("Failed starting docker provider: %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

On the Beats code based we tried no to use Warn as it is this strange middleground between info and error. Is user interaction needed or not? Is it a bug or not? I would suggest to move this to the Info level and phrase it more something like: Docker provider not started as docker not available (if we can make this distinction).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@blakerouse
Copy link
Contributor Author

Nice. How similar is this code from our Beats autodiscovery code?

This is very, very similar. Because the docker code was so well encapsulated in libbeat/common/docker.NewWatcher it was very easy to add. That is also why this module can easily be implemented in less than 200 lines of code.

I really like that this allows us to start playing around with it.

@ph
Copy link
Contributor

ph commented Aug 31, 2020

cc @exekias You might want to watch this :)

@exekias
Copy link
Contributor

exekias commented Sep 2, 2020

Nice job! Maybe this means we don't really need to refactor current providers #19271?

I see it's possible to pass the config for the provider, how would that look in the YAML?

@blakerouse
Copy link
Contributor Author

@exekias I think creating the new providers in Agent will be the most amount of work, but if most of the watcher logic is in libbeat then Agent can easily pull it in and use it.

For configuring it looks like:

providers:
  docker:
    host: "unix:///var/run/docker.sock"
    tls:
      verification_mode: none

}

// Run runs the environment context provider.
func (c *dynamicProvider) Run(comm composable.DynamicProviderComm) error {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would call it Start instead of Run as it is non blocking, but we change that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not called Start because there is no Stop. It is stopped only by the cancelling of the passed context. It is up to the provider on how it wants to implement. Example is the local_dynamic provider it just reads the config and returns, as it does need a goroutine to always be running.

"id": container.ID,
"name": container.Name,
"image": container.Image,
"labels": processorLabelMap,
Copy link
Member

Choose a reason for hiding this comment

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

We should revisit later on what the default info is that we ship, if we potentially should reduce it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for me.

Copy link
Member

Choose a reason for hiding this comment

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

@blakerouse Could you follow up with an issue? I'm pretty sure we will forget otherwise :-D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #21135

@blakerouse blakerouse merged commit f017e24 into elastic:master Sep 16, 2020
@blakerouse blakerouse deleted the agent-docker-provider branch September 16, 2020 16:34
blakerouse added a commit to blakerouse/beats that referenced this pull request Sep 16, 2020
* Add docker provider.

* Add changelog.

* Update docker start message to info.

(cherry picked from commit f017e24)
blakerouse added a commit that referenced this pull request Sep 16, 2020
…mic provider. (#21117)

* [Elastic Agent] Add docker composable dynamic provider. (#20842)

* Add docker provider.

* Add changelog.

* Update docker start message to info.

(cherry picked from commit f017e24)

* Fix docker provider builder.
v1v added a commit to v1v/beats that referenced this pull request Sep 18, 2020
…ne-2.0

* upstream/master: (44 commits)
  Update users.asciidoc (elastic#20802) (elastic#21108)
  Fix docker provider builder. (elastic#21118)
  [Elastic Agent] Add docker composable dynamic provider. (elastic#20842)
  Add new modules/filesets from rsa2elk for 7.10 (elastic#20820)
  Fix broken links to external websites (elastic#21061)
  [docs] typo in the command line (elastic#20799)
  [Filebeat] add panos type and sub_type (elastic#20912)
  Move the `compute_vm_scalset` to  a light metricset and map the cloud metadata (elastic#21038)
  [Filebeat] Add support for Cloudtrail digest files (elastic#21086)
  Add metrics collection from cost explorer into aws/billing metricset (elastic#20527)
  Add vendoring to Google Cloud Functions again (elastic#21070)
  [Elastic Agent] Add fleet.host.id for sending to endpoint. (elastic#21042)
  Do not need Google credentials before using it (elastic#21072)
  [Filebeat][New Module] Zoom webhook module (elastic#20414)
  Add support for GMT timezone offset in decode_cef (elastic#20993)
  Filebeat: Fix random error on harvester close (elastic#21048)
  Add ingress controller dashboards (elastic#21052)
  Fix loggers in composable module. (elastic#21047)
  [Ingest Manager] Increase kibana client timeout to 5 minutes (elastic#21037)
  Add changelog. (elastic#21041)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants