Add status messages for traceflow graph #1277
Conversation
|
Thanks for your PR. The following commands are available:
|
Codecov Report
@@ Coverage Diff @@
## master #1277 +/- ##
==========================================
- Coverage 55.02% 55.00% -0.03%
==========================================
Files 110 110
Lines 10573 10596 +23
==========================================
+ Hits 5818 5828 +10
- Misses 4183 4192 +9
- Partials 572 576 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pkg/graphviz/traceflow.go
Outdated
| } else { | ||
| wStr += string(str[i]) | ||
| } | ||
| } |
There was a problem hiding this comment.
could it use strings.ReplaceAll instead?
There was a problem hiding this comment.
Used strings.ReplaceAll instead.
pkg/graphviz/traceflow.go
Outdated
| @@ -309,6 +334,12 @@ func GenGraph(tf *opsv1alpha1.Traceflow) (string, error) { | |||
|
|
|||
| senderRst := getNodeResult(tf, isSender) | |||
| receiverRst := getNodeResult(tf, isReceiver) | |||
| if tf.Status.Phase != opsv1alpha1.Succeeded { | |||
| err := getTraceflowStatusMessage(tf, graph) | |||
There was a problem hiding this comment.
it doesn't seem that the function will return an error, and having a side effect in a get function is obscure, maybe just let the function return the message and set the label in GenGraph, which could be more consistent as well?
There was a problem hiding this comment.
Function modified.
ZhangYW18
force-pushed
the
tf-ui-fix
branch
from
c0fac4d
to
a713dd4
Compare
pkg/graphviz/traceflow.go
Outdated
| case opsv1alpha1.Pending: | ||
| return getWrappedStr(fmt.Sprintf("traceflow %s is pending...", tf.Name)) | ||
| default: | ||
| return "Unknown traceflow status, please check whether Antrea is running and traceflow is enabled." |
There was a problem hiding this comment.
doesn't this message need to be wrapped by "?
There was a problem hiding this comment.
Added getWrappedStr.
ZhangYW18
force-pushed
the
tf-ui-fix
branch
from
a713dd4
to
f0cad63
Compare
pkg/graphviz/traceflow.go
Outdated
| func getTraceflowStatusMessage(tf *opsv1alpha1.Traceflow) string { | ||
| switch tf.Status.Phase { | ||
| case opsv1alpha1.Failed: | ||
| return getWrappedStr(fmt.Sprintf("traceflow %s failed: %s", tf.Name, tf.Status.Reason)) |
There was a problem hiding this comment.
I feel "Traceflow" is better.
There was a problem hiding this comment.
"traceflow" changed to "Traceflow".
pkg/graphviz/traceflow.go
Outdated
| case opsv1alpha1.Pending: | ||
| return getWrappedStr(fmt.Sprintf("traceflow %s is pending...", tf.Name)) | ||
| default: | ||
| return getWrappedStr("Unknown traceflow status, please check whether Antrea is running and traceflow is enabled.") |
There was a problem hiding this comment.
Suggestion - Unknown Traceflow status. Please check Antrea is running with Traceflow feature gate enabled.
ZhangYW18
force-pushed
the
tf-ui-fix
branch
from
f0cad63
to
d5b013f
Compare
|
/test-all |
When users start a traceflow on the Antrea Octant plugin, they need to wait for several seconds before the traceflow graph finishes., which may be considered a UI bug. Also, when something's wrong with traceflow, users will see a blank graph, which is confusing.
This patch shows status messages on traceflow graph when traceflow is running, pending or stopped for some reason. When users start a traceflow, they will see the graph showing "traceflow xxx is running" in the first seconds.
Example graphs:
Traceflow is running:
Showing error messages:
According to Octant team, the next Octant release is running into some delays and pushing into early this week. Alerts with traceflow error messages will be added after the next Octant release.
Fixes #1175