Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Add integration tests and other tweaks for filebeat #121

Merged
merged 12 commits into from
May 9, 2019
Merged

Conversation

Crazybus
Copy link
Contributor

@Crazybus Crazybus commented May 3, 2019

Apart from adding some integration tests and more templating tests I also made a few other changes:

  • Added a default working filebeat config into the default values.yaml. This means the default install without any changes will result in logs being sent to the default elasticsearch chart.
  • Switched the livenessProbe to a curl since httpGet cannot check localhost. We could bind the monitoring API to a non-localhost port but this has security implications
  • Added integration tests for the oss release of Filebeat/Kibana and Elasticsearch. I did this mostly because I wasn't 100% sure if the monitoring port was available in the oss release. And added it for the other charts to make sure features don't get introduced that would break the oss version.
  • Added a readinessProbe that checks it can connect to its output. The livesnessProbe will restart filebeat if it isn't responding, and this will prevent upgrades from happening if the new configuration can't connect to its output. And also makes it easy for monitoring tools (like the data we get from metricbeat) to detect if a filebeat container is not healthy.
  • Fixed various name collisions. A few resources had a hardcoded "filebeat" name which would conflict if multiple filebeat instances were deployed to the namespace. When would this happen? One example is our integration testing, however I can also imagine blue/green deploy strategies or use cases where multiple filebeat daemonsets are deployed to the same cluster and namespace.
  • Added "namespace" to the hostPath that is being mounted. To make sure there are no conflicts if filebeat is deployed in two different namespaces with the same release name.

@Crazybus Crazybus changed the title WIP: Add integration tests and other tweaks for filebeat Add integration tests and other tweaks for filebeat May 8, 2019
@Crazybus Crazybus requested a review from tylerjl May 8, 2019 18:47
Copy link
Contributor

@tylerjl tylerjl left a comment

Choose a reason for hiding this comment

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

All the changes make sense to me, agreed on all counts 👍

I think that having a "semi-opinionated" config out-of-the-box makes sense here since the alternative is basically a useless go process, it's probably worth a minor note in the filebeat chart's README so that people can get started quickly by setting the appropriate ELASTICSEARCH_HOSTS extraEnv and go. I can PR that later though.

@Crazybus
Copy link
Contributor Author

Crazybus commented May 9, 2019

it's probably worth a minor note in the filebeat chart's README

Agreed! I think adding this to a Usage notes and getting started section like the Elasticsearch readme makes the most sense.

Potentially giving an example of how to configure one of the other many filebeat ouputs could also be useful. But hopefully an example of how to configure filebeat.yml for Elasticsearch and add the right environment variable should be enough.

I can PR that later though.

🙏

@Crazybus Crazybus merged commit 9b406f8 into master May 9, 2019
@Crazybus Crazybus deleted the beam_em_up branch May 9, 2019 06:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants