enhancement(spd): set default baselint ratio by qos level #375
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #375 +/- ##
==========================================
+ Coverage 53.36% 53.42% +0.05%
==========================================
Files 439 439
Lines 48322 48372 +50
==========================================
+ Hits 25789 25842 +53
+ Misses 19622 19611 -11
- Partials 2911 2919 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cheney-lin
added
enhancement
| // for indicator get functions, IndicatorUpdater will return a channel to obtain the merged results. | ||
| GetIndicatorSpec(_ types.NamespacedName) *apiworkload.ServiceProfileDescriptorSpec | ||
| GetIndicatorStatus(_ types.NamespacedName) *apiworkload.ServiceProfileDescriptorStatus | ||
| GetSPDSpec(_ types.NamespacedName) *apiworkload.ServiceProfileDescriptorSpec |
There was a problem hiding this comment.
maybe it's better to keep GetIndicatorSpec, since the logic is for updating indicator spec (despite that the returned value is entire spec for spd)
pkg/controller/spd/spd.go
Outdated
| @@ -539,6 +542,25 @@ func (sc *SPDController) getWorkload(gvr schema.GroupVersionResource, namespace, | |||
| return workload, nil | |||
| } | |||
|
|
|||
| func (sc *SPDController) defaultBaselineRatio(workload *unstructured.Unstructured) *float32 { | |||
There was a problem hiding this comment.
for safety reasons, the default value when error appears should be 100 rather than nil (which actually equals 0)
There was a problem hiding this comment.
and remember to add some comments for why we should return 100
| @@ -39,6 +40,8 @@ type IndicatorUpdater interface { | |||
| AddBusinessIndicatorSpec(_ types.NamespacedName, _ []apiworkload.ServiceBusinessIndicatorSpec) | |||
| AddSystemIndicatorSpec(_ types.NamespacedName, _ []apiworkload.ServiceSystemIndicatorSpec) | |||
| AddBusinessIndicatorStatus(_ types.NamespacedName, _ []apiworkload.ServiceBusinessIndicatorStatus) | |||
|
|
|||
| UpdateBaselineRatio(_ types.NamespacedName, _ float32) | |||
There was a problem hiding this comment.
make the naming aligned, either by change the origin AddBusinessIndicatorSpec to Updatexxx or UpdateBaselineRatio to AddBaselineRatio
cheney-lin
force-pushed
the
dev/spd
branch
5 times, most recently
from
8a2c251 to
85ecf34
Compare
Signed-off-by: linzhecheng <linzhecheng@bytedance.com>
cheney-lin
added
workflow/merge-ready
What type of PR is this?
Enhancements
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer: