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

Add daemonset datastream for k8s package & system tests #569

Merged
merged 10 commits into from
Feb 2, 2021

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jan 26, 2021

What does this PR do?

This PR:

  1. adds daemonset datastream introduced by Kubernetes state_daemonset metricset for Metricbeat beats#20649 as part of Migrate Kubernetes integration #511, based on migration assessment
  2. Adds system tests for the datastreams using mock k8s APIs

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all datasets collect metrics or logs.

@ChrsMark ChrsMark self-assigned this Jan 26, 2021
@elasticmachine
Copy link

elasticmachine commented Jan 26, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #569 updated

    • Start Time: 2021-02-02T08:10:44.140+0000
  • Duration: 29 min 28 sec

  • Commit: f0b7841

Test stats 🧪

Test Results
Failed 0
Passed 87
Skipped 0
Total 87

Signed-off-by: ChrsMark <[email protected]>
@ChrsMark ChrsMark changed the title Add daemonset datastream for k8s package Add daemonset datastream for k8s package & system tests Jan 29, 2021
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
@ChrsMark
Copy link
Member Author

@mtojek it could be merged as is but not sure if we should merge yet. However, I would like your eyes on this at this stage.

@mtojek mtojek self-requested a review February 1, 2021 15:08
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

LGTM. I think we can merge it and even push it to snapshot. We don't need to promote it further.

@ChrsMark
Copy link
Member Author

ChrsMark commented Feb 2, 2021

OK merging this one for now, thanks for reviewing!

@ChrsMark
Copy link
Member Author

ChrsMark commented Feb 2, 2021

Ah, CI failed. Checking...

@mtojek
Copy link
Contributor

mtojek commented Feb 2, 2021

Hm.. it looks like it passed.

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

Successfully merging this pull request may close these issues.

3 participants