Add support for docker autodiscover to monitor containers on host network #6708

vjsamuel · 2018-03-30T03:52:28Z

In today's approach autodiscover docker provider can not monitor containers running on Host network. If a container runs on host network it would not have .NetworkSettings.IPAddress as seen below:

# docker inspect --format '{{ .NetworkSettings.IPAddress }}' jobmanager #runs as --net=host

# docker inspect --format '{{ .NetworkSettings.IPAddress }}' filebeat #normal container
172.17.0.2

To be able to monitor these containers we detect a host address that can be used be the docker auto discover as follows:

If filebeat/metricbeat runs as a container, try to check with the the Docker daemon if the hostname is the prefix of one of the running containers. If yes, get the Gateway of the network interface. That will be the host address that can be used.
If filebeat/metricbeat are running as processes on the host, get an IP address on the host that is not a loop back address and use them to monitor.

elasticmachine · 2018-03-30T03:52:30Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2018-03-30T03:52:37Z

libbeat/common/docker/util.go

+
+// GetHostIp can look at the hostname and give the container gateway/host IP based on where the given
+// beat is running.
+func GetHostIp(watcher Watcher) string {


func GetHostIp should be GetHostIP

houndci-bot · 2018-03-30T03:52:37Z

libbeat/autodiscover/providers/docker/docker.go

@@ -65,13 +66,16 @@ func AutodiscoverBuilder(bus bus.Bus, c *common.Config) (autodiscover.Provider,
 		return nil, err
 	}

+	hostIp := docker.GetHostIp(watcher)


var hostIp should be hostIP

houndci-bot · 2018-03-30T03:52:37Z

libbeat/autodiscover/providers/docker/docker.go

@@ -24,6 +24,7 @@ type Provider struct {
 	appenders     autodiscover.Appenders
 	watcher       docker.Watcher
 	templates     *template.Mapper
+	hostIp        string


struct field hostIp should be hostIP

ruflin · 2018-03-30T08:11:20Z

As we already had this feature in 6.2, could you add a CHANLOGE entry? As far as I understand the change, it's an addition / enhancement and will not break anything existant.

For the data.host change: This change is only indirectly related to the previous one. So probably worth 2 changelog entries?

ruflin · 2018-03-30T08:11:29Z

jenkins, test it

vjsamuel · 2018-03-30T16:53:47Z

@ruflin i think it is necessary to break this PR into 2. Otherwise you wont be able to back port this fix to 6.2. Thoughts?

ruflin · 2018-03-30T18:31:00Z

I'm always up for 2 PR's if they can be separated, especially if one of them needs backport.

vjsamuel · 2018-04-02T00:12:12Z

@ruflin this PR has to be back ported to 6.2 and I have pulled out the hints builder changes to a separate PR[https://github.com//pull/6727] to allow the same.

ruflin

LGTM. @jsoriano would you mind having a look?

ruflin · 2018-04-03T11:54:27Z

jenkins, test it

jsoriano · 2018-04-03T15:57:45Z

libbeat/common/docker/util.go

+	// Gateway address of a container is one of the interfaces on the host that can be used to connect to
+	// containers that are being run on host network.
+	for cid, container := range watcher.Containers() {
+		if strings.Index(cid, hostname) == 0 && container != nil {


In docker inspect the hostname is available under Config, I guess that it is also in the API, could we store it when requesting containers information so we don't need to assume anything? 🙂

jsoriano · 2018-04-03T16:03:50Z

libbeat/common/docker/util.go

+	for _, address := range addrs {
+		// Check the address type and if it is not a loop back then return it
+		if ipnet, ok := address.(*net.IPNet); ok && !ipnet.IP.IsLoopback() {
+			if ipnet.IP.To4() != nil {


Any reason to limit to IPv4 only?

jsoriano · 2018-04-03T16:08:11Z

libbeat/common/docker/watcher.go

+
+			if net.Gateway != "" {
+				gateways = append(gateways, net.Gateway)
+			}


Here, when requesting information from the daemon we could also get the Hostname, at least if we use inspect, not sure with list.

we wont get a hostname for a container running on host network.

A container running in host network gets the hostname of the host.

jsoriano · 2018-04-03T16:39:32Z

libbeat/common/docker/util.go

+	return getLocalIP()
+}
+
+func getLocalIP() string {


This method is a bit tricky, are we sure that net.InterfaceAddrs() always returns the addresses in the same order? what happens if an address is added or removed?

I think there is no safe method to guess the address of a host with multiple addresses 😐 maybe for that case we should allow to configure a host address in metricbeat (e.g. sensu has the address setting, and kubelet has the --node-ip flag), and take it from there @ruflin wdyt?

I like the idea of making it configurable. We can try to guess by default but we allow the user to overwrite it if the guessing is wrong. Normally I would suggest to go the other way and have the configuration by default and require the user to enable the magic part but as this is part of auto discovery the user expects no configuration and magic I would say.

I talked with @vjsamuel offline and sory, I had misunderstood something, as the IP is only required to connect with the host namespace then yes, any IP of the host is equally valid :)

ruflin · 2018-04-06T06:42:01Z

jenkins, test it

jsoriano · 2018-04-06T10:23:16Z

libbeat/common/docker/watcher.go

+		if len(ipaddresses) == 0 {
+			info, err := w.client.ContainerInspect(w.ctx, c.ID)
+			if err == nil {
+				ipaddresses = append(ipaddresses, info.Config.Hostname)


metricbeat running in docker will only be able to resolve the hostname of the host if it is in the DNS or if the host exists in /etc/hosts in the container, what is not always necessarily true.

I think we should check here if metricbeat is running in docker (or in a network namespace).
If it is, then we use its gateway, that should be an IP in the host, as you proposed in a previous version, but now we are sure of knowing the hostname.
If it isn't running as docker it is also running in host mode and localhost or a local IP can be used.

i checked this behavior and it seems to work.

neither approach work (using hostname and gateway ip) on mac however

ruflin · 2018-04-10T11:09:17Z

@jsoriano @vjsamuel Can you push this one forward together? @exekias FYI

exekias · 2018-04-10T11:12:22Z

I'm ok with this change if we don't find something more reliable. I was planning to do some research on this and try to come up with some alternative way to retrieve host IP, not sure it's possible though :)

exekias · 2018-04-10T21:57:12Z

I've been researching this with @vjsamuel, it seems we can get host IP by checking the default gateway:

root@85ae06759fea:/# netstat -nr | grep '^0\.0\.0\.0' | awk '{print $2}'
172.17.0.1

It should be possible to read it from proc filesystem, I guess this will support more use cases, still to be tested

…work

vjsamuel · 2018-06-20T00:13:26Z

@exekias i was revisiting the code today, i feel that this might be the cleanest solution code-wise. what do you think? if you are not convinced, i can make the above discussed change

exekias

LGTM @vjsamuel, this already improves the situation. I left some comments

exekias · 2018-06-20T08:56:51Z

CHANGELOG.asciidoc

@@ -161,6 +161,9 @@ https://github.com/elastic/beats/compare/v6.2.3...master[Check the HEAD diff]
 - Add experimental Jolokia Discovery autodiscover provider. {pull}7141[7141]
 - Add owner object info to Kubernetes metadata. {pull}7231[7231]
 - Add beat export dashboard command. {pull}7239[7239]
+- Add a default seccomp (secure computing) filter on Linux that prohibits
+  execve, execveat, fork, and vfork syscalls. A custom policy can be configured. {issue}5213[5213]
+- Add support for docker autodiscover to monitor containers on host network {pull}6708[6708]


some entries sneaked in here

exekias · 2018-06-20T08:57:28Z

libbeat/common/docker/watcher.go

+			if err == nil {
+				ipaddresses = append(ipaddresses, info.Config.Hostname)
+			} else {
+				logp.Debug("docker", "unable to inspect container %s due to error %v", c.ID, err)


this should probably be a warning message? as it should not happen frequently, and can cause some debugging nightmares

exekias

WFG

exekias · 2018-06-20T16:10:42Z

jenkins, test this please

houndci-bot reviewed Mar 30, 2018

View reviewed changes

vjsamuel force-pushed the add_host_network_support branch 2 times, most recently from 53cb8cf to 3c1afca Compare March 30, 2018 06:34

ruflin added review libbeat labels Mar 30, 2018

vjsamuel force-pushed the add_host_network_support branch 2 times, most recently from a668182 to 9b02d7d Compare April 2, 2018 00:11

ruflin added the needs_backport PR is waiting to be backported to other branches. label Apr 3, 2018

ruflin approved these changes Apr 3, 2018

View reviewed changes

jsoriano reviewed Apr 3, 2018

View reviewed changes

vjsamuel force-pushed the add_host_network_support branch 2 times, most recently from e9775f1 to 04bd3ed Compare April 5, 2018 21:36

jsoriano reviewed Apr 6, 2018

View reviewed changes

vjsamuel force-pushed the add_host_network_support branch from 04bd3ed to 5ee2645 Compare April 10, 2018 21:02

jsoriano mentioned this pull request May 11, 2018

The "add_kubernetes_metadata" processor can't get meta when the pod use hostNetwork #7070

Closed

Add support for docker autodiscover to monitor containers on host net…

300cf63

…work

vjsamuel force-pushed the add_host_network_support branch from 5ee2645 to e0556e8 Compare June 20, 2018 00:12

vjsamuel force-pushed the add_host_network_support branch from e0556e8 to e31b456 Compare June 20, 2018 00:23

exekias reviewed Jun 20, 2018

View reviewed changes

Incorporating review comments

b182429

vjsamuel force-pushed the add_host_network_support branch from e31b456 to b182429 Compare June 20, 2018 15:32

exekias approved these changes Jun 20, 2018

View reviewed changes

exekias merged commit 443c39d into elastic:master Jun 20, 2018

vjsamuel deleted the add_host_network_support branch June 20, 2018 23:12

exekias removed the needs_backport PR is waiting to be backported to other branches. label Jun 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for docker autodiscover to monitor containers on host network #6708

Add support for docker autodiscover to monitor containers on host network #6708

vjsamuel commented Mar 30, 2018 •

edited

Loading

elasticmachine commented Mar 30, 2018

houndci-bot Mar 30, 2018

houndci-bot Mar 30, 2018

houndci-bot Mar 30, 2018

ruflin commented Mar 30, 2018

ruflin commented Mar 30, 2018

vjsamuel commented Mar 30, 2018

ruflin commented Mar 30, 2018

vjsamuel commented Apr 2, 2018

ruflin left a comment

ruflin commented Apr 3, 2018

jsoriano Apr 3, 2018

jsoriano Apr 3, 2018

jsoriano Apr 3, 2018

vjsamuel Apr 5, 2018

jsoriano Apr 5, 2018

jsoriano Apr 3, 2018

ruflin Apr 4, 2018

jsoriano Apr 5, 2018

ruflin commented Apr 6, 2018

jsoriano Apr 6, 2018

vjsamuel Apr 6, 2018

vjsamuel Apr 6, 2018

ruflin commented Apr 10, 2018

exekias commented Apr 10, 2018

exekias commented Apr 10, 2018

vjsamuel commented Jun 20, 2018

exekias left a comment

exekias Jun 20, 2018

exekias Jun 20, 2018

exekias left a comment

exekias commented Jun 20, 2018

Add support for docker autodiscover to monitor containers on host network #6708

Add support for docker autodiscover to monitor containers on host network #6708

Conversation

vjsamuel commented Mar 30, 2018 • edited Loading

elasticmachine commented Mar 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Mar 30, 2018

ruflin commented Mar 30, 2018

vjsamuel commented Mar 30, 2018

ruflin commented Mar 30, 2018

vjsamuel commented Apr 2, 2018

ruflin left a comment

Choose a reason for hiding this comment

ruflin commented Apr 3, 2018

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

ruflin commented Apr 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Apr 10, 2018

exekias commented Apr 10, 2018

exekias commented Apr 10, 2018

vjsamuel commented Jun 20, 2018

exekias left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

exekias left a comment

Choose a reason for hiding this comment

exekias commented Jun 20, 2018

vjsamuel commented Mar 30, 2018 •

edited

Loading