Add autodiscover for kubernetes #6055

vjsamuel · 2018-01-11T18:20:55Z

This PR adds auto discovery for kubernetes. Some sample specs for using this feature:

Filebeat: (discover all workloads in default namespace)

filebeat.autodiscover:
  providers:
    - type: kubernetes
      in_cluster: false
      kube_config: /home/docker/config
      host: minikube
      templates:
        - condition:
            equals:
              kubernetes.namespace: default
          config:
            - type: docker
              containers.ids:
                - "${data.kubernetes.container.id}"


output.console.pretty: true

Metricbeat: query for pods with label project: prometheus

metricbeat.autodiscover:
  providers:
    - type: kubernetes
      in_cluster: false
      kube_config: /home/docker/config
      host: minikube
      templates:
        - condition:
            equals:
              kubernetes.labels.project: prometheus
          config:
            - module: prometheus
              metricsets: ["stats"]
              hosts: "${data.host}:9090"

output.console.pretty: true

elasticmachine · 2018-01-11T18:20:56Z

Can one of the admins verify this patch?

houndci-bot · 2018-01-11T18:21:03Z

libbeat/common/kubernetes/util.go

+	return client, nil
+}
+
+func DiscoverKubernetesNode(host string, client *k8s.Client) string {


exported function DiscoverKubernetesNode should have comment or be unexported

houndci-bot · 2018-01-11T18:21:03Z

libbeat/common/kubernetes/util.go

+	"github.com/elastic/beats/libbeat/logp"
+)
+
+func GetKubernetesClient(in_cluster bool, kube_config string) (client *k8s.Client, err error) {


exported function GetKubernetesClient should have comment or be unexported
don't use underscores in Go names; func parameter in_cluster should be inCluster
don't use underscores in Go names; func parameter kube_config should be kubeConfig

houndci-bot · 2018-01-11T18:21:04Z

libbeat/common/kubernetes/types.go

@@ -107,6 +108,17 @@ type Pod struct {
 	Status     PodStatus  `json:"status"`
 }

+func (s *PodContainerStatus) GetContainerID() string {


exported method PodContainerStatus.GetContainerID should have comment or be unexported

houndci-bot · 2018-01-11T18:21:04Z

libbeat/autodiscover/providers/kubernetes/kubernetes.go

+	p.bus.Publish(event)
+}
+
+func (p *Provider) Stop() {


exported method Provider.Stop should have comment or be unexported

houndci-bot · 2018-01-11T18:21:04Z

libbeat/autodiscover/providers/kubernetes/kubernetes.go

+	}, nil
+}
+
+func (p *Provider) Start() {


exported method Provider.Start should have comment or be unexported

@vjsamuel lately we have been trying to make hound happy for most of its messages, could you add these comments?

ruflin

LGTM. Would like to also get a review from @exekias o this.

One part that is missing is docs. @exekias Should we open a meta issue to track the changes to make sure we don't forget about docs afterwards?

ruflin · 2018-01-12T01:54:42Z

libbeat/common/kubernetes/metadata.go

@@ -47,6 +47,9 @@ func (g *metaGenerator) PodMetadata(pod *Pod) common.MapStr {
 		"pod": common.MapStr{
 			"name": pod.Metadata.Name,
 		},
+		"node": common.MapStr{


Will this need an update to our fields.yml or is node already in there?

ruflin · 2018-01-12T01:55:40Z

libbeat/common/kubernetes/util.go

+)
+
+func GetKubernetesClient(in_cluster bool, kube_config string) (client *k8s.Client, err error) {
+	if in_cluster == true {


We normally use inClusterfor naming, same for kubeConfig

ruflin · 2018-01-12T01:57:30Z

libbeat/common/kubernetes/util.go

+		if podName == "localhost" {
+			host = "localhost"
+		} else {
+			pod, error := client.CoreV1().GetPod(ctx, podName, client.Namespace)


nit: We often use err instead of error

ruflin · 2018-01-12T02:00:19Z

jenkins, test it

walktall · 2018-01-12T02:08:57Z

libbeat/processors/add_kubernetes_metadata/kubernetes.go

-
-		}
-	}
+	config.Host = kubernetes.DiscoverKubernetesNode(config.Host, client)


The behavior of this Func is odd when beat is put outside of k8s cluster. As far as I know, host is used as a node selector while making watch requests. So when beat is deployed outside the cluster, host should be ignored or it won't get any watched info from k8s. And if beat is put in cluster, host should be set by k8s downward api I think. So should we remove this host config since it is either wrong in case outside cluster or error-prone while user specify a wrong host in config?

@walktall, this piece of code when provided with a host just uses the host. it is good especially when you are testing locally without having to deploy the beat onto Kubernetes. when running in cluster, this code, gets the pod name and uses it to get the pod spec and figures out what host the pod is deployed on.

Yes, it is useful while test beat locally since user can set host to k8s node name. But if it is deployed outside the cluster or as a k8s deployment, we still need to ignore host anyway. Say I put beat in another k8s cluster, the result will be wrong here.

when running in cluster, this code, gets the pod name and uses it to get the pod spec and figures out what host the pod is deployed on.

I prefer using k8s downward api here to get pod name though.

In general, watching all nodes from outside the cluster may suffer from other issues, like scalability.

These settings work best when running from inside the cluster, or orchestrating beats as part of the deployment (one per node, with the host value set).

We can discuss this in a new issue if you find the idea worth exploring, as I want to move this PR forward, and it only moved code around to get a cleaner interface.

houndci-bot · 2018-01-12T05:45:51Z

libbeat/common/kubernetes/types.go

@@ -98,3 +107,32 @@ type Pod struct {
 	Spec       PodSpec    `json:"spec"`
 	Status     PodStatus  `json:"status"`
 }
+
+func (s *PodContainerStatus) GetContainerID() string {


exported method PodContainerStatus.GetContainerID should have comment or be unexported

houndci-bot · 2018-01-12T05:45:52Z

metricbeat/module/uwsgi/uwsgi.go

+	}
+}
+
+func NewModule(base mb.BaseModule) (mb.Module, error) {


exported function NewModule should have comment or be unexported

houndci-bot · 2018-01-12T05:45:52Z

libbeat/common/kubernetes/types.go

@@ -98,3 +107,32 @@ type Pod struct {
 	Spec       PodSpec    `json:"spec"`
 	Status     PodStatus  `json:"status"`
 }
+
+func (s *PodContainerStatus) GetContainerID() string {


exported method PodContainerStatus.GetContainerID should have comment or be unexported

houndci-bot · 2018-01-12T05:45:52Z

metricbeat/module/uwsgi/uwsgi.go

+	}
+}
+
+func NewModule(base mb.BaseModule) (mb.Module, error) {


exported function NewModule should have comment or be unexported

exekias

LGTM, nice helper functions, and the node name addition.

We will need to open a follow-up PR for documentation

exekias · 2018-01-12T15:36:05Z

ok to test

houndci-bot · 2018-01-12T17:01:23Z

libbeat/common/kubernetes/types.go

@@ -107,6 +108,18 @@ type Pod struct {
 	Status     PodStatus  `json:"status"`
 }

+// Parse the container ID to get the actual ID string


comment on exported method PodContainerStatus.GetContainerID should be of the form "GetContainerID ..."

houndci-bot · 2018-01-12T17:01:23Z

libbeat/common/kubernetes/types.go

@@ -107,6 +108,18 @@ type Pod struct {
 	Status     PodStatus  `json:"status"`
 }

+// Parse the container ID to get the actual ID string


comment on exported method PodContainerStatus.GetContainerID should be of the form "GetContainerID ..."

exekias · 2018-01-12T19:50:55Z

ok to test

exekias · 2018-01-12T20:49:18Z

Awesome work @vjsamuel, as usual, thank you! 🎉

dedemorton · 2018-01-22T19:04:10Z

I see that docs are added in #6065, so I'm removing the needs_docs label from this PR. Please open an issue against docs if additional changes are required.

houndci-bot reviewed Jan 11, 2018

View reviewed changes

vjsamuel force-pushed the kubernetes_autodiscover branch from 327c662 to 27a1825 Compare January 11, 2018 18:27

ruflin added review libbeat labels Jan 12, 2018

ruflin requested a review from exekias January 12, 2018 01:53

ruflin reviewed Jan 12, 2018

View reviewed changes

walktall reviewed Jan 12, 2018

View reviewed changes

vjsamuel force-pushed the kubernetes_autodiscover branch 3 times, most recently from 61a916a to e487576 Compare January 12, 2018 05:45

houndci-bot reviewed Jan 12, 2018

View reviewed changes

vjsamuel added 3 commits January 11, 2018 22:10

Add autodiscover for kubernetes

9123dcc

Fixing failed Indexers UT

012f5a2

Incorporating review comments

8415b40

vjsamuel force-pushed the kubernetes_autodiscover branch from e487576 to 8415b40 Compare January 12, 2018 06:22

exekias approved these changes Jan 12, 2018

View reviewed changes

exekias added the needs_docs label Jan 12, 2018

houndci-bot reviewed Jan 12, 2018

View reviewed changes

Make hound happy

37cfd21

vjsamuel force-pushed the kubernetes_autodiscover branch from db2cf2b to 37cfd21 Compare January 12, 2018 17:02

exekias merged commit 7fbd75d into elastic:master Jan 12, 2018

vjsamuel deleted the kubernetes_autodiscover branch January 12, 2018 22:10

dedemorton mentioned this pull request Jan 17, 2018

Beats doc updates in 6.x #5632

Closed

37 tasks

dedemorton removed the needs_docs label Jan 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autodiscover for kubernetes #6055

Add autodiscover for kubernetes #6055

vjsamuel commented Jan 11, 2018

elasticmachine commented Jan 11, 2018

houndci-bot Jan 11, 2018

houndci-bot Jan 11, 2018

houndci-bot Jan 11, 2018

houndci-bot Jan 11, 2018

houndci-bot Jan 11, 2018

exekias Jan 12, 2018

vjsamuel Jan 12, 2018

ruflin left a comment

ruflin Jan 12, 2018

ruflin Jan 12, 2018

ruflin Jan 12, 2018

ruflin commented Jan 12, 2018

walktall Jan 12, 2018 •

edited

Loading

vjsamuel Jan 12, 2018

walktall Jan 12, 2018

walktall Jan 12, 2018

exekias Jan 12, 2018

houndci-bot Jan 12, 2018

houndci-bot Jan 12, 2018

houndci-bot Jan 12, 2018

houndci-bot Jan 12, 2018

exekias left a comment

exekias commented Jan 12, 2018

houndci-bot Jan 12, 2018

houndci-bot Jan 12, 2018

exekias commented Jan 12, 2018

exekias commented Jan 12, 2018

dedemorton commented Jan 22, 2018

Add autodiscover for kubernetes #6055

Add autodiscover for kubernetes #6055

Conversation

vjsamuel commented Jan 11, 2018

elasticmachine commented Jan 11, 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

ruflin 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

ruflin commented Jan 12, 2018

walktall Jan 12, 2018 • edited Loading

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

exekias left a comment

Choose a reason for hiding this comment

exekias commented Jan 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

exekias commented Jan 12, 2018

exekias commented Jan 12, 2018

dedemorton commented Jan 22, 2018

walktall Jan 12, 2018 •

edited

Loading