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 option for configuring the location of the container's host filesystem #2187

Merged
merged 1 commit into from
Aug 10, 2016

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Aug 8, 2016

Commit to review: 4b48e2f (isolates these changes from those added in #2184)

This allows the system module to read metrics related to the host machine rather than the container. When using the -system.hostfs flag, directories like /proc and /sys are assumed to be mounted relative to the path given in the value. Here's an example usage.

docker run --volume=/:/hostfs:ro --net=host --name=metricbeat -d metricbeat -system.hostfs=/hostfs

And here is a more restrictive example which does not mount all of the host's filesystems inside the container. system/filesystem will not be able to report metrics for the host's filesystem.

docker run --volume=/proc:/hostfs/proc:ro --volume=/sys/fs/cgroup:/hostfs/sys/fs/cgroup --net=host --name=metricbeat -d metricbeat -system.hostfs=/hostfs

@ruflin
Copy link
Contributor

ruflin commented Aug 9, 2016

@andrewkroh I merged #2184 so this now needs a rebase.

@ruflin
Copy link
Contributor

ruflin commented Aug 9, 2016

  • Is it required to have this as a flag or could we somehow enabled this also as a config option?
  • In the commit message you described very nicely how it works and what the options are. This should be added to the docs.

@andrewkroh andrewkroh force-pushed the feature/mb/configurable-procd branch from 4b48e2f to cab1f1b Compare August 9, 2016 13:26
@andrewkroh
Copy link
Member Author

andrewkroh commented Aug 9, 2016

Rebased.

Is it required to have this as a flag or could we somehow enabled this also as a config option?

I did see your comment about this in issue #2137, but I already had it implemented it like this so I put out the PR, but I do think it could be config file option since it is so easy to set config variables on the CLI now. The one issue I ran into was figuring out how to best route the configuration value down into the module without making a global variable. Probably the best option is to write the value to ModuleConfig, but not make it configurable by the individual modules.

In the commit message you described very nicely how it works and what the options are. This should be added to the docs.

Yeah, I haven't updated any of the docs to discuss cgroups yet either. I updated the meta issue with a task for adding info about how to run Metricbeat in a container.

@ruflin
Copy link
Contributor

ruflin commented Aug 9, 2016

If somehow possible, we should make all options part of the config file, especially with centralised configuration management in mind. I see the command line flags mainly for overwriting existing config options if needed. If it is a config file option, no special flag is needed as the default "overload flag options" can be used.

Alternatively metricbeat.system.hostfs could be used. I think it is fine to use the global namespace for global options?

Update: I see there is an issue with local metricsets knowning about global variables.

…ystem

This allows the system module to read metrics related to the host machine rather than the container. When using the -system.hostfs flag, directories like /proc and /sys are assumed to be mounted relative to the path given in the value. Here's an example usage.

`docker run --volume=/:/hostfs:ro --net=host --name=metricbeat -d metricbeat -system.hostfs=/hostfs`

And here is a more restrictive example which does not mount all of the host's filesystems inside the container. system/filesystem will not be able to report metrics for the host's filesystem.

`docker run --volume=/proc:/hostfs/proc:ro --volume=/sys/fs/cgroup:/hostfs/sys/fs/cgroup --net=host --name=metricbeat -d metricbeat -system.hostfs=/hostfs`
@andrewkroh andrewkroh force-pushed the feature/mb/configurable-procd branch from cab1f1b to dce6786 Compare August 9, 2016 21:20
@ruflin ruflin merged commit 5ae6d88 into elastic:master Aug 10, 2016
@ruflin
Copy link
Contributor

ruflin commented Aug 10, 2016

@andrewkroh I merged this one. I would be nice if we could find a "nice" option to have it in the config file and somehow pass it to the Module.

@andrewkroh andrewkroh deleted the feature/mb/configurable-procd branch August 11, 2016 21:57
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.

2 participants