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

Remove prospector support in Filebeat #8909

Merged
merged 11 commits into from
Nov 6, 2018

Conversation

ph
Copy link
Contributor

@ph ph commented Nov 2, 2018

Add CheckRemoved6xSettings and CheckRemoved6xSettings

Add new methods to verify obsolete configuration for 7.0,
added missing test for them.

Remove the prospector option in the configuration.

In 6.3 we have deprecated the usage of the 'prospector' options, in 7.0
its now obsolete.

This commit does the following:

  • Remove any backward compatibility fixes.
  • Add warning when using the prospector key in configuration.
  • Remove any usage of prospector in tests.
  • Remove any usage or prospector in module expectation files.
  • Adjust any integration tests to not check for prospector.type.
  • Remove the prospector type from the fields.
  • Remove any shims created by the prospector package.

@ph ph added in progress Pull request is currently in progress. Filebeat Filebeat labels Nov 2, 2018
@ph
Copy link
Contributor Author

ph commented Nov 2, 2018

There is a bit more changes in the PR that I would like to, mainly due to the trailing chars, please review it with NO WHITESPACE

"close_eof": true,
},
},
},
{
name: "input overrides",
fcfg: FilesetConfig{},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is another test for inputs.

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.

Did a review even though it's still in progress. Overall LGTM.

The files that you adjusted with the whitespaces are auto genereated. Did you modify them manually? To just generated them run

INTEGRATION_TESTS=1 GENERATE=1 nosetests system/tests/test_modules.py

Make sure to have an Elasticsearch instance running.

filebeat/Makefile Outdated Show resolved Hide resolved
@@ -64,3 +63,23 @@ func CheckRemoved5xSetting(cfg *common.Config, setting string) error {

return fmt.Errorf("setting '%v' has been removed", current.PathOf(name))
}

// CheckRemoved5xSettings prints a warning if the obsolete setting is used.
func CheckRemoved5xSettings(cfg *common.Config, settings ...string) error {
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove now in master all 5.x warnings? I don't expect people to upgrade from 5.x to 7.x directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I wanted to do it in another PR

@ph
Copy link
Contributor Author

ph commented Nov 5, 2018

The files that you adjusted with the whitespaces are auto genereated. Did you modify them manually? To just generated them run

No I did a quick search/replace with my editor.. and the save just fix the whitespace, I will regenerate them.

@ph ph added review and removed in progress Pull request is currently in progress. labels Nov 5, 2018
@ph
Copy link
Contributor Author

ph commented Nov 5, 2018

@ruflin I've recreated the modules expected files for Filebeat, I also removed the mention of prospector for Surricata in xpack/filebeat.

It's ready for a review.

@ph
Copy link
Contributor Author

ph commented Nov 5, 2018

hold on.. I need to generate the file with a ES master
to make sure the geocoords are the same.

@ph
Copy link
Contributor Author

ph commented Nov 5, 2018

@ruflin I've regenerated the expectation with a ES snapshot The tests passed locally so we will see on CI.

ph and others added 8 commits November 5, 2018 12:38
Add new methods to verify obsolete configuration for 7.0,
added missing test for them.
In 6.3 we have deprecated the usage of the 'prospector' options, in 7.0
its now obsolete.

This commit does the following:

- Remove any backward compatibility fixes.
- Add warning when using the prospector key in configuration.
- Remove any usage of prospector in tests.
- Remove any usage or prospector in module expectation files.
- Adjust any integration tests to not check for prospector.type.
- Remove the prospector type from the fields.
- Remove any shims created by the prospector package.
Co-Authored-By: ph <phpellerin@gmail.com>
@ph ph force-pushed the fix/remove-prospector-support-filebeat branch from 79fc8a4 to 5b6e247 Compare November 5, 2018 19:17
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.

WFG

@ph
Copy link
Contributor Author

ph commented Nov 5, 2018

jenkins test this please

@ruflin ruflin merged commit 755831e into elastic:master Nov 6, 2018
@ruflin
Copy link
Member

ruflin commented Nov 6, 2018

@ph I merged this one as it conflicts with all my current PR's and I would like to continue with my PR's. Hope it's ok for you.

@ph
Copy link
Contributor Author

ph commented Nov 6, 2018

🤗 @ruflin All is OK I was waiting for a Jenkins build to complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants