Skip to content

Commit

Permalink
address comments UPDATE change
Browse files Browse the repository at this point in the history
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
  • Loading branch information
qiyueyao committed Aug 8, 2022
1 parent 9cde14b commit 3493743
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 10 deletions.
2 changes: 0 additions & 2 deletions docs/antrea-network-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -659,8 +659,6 @@ for all NetworkPolicies in the Namespace. Packets of any connection that match
a NetworkPolicy rule will be logged with a reference to the NetworkPolicy name,
but packets dropped by the implicit "default drop" (not allowed by any NetworkPolicy)
will only be logged with consistent name `K8sNetworkPolicy` for reference.
Note that currently, Antrea only retrieves the logging Annotation once when adding
NetworkPolicies and in case of agent restart, it does not monitor Annotation updates.
The rules are logged in the following format:

```text
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/openflow/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func prepareTraceflowFlow(ctrl *gomock.Controller) *client {
c.bridge = ovsoftest.NewMockBridge(ctrl)

mFlow := ovsoftest.NewMockFlow(ctrl)
ctx := &conjMatchFlowContext{dropFlow: mFlow}
ctx := &conjMatchFlowContext{dropFlow: mFlow, dropFlowEnableLogging: false}
mFlow.EXPECT().FlowProtocol().Return(binding.Protocol("ip"))
mFlow.EXPECT().CopyToBuilder(priorityNormal+2, false).Return(EgressDefaultTable.ofTable.BuildFlow(priorityNormal + 2)).Times(1)
c.featureNetworkPolicy.globalConjMatchFlowCache["mockContext"] = ctx
Expand Down
25 changes: 19 additions & 6 deletions pkg/agent/openflow/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,8 @@ type conjMatchFlowContext struct {
// empty, and uninstalled when both two are empty. When the dropFlow is uninstalled from the switch, the
// conjMatchFlowContext is removed from the cache.
dropFlow binding.Flow
// dropFlowEnableLogging describes the logging requirement of the dropFlow.
dropFlowEnableLogging bool
}

// createOrUpdateConjunctiveMatchFlow creates or updates the conjunctive match flow with the latest actions. It returns
Expand Down Expand Up @@ -718,9 +720,10 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol
context, found = featureNetworkPolicy.globalConjMatchFlowCache[matcherKey]
if !found {
context = &conjMatchFlowContext{
conjunctiveMatch: match,
actions: make(map[uint32]*conjunctiveAction),
featureNetworkPolicy: featureNetworkPolicy,
conjunctiveMatch: match,
actions: make(map[uint32]*conjunctiveAction),
featureNetworkPolicy: featureNetworkPolicy,
dropFlowEnableLogging: enableLogging,
}
ctxType = insertion

Expand All @@ -731,6 +734,15 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol
changeType: insertion,
}
}
} else if context.dropFlowEnableLogging != enableLogging {
// Logging requirement of the rule has changed, modify default drop flow accordingly.
context.dropFlowEnableLogging = enableLogging
if c.dropTable != nil && context.dropFlow != nil {
dropFlow = &flowChange{
flow: context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs, enableLogging),
changeType: modification,
}
}
}

// Calculate the change on the conjMatchFlowContext.
Expand Down Expand Up @@ -1178,9 +1190,10 @@ func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match
context, found = f.globalConjMatchFlowCache[matcherKey]
if !found {
context = &conjMatchFlowContext{
conjunctiveMatch: match,
actions: make(map[uint32]*conjunctiveAction),
featureNetworkPolicy: f,
conjunctiveMatch: match,
actions: make(map[uint32]*conjunctiveAction),
featureNetworkPolicy: f,
dropFlowEnableLogging: enableLogging,
}
// Generate the default drop flow if dropTable is not nil.
if clause.dropTable != nil {
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/networkpolicy/clusternetworkpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,12 @@ func (n *NetworkPolicyController) updateNamespace(oldObj, curObj interface{}) {
n.reprocessCNP(cnp, false)
}
}
// TODO: Reprocess K8s NetworkPolicies when logging annotation has changed.
if oldNamespace.Annotations[EnableNPLoggingAnnotationKey] != curNamespace.Annotations[EnableNPLoggingAnnotationKey] {
affectedNPs, _ := n.networkPolicyLister.NetworkPolicies(curNamespace.Name).List(labels.Everything())
for _, np := range affectedNPs {
n.updateNetworkPolicy(np, np)
}
}
}

// deleteNamespace receives Namespace DELETE events and triggers all ClusterNetworkPolicies that have a
Expand Down

0 comments on commit 3493743

Please sign in to comment.