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.

Sign up for GitHub

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

Jump to bottom

Added kube_scheduler check #3371

Merged
merged 7 commits into from
Mar 29, 2019
Merged

Added kube_scheduler check #3371

merged 7 commits into from
Mar 29, 2019

Conversation

clamoriniere
Copy link
Contributor

@clamoriniere clamoriniere commented Mar 25, 2019 •
edited
Loading

What does this PR do?

Introduce a new check: kube_scheduler

Motivation

Improves the Kubernetes core components integration

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • Feature or bugfix must have tests
  • Git history must be clean
  • If PR adds a configuration option, it must be added to the configuration file.

@clamoriniere clamoriniere requested review from mfpierre and xvello March 25, 2019 11:15
@clamoriniere clamoriniere requested review from a team as code owners March 25, 2019 11:15
@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch 4 times, most recently from a82da7c to 0aa1972 Compare March 25, 2019 12:39
@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch from 0aa1972 to f0ce07e Compare March 25, 2019 12:40
@codecov
Copy link

codecov bot commented Mar 25, 2019 •
edited
Loading

Codecov Report

Merging #3371 into master will increase coverage by 4.38%.
The diff coverage is 90.34%.

@@            Coverage Diff             @@
##           master    #3371      +/-   ##
==========================================
+ Coverage   85.95%   90.34%   +4.38%     
==========================================
  Files         722        5     -717     
  Lines       36897      176   -36721     
  Branches     4355       12    -4343     
==========================================
- Hits        31714      159   -31555     
+ Misses       3959       14    -3945     
+ Partials     1224        3    -1221

l0k0ms
l0k0ms reviewed Mar 25, 2019
View reviewed changes
kube_scheduler/README.md Outdated Show resolved Hide resolved
l0k0ms
l0k0ms reviewed Mar 25, 2019
View reviewed changes
kube_scheduler/README.md Outdated Show resolved Hide resolved
@l0k0ms
Copy link
Contributor

l0k0ms commented Mar 25, 2019

nice PR from a doc perspective :) I left some comments.

@clamoriniere
Copy link
Contributor Author

thanks @l0k0ms for the review.
I updated the PR with all your comments.

@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch from 64a0493 to 6975487 Compare March 25, 2019 13:47
xvello
xvello reviewed Mar 25, 2019
View reviewed changes
kube_scheduler/datadog_checks/kube_scheduler/kube_scheduler.py Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch from b3ef4b3 to f2c88ea Compare March 26, 2019 16:00
@clamoriniere
Copy link
Contributor Author

@xvello PR updated following your comments

xvello
xvello reviewed Mar 27, 2019
View reviewed changes
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

Thanks for testing the two versions!
Some comments on the metadata + one nit on _summary_from_microseconds_to_second

kube_scheduler/datadog_checks/kube_scheduler/kube_scheduler.py Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
kube_scheduler/metadata.csv Outdated Show resolved Hide resolved
@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch from f2c88ea to bc2d309 Compare March 27, 2019 11:10
@clamoriniere clamoriniere force-pushed the clamoriniere/kube-scheduler branch from bc2d309 to 82bcc4a Compare March 27, 2019 12:08
@clamoriniere
Copy link
Contributor Author

@xvello, it should be good now :)

@clamoriniere clamoriniere changed the title start kube-scheduler check implementation Added kube-scheduler check Mar 27, 2019
@clamoriniere clamoriniere changed the title Added kube-scheduler check Added kube_scheduler check Mar 27, 2019
xvello
xvello previously approved these changes Mar 27, 2019
View reviewed changes
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

Looking great!

Let's get a final decision on the apiserver client metrics before merging

@clamoriniere
Copy link
Contributor Author

clamoriniere commented Mar 27, 2019 •
edited
Loading

@xvello, after a discussion with @CharlyF, I removed the apiserver metrics from this check and will see later if it worth to add it back.

xvello
xvello previously approved these changes Mar 28, 2019
View reviewed changes
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

👍

ofek
ofek requested changes Mar 28, 2019
View reviewed changes
kube_scheduler/tox.ini Outdated Show resolved Hide resolved
kube_scheduler/tox.ini Outdated Show resolved Hide resolved
@ofek @clamoriniere
change basepython version
c67f51c 
move from py36 to py37

Co-Authored-By: clamoriniere <cedric.lamoriniere@gmail.com>
@ofek @clamoriniere
Update kube_scheduler/tox.ini
eaf2342 
move from py36 to py37

Co-Authored-By: clamoriniere <cedric.lamoriniere@gmail.com>
@clamoriniere clamoriniere requested a review from ofek March 28, 2019 15:36
@clamoriniere
Copy link
Contributor Author

thanks @ofek for the review. PR updated with your suggestions.

xvello
xvello approved these changes Mar 29, 2019
View reviewed changes
Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

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

🚢

@clamoriniere clamoriniere merged commit 90dc3b6 into master Mar 29, 2019
@clamoriniere clamoriniere deleted the clamoriniere/kube-scheduler branch March 29, 2019 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@l0k0ms l0k0ms l0k0ms left review comments

@ofek ofek ofek approved these changes

@xvello xvello xvello approved these changes

@mfpierre mfpierre Awaiting requested review from mfpierre

Assignees
No one assigned
Labels
containers integration/kube_scheduler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@clamoriniere @l0k0ms @xvello @ofek