[ANP] ExternalEntity support for agent #1056

Dyanngg · 2020-08-07T23:39:43Z

This PR is rebased from and replaces #782.
It moves ToAddresses/FromAddresses in CompletedRule and AddressSetByGroup in ruleCache to use GroupMemberSet instead of GroupMmeberPodSet. Thus both Pods and ExternalEntities are expressed as GroupMember when in these fields.
Pods in appliedTo field continue be expressed by existing GroupMemberPod, and migration to GroupMember shall
be done in a subsequent PR.

antrea-bot · 2020-08-07T23:39:59Z

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.

antoninbas

LGTM, but someone else should review

pkg/agent/controller/networkpolicy/cache.go

suwang48404 · 2020-08-08T00:21:22Z

@abhiraut @tnqn @jianjuns can u guys please help review

Thx

antrea-bot · 2020-08-10T21:46:37Z

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

This commit moves ToAddresses/FromAddresses in CompletedRule and AddressSetByGroup in ruleCache to use GroupMemberSet instead of GroupMmeberPodSet. Thus both Pods and ExternalEntities are expressed as GroupMember when in these fields. Pods in appliedTo field continue be expressed by existing GroupMemberPod, and migration to GroupMember shall be done in a subsequent PR.

suwang48404 · 2020-08-11T16:34:24Z

Hi @abhiraut @tnqn @jianjuns,

ping ?

jianjuns · 2020-08-11T20:46:13Z

pkg/agent/controller/networkpolicy/cache.go

@@ -417,16 +432,23 @@ func (c *ruleCache) PatchAddressGroup(patch *v1beta1.AddressGroupPatch) error {
 	c.addressSetLock.Lock()
 	defer c.addressSetLock.Unlock()

-	podSet, exists := c.addressSetByGroup[patch.Name]
+	groupMemberSet, exists := c.addressSetByGroup[patch.Name]
 	if !exists {
 		return fmt.Errorf("AddressGroup %v doesn't exist in cache, can't be patched", patch.Name)
 	}
 	for i := range patch.AddedPods {


Question - we still keep AddedPods/RemovedPods, after add AddedGroupMembers/RemovedGroupMembers?

The plan is to deprecate GroupMemberPod and GroupMemberPodSet in the future, so those funcs will be unified.

But as of now when controller will send AddedPods/RemovedPod, when AddedGroupMembers/RemovedGroupMembers?

jianjuns · 2020-08-11T20:52:36Z

pkg/agent/controller/networkpolicy/cache.go

+	// Source GroupMembers of this rule, can't coexist with ToAddresses.
+	FromAddresses v1beta1.GroupMemberSet
+	// Destination GroupMembers of this rule, can't coexist with FromAddresses.
+	ToAddresses v1beta1.GroupMemberSet
 	// Target Pods of this rule.
 	Pods v1beta1.GroupMemberPodSet


Why target cannot be external endpoints?

I believe the reason is that for the externalEntities in the appliedTo, there's nothing to be enforced from the Antrea agent side. The antrea-cloud controller will enforce firewall rules on the cloud provider instead.

Ok. I thought we will support agented model together.

would let @suwang48404 comment on that

If we do not support agented model for external Nodes, then we need not any code in agent to handle ExternalEndpoints/Nodes (I think you still have such code?)?

yes.
use case is that use ANP to control Pods' access to services on cloud (VMs or cloud Native Services). This was part of antrea+ demo.

Ok. Then for Antrea perspective, it is more about supporting externalEntity in from/to. Still do not feel it is very useful alone. If am ok to make it the first step, but we should either have a way to validate externalEntity cannot be in appliedTo, or have a featureGate to disable the whole externalEntity support?
Let us talk offline.

Hi @jianjuns

@Dyanngg has kindly taking over antrea externalEntity support. If u have any concrete suggestions, he will help out.

Regarding to ur suggestion about " ... either have a way to validate externalEntity cannot be in appliedTo, or ...", if I recall correctly, if user specifies ExternalEntity in appliedTo field of an ANP

controller will not forward the AppliedToGroup (contain externalEntity) to agent

even if controller (via bug) forward AppliedToGroup( contain externalEntity) to agent, agent ignores externalEntity in AppliedToGroup.

I hope this satisfies ur requirements.

I would say it is very confusing to users. We should document we do not support appliedTo and validate it, or disable the externalEntity feature until we really support agent on external Nodes.

I was chatting with @abhiraut on this. We also feel namespaced externalEntity does not make much sense from Antrea perspective. But anyway we might start from what we have now, but at least we should add some validation no externalEntity in appliedTo.

Dyanngg · 2020-08-26T23:32:19Z

Superseded by #1084

vmwclabot added the cla-not-required label Aug 7, 2020

Dyanngg changed the title ~~Agent supports ExternalEntity.~~ [ANP] ExternalEntity support for agent Aug 7, 2020

Dyanngg requested review from tnqn, abhiraut, jianjuns and suwang48404 and removed request for tnqn August 7, 2020 23:40

antoninbas reviewed Aug 8, 2020

View reviewed changes

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

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

Dyanngg force-pushed the agent-ee branch 2 times, most recently from cb2e24e to 58468f4 Compare August 10, 2020 21:46

Dyanngg force-pushed the agent-ee branch from 58468f4 to aaaf100 Compare August 10, 2020 22:05

suwang48404 approved these changes Aug 11, 2020

View reviewed changes

jianjuns reviewed Aug 11, 2020

View reviewed changes

Dyanngg closed this Aug 26, 2020

Dyanngg deleted the agent-ee branch September 30, 2020 23:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ANP] ExternalEntity support for agent #1056

[ANP] ExternalEntity support for agent #1056

Dyanngg commented Aug 7, 2020

antrea-bot commented Aug 7, 2020

antoninbas left a comment

suwang48404 commented Aug 8, 2020

antrea-bot commented Aug 10, 2020

suwang48404 commented Aug 11, 2020

jianjuns Aug 11, 2020

Dyanngg Aug 11, 2020

jianjuns Aug 11, 2020

jianjuns Aug 11, 2020

Dyanngg Aug 11, 2020

jianjuns Aug 11, 2020

Dyanngg Aug 11, 2020

jianjuns Aug 11, 2020

suwang48404 Aug 11, 2020 •

edited

Loading

jianjuns Aug 11, 2020

suwang48404 Aug 11, 2020

jianjuns Aug 11, 2020

jianjuns Aug 17, 2020

Dyanngg commented Aug 26, 2020

[ANP] ExternalEntity support for agent #1056

[ANP] ExternalEntity support for agent #1056

Conversation

Dyanngg commented Aug 7, 2020

antrea-bot commented Aug 7, 2020

antoninbas left a comment

Choose a reason for hiding this comment

suwang48404 commented Aug 8, 2020

antrea-bot commented Aug 10, 2020

suwang48404 commented Aug 11, 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

suwang48404 Aug 11, 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

Choose a reason for hiding this comment

Dyanngg commented Aug 26, 2020

suwang48404 Aug 11, 2020 •

edited

Loading