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

Fixing configuration documentation for kubernetes processor #4313

Merged
merged 1 commit into from
May 15, 2017

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented May 15, 2017

Fixing configuration documentation for kubernetes processor

@vjsamuel vjsamuel force-pushed the avoid_duplicate_pod_updates branch from 8318397 to 6362558 Compare May 15, 2017 05:57
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@vjsamuel
Copy link
Contributor Author

@exekias, im still on the fence with this change. this would make sure that we always start at the most recent resource version. but this would also mean that we always miss events that were written by the API server when the watch was broken at our end.

@vjsamuel vjsamuel changed the title Avoid duplicate watch events from being processed WIP: Avoid duplicate watch events from being processed May 15, 2017
@vjsamuel vjsamuel force-pushed the avoid_duplicate_pod_updates branch from 6362558 to 8f39c8f Compare May 15, 2017 08:14
@vjsamuel vjsamuel changed the title WIP: Avoid duplicate watch events from being processed Fixing configuration documentation for kubernetes processor May 15, 2017
@vjsamuel
Copy link
Contributor Author

@exekias Updated the PR to just fix an issue in the configuration documentation. I dont think its a good idea to do the check based on last resource version. would result in losing metadata update events.

@exekias
Copy link
Contributor

exekias commented May 15, 2017

@vjsamuel podwatcher stores last seen resource version, you could use that :) https://github.com/elastic/beats/blob/master/libbeat/processors/kubernetes/podwatcher.go#L22

Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

WFG

@@ -215,6 +215,7 @@ https://github.com/elastic/beats/compare/v5.3.2...v5.4.0[View commits]
- Fix potential Elasticsearch output URL parsing error if protocol scheme is missing. {pull}3671[3671]
- Downgrade Elasticsearch per batch item failure log to debug level. {issue}3953[3953]
- Make `@timestamp` accessible from format strings. {pull}3721[3721]
- Fixing configuration documentation for kubernetes processor {pull}4313[4313]
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I just noticed this line went to an already released version, could you move it to the HEAD? also it would be great if you rename Fixing -> Fix

@vjsamuel vjsamuel force-pushed the avoid_duplicate_pod_updates branch 2 times, most recently from fca62c9 to 02913cf Compare May 15, 2017 15:04
@vjsamuel vjsamuel force-pushed the avoid_duplicate_pod_updates branch from 02913cf to 32fe106 Compare May 15, 2017 15:21
@exekias exekias merged commit 723903d into elastic:master May 15, 2017
@exekias exekias deleted the avoid_duplicate_pod_updates branch May 15, 2017 16:21
@exekias exekias restored the avoid_duplicate_pod_updates branch May 15, 2017 16:21
@vjsamuel vjsamuel deleted the avoid_duplicate_pod_updates branch May 15, 2017 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants