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

Prevent to start filebeat when STDIN is used with other inputs #6463

Merged
merged 1 commit into from
Mar 28, 2018

Conversation

ph
Copy link
Contributor

@ph ph commented Feb 23, 2018

  • breaking changes: Stdin will now requires exclusive mode to run.
  • Added helpers on top of the config class to retrieve enabled plugins.
  • Refactored the stdin python integration testing into their own file.
  • Added tests to check the combination of STDIN with other plugins
  • Loop once on the enabled inputs

@ph ph added in progress Pull request is currently in progress. Filebeat Filebeat labels Feb 23, 2018
@ph ph force-pushed the fix/stdin-as-single-input branch from 8dd3362 to 6597fe9 Compare February 23, 2018 20:44
@ph
Copy link
Contributor Author

ph commented Feb 23, 2018

Due to the nature of stdin related to EOF, it more appropriate to make sure the Stdin input is run in exclusive mode.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM.

@tsg Ok to merge? This will probably be in 6.3.

@@ -47,6 +47,8 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Refactor the usage of prospector to input in the YAML reference and
the system test. {pull}6121[6121]
- Add IIS module to parse access log and error log. {pull}6127[6127]
- Add validation for Stdin, when Filebeat is configured with Stdin and any other inputs, Filebeat
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is a bug fix or a breaking change :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its kind of both 🍁

@ruflin
Copy link
Member

ruflin commented Feb 26, 2018

@ph We should add a note about this also to the docs.

@ph
Copy link
Contributor Author

ph commented Feb 26, 2018

@ruflin I need to take a look at config reload, maybe we should do some logic to prevent it there too can be a separate PR.

@ph ph force-pushed the fix/stdin-as-single-input branch from 2a60992 to 4c89a4b Compare March 26, 2018 15:34
@ph
Copy link
Contributor Author

ph commented Mar 26, 2018

I have added a note to the documentation.

@tsg could you take a look?

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Needs again a rebase.

{name: "input exists and enabled", input: "stdin", expected: true, config: config},
{name: "input exists and disabled", input: "udp", expected: false, config: config},
{name: "input doesn't exist", input: "redis", expected: false, config: config},
{name: "not inputs are enabled", input: "redis", expected: false, config: &Config{}},
Copy link
Member

Choose a reason for hiding this comment

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

s/not/no/

@@ -9,6 +9,8 @@

Use the `stdin` input to read events from standard in.

Note: This input cannot be run at the same time of other inputs type.
Copy link
Member

Choose a reason for hiding this comment

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

at the same time "with" other input types? (plural types, singluar input?)

@ph ph force-pushed the fix/stdin-as-single-input branch from 4c89a4b to 9d7dd34 Compare March 27, 2018 13:40
- **breaking changes:** Stdin will now requires exclusive mode to run.
- Added helpers on top of the config class to retrieve enabled plugins.
- Refactored the stdin python integration testing into their own file.
- Added tests to check the combination of STDIN with other plugins
- Loop once on the enabled inputs
@ph ph force-pushed the fix/stdin-as-single-input branch from 9d7dd34 to 327d202 Compare March 27, 2018 13:41
@ph
Copy link
Contributor Author

ph commented Mar 27, 2018

@ruflin typos fixed, rebased with master.

@ruflin ruflin merged commit 91b2f8e into elastic:master Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Filebeat Filebeat in progress Pull request is currently in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants