From bb47272882e4a1c7a957886c2b21e1d8330eeca3 Mon Sep 17 00:00:00 2001 From: CK Date: Tue, 14 Feb 2023 12:54:39 -0600 Subject: [PATCH 1/9] update endpointcache --- npm/pkg/dataplane/dataplane.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 71218aaa4b..32dd949aee 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -284,15 +284,29 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { // because policy Manager will remove from policy from cache // keep a local copy to remove references for ipsets policy, ok := dp.policyMgr.GetPolicy(policyKey) + endpoints := policy.PodEndpoints if !ok { klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policyKey) return nil } + + dp.updatePodCache.Lock() + // Use the endpoint list saved in cache for this network policy to remove err := dp.policyMgr.RemovePolicy(policy.PolicyKey) if err != nil { return fmt.Errorf("[DataPlane] error while removing policy: %w", err) } + + for npmEndpoint := range endpoints { + // if the endpoint is not in the policy's endpoint list, delete from cache + if _, ok := policy.PodEndpoints[npmEndpoint]; !ok { + delete(dp.endpointCache.cache, npmEndpoint) + } + } + + dp.updatePodCache.Unlock() + // Remove references for Rule IPSets first err = dp.deleteIPSetsAndReferences(policy.RuleIPSets, policy.PolicyKey, ipsets.NetPolType) if err != nil { From faf3be01c2fc47e0562050b616f3dc1eac11c63f Mon Sep 17 00:00:00 2001 From: ck319 Date: Fri, 17 Feb 2023 12:25:21 -0600 Subject: [PATCH 2/9] update delete logic --- npm/pkg/dataplane/dataplane.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 32dd949aee..e1938be0a3 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -284,12 +284,16 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { // because policy Manager will remove from policy from cache // keep a local copy to remove references for ipsets policy, ok := dp.policyMgr.GetPolicy(policyKey) - endpoints := policy.PodEndpoints + endpoints := make(map[string]string, len(policy.PodEndpoints)) + + for podIP,endpointId := range policy.PodEndpoints { + endpoints[podIP] = endpointId + } + if !ok { klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policyKey) return nil } - dp.updatePodCache.Lock() // Use the endpoint list saved in cache for this network policy to remove @@ -298,10 +302,11 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { return fmt.Errorf("[DataPlane] error while removing policy: %w", err) } - for npmEndpoint := range endpoints { - // if the endpoint is not in the policy's endpoint list, delete from cache - if _, ok := policy.PodEndpoints[npmEndpoint]; !ok { - delete(dp.endpointCache.cache, npmEndpoint) + for podIP := range endpoints { + // if the endpoint is not in the policy's endpoint list, delete policy reference from cache + if _, ok := policy.PodEndpoints[podIP]; !ok { + endpoint := dp.endpointCache.cache[podIP] + delete(endpoint.netPolReference, policyKey) } } From df05f1adb531e60e48085c72e887595e50446a22 Mon Sep 17 00:00:00 2001 From: Cristina Kovacs Date: Fri, 17 Feb 2023 12:39:26 -0600 Subject: [PATCH 3/9] fix lint issue --- npm/pkg/dataplane/dataplane.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index e1938be0a3..95981f4a66 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -285,11 +285,11 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { // keep a local copy to remove references for ipsets policy, ok := dp.policyMgr.GetPolicy(policyKey) endpoints := make(map[string]string, len(policy.PodEndpoints)) - - for podIP,endpointId := range policy.PodEndpoints { + + for podIP, endpointId := range policy.PodEndpoints { endpoints[podIP] = endpointId - } - + } + if !ok { klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policyKey) return nil @@ -309,7 +309,7 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { delete(endpoint.netPolReference, policyKey) } } - + dp.updatePodCache.Unlock() // Remove references for Rule IPSets first From 69c5e0f661590b025981cd1f429f082f047e0333 Mon Sep 17 00:00:00 2001 From: ck319 Date: Fri, 17 Feb 2023 12:58:27 -0600 Subject: [PATCH 4/9] added check for endpoint in cache --- npm/pkg/dataplane/dataplane.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 95981f4a66..053b6e99f9 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -305,8 +305,10 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { for podIP := range endpoints { // if the endpoint is not in the policy's endpoint list, delete policy reference from cache if _, ok := policy.PodEndpoints[podIP]; !ok { - endpoint := dp.endpointCache.cache[podIP] - delete(endpoint.netPolReference, policyKey) + //check if the endpoint is in the cache + if endpoint, ok := dp.endpointCache.cache[podIP]; ok { + delete(endpoint.netPolReference, policyKey) + } } } From 4996e9d1f30bd99983083c865bbce88d14844d89 Mon Sep 17 00:00:00 2001 From: ck319 Date: Wed, 22 Feb 2023 15:36:58 -0600 Subject: [PATCH 5/9] add unit test --- .../dataplane-test-cases_windows_test.go | 59 +++++++++++++++++++ npm/pkg/dataplane/types_linux.go | 4 +- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index e403eb8dd1..8d17c0922e 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -511,6 +511,65 @@ func getAllSerialTests() []*SerialTestCase { }, }, }, + { + Description: "pod created to satisfy policy, then policy deleted, then pod relabeled to no longer satisfy policy, then policy re-created and pod relabeled to satisfy policy", + Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + // will apply dirty ipsets from CreatePod + UpdatePolicy(policyXBaseOnK1V1()), + DeletePolicyByObject(policyXBaseOnK1V1()), + UpdatePodLabels("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}, map[string]string{"k2": "v2"}), + ApplyDP(), + UpdatePolicy(policyXBaseOnK1V1()), + ApplyDP(), + UpdatePodLabels("x", "a", ip1, thisNode, map[string]string{"k2": "v2"}, map[string]string{"k1": "v1"}), + ApplyDP(), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), + dptestutils.SetPolicy(podK2Set), + dptestutils.SetPolicy(podK2V2Set), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + }, } } diff --git a/npm/pkg/dataplane/types_linux.go b/npm/pkg/dataplane/types_linux.go index ec0a7c019e..fbeeccace5 100644 --- a/npm/pkg/dataplane/types_linux.go +++ b/npm/pkg/dataplane/types_linux.go @@ -1,4 +1,6 @@ package dataplane // npmEndpoint holds info relevant for endpoints in windows -type npmEndpoint struct{} +type npmEndpoint struct{ + netPolReference: make(map[string]struct{}), +} From 2fc4be4c458a4c4954fb53649d548c3f5f8eecc2 Mon Sep 17 00:00:00 2001 From: Cristina Kovacs Date: Wed, 22 Feb 2023 15:42:58 -0600 Subject: [PATCH 6/9] fix type lint issue --- npm/pkg/dataplane/types_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/types_linux.go b/npm/pkg/dataplane/types_linux.go index fbeeccace5..6275283ab1 100644 --- a/npm/pkg/dataplane/types_linux.go +++ b/npm/pkg/dataplane/types_linux.go @@ -1,6 +1,6 @@ package dataplane // npmEndpoint holds info relevant for endpoints in windows -type npmEndpoint struct{ - netPolReference: make(map[string]struct{}), +type npmEndpoint struct { + netPolReference map[string]struct{} } From 1fa066d1960b911bb82665089c9c566a44bbce4e Mon Sep 17 00:00:00 2001 From: ck319 Date: Wed, 22 Feb 2023 15:54:15 -0600 Subject: [PATCH 7/9] fix naming lint error --- npm/pkg/dataplane/dataplane.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 053b6e99f9..b02e380adb 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -286,8 +286,8 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { policy, ok := dp.policyMgr.GetPolicy(policyKey) endpoints := make(map[string]string, len(policy.PodEndpoints)) - for podIP, endpointId := range policy.PodEndpoints { - endpoints[podIP] = endpointId + for podIP, endpointID := range policy.PodEndpoints { + endpoints[podIP] = endpointID } if !ok { @@ -305,7 +305,7 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { for podIP := range endpoints { // if the endpoint is not in the policy's endpoint list, delete policy reference from cache if _, ok := policy.PodEndpoints[podIP]; !ok { - //check if the endpoint is in the cache + // check if the endpoint is in the cache if endpoint, ok := dp.endpointCache.cache[podIP]; ok { delete(endpoint.netPolReference, policyKey) } From e5b02794e51739bc9f36e070aa73feb78636525c Mon Sep 17 00:00:00 2001 From: ck319 Date: Wed, 22 Feb 2023 17:09:27 -0600 Subject: [PATCH 8/9] updated endpoint cache lock --- npm/pkg/dataplane/dataplane.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index b02e380adb..e48aaeb687 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -294,7 +294,7 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policyKey) return nil } - dp.updatePodCache.Lock() + dp.endpointCache.Lock() // Use the endpoint list saved in cache for this network policy to remove err := dp.policyMgr.RemovePolicy(policy.PolicyKey) @@ -312,7 +312,7 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { } } - dp.updatePodCache.Unlock() + dp.endpointCache.Unlock() // Remove references for Rule IPSets first err = dp.deleteIPSetsAndReferences(policy.RuleIPSets, policy.PolicyKey, ipsets.NetPolType) From b65ba11ca4700e84727aaed332cfb4f3faf9d9e3 Mon Sep 17 00:00:00 2001 From: Cristina Kovacs Date: Thu, 23 Feb 2023 14:55:38 -0600 Subject: [PATCH 9/9] moved cache lock down and added check for windows --- npm/pkg/dataplane/dataplane.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index e48aaeb687..995b6a11ce 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -294,7 +294,6 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { klog.Infof("[DataPlane] Policy %s is not found. Might been deleted already", policyKey) return nil } - dp.endpointCache.Lock() // Use the endpoint list saved in cache for this network policy to remove err := dp.policyMgr.RemovePolicy(policy.PolicyKey) @@ -302,17 +301,23 @@ func (dp *DataPlane) RemovePolicy(policyKey string) error { return fmt.Errorf("[DataPlane] error while removing policy: %w", err) } - for podIP := range endpoints { - // if the endpoint is not in the policy's endpoint list, delete policy reference from cache - if _, ok := policy.PodEndpoints[podIP]; !ok { - // check if the endpoint is in the cache - if endpoint, ok := dp.endpointCache.cache[podIP]; ok { - delete(endpoint.netPolReference, policyKey) + if dp.shouldUpdatePod() { + + dp.endpointCache.Lock() + + for podIP := range endpoints { + // if the endpoint is not in the policy's endpoint list, delete policy reference from cache + if _, ok := policy.PodEndpoints[podIP]; !ok { + // check if the endpoint is in the cache + if endpoint, ok := dp.endpointCache.cache[podIP]; ok { + delete(endpoint.netPolReference, policyKey) + } } } - } - dp.endpointCache.Unlock() + dp.endpointCache.Unlock() + + } // Remove references for Rule IPSets first err = dp.deleteIPSetsAndReferences(policy.RuleIPSets, policy.PolicyKey, ipsets.NetPolType)