[Helm] Add Logstash Output to Elastic Agent Standalone #10644
Conversation
|
This pull request does not have a backport label. Could you fix it @btrieger? 🙏
|
cmacknz
added
the
Team:Elastic-Agent-Control-Plane
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
Hey @btrieger |
|
@pierrehilbert added the changelog fragment |
…l to avoid removal of quotes around strings
|
I added the remaining ssl flags and they should work with any of the outputs. I didn't provide an example of using them as I don't have a functioning logstash cluster that needs them but I did validate that they are being set as expected with a template command. Was thinking maybe I could document all the possible input variables outside the values schema json for every day users. Would it be best to put this in the examples readme for logstash? On a side note the rest of the outputs should remove the fromYaml and toYaml around the the call to the SSLConfig rendering as it removes the quotes and in the case of ca fingerprints this could prove problematic. |
💚 Build Succeeded
History
cc @btrieger |
There was a problem hiding this comment.
thanks for the contribution @btrieger . This is a big one so I will have to go through that one more time but in the meantime I left some comments. Also please any change that you do for the k8s templates try to introduce it also for the eck ones, from what I can tell everything here is compatible with both
| {{- with index $outputVal "queue.mem.flush.timeout" }} | ||
| queue.mem.flush.timeout: {{. | quote}} | ||
| {{- end }} | ||
| {{- with index $outputVal "queue.mem.flush.min_events" }} | ||
| queue.mem.flush.min_events: {{.}} | ||
| {{- end }} | ||
| {{- with index $outputVal "queue.mem.events" }} | ||
| queue.mem.events: {{.}} | ||
| {{- end }} |
There was a problem hiding this comment.
I believe that there shouldn't be any fields with dot notation in them here. If a user sets through cli such values they will appear as proper yaml and then this here won't work
There was a problem hiding this comment.
Would it be best to build out the schema to make them all objects? I think it is best to keep variable names in line with what they are in our docs for clarity? You can still set these via the cli if you use quotes around the variable name. I have done it with other charts in the past.
Speaking of keeping variable names in line. What are your thoughts on making a 2nd copy of certificate_authorities and verification_mode and deprecating the others. I would make a clean switch but don't want to break anyones chart that is using this one.
There was a problem hiding this comment.
Would it be best to build out the schema to make them all objects? I think it is best to keep variable names in line with what they are in our docs for clarity?
yes they are like that in our docs but elastic-agent does under the hood the splitting up of dot notation, I do believe that is more error prone to say to the user that a key has to wrapped across some quotes for it to find it's appropriate place in the values, thus I am inclining of saying to expand all of them as objects
Speaking of keeping variable names in line. What are your thoughts on making a 2nd copy of certificate_authorities and verification_mode and deprecating the others. I would make a clean switch but don't want to break anyones chart that is using this one.
sure mark the existing ones as deprecated but they still have to render properly and after some versions these can go away
There was a problem hiding this comment.
I think doing the deprecation may be best in a separate PR otherwise this one will get event longer. I have made the other changes suggested.
| {{- with index $outputVal "backoff.max" }} | ||
| backoff.max: {{. | quote }} | ||
| {{- end }} | ||
| {{- with index $outputVal "backoff.init" }} | ||
| backoff.init: {{. | quote }} | ||
| {{- end }} |
|
@pkoutsovasilis will do I was tyring to keep it small but it seemed kind of unavoidable if I wanted to add all the variables available for logstash. |
5 participants
What does this PR do?
This PR adds in the Logstash output for elastic agents to the standalone helm chart.
Why is it important?
Without this, standalone agents cannot connect to Logstash clusters. This makes the chart unusable for end users who have logstash in between agent and elasticsearch.
Checklist
./changelog/fragmentsusing the changelog toolDisruptive User Impact
This should have no user impact and be backward compatable.
How to test this PR locally
Follow steps in examples/kubernetes-logstash-output
Related issues