-
Notifications
You must be signed in to change notification settings - Fork 558
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is a working PR, confirmed:
|
@sharmasushant Can you confirm the above pod description is what you want to see on a k8s cluster? |
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.
lgtm
@sharmasushant @tamilmani1989 for an lgtm from Azure CNI team |
835989e
to
db387ab
Compare
Codecov Report
@@ Coverage Diff @@
## master #3198 +/- ##
==========================================
- Coverage 52.66% 52.39% -0.27%
==========================================
Files 103 103
Lines 15505 15486 -19
==========================================
- Hits 8165 8114 -51
- Misses 6600 6643 +43
+ Partials 740 729 -11 |
@jackfrancis Thanks for this. We were thinking that may be we can make it part of the CNI itself. So when CNI is used on the node, it can start the monitoring service in case it is not already running. Let me get back to you on this. |
@sharmasushant I'm going to merge this in in the meantime as we've seen this addon help in our testing. We can remove it later if we have the same monitoring functionality baked into the binaries. Thanks! |
@jackfrancis sounds good. Please be aware that this right now is considered experimental. So, let's not enable by default. We would like to go through some more testing on this. |
What this PR does / why we need it: Installs an Azure CNI "networkmonitor" addon whenever Azure CNI is present
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: