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

Make k8s meta processor initialisation asynchronous #16373

Merged
merged 7 commits into from
Feb 19, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Feb 17, 2020

What does this PR do?

This PR changes add_kubernetes_metadata processor so as to get initialised asynchronously and being able to handle node not ready situations.

Why is it important?

This will be solve the problem of having to restart Beat in case the processor was not initialised if the node was not ready yet when Beat started.

How to test this PR locally

With the use of Minikube:

  1. minikube start
  2. Have a minikube ssh in one tab so as to handle k8s health up and down
  3. Build Filebeat on the host where Minikube runs
  4. Use the kubeconfig of Minikube: export KUBECONFIG=~/.kube/config
  5. Make sure that add_kubernetes_metadata processor is enabled:
processors:
  - add_kubernetes_metadata: ~
  1. Start filebeat and expect to see 2020-02-18T12:25:12.825+0200 INFO add_kubernetes_metadata/kubernetes.go:77 add_kubernetes_metadata: kubernetes env detected, with version: v1.17.0 in your logs, meaning that processor was able to reach k8s API.
  2. Stop filebeat
  3. In the session you have inside Minikube vm run sudo systemctl stop docker to bring docker down and make k8s API unreachable
  4. Start filebeat again and you should see (after ~30secs) sth like 2020-02-18T12:27:16.958+0200 INFO add_kubernetes_metadata/kubernetes.go:89 add_kubernetes_metadata: could not detect kubernetes env: Get https://192.168.64.6:8443/version?timeout=32s: dial tcp 192.168.64.6:8443: connect: connection refused
  5. Stop filebeat and start it again.
  6. Wait 3-4 seconds and go to Minkube vm and start docker daemon again: sudo systemctl start docker.
  7. In Filebeat's logs you should expect to see that the processor reached k8s API like in step 6.

Related issues

@ChrsMark ChrsMark added enhancement containers Related to containers use case [zube]: In Review Team:Integrations Label for the Integrations team autodiscovery Team:Platforms Label for the Integrations - Platforms team labels Feb 17, 2020
@ChrsMark ChrsMark self-assigned this Feb 17, 2020
Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark changed the title Make k8s meta processor initialisation asyncronous Make k8s meta processor initialisation asynchronous Feb 17, 2020
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.

Only some small details, and there seems to be some format error.

Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark added v7.7.0 needs_backport PR is waiting to be backported to other branches. labels Feb 18, 2020
@ChrsMark
Copy link
Member Author

Changed some stuff and added testing notes. Tested it with minikube with the process described in the PR's description.

"time"

"github.com/cloudflare/cfssl/log"

"github.com/elastic/beats/libbeat/common/kubernetes/metadata"
Copy link
Member

Choose a reason for hiding this comment

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

Nit. Imports order, this libbeat import should be below with the other ones.

Signed-off-by: chrismark <[email protected]>
@ChrsMark
Copy link
Member Author

ChrsMark commented Feb 19, 2020

While fixing the import order I noticed that one import("github.com/cloudflare/cfssl/log") was not used 🤔 so I removed it and also tried to unify the logging using the processor's logger along. Sorry for the back-and-forth 🙂 .

@ChrsMark ChrsMark requested a review from jsoriano February 19, 2020 09:44
@jsoriano
Copy link
Member

While fixing the import order I noticed that one import("github.com/cloudflare/cfssl/log") was not used thinking so I removed it and also tried to unify the logging using the processor's logger along. Sorry for the back-and-forth slightly_smiling_face .

Good catch!

Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark merged commit 4891c04 into elastic:master Feb 19, 2020
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Feb 19, 2020
@ChrsMark ChrsMark added test-plan Add this PR to be manual test plan and removed needs_backport PR is waiting to be backported to other branches. labels Feb 19, 2020
ChrsMark added a commit that referenced this pull request Feb 20, 2020
kvch pushed a commit to kvch/beats that referenced this pull request Feb 20, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autodiscovery containers Related to containers use case enhancement Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat] pods on new nodes sometimes don't detect the Kubernetes environment
3 participants