-
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
Fix ordering and duplicate configs on autodiscover #19317
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💔 Tests FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
962a624
to
17c0ba6
Compare
jenkins, run the tests please |
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 the fix. I have a couple of questions but in general it looks good.
@vjsamuel failures in CI seem related |
17c0ba6
to
9a90185
Compare
Fixed and updated the tests to cover more scenarios. |
jenkins run the tests please |
jenkins run the tests please |
jenkins run the tests again please |
9a90185
to
7c1bd02
Compare
rebased with master again. |
Not sure why CI is failing so much on this branch, doesn't seem related... jenkins run the tests please |
Oh, this branch is going to need another update after #19478 |
7c1bd02
to
5028a33
Compare
@jsoriano can you help close on this please? |
Could you please update the branch again? #19535 should finally solve the linting issue 🤞 |
5028a33
to
76117c5
Compare
jenkins run the tests please 🤞 |
elastic#18979 introduced a pod level event which is generated after all container events. The ordering is wrong in that pod events are sent last which would generate a valid event similar to container events. The ordering needs to be pod first and container events next so that pod events dont override valid container events. One other issue was that the pod level hint generates a single config with all hosts and it wont get over written by container hints causing more than one config to be spun up for the same hint (one with a container meta and one without). (cherry picked from commit 58edbb4)
…ne-beats * upstream/master: (35 commits) [ci] fix env variable name for xpack filebeats (elastic#19617) Cache error responses for cloudfoundry apps metadata (elastic#19181) ci: user fixed type of agent (elastic#19625) Input v2 cursor testing (elastic#19573) Update Jenkinsfile to not inspect removed vendor (elastic#19610) Fix ordering and duplicate configs on autodiscover (elastic#19317) Prepare input/file for changes in the registrar (elastic#19516) Cursor input and manager implementation (elastic#19571) [Filebeat] Fix tls mapping in suricata module (elastic#19494) [Ingest Manager] Make Agent beta and Constraints experimental (elastic#19586) Accept prefix as metric_types for stackdriver metricset in GCP (elastic#19345) Implement memlog store operations (elastic#19533) introduce journalbeat/pkg in order to provide reusable shared code (elastic#19581) Add descriptions to HAProxy fields in Metricbeat (elastic#19561) ci: apm-server-update trigered only on upstream, comments, and manual triggered (elastic#19590) ci: enable upstream triggering on the packaging job (elastic#19589) ci: some jjbb improvements (elastic#19588) [MetricBeat] set tags correctly if the dimension value is ARN (elastic#19433) [Filebeat] Add default_fields: false to fields.yml in aws module (elastic#19568) Add publisher implementation for stateful inputs (elastic#19530) ...
#18979 introduced a pod level event which is generated after all container events. The ordering is wrong in that pod events are sent last which would generate a valid event similar to container events. The ordering needs to be pod first and container events next so that pod events dont override valid container events. One other issue was that the pod level hint generates a single config with all hosts and it wont get over written by container hints causing more than one config to be spun up for the same hint (one with a container meta and one without). (cherry picked from commit 58edbb4) Co-authored-by: Vijay Samuel <vjsamuel@ebay.com>
elastic#18979 introduced a pod level event which is generated after all container events. The ordering is wrong in that pod events are sent last which would generate a valid event similar to container events. The ordering needs to be pod first and container events next so that pod events dont override valid container events. One other issue was that the pod level hint generates a single config with all hosts and it wont get over written by container hints causing more than one config to be spun up for the same hint (one with a container meta and one without).
Regression
What does this PR do?
#18979 introduced a pod level event which is generated after all container events. The ordering is wrong in that pod events are sent last which would generate a valid event similar to container events. The ordering needs to be pod first and container events next so that pod events dont override valid container events. One other issue was that the pod level hint generates a single config with all hosts and it wont get over written by container hints causing more than one config to be spun up for the same hint (one with a container meta and one without)
Why is it important?
Without this, container metadata will never be emitted even if a port belongs to a valid exposed port.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesI have added an entry inCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Generate any event for an exposed port and make sure that container metadata is visible.
Related issues
Use cases
Screenshots
Logs