Added separate knobs to configure agent side networkPolicyController. #1753
Conversation
Knobs added: statusMgr, logging, and appliedToExternalEntity.
suwang48404
force-pushed
the
releae-0.12-patch-merge
branch
from
901d09b to
a245cc6
Compare
Codecov Report
@@ Coverage Diff @@
## master #1753 +/- ##
==========================================
- Coverage 63.31% 61.85% -1.47%
==========================================
Files 170 192 +22
Lines 14250 16377 +2127
==========================================
+ Hits 9023 10130 +1107
- Misses 4292 5184 +892
- Partials 935 1063 +128
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| memberSet.Insert(&group.GroupMembers[i]) | ||
| m := &group.GroupMembers[i] | ||
| if c.appliedToExternalEntity && m.Pod == nil && m.ExternalEntity != nil { | ||
| // Convert ExternalEntity to PodEntity so that rules applied to ExternalEntity. |
There was a problem hiding this comment.
Having separate knobs for Antrea-native policy features sounds fine by me. However, I think converting externalEntityReference to podReference here is too much a hack. If we know that ExternalEntities will be configured with container interfaces just like Pods, we could refactor GetContainerInterfacesByPod() method to be more generic and be applicable to EEs as well. The same goes for getPodOFPorts and getPodIPs. @jianjuns @tnqn
There was a problem hiding this comment.
I would agree what Yang said. We could revise the struct or add a new to support ExternalEntity.
There was a problem hiding this comment.
Agree with @Dyanngg, I think that's also why we replaced GroupMemberPodSet with GroupMemberSet in many places.
There was a problem hiding this comment.
appliedToExternalEntity is not really needed, right? If there's ExternalEntity in the group, I think there's no reason to ignore it?
There was a problem hiding this comment.
I took approach with the least change/resistence to allow appliedToExternalEntity supportability, as I am not very familiar with the implementation, and rather not to making sweeping changes.
In my view, if to address this properly, there are two approaches:
- Using ANP CRD ( from antrea manifest) as cue to support appliedToExternalEntity or not in antrea. On agent side we will support AppliedToExternalEntity based on AppliedToGroup content derived from ANP and received from controller.
- In addition to 1, to be explicit , so that user cannot accidentally change AppliedToExternalEntity supportability by change antrea manifest. An compilation time flag is needed on agent side to indicate supportability on agent regardless of AppliedToGroup content.
@jianjuns @tnqn @Dyanngg what do u think?
And if we do choose of the two above approaches, wonder if I get some help from u guys. It will definitely much efficient if @Dyanngg or @tnqn has a look?
There was a problem hiding this comment.
I am not sure if I got your #2, but definitely I would agree with your #1 that Agent should support ExternalEntity. But in your mind, if Agent does not really implements NetworkPolicy for ExternalEntity, what will be the interface it exposes to you? Probably we should discuss offline to understand your requirements.
The change is made so that networkPolicy controller may be reused to support ANP CRD but perhaps outside antrea CNI itself. |
|
superseded by #1775 |
Knobs added: statusMgr, logging, and appliedToExternalEntity.