-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Filebeat] Support for Deployment Kubernetes resource #822
[Filebeat] Support for Deployment Kubernetes resource #822
Conversation
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? |
💚 CLA has been signed |
This commit indtroduces the feature of deploying a Kubernetes deployment instead of a Daemonset using Filebeat, using a `values.yaml` syntax as below: `values.yaml` --- ```yaml [...] deploymentType: [daemonset|deployment] [...] ``` Specifically, this is used for creation of Filebeat instances not bound to each Worker, conducting non-Worker-related work, such as collection of AWS CloudTrail logs as described in [1]. [1]:#821
This commit adds a default value test for `deploymentType`. Additionally, * `test_deployment_type_deployment` Checks if a `Deployment` is created but NOT a `DaemonSet` * `test_deployment_type_daemonset` Checks if a `DaemonSet` is created but NOT a `Deployment` * `test_deployment_type_case_insensitive` Checks if `deploymentType` value is accepted in a case-insensitive way.
Hello people! Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @operatorequals, thanks for submitting this PR.
Metricbeat chart is already managing both daemonset and deployment resources using daemonset.enable
and deployment.enable
values.
For consistency reason between Elastic charts, I would prefer having the same behavior in Filebeat chart if we had the feature to manage Filebeat deployment resources.
Can you refactor your PR according to Metricbeat chart code?
Absolutely! |
This commit uses the MetricBeat Helm chart to create a Daemonset/Deployment Helm chart for Filebeat. Uses the ```yaml daemonset: [...] deployment: [...] ``` structure falling back to root key defaults.
I did refactor the PR in a way that accepts defaults from the YAML root key, and also get overriden by |
The value: ```yaml serviceAccount: "" ``` was existing twice in the `filebeat/values.yaml` file.
jenkins test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this great work 👍
Co-authored-by: Julien Mailleret <[email protected]>
Co-authored-by: Julien Mailleret <[email protected]>
On it! |
Co-authored-by: Julien Mailleret <[email protected]>
Please revisit all your previous Comments. I think I fixed them all! |
jenkins test this please |
As of Jenkins the 16:42:29 > assert "tolerations" not in r["daemonset"][name]["spec"]["template"]["spec"]
16:42:29 E AssertionError: assert 'tolerations' not in {'affinity': {}, 'containers': [{'args': ['-e', '-E', 'http.enabled=true'], 'env': [{'name': 'POD_NAMESPACE', 'valueFr...astic.co/beats/filebeat:8.0.0-SNAPSHOT', ...}], 'nodeSelector': {}, 'serviceAccountName': 'release-name-filebeat', ...}
16:42:29 The line should be replaced with: assert (
r["daemonset"][name]["spec"]["template"]["spec"]["tolerations"]
== []
) (taken from Metricbeat). Yet, I lost access to the fork due to an accident and I cannot push another commit: After that, the tests will have lower coverage but they will pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensuring that everything is fin for this PR is a very tedious work so many thanks for it 👍
Yep tests will also need some refactoring pretty similar to what I did for |
This commit introduces the feature of deploying a Kubernetes deployment
instead of a Daemonset using Filebeat, using a
values.yaml
syntaxas below:
values.yaml
Specifically, this is used for creation of Filebeat instances not
bound to each Worker, conducting non-Worker-related work, such as
collection of AWS CloudTrail logs as described in 1.
${CHART}/tests/*.py
${CHART}/examples/*/test/goss.yaml
Fix #821