add cpu qos and mv nodeslo informer to states informer #153
Conversation
|
@zwzhang0107 can you split this into separate PRs? one for CPU QoS, and the other for states_informer. |
Codecov Report
@@ Coverage Diff @@
## main #153 +/- ##
==========================================
+ Coverage 56.89% 57.56% +0.67%
==========================================
Files 86 89 +3
Lines 7887 7953 +66
==========================================
+ Hits 4487 4578 +91
+ Misses 3006 2979 -27
- Partials 394 396 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
it's already a separation of group identity hook implement(#154), since cpu qos definition dose not include much logic |
|
could you also fix #77 in this PR? Thanks |
| return | ||
| } | ||
| for _, c := range callbacks { | ||
| klog.Infof("start running callback function %v for type %v", c.name, objType) |
There was a problem hiding this comment.
Is this supposed to be a v(x) log ?
| ) | ||
|
|
||
| var singleton StatesInformer |
There was a problem hiding this comment.
Could you avoid using singleton pattern?
done |
| func (s *statesInformer) RegisterCallbacks(objType reflect.Type, name, description string, callbackFn UpdateCbFn) { | ||
| callbacks, legal := s.stateUpdateCallbacks[objType] | ||
| if !legal { | ||
| panic(fmt.Sprintf("states informer callback register with type %v is illegal", objType)) |
Signed-off-by: zwzhang0107 <zuoweizhang@outlook.com>
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hormes 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 |
Ⅰ. Describe what this PR does
#154
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews