Support for batch OVS flow updates #844

Dyanngg · 2020-06-17T00:32:07Z

Currently in Antrea, OVS flows for NetworkPolicies are installed asynchronously. For agent restart cases however, the initial NetworkPolicy events before the Bookmark (#704) can be processed in batch and be sent to OVS bridge in a single bundle. This PR implements this methodology.

antrea-bot · 2020-06-17T00:32:20Z

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-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-all: to trigger all tests.
/skip-all: to skip all tests.

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

tnqn

I only reviewed the commit of batch update, though not finished too.

pkg/agent/controller/networkpolicy/networkpolicy_controller.go

antrea-bot · 2020-06-19T21:09:03Z

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-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.

Dyanngg · 2020-07-08T20:51:59Z

/test-all

pkg/agent/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/controller/networkpolicy/reconciler.go

wenyingd · 2020-07-14T11:33:19Z

pkg/agent/openflow/network_policy.go

@@ -711,21 +711,41 @@ func (c *policyRuleConjunction) getAddressClause(addrType types.AddressType) *cl
 // If there is an error in any clause's addAddrFlows or addServiceFlows, the conjunction action flow will never be hit.
 // If the default drop flow is already installed before this error, all packets will be dropped by the default drop flow,
 // Otherwise all packets will be allowed.
-func (c *client) InstallPolicyRuleFlows(ruleID uint32, rule *types.PolicyRule, npName, npNamespace string) error {
+func (c *client) InstallPolicyRuleFlows(ofPolicyRule types.OFPolicyRule) error {


Is it better to flow the workflow of BatchInstallPolicyRuleFlows? I mean we don't need to implement two workflows.

I intentionally kept those two workflows separate. The reason is:

For InstallPolicyRuleFlows, the conjMatchFlowLock is needed to protect from concurrency issues. This lock is not needed for batch install because the rules will be processed in sequence.

For BatchInstallPolicyRuleFlows, for each rule it is processing, as soon as the context changes are calculated, ctxChange.updateContextStatus() needs to be called to ensure that when we process the next rule, the "conjuctive contexts" are up to date before the next round of context change calculation. I do not want to change the behavior of InstallPolicyRuleFlows as it would cause regression issues.

antrea-bot · 2020-07-14T22:25:27Z

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.

Dyanngg · 2020-07-14T22:27:17Z

/test-all

pkg/agent/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/controller/networkpolicy/reconciler.go

Dyanngg · 2020-07-16T22:24:08Z

/test-all

Dyanngg · 2020-07-16T23:59:27Z

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

Dyanngg · 2020-07-17T00:28:52Z

Looks like the clusterAPI for Jenkins e2e builds are failing @lzhecheng

=== Wait for workload cluster secret for 10 min ===
=== Get kubeconfig (try for 1m) ===
Error from server (NotFound): secrets "antrea-networkpolicy-for-pull-request-716-kubeconfig" not found
=== Get kubeconfig (try for 1m) ===
Error from server (NotFound): secrets "antrea-networkpolicy-for-pull-request-716-kubeconfig" not found
=== Get kubeconfig (try for 1m) ===
Error from server (NotFound): secrets "antrea-networkpolicy-for-pull-request-716-kubeconfig" not found
=== Get kubeconfig (try for 1m) ===

lzhecheng · 2020-07-17T02:25:33Z

@Dyanngg Yes, this may relate to ip allocation. Could you try again? It should work now.

This is an optimization for NetworkPolicy OVS flows to be collectively installed in agent restart case, so that those flows will rolled out at once and avoid potential priority re-assignments. In the future, mechanisms can also be added so that once this operation is done, NetworkPolicy flows from the previous round are deleted immediately after.

Dyanngg · 2020-07-29T00:08:37Z

/test-all

... as well as resolving comments

Dyanngg · 2020-07-29T00:33:48Z

/test-all

Dyanngg · 2020-07-29T03:20:49Z

/test-windows-networkpolicy

Dyanngg · 2020-07-29T04:07:36Z

/test-windows-networkpolicy

antrea-bot · 2020-07-29T11:26:36Z

Can one of the admins verify this patch?

tnqn

LGTM except two minor suggestions. If you apply the suggestions, feel free to merge once you get other reviewers' approval back.

pkg/agent/openflow/network_policy.go

Dyanngg · 2020-07-29T20:19:06Z

/test-all

Dyanngg · 2020-07-29T20:59:22Z

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

Dyanngg · 2020-07-29T21:35:20Z

/test-windows-networkpolicy

abhiraut · 2020-07-29T23:29:05Z

/test-windows-networkpolicy

abhiraut · 2020-07-30T01:28:43Z

looks like flaky test result from windows np ..

kubectl rollout status deployment/coredns -n kube-system
Waiting for deployment spec update to be observed...
Waiting for deployment "coredns" rollout to finish: 0 out of 2 new replicas have been updated...
Waiting for deployment "coredns" rollout to finish: 0 out of 2 new replicas have been updated...
Waiting for deployment "coredns" rollout to finish: 1 old replicas are pending termination...
error: deployment "coredns" exceeded its progress deadline
Build step 'Execute shell' marked build as failure

merging as it is unrelated

abhiraut · 2020-07-30T01:30:23Z

/cc @lzhecheng

lzhecheng · 2020-07-30T05:59:28Z

@abhiraut Thanks. There is something wrong on one testbed. I disconnected it and is investigating.

Support for batch OVS flow updates This is an optimization for NetworkPolicy OVS flows to be collectively installed in agent restart case, so that those flows will rolled out at once and avoid potential priority re-assignments. In the future, mechanisms can also be added so that once this operation is done, NetworkPolicy flows from the previous round are deleted immediately after.

vmwclabot added the cla-not-required label Jun 17, 2020

Dyanngg force-pushed the batch-flow branch from 6683d55 to 2b3d9a3 Compare June 17, 2020 06:04

tnqn reviewed Jun 17, 2020

View reviewed changes

Dyanngg force-pushed the batch-flow branch 4 times, most recently from 4c52c27 to ef7c13e Compare June 19, 2020 21:08

Dyanngg force-pushed the batch-flow branch 2 times, most recently from 5ed4ccb to c3b9e96 Compare June 19, 2020 21:37

Dyanngg force-pushed the batch-flow branch 4 times, most recently from ef9112b to 78243a2 Compare July 8, 2020 06:17

Dyanngg requested a review from tnqn July 8, 2020 19:08

Dyanngg changed the title ~~[WIP] Add support for batch OVS flow updates~~ Support for batch OVS flow updates Jul 8, 2020

wenyingd reviewed Jul 14, 2020

View reviewed changes

Dyanngg force-pushed the batch-flow branch from 78243a2 to 8576044 Compare July 14, 2020 22:25

Dyanngg requested a review from wenyingd July 15, 2020 22:06

wenyingd reviewed Jul 16, 2020

View reviewed changes

pkg/agent/controller/networkpolicy/networkpolicy_controller.go Outdated Show resolved Hide resolved

pkg/agent/controller/networkpolicy/networkpolicy_controller.go Outdated Show resolved Hide resolved

pkg/agent/controller/networkpolicy/reconciler.go Outdated Show resolved Hide resolved

Dyanngg force-pushed the batch-flow branch 2 times, most recently from 8f50c85 to 8df0b15 Compare July 16, 2020 22:21

Dyanngg dismissed stale reviews from antoninbas and abhiraut via 113954a July 29, 2020 00:05

Dyanngg force-pushed the batch-flow branch from fe94de6 to 113954a Compare July 29, 2020 00:05

Dyanngg requested review from antoninbas, tnqn and abhiraut July 29, 2020 00:08

Unify PolicyRule and OFPolicyRule struct

10942df

... as well as resolving comments

Dyanngg force-pushed the batch-flow branch from 113954a to 10942df Compare July 29, 2020 00:33

tnqn previously approved these changes Jul 29, 2020

View reviewed changes

pkg/agent/openflow/network_policy.go Outdated Show resolved Hide resolved

pkg/agent/openflow/network_policy.go Outdated Show resolved Hide resolved

Addressing remaining comments for network_policy.go

b0a9108

Dyanngg dismissed tnqn’s stale review via b0a9108 July 29, 2020 20:18

abhiraut approved these changes Jul 29, 2020

View reviewed changes

Dyanngg requested a review from tnqn July 30, 2020 01:10

antoninbas approved these changes Jul 30, 2020

View reviewed changes

abhiraut merged commit d214b52 into antrea-io:master Jul 30, 2020

Dyanngg deleted the batch-flow branch September 30, 2020 23:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for batch OVS flow updates #844

Support for batch OVS flow updates #844

Dyanngg commented Jun 17, 2020 •

edited

Loading

antrea-bot commented Jun 17, 2020

tnqn left a comment

antrea-bot commented Jun 19, 2020

Dyanngg commented Jul 8, 2020

wenyingd Jul 14, 2020

Dyanngg Jul 15, 2020 •

edited

Loading

antrea-bot commented Jul 14, 2020

Dyanngg commented Jul 14, 2020

Dyanngg commented Jul 16, 2020

Dyanngg commented Jul 16, 2020

Dyanngg commented Jul 17, 2020

lzhecheng commented Jul 17, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

antrea-bot commented Jul 29, 2020

tnqn left a comment

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

abhiraut commented Jul 29, 2020

abhiraut commented Jul 30, 2020

abhiraut commented Jul 30, 2020

lzhecheng commented Jul 30, 2020

Support for batch OVS flow updates #844

Support for batch OVS flow updates #844

Conversation

Dyanngg commented Jun 17, 2020 • edited Loading

antrea-bot commented Jun 17, 2020

tnqn left a comment

Choose a reason for hiding this comment

antrea-bot commented Jun 19, 2020

Dyanngg commented Jul 8, 2020

wenyingd Jul 14, 2020

Choose a reason for hiding this comment

Dyanngg Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

antrea-bot commented Jul 14, 2020

Dyanngg commented Jul 14, 2020

Dyanngg commented Jul 16, 2020

Dyanngg commented Jul 16, 2020

Dyanngg commented Jul 17, 2020

lzhecheng commented Jul 17, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

antrea-bot commented Jul 29, 2020

tnqn left a comment

Choose a reason for hiding this comment

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

Dyanngg commented Jul 29, 2020

abhiraut commented Jul 29, 2020

abhiraut commented Jul 30, 2020

abhiraut commented Jul 30, 2020

lzhecheng commented Jul 30, 2020

Dyanngg commented Jun 17, 2020 •

edited

Loading

Dyanngg Jul 15, 2020 •

edited

Loading