add livetraffic support in octant plugin #2124

luolanzone · 2021-04-25T09:29:55Z

add new tab to support live traffic UI

refine 'Generate Trace Graph' tab to allow user to chose from CRD list instead of input a traceflow name directly

refine log and alert output

Implementation as part of issue #2030

codecov-commenter · 2021-04-25T09:34:28Z

Codecov Report

Merging #2124 (f68d305) into main (e50142c) will not change coverage.
The diff coverage is n/a.

❗ Current head f68d305 differs from pull request most recent head 2f9164e. Consider uploading reports for the commit 2f9164e to get more accurate results

@@           Coverage Diff           @@
##             main    #2124   +/-   ##
=======================================
  Coverage   61.26%   61.26%           
=======================================
  Files         270      270           
  Lines       20478    20478           
=======================================
  Hits        12545    12545           
  Misses       6640     6640           
  Partials     1293     1293

Flag	Coverage Δ
kind-e2e-tests	`52.11% <ø> (+<0.01%)`	⬆️
unit-tests	`41.33% <ø> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/apiserver/handlers/endpoint/handler.go	`58.82% <0.00%> (-11.77%)`	⬇️
pkg/apiserver/certificate/cacert_controller.go	`58.95% <0.00%> (-8.96%)`	⬇️
pkg/apiserver/apiserver.go	`89.43% <0.00%> (-1.63%)`	⬇️
pkg/controller/networkpolicy/status_controller.go	`85.16% <0.00%> (-1.30%)`	⬇️
pkg/ovs/ovsconfig/ovs_client.go	`49.37% <0.00%> (+0.62%)`	⬆️
...ntroller/networkpolicy/networkpolicy_controller.go	`70.96% <0.00%> (+0.64%)`	⬆️
pkg/agent/cniserver/pod_configuration.go	`57.42% <0.00%> (+3.51%)`	⬆️
...gent/controller/networkpolicy/status_controller.go	`78.08% <0.00%> (+5.47%)`	⬆️

antoninbas

I like the idea of having a new tab for live-traffic traceflow.

I tried to do a traceflow without specifying the destination, but it failed with the following error:

Invalid destination namespace string, please check your input and submit again. err: [a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')]

Same thing if I don't specify a source. But for live-traffic traceflow one can omit one of source or destination. Maybe you just didn't get to that part yet?

luolanzone · 2021-04-27T02:55:29Z

@antoninbas , I just updated the code, I move the validation to the wrong place before. you can try again. :)

antoninbas · 2021-04-27T03:04:22Z

@luolanzone it seems I am still getting the same error for empty source / destination? could you point me to the part of your code that handles an empty source field differently for live-traffic traceflow?

luolanzone · 2021-04-27T03:08:26Z

@antoninbas ah, sorry, I forgot the most important empty validation logic for live-traffic, please hold review for this PR, I will update it and require review later.

jianjuns · 2021-04-27T04:36:32Z

@luolanzone : thanks for working on this! Could you also paste the screen shoots for the live-traffic flow tab? Or maybe you can update docs/traceflow-guide.md and add the screenshot there.

luolanzone · 2021-04-27T04:42:14Z

@jianjuns I just added the screen shot to summary, since you are working on the doc update already, maybe I should add it later after your PR is merged?
@antoninbas @mengdie-song I think the PR is ready for review now, please help to check. thanks.

mengdie-song

Hi @luolanzone,

I have concern about the validation part for optional field for live-traffic traceflow and put my comment to the corresponding line. Could you please take a look and check on the live set up?

mengdie-song · 2021-04-27T06:33:08Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

 		}

-		srcPod, err := request.Payload.String(srcPodCol)
+		srcNamespace, err := request.Payload.String(srcNamespaceCol)


For live-traffic traceflow, the column can be empty, right? Could you confirm that empty input will not cause any error here? I am asking because that we used to use request.Payload.OptionalString() for optional field.

@mengdie-song yes, it works with empty source without any error when the destination has been provided, I think it's not a pure optional string because it depends on destination. per may understanding, the OptionalString() should be used for those optional fields under any circumstance.

OK, got it. I agree that OptionalString() should be used for those optional fields under any circumstance and just want to make sure empty input gets no errors here. Thanks.

mengdie-song · 2021-04-27T09:29:06Z

plugins/octant/cmd/antrea-octant-plugin/main.go

@@ -22,7 +22,7 @@ import (
 	"github.com/vmware-tanzu/octant/pkg/navigation"
 	"github.com/vmware-tanzu/octant/pkg/plugin"
 	"github.com/vmware-tanzu/octant/pkg/plugin/service"
-	"k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"


nit: no need to add v1 here

mengdie-song · 2021-04-27T09:29:21Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

@@ -28,14 +29,15 @@ import (
 	"github.com/vmware-tanzu/octant/pkg/view/component"
 	"github.com/vmware-tanzu/octant/pkg/view/flexlayout"
 	"k8s.io/apimachinery/pkg/api/validation"
-	"k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"


mengdie-song · 2021-04-27T09:39:00Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

-				"please check your input and submit again."), action.DefaultAlertExpiration)
-			request.DashboardClient.SendAlert(request.Context(), request.ClientID, alert)
-			return nil
+	case addTfAction, addLiveTfAction:


I understand there can be some common code for these two cases, but I am still thinking if we should split these two cases to do validation. Because it seems that there are multiple boolean flags and goto clause here. What do you think?

that make sense, I will check, maybe I can move more common codes as functions and make thess two actions into two cases which should help other to understand the logic better.

antoninbas · 2021-04-27T18:15:37Z

@luolanzone I ran a manual test for a live-traffic traceflow with a source and no destination. The traceflow is created and succeeds, but the UI is never updated:

But the CR Status has been updated successfully:

$ kubectl get tf/live-src-toolbox-z5wjn-20210427-111158 -o yaml
apiVersion: crd.antrea.io/v1alpha1
kind: Traceflow
metadata:
  creationTimestamp: "2021-04-27T18:11:58Z"
  generation: 1
  managedFields:
  - apiVersion: crd.antrea.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:spec:
        .: {}
        f:destination: {}
        f:liveTraffic: {}
        f:packet:
          .: {}
          f:ipHeader:
            .: {}
            f:protocol: {}
          f:transportHeader:
            .: {}
            f:icmp: {}
        f:source:
          .: {}
          f:namespace: {}
          f:pod: {}
    manager: antrea-octant-plugin
    operation: Update
    time: "2021-04-27T18:11:58Z"
  - apiVersion: crd.antrea.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:status:
        f:capturedPacket:
          .: {}
          f:dstIP: {}
          f:ipHeader:
            .: {}
            f:flags: {}
            f:protocol: {}
            f:ttl: {}
          f:length: {}
          f:srcIP: {}
          f:transportHeader: {}
        f:results: {}
    manager: antrea-agent
    operation: Update
    time: "2021-04-27T18:12:02Z"
  - apiVersion: crd.antrea.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:status:
        .: {}
        f:phase: {}
    manager: antrea-controller
    operation: Update
    time: "2021-04-27T18:12:02Z"
  name: live-src-toolbox-z5wjn-20210427-111158
  resourceVersion: "426149"
  uid: 264dff46-2151-403c-9124-ce2996453858
spec:
  destination: {}
  liveTraffic: true
  packet:
    ipHeader:
      protocol: 1
    transportHeader:
      icmp: {}
  source:
    namespace: default
    pod: toolbox-z5wjn
status:
  capturedPacket:
    dstIP: 10.10.0.2
    ipHeader:
      flags: 2
      protocol: 1
      ttl: 63
    length: 84
    srcIP: 10.10.1.4
    transportHeader: {}
  phase: Succeeded
  results:
  - node: k8s-node-worker-1
    observations:
    - action: Forwarded
      component: SpoofGuard
    - action: Forwarded
      component: Forwarding
      componentInfo: Output
      tunnelDstIP: 192.168.77.100
    timestamp: 1619547122
  - node: k8s-node-control-plane
    observations:
    - action: Received
      component: Forwarding
      componentInfo: Classification
    - action: Delivered
      component: Forwarding
      componentInfo: Output
    timestamp: 1619547122

Could you make sure you run all these tests manually on your cluster?

jianjuns · 2021-04-27T18:15:33Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

-		component.NewFormFieldText(dstCol, dstCol, ""),
-		component.NewFormFieldNumber(dstPortCol, dstPortCol, ""),
-		component.NewFormFieldSelect(protocolCol, protocolCol, protocolSelect, false),
+	srcNamespaceField := component.NewFormFieldText(srcNamespaceCol, srcNamespaceCol, "")


Add "(Not required when destination is an IP)" too.

jianjuns · 2021-04-27T18:19:28Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+	dstField := component.NewFormFieldText(dstCol, dstCol, "")
+	dstPortField := component.NewFormFieldNumber(dstPortCol, dstPortCol, "")
+	protocolField := component.NewFormFieldSelect(protocolCol, protocolCol, protocolSelect, false)
+	timeoutField := component.NewFormFieldNumber(timeoutCol+" (Default value is 15 seconds)", timeoutCol, "15")


Let us use DefaultTraceflowTimeout defined in apis/crd/v1alpha1/types.go.

luolanzone · 2021-04-28T02:22:16Z

@antoninbas thanks for the review, I was focus on the field validation, I will take a look why it's not updating when it succeed, It was working before, now I notice the graph will only update if it's timeout result, probably some issues introduced when I refactor the code.

luolanzone · 2021-04-28T05:37:52Z

@antoninbas , I have fixed the graph issue for your use case, but there are a few cases I need to clarify with @jianjuns , when the destination is pod, looks like no matter source is empty or not, the live traffic result will only contain receiver side info, which causes the graph failed because no available source node info. jianjun, Could you help to check below results are expected or not?

empty source

spec:
  destination:
    namespace: kube-system
    pod: pod2
  liveTraffic: true
  packet:
    ipHeader:
      protocol: 1
    transportHeader:
      icmp: {}
  source: {}
status:
  capturedPacket:
    dstIP: 192.168.226.5
    ipHeader:
      flags: 2
      protocol: 1
      ttl: 62
    length: 84
    srcIP: 192.168.225.5
    transportHeader: {}
  phase: Succeeded
  results:
  - node: lan-k8s-0-2
    observations:
    - action: Received
      component: Forwarding
    - action: Delivered
      component: Forwarding
      componentInfo: Output
    timestamp: 1619587044

source is IP

spec:
  destination:
    namespace: kube-system
    pod: pod2
  liveTraffic: true
  packet:
    ipHeader:
      protocol: 1
    transportHeader:
      icmp: {}
  source:
    ip: 192.168.225.5
status:
  capturedPacket:
    dstIP: 192.168.226.5
    ipHeader:
      flags: 2
      protocol: 1
      ttl: 62
    length: 84
    srcIP: 192.168.225.5
    transportHeader: {}
  phase: Succeeded
  results:
  - node: lan-k8s-0-2
    observations:
    - action: Received
      component: Forwarding
    - action: Delivered
      component: Forwarding
      componentInfo: Output
    timestamp: 1619586221

special case, use IP to ping the pod itself get timeout

spec:
  destination:
    namespace: kube-system
    pod: pod1
  liveTraffic: true
  packet:
    ipHeader:
      protocol: 1
    transportHeader:
      icmp: {}
  source:
    ip: 192.168.225.5
status:
  phase: Failed
  reason: Traceflow timeout

antoninbas · 2021-04-28T21:08:02Z

when the destination is pod, looks like no matter source is empty or not, the live traffic result will only contain receiver side info

This is the expected behavior, you can refer to Jianjun's documentation PR which has more information: #2126

If the source is not a Pod, you will only get information from the receiver Node.

which causes the graph failed because no available source node info

The graph logic needs to be updated to support that case for live-traffic Traceflow

jianjuns · 2021-04-28T21:47:41Z

For this one: "3. special case, use IP to ping the pod itself get timeout". If you mean ping the Pod's own IP inside the Pod, yes it is expected Traceflow will not capture the packet (as it never leaves the Pod and enters OVS).

luolanzone · 2021-04-29T10:44:19Z

@mengdie-song I have fixed the graph issue for live traffic, but now I encountered a problem to make 'source' to be the left of 'destination', looks like existing function doesn't work as expected to adjust the sequence, I know it's a dot language sequence problem, but I haven't figured out where's the code issue, Could you help to review and give me some insights? thanks!

jianjuns

Could we have a screenshot of Traceflow graph for the receiver-only case?

jianjuns · 2021-04-29T16:52:46Z

pkg/graphviz/traceflow.go

@@ -341,6 +350,55 @@ func GenGraph(tf *crdv1alpha1.Traceflow) (string, error) {
 		graph.Attrs[gographviz.Label] = getTraceflowStatusMessage(tf)
 	}
 	if tf == nil || senderRst == nil || tf.Status.Phase != crdv1alpha1.Succeeded || len(senderRst.Observations) == 0 {
+		// for live traffic, when the source is IP or empty, there is no sender's node result from traceflow status.


Nit: capitalize the first letter.

antoninbas

I tested a few things manually (probably not all cases) and didn't have any issue.

@jianjuns this is the graph when the source is not specified:

Unfortunately there is no easy way to map the IP to a Pod reference.

@luolanzone we should have a follow-up PR to show the contents of the captured packet when doing a live traceflow.

Finally, it feels like we should have a semi-automated way of testing the UI, at least for the Traceflow feature. I feel like at the very least we should have a list with all the difference cases someone should test when validating the UI support. I don't know if there is a better way that doesn't require a large development effort...

antoninbas · 2021-04-29T21:56:54Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

@@ -88,292 +95,236 @@ func getDstType(tf *crdv1alpha1.Traceflow) string {
 	return ""
 }

-// actionHandler handlers clicks and actions from "Start New Trace" and "Generate Trace Graph" buttons.
+// actionHandler handlers clicks and actions from "Start New Trace", "START NEW LIVE-TRAFFIC TRACE" and "Generate Trace Graph" buttons.


why is START NEW LIVE-TRAFFIC TRACE capitalized here?

I think it is just to follow other tab titles. I agree "Start new live-traffic Traceflow" looks better to me.

antoninbas · 2021-04-29T21:57:52Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

@@ -383,6 +334,273 @@ func (p *antreaOctantPlugin) actionHandler(request *service.ActionRequest) error
 	}
 }

+func checkNameSpace(request *service.ActionRequest) (string, error) {


s/checkNameSpace/checkNamespace

antoninbas · 2021-04-29T21:58:35Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+	if tfOld.Name == tfName {
+		alertPrinter(request, invalidInputMsg+
+			fmt.Sprintf("duplicate traceflow \"%s\": same source pod and destination pod in less than one second: %+v. ", tfName, tfOld),
+			"Duplicate traceflow: same source pod and destination pod in less than one second", nil, nil)


we typically capitalize K8s terms such as Pod, Namespace, Service, Node, etc.

jianjuns · 2021-04-29T22:46:09Z

Thanks @antoninbas for the screenshot. It looks ok to me.

luolanzone · 2021-05-03T13:35:35Z

@antoninbas thanks for the review comments, I will check how the content of captured package can be added in the graph. and for the UI testing automation, not sure how much effort it will be, but I guess it's not an easy task and error-prone.

antoninbas

some nits, otherwise LGTM

I gave the plugin another try and it worked fine

looking forward to having the captured packet as part of the live traffic result in a future PR

antoninbas · 2021-05-06T21:24:33Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+			dropOnlyChecked = true
+		}
+
+		// Judge whether the name of trace flow is duplicated.


maybe use "traceflow" consistently instead of "trace flow"?

antoninbas · 2021-05-06T21:25:20Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+		}
+
+		// Judge whether the name of trace flow is duplicated.
+		// If it is, then the user creates more than one traceflows in one second, which is not allowed.


Suggested change

// If it is, then the user creates more than one traceflows in one second, which is not allowed.

// If it is, then the user created more than one traceflow in one second, which is not allowed.

antoninbas · 2021-05-06T21:26:33Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+	ctx := context.Background()
+	tfs, err := p.client.CrdV1alpha1().Traceflows().List(ctx, v1.ListOptions{ResourceVersion: "0"})
+	if err != nil {
+		log.Fatalf("Failed to get Traceflows %v", err)


you don't capitalize "traceflow" in other places?

antoninbas · 2021-05-06T21:29:17Z

pkg/graphviz/traceflow.go

@@ -341,6 +350,53 @@ func GenGraph(tf *crdv1alpha1.Traceflow) (string, error) {
 		graph.Attrs[gographviz.Label] = getTraceflowStatusMessage(tf)
 	}
 	if tf == nil || senderRst == nil || tf.Status.Phase != crdv1alpha1.Succeeded || len(senderRst.Observations) == 0 {
+		// For live traffic, when the source is IP or empty, there is no sender's Node result from traceflow status.


Suggested change

// For live traffic, when the source is IP or empty, there is no sender's Node result from traceflow status.

// For live traffic, when the source is IP or empty, there is no result from the sender Node result in the traceflow status.

luolanzone · 2021-05-10T03:46:34Z

@jianjuns @antoninbas here is the new added capturedPacket info screenshot:

antoninbas

I tried a few times but the only way I could get the captured packet to work was if the source was an IP address and the destination was a Pod. In particular, it never worked for me when the source was a Pod instead of an IP address, even though the captured packet shows up correctly in the Traceflow CR. This makes sense based on the code (we only include the packet when the source is an IP or is empty), but I don't now why you are only handling that case?

BTW, I think we were really close to merging your original PR. I think it would have been better to add support for displaying the captured packet in a follow-up PR.

antoninbas · 2021-05-10T22:42:41Z

pkg/graphviz/traceflow.go

@@ -422,3 +488,37 @@ func GenGraph(tf *crdv1alpha1.Traceflow) (string, error) {

 	return genOutput(graph, false), nil
 }
+
+func getCapturedPacketLabel(tf *crdv1alpha1.Traceflow) string {
+	label := "caputredPacket:\\ldstIP:" + tf.Status.CapturedPacket.DstIP + "\\l"


s/caputredPacket/capturedPacket

antoninbas · 2021-05-11T00:03:34Z

pkg/graphviz/traceflow.go

+
+	if tf.Status.CapturedPacket.IPHeader != (crdv1alpha1.IPHeader{}) {
+		label = label + "ipHeader: " + "\\l" +
+			"    flags: " + strconv.Itoa(int(tf.Status.CapturedPacket.IPHeader.Flags)) + "\\l" +


IMO you should use fmt.Sprintf instead of these strconv.Itoa(int(...)) calls

antoninbas · 2021-05-11T00:06:19Z

pkg/graphviz/traceflow.go

+				return "", err
+			}
+
+			// create an invisiable edge before destination cluster, otherwise the source cluster will


s/invisiable/invisble

antoninbas · 2021-05-11T00:21:19Z

pkg/graphviz/traceflow.go

+			// Add captured packet info to destination cluster.
+			err = graph.AddNode(dstCluster.Name, "capturedPacket", map[string]string{


why only for this case (senderRst == nil)?

it's a mistake, I will refine it in a following PR.

luolanzone · 2021-05-11T01:05:01Z

@antoninbas you are right, I didn't realize that I placed the code in the wrong place, so it only handle the special case, I will revert it and submit the change in a new PR.

luolanzone · 2021-05-12T01:49:49Z

@antoninbas @jianjuns Could you help to check and confirm if this PR is ready for merge or not? for the captured packet info, I have discarded it in this PR, plan to rework it after it is merged. thanks.

jianjuns · 2021-05-12T04:07:07Z

The current design looks good to me. I am ok to add captured packet info in a separate PR.

antoninbas

LGTM
@mengdie-song do you think you could do another round of review?

mengdie-song

I am good to merge this.

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

tnqn · 2021-05-12T11:02:56Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+		// If it is, then the user created more than one traceflow in one second, which is not allowed.
+		tfName := ""
+		if srcLen == 0 {
+			tfName = "live-dst-" + dst + "-" + time.Now().Format(TIME_FORMAT_YYYYMMDD_HHMMSS)


nit:

tfName := "live-" if srcLen == 0 { tfName += "dst-" + dst } else if dstLen == 0 { tfName += "src-" + sourceName } else { tfName += sourceName + "-" + dst } tfName += "-" + time.Now().Format(TIME_FORMAT_YYYYMMDD_HHMMSS)

tnqn · 2021-05-12T11:06:19Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+			tfName = "live-" + sourceName + "-" + dst + "-" + time.Now().Format(TIME_FORMAT_YYYYMMDD_HHMMSS)
+		}
+		ctx := context.Background()
+		err = p.checkDuplicateTf(tfName, ctx, request)


Not introduced by this PR, but there is no point to check duplicate name before creating it and it doesn't really help prevent conflict as the same name resource can be created after the check is done.
If it's duplicate, creating it will return 409 Conflict, and the code can check the error and report duplication.

it will actually skip tf creation and return error immediately, so I think it's worthy to do the check instead of posting the data to backend to get conflict error.

Getting it and creating it are two API calls, how is it worthy when the single create API already does the same check?

There is no any lock mechanism to prevent another client from creating the resource with same name between this GET and POST call, the code still needs to handle the create error anyway.

ah, Ok, I got your point now, I will check and refine this part. thanks.

@tnqn I just updated the code, remove unnecessary check and leave createTfCR() to raise the error during creation.

tnqn · 2021-05-12T11:11:03Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

-			request.DashboardClient.SendAlert(request.Context(), request.ClientID, alert)
+		traceName, err := request.Payload.StringSlice(traceNameCol)
+		if err != nil || len(traceName) == 0 {
+			alertPrinter(request, invalidInputMsg+"failed to get graph name as",


tnqn · 2021-05-12T11:14:02Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+func checkDstType(request *service.ActionRequest) (string, error) {
+	dstType, err := request.Payload.StringSlice(dstTypeCol)
+	if err != nil || len(dstType) == 0 {
+		alertPrinter(request, invalidInputMsg+"failed to get dstType as string slice:",


I'm a little confused about the style of the log message: some messages have trailing ":" while some don't, some have a trailing space while some don't.
And all wrapped messages should not be capitalized like here, but they are capitalized in some places.

I refined the log print via a shared method but keep the original data, I will revisit those logs and make them consistent.

and for the second message, it's an alert message which will be printed in UI, I suppose it should be capitalized?

some of error messages are just printed directly due to errs and err are both nil, so no need to have trailing ":"

I refined the alertPrinter and remove the trailing ":" for most cases.

tnqn · 2021-05-12T11:23:56Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+func updateIPHeader(tf *crdv1alpha1.Traceflow, hasSrcPort bool, hasDstPort bool, srcPort uint16, dstPort uint16) {
+	switch tf.Spec.Packet.IPHeader.Protocol {
+	case crdv1alpha1.TCPProtocol:
+		{


not common to wrap the code with an extra brace, which causes unnecessary indent.

tnqn · 2021-05-12T11:27:35Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+	srcField := component.NewFormFieldText(srcCol, srcCol, "")
+	dropOnlyField := component.NewFormFieldSelect(dropOnlyCol+" (Only capture packets dropped by NetworkPolicies)", dropOnlyCol, dropOnlySelect, false)
+
+	livetfFields := []component.FormField{


Suggested change

livetfFields := []component.FormField{

liveTfFields := []component.FormField{

tnqn · 2021-05-12T11:28:52Z

plugins/octant/cmd/antrea-octant-plugin/traceflow.go

+	ctx := context.Background()
+	tfs, err := p.client.CrdV1alpha1().Traceflows().List(ctx, v1.ListOptions{ResourceVersion: "0"})
+	if err != nil {
+		log.Fatalf("Failed to get traceflows %v", err)


Suggested change

log.Fatalf("Failed to get traceflows %v", err)

log.Fatalf("Failed to get traceflows: %v\n", err)

tnqn · 2021-05-13T14:29:24Z

/test-all

tnqn

LGTM, thanks for addressing my comments

vmwclabot added the cla-not-required label Apr 25, 2021

luolanzone requested review from mengdie-song and removed request for mengdie-song April 25, 2021 09:34

luolanzone force-pushed the octant-livetf branch 3 times, most recently from 3c2ecb7 to 15182d5 Compare April 27, 2021 02:38

luolanzone requested review from mengdie-song and jianjuns April 27, 2021 02:38

luolanzone force-pushed the octant-livetf branch from 15182d5 to f37c5a8 Compare April 27, 2021 02:53

antoninbas reviewed Apr 27, 2021

View reviewed changes

luolanzone force-pushed the octant-livetf branch 3 times, most recently from 9c69bc0 to df7cb41 Compare April 27, 2021 04:28

mengdie-song reviewed Apr 27, 2021

View reviewed changes

jianjuns reviewed Apr 27, 2021

View reviewed changes

luolanzone force-pushed the octant-livetf branch from 9292ff6 to f3d86f2 Compare April 28, 2021 05:26

luolanzone force-pushed the octant-livetf branch from f3d86f2 to 83e9eb0 Compare April 29, 2021 10:40

jianjuns reviewed Apr 29, 2021

View reviewed changes

antoninbas reviewed Apr 29, 2021

View reviewed changes

luolanzone force-pushed the octant-livetf branch from d2b0fa4 to d9fba69 Compare May 3, 2021 13:19

antoninbas reviewed May 6, 2021

View reviewed changes

luolanzone force-pushed the octant-livetf branch 2 times, most recently from 027318f to 750c93d Compare May 10, 2021 05:54

antoninbas reviewed May 11, 2021

View reviewed changes

luolanzone force-pushed the octant-livetf branch 2 times, most recently from f68d305 to 2f9164e Compare May 11, 2021 01:17

antoninbas previously approved these changes May 12, 2021

View reviewed changes

mengdie-song previously approved these changes May 12, 2021

View reviewed changes

mengdie-song reviewed May 12, 2021

View reviewed changes

plugins/octant/cmd/antrea-octant-plugin/traceflow.go Show resolved Hide resolved

tnqn reviewed May 12, 2021

View reviewed changes

add livetraffic support in octant plugin

96b2ea0

luolanzone dismissed stale reviews from mengdie-song and antoninbas via 96b2ea0 May 13, 2021 02:14

luolanzone force-pushed the octant-livetf branch 2 times, most recently from 081cc15 to 3adce2c Compare May 13, 2021 06:26

remove unnecessary duplication check

5df0123

luolanzone force-pushed the octant-livetf branch from 3adce2c to 5df0123 Compare May 13, 2021 06:27

tnqn approved these changes May 13, 2021

View reviewed changes

tnqn merged commit 71e0fab into antrea-io:main May 13, 2021

luolanzone deleted the octant-livetf branch September 29, 2022 08:15

	// If it is, then the user creates more than one traceflows in one second, which is not allowed.
	// If it is, then the user created more than one traceflow in one second, which is not allowed.

	// For live traffic, when the source is IP or empty, there is no sender's Node result from traceflow status.
	// For live traffic, when the source is IP or empty, there is no result from the sender Node result in the traceflow status.

		// Add captured packet info to destination cluster.
		err = graph.AddNode(dstCluster.Name, "capturedPacket", map[string]string{

	livetfFields := []component.FormField{
	liveTfFields := []component.FormField{

	log.Fatalf("Failed to get traceflows %v", err)
	log.Fatalf("Failed to get traceflows: %v\n", err)

add livetraffic support in octant plugin #2124

add livetraffic support in octant plugin #2124

Conversation

luolanzone commented Apr 25, 2021 • edited Loading

codecov-commenter commented Apr 25, 2021 • edited Loading

Codecov Report

antoninbas left a comment

Choose a reason for hiding this comment

luolanzone commented Apr 27, 2021

antoninbas commented Apr 27, 2021

luolanzone commented Apr 27, 2021

jianjuns commented Apr 27, 2021 • edited Loading

luolanzone commented Apr 27, 2021

mengdie-song left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone Apr 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas commented Apr 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone commented Apr 28, 2021

luolanzone commented Apr 28, 2021

antoninbas commented Apr 28, 2021

jianjuns commented Apr 28, 2021

luolanzone commented Apr 29, 2021

jianjuns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns commented Apr 29, 2021

luolanzone commented May 3, 2021

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone commented May 10, 2021

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone commented May 11, 2021

luolanzone commented May 12, 2021

jianjuns commented May 12, 2021

antoninbas left a comment

Choose a reason for hiding this comment

mengdie-song left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn May 12, 2021 • edited Loading

Choose a reason for hiding this comment

luolanzone May 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone commented Apr 25, 2021 •

edited

Loading

codecov-commenter commented Apr 25, 2021 •

edited

Loading

jianjuns commented Apr 27, 2021 •

edited

Loading

luolanzone Apr 27, 2021 •

edited

Loading

tnqn May 12, 2021 •

edited

Loading

luolanzone May 13, 2021 •

edited

Loading