✨ Configurable qos resources for containers managed by cluster-manager and klusterlet #351
Conversation
|
/hold |
dongbeiqing91
force-pushed
the
dev/qos
branch
from
4b3664b to
3df936d
Compare
dongbeiqing91
force-pushed
the
dev/qos
branch
from
3df936d to
813421c
Compare
dongbeiqing91
force-pushed
the
dev/qos
branch
from
813421c to
fa5140e
Compare
|
/hold cancel |
dongbeiqing91
force-pushed
the
dev/qos
branch
from
fa5140e to
582adb1
Compare
|
/hold |
pkg/common/helpers/clusters_test.go
Outdated
| @@ -1,114 +0,0 @@ | |||
| package helpers | |||
There was a problem hiding this comment.
The PlacementDecisionGetter is being used by other controllers, GetClusterChanges and the clusters_test.go is used to test the PlacementDecisionGetter I think. Any reason want to remove it?
There was a problem hiding this comment.
I removed this because functions used in pkg/common/helpers/clusters.go are not found after I updated open-cluster-management.io/api to the latest commit in go.mod
Confirmed with @qiujian16, the not found issue will be fixed by open-cluster-management-io/addon-framework#232.
I will update this PR then after the above PR is merged.
There was a problem hiding this comment.
file restored
| } | ||
|
|
||
| func TestRenderingResourceRequirements(t *testing.T) { | ||
| r := &operatorapiv1.ResourceRequirement{ |
There was a problem hiding this comment.
seems to me the three tests can be put in one test func with different cases. It also worth a test case with type is ResourceRequirement and ResourceRequirements is nil or empty
There was a problem hiding this comment.
merged into one unit test with 3 cases
dongbeiqing91
force-pushed
the
dev/qos
branch
from
582adb1 to
b712806
Compare
|
/hold cancel |
…and klusterlet Signed-off-by: Dong Beiqing <350758787@qq.com>
dongbeiqing91
force-pushed
the
dev/qos
branch
from
b712806 to
3c882f1
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #351 +/- ##
==========================================
- Coverage 61.35% 61.29% -0.06%
==========================================
Files 132 132
Lines 13996 14020 +24
==========================================
+ Hits 8587 8594 +7
- Misses 4657 4672 +15
- Partials 752 754 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
LGTM |
Signed-off-by: Dong Beiqing <350758787@qq.com>
|
executed |
|
/approve I am fine to merge this, I think we also need to add an integration test case as a followup |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: promid, qiujian16 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 |
openshift-merge-bot
bot
merged commit 5b9b566
into
open-cluster-management-io:main
Summary
Configurable qos resources for containers managed by cluster-manager and klusterlet
Related issue(s)
Fixes #
Related PR
open-cluster-management-io/api#316