-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Align elastic-agent-standalone manifest with the kubernetes package changes #29595
Align elastic-agent-standalone manifest with the kubernetes package changes #29595
Conversation
Signed-off-by: Tetiana Kravchenko <[email protected]>
This pull request does not have a backport label. Could you fix it @tetianakravchenko? 🙏
NOTE: |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
Signed-off-by: Tetiana Kravchenko <[email protected]>
type: logs | ||
prospector.scanner.symlinks: true | ||
parsers: | ||
- container: |
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.
I believe we should keep the manifest as simple as possible and leave the defaults with not comments. The extra fields are documented for anyone who wants to change settings. I would just leave
parsers:
- container: ~
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.
it is current default - see https://github.com/elastic/integrations/blob/master/packages/kubernetes/data_stream/container_logs/agent/stream/stream.yml.hbs#L7-L10, was introduced in elastic/integrations#2345 to make it possible to configure those default values
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.
Defaults are all
streams and auto format. See in https://github.com/elastic/integrations/blob/master/packages/kubernetes/data_stream/container_logs/manifest.yml#L27.
For filestream input if we don't set them like
- container: ~
it takes the defaults.
For additionalParsersConfig
we just added the possibility for a user to add another parser via advanced options.
The default is still commented out.
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.
ah, ok, I understood what do you mean, I thought to set it the same way as done in the package, but you are right - in the end it is the same.
setting default values implicitly - 7c00ce9
@@ -482,7 +547,7 @@ spec: | |||
- name: ES_PASSWORD | |||
value: "" | |||
- name: ES_HOST | |||
value: "" | |||
value: "elastic-package-stack_elasticsearch_1" |
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.
Was this change intentional ? This is a proposed manifest and I don't think elastic-package-stack_elasticsearch_1
is of any value to a customer.
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.
I've already removed it - it was used to test locally
Signed-off-by: Tetiana Kravchenko <[email protected]>
Signed-off-by: Tetiana Kravchenko <[email protected]>
Pinging @elastic/integrations (Team:Integrations) |
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 working on this! I left one small comment about the skip_older
which is mostly to provide option for disabling this and hence we should better avoid exposing this with the default value.
Signed-off-by: Tetiana Kravchenko <[email protected]>
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.
🚀
Signed-off-by: Tetiana Kravchenko [email protected]
What does this PR do?
In kubernetes package were added some data-streams configuration adjustments. This PR backports those changes to elastic-agent standalone manifest.
Why is it important?
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs