Improve workloads statuses graphs on overview page #2598
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Great! |
k8s-ci-robot
added
size/XL
| package common | ||
|
|
||
| // Status provides basic information about resources status on the list. | ||
| type Status struct { |
| Failed int32 `json:"failed"` | ||
|
|
||
| // Number of resources that are in succeeded state. | ||
| Succeeded int32 `json:"succeeded"` |
There was a problem hiding this comment.
I would prefer map[string]int32 here, because it may be misleading in case of Cron Jobs for example, which are Active or Suspended only. And these labels are not translated anyways.
There was a problem hiding this comment.
There might be an issue with colouring graph slices... Perhaps some constant values for common names like active, failed, running, pending etc. would make it better.
There was a problem hiding this comment.
I was considering map but it requires introducing yet another portions of strings that would need to be hardcoded in backend and in frontend, and kept in sync. Using generic names like that and mapping them to specific resources seems like better idea. Map would complicate the code even more.
For CronJob Running can be easily considered as Active state and Failed as Suspended state. Icons used to show CronJob state in frontend are the same as for Running/Failed resource anyway.
There was a problem hiding this comment.
Syncing frontend with the backend is required at the moment too. You need to keep in mind, that for Cron Job active = running and suspended = failed during development. Both have their own drawbacks. We can stick to the current implementation for now and make the release.
|
|
||
| for _, cronJob := range cronJobList.Items { | ||
| if cronJob.Spec.Suspend != nil && !(*cronJob.Spec.Suspend) { | ||
| info.Running++ |
There was a problem hiding this comment.
This is what I have mentioned in my earlier comment.
| info.Pending++ | ||
| } else { | ||
| info.Running++ | ||
| } |
There was a problem hiding this comment.
Export to a function if it repeats somewhere?
k8s-ci-robot
added
size/XXL
floreks
force-pushed
the
new-branch
branch
2 times, most recently
from
8a0d1b1
to
beed590
Compare
Codecov Report
@@ Coverage Diff @@
## master #2598 +/- ##
==========================================
+ Coverage 53.63% 54.18% +0.55%
==========================================
Files 562 563 +1
Lines 11955 12108 +153
==========================================
+ Hits 6412 6561 +149
+ Misses 5322 5289 -33
- Partials 221 258 +37
Continue to review full report at Codecov.
|
Fixes #2593
Changes
Running: 4.IsTypeFilledcalls on backend.Statusstruct to all resource list objects that contain basic info about the state of resources on the list.There is an issue (visible on below ss) with graph not being fully filled. It's related to how nvd3 handles showing charts. There is not much we can do right now.
TODO
Before
After