Support service destination in traceflow #979

gran-vmv · 2020-07-23T07:57:39Z

Result example:

    results:
    - node: gran-k8s0-1
      observations:
      - action: Forwarded
        component: SpoofGuard
      - action: Forwarded
        component: LB
        pod: kube-system/antrea-octant-7cb57d66bc-vtvmz
        translatedDstIP: 192.168.0.69
      - action: Forwarded
        component: NetworkPolicy
        componentInfo: EgressRule
        networkPolicy: default/allow-all
      - action: Forwarded
        component: Forwarding
        componentInfo: Output
        tunnelDstIP: 10.176.26.135
      timestamp: 1595490479

antrea-bot · 2020-07-23T07:57:53Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

antoninbas

As a general comment, make sure that traceflow error messages are "wrapped" appropriately to provide enough context.

pkg/agent/controller/traceflow/packetin.go

cmd/antrea-controller/controller.go

pkg/controller/traceflow/controller.go

pkg/agent/controller/traceflow/traceflow_controller.go

pkg/controller/traceflow/controller.go

antrea-bot · 2020-07-29T11:25:48Z

Can one of the admins verify this patch?

antrea-bot · 2020-07-30T06:43:32Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

pkg/agent/controller/traceflow/packetin.go

pkg/agent/controller/traceflow/traceflow_controller.go

jianjuns · 2020-08-01T04:57:08Z

pkg/controller/traceflow/controller.go

 			if ob.Component == opsv1alpha1.SpoofGuard {
 				sender = true
 			}
 			if ob.Action == opsv1alpha1.Delivered || ob.Action == opsv1alpha1.Dropped {
 				receiver = true
 			}
+			if ob.TranslatedDstIP != "" {


Humm.. Feel not certain if worthwhile to figure out the Pod from IP. Do you think good enough to use IP to indicate the destination here?

Yes, if a translated IP is a Pod IP, it can be retrieved here.
ob.TranslatedDstIP will exist only when packet is processed by DNAT.

I meant there will be costs to watch Pods and build IP -> Pod indexes. So, I feel maybe not worthwhile to translate IP back to Pods.

Pods will be always watched in AntreaController (by NetworkPolicyController), thus the only extra cost is the PodIP index. I feel an additional index is acceptable.

Could you share the memory overhead of the new index your saw?
BTW, I know for networkpolicy query, I remember we plan to add Pod IP index too, but still want to avoid obvious mem increase. We already saw some mem issues in scale tests.

If you can send me details of the specific case which worries you, I can add an additional test to networkpolicy_controller_perf_test/controller_query_perf_test to directly test memory consumption.

131.3M v.s. 137.7M sounds ok. But in our scale tests, we saw mem usage of ~10G with 150K Pods and 40K NetworkPolicies. Do you think in that case, the extra index will lead to Gs of more mem consumption?

I think the major different point is that I only added many Pods into cache and didn't touch NetworkPolicy. For NetworkPolicy, many additional elements will be cached such as rules and OVS flows.

The price of the extra PodIP index is a map like below will be created and maintained when adding/updating/deleting pods. But unlike the Pod index of AddressGroup and AppliedToGroup that could have large number of Pods, each Pod has only 1 IP, the extra overhead is constant.

{ "1.1.1.1": []string{"ns1/pod1"}, "1.1.1.2": []string{"ns1/pod2"}, }

BenchmarkAddPortWithPodIPIndex-4 252216 4207 ns/op 1608 B/op 17 allocs/op BenchmarkAddPortWithoutPodIPIndex-4 408417 3234 ns/op 1144 B/op 12 allocs/op

Given the difference is 3us vs 4us per pod, i.e. 300ms vs 400ms for 100k Pods, I think it should be ok.

Ok. But can we add some comments at where we create the IP-Pod index to explain why we add the index and we evaluated the overhead is acceptable?

Added comments to:
pkg/controller/traceflow/controller.go

// Add IP-Pod index. Each Pod has only 1 IP, the extra overhead is constant and acceptable. // @tnqn evaluated the performance without/with IP index is 3us vs 4us per pod, i.e. 300ms vs 400ms for 100k Pods. podInformer.Informer().AddIndexers(cache.Indexers{podIPIndex: podIPIndexFunc})

pkg/controller/traceflow/controller.go

tnqn · 2020-08-07T07:38:35Z

pkg/agent/controller/traceflow/traceflow_controller.go

 	}
-	if dstNodeIP != "" {
+	// Check encap status if Node of destination Pod is remote or destination is IP/Service
+	if dstNodeIP != "" || tf.Spec.Destination.Pod == "" {


Doesn't it need to wait for a while if the destination is service? what if the target pod is on another Node? so do IP on another Node case.

Yes. If destination is Service or IP/Pod not on current Node, then verify the tunnel/encap status and wait.
Change
if dstNodeIP != "" || tf.Spec.Destination.Pod == "" {
To
if dstMAC == "" {
And added comments.

pkg/agent/controller/traceflow/traceflow_controller.go

pkg/controller/traceflow/controller.go

antrea-bot · 2020-08-10T01:59:04Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

gran-vmv · 2020-08-10T02:01:14Z

/test-all

gran-vmv · 2020-08-10T02:34:53Z

/test-windows-networkpolicy

gran-vmv · 2020-08-10T02:38:36Z

/test-windows-conformance
/test-windows-networkpolicy

gran-vmv · 2020-08-10T02:56:55Z

/test-windows-networkpolicy

gran-vmv · 2020-08-10T05:25:15Z

/test-windows-networkpolicy

gran-vmv · 2020-08-10T06:44:58Z

/test-windows-networkpolicy

gran-vmv · 2020-08-10T07:25:36Z

/test-windows-networkpolicy

gran-vmv · 2020-08-10T09:31:07Z

/test-windows-networkpolicy

tnqn

LGTM

gran-vmv added this to the Antrea v0.9.0 release milestone Jul 23, 2020

gran-vmv requested review from antoninbas, wenyingd, tnqn, weiqiangt, lzhecheng and jianjuns July 23, 2020 07:57

vmwclabot added the cla-not-required label Jul 23, 2020

antoninbas reviewed Jul 27, 2020

View reviewed changes

gran-vmv force-pushed the tf-svc branch 4 times, most recently from 71c9187 to 025713f Compare July 29, 2020 09:13

gran-vmv force-pushed the tf-svc branch from 025713f to eae46c3 Compare July 30, 2020 06:43

gran-vmv force-pushed the tf-svc branch from eae46c3 to adcce44 Compare July 31, 2020 02:13

jianjuns reviewed Aug 1, 2020

View reviewed changes

gran-vmv force-pushed the tf-svc branch 5 times, most recently from 74215ac to faa088d Compare August 7, 2020 07:06

tnqn reviewed Aug 7, 2020

View reviewed changes

gran-vmv force-pushed the tf-svc branch from faa088d to af5d168 Compare August 7, 2020 08:19

Support service destination in traceflow

c7744d0

gran-vmv force-pushed the tf-svc branch from af5d168 to c7744d0 Compare August 10, 2020 01:58

tnqn approved these changes Aug 10, 2020

View reviewed changes

gran-vmv merged commit b0426e1 into antrea-io:master Aug 11, 2020

GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020

Support service destination in traceflow (antrea-io#979)

4964da9

gran-vmv deleted the tf-svc branch October 12, 2020 09:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support service destination in traceflow #979

Support service destination in traceflow #979

gran-vmv commented Jul 23, 2020 •

edited

Loading

antrea-bot commented Jul 23, 2020

antoninbas left a comment

antrea-bot commented Jul 29, 2020

antrea-bot commented Jul 30, 2020

jianjuns Aug 1, 2020

gran-vmv Aug 3, 2020

jianjuns Aug 3, 2020

gran-vmv Aug 4, 2020

jianjuns Aug 4, 2020

jakesokol1 Aug 4, 2020

gran-vmv Aug 5, 2020

tnqn Aug 7, 2020

jianjuns Aug 7, 2020 •

edited

Loading

gran-vmv Aug 10, 2020

tnqn Aug 7, 2020

gran-vmv Aug 7, 2020

antrea-bot commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

tnqn left a comment

Support service destination in traceflow #979

Support service destination in traceflow #979

Conversation

gran-vmv commented Jul 23, 2020 • edited Loading

antrea-bot commented Jul 23, 2020

antoninbas left a comment

Choose a reason for hiding this comment

antrea-bot commented Jul 29, 2020

antrea-bot commented Jul 30, 2020

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

jianjuns Aug 7, 2020 • 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

antrea-bot commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

gran-vmv commented Aug 10, 2020

tnqn left a comment

Choose a reason for hiding this comment

gran-vmv commented Jul 23, 2020 •

edited

Loading

jianjuns Aug 7, 2020 •

edited

Loading