From e141679c69552d9132bcaaa7cf3a5f2401b30026 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 28 Oct 2021 14:01:31 -0700 Subject: [PATCH 01/15] [NPM] Expanding HNS fake usage with internal state --- network/hnswrapper/hnsv2wrapperfake.go | 162 ++++++++++++++++++++++--- 1 file changed, 148 insertions(+), 14 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index a368eb3305..f8a72872f4 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -7,13 +7,37 @@ package hnswrapper import ( + "encoding/json" + "reflect" + "sync" + "github.com/Microsoft/hcsshim/hcn" ) +type fakeHNSCache struct { + networks map[string]hcn.HostComputeNetwork + endpoints map[string]hcn.HostComputeEndpoint +} + type Hnsv2wrapperFake struct { + cache fakeHNSCache + sync.Mutex +} + +func NewHnsv2wrapperFake() *Hnsv2wrapperFake { + return &Hnsv2wrapperFake{ + cache: fakeHNSCache{ + networks: map[string]hcn.HostComputeNetwork{}, + endpoints: map[string]hcn.HostComputeEndpoint{}, + }, + } } func (f Hnsv2wrapperFake) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.HostComputeNetwork, error) { + f.Lock() + defer f.Unlock() + + f.cache.networks[network.Name] = *network return network, nil } @@ -21,7 +45,54 @@ func (f Hnsv2wrapperFake) DeleteNetwork(network *hcn.HostComputeNetwork) error { return nil } -func (Hnsv2wrapperFake) ModifyNetworkSettings(network *hcn.HostComputeNetwork, request *hcn.ModifyNetworkSettingRequest) error { +func (f Hnsv2wrapperFake) ModifyNetworkSettings(network *hcn.HostComputeNetwork, request *hcn.ModifyNetworkSettingRequest) error { + f.Lock() + defer f.Unlock() + switch request.RequestType { + case hcn.RequestTypeAdd: + var setPolSettings []hcn.NetworkPolicy + err := json.Unmarshal(request.Settings, &setPolSettings) + if err != nil { + return err + } + for _, setPolSetting := range setPolSettings { + if setPolSetting.Type == hcn.SetPolicy { + network.Policies = append(network.Policies, setPolSetting) + } + } + case hcn.RequestTypeRemove: + newtempPolicies := network.Policies + var setPolSettings []hcn.NetworkPolicy + err := json.Unmarshal(request.Settings, &setPolSettings) + if err != nil { + return err + } + for i, policy := range network.Policies { + for _, newPolicy := range setPolSettings { + if policy.Type != newPolicy.Type { + continue + } + if reflect.DeepEqual(policy.Settings, newPolicy.Settings) { + newtempPolicies = append(newtempPolicies[:i], newtempPolicies[i+1:]...) + break + } + } + } + network.Policies = newtempPolicies + case hcn.RequestTypeUpdate: + network.Policies = []hcn.NetworkPolicy{} + var setPolSettings []hcn.NetworkPolicy + err := json.Unmarshal(request.Settings, &setPolSettings) + if err != nil { + return err + } + for _, setPolSetting := range setPolSettings { + if setPolSetting.Type == hcn.SetPolicy { + network.Policies = append(network.Policies, setPolSetting) + } + } + } + return nil } @@ -33,25 +104,46 @@ func (Hnsv2wrapperFake) RemoveNetworkPolicy(network *hcn.HostComputeNetwork, net return nil } -func (Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) { +func (f Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) { + f.Lock() + defer f.Unlock() + if network, ok := f.cache.networks[networkName]; ok { + return &network, nil + } return &hcn.HostComputeNetwork{}, nil } func (f Hnsv2wrapperFake) GetNetworkByID(networkID string) (*hcn.HostComputeNetwork, error) { - network := &hcn.HostComputeNetwork{Id: networkID} - return network, nil + f.Lock() + defer f.Unlock() + for _, network := range f.cache.networks { + if network.Id == networkID { + return &network, nil + } + } + return &hcn.HostComputeNetwork{}, nil } func (f Hnsv2wrapperFake) GetEndpointByID(endpointID string) (*hcn.HostComputeEndpoint, error) { - endpoint := &hcn.HostComputeEndpoint{Id: endpointID} - return endpoint, nil -} - -func (Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { + f.Lock() + defer f.Unlock() + if ep, ok := f.cache.endpoints[endpointID]; ok { + return &ep, nil + } + return &hcn.HostComputeEndpoint{}, nil +} + +func (f Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { + f.Lock() + defer f.Unlock() + f.cache.endpoints[endpoint.Id] = *endpoint return endpoint, nil } -func (Hnsv2wrapperFake) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error { +func (f Hnsv2wrapperFake) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error { + f.Lock() + defer f.Unlock() + delete(f.cache.endpoints, endpoint.Id) return nil } @@ -68,10 +160,52 @@ func (Hnsv2wrapperFake) RemoveNamespaceEndpoint(namespaceId string, endpointId s return nil } -func (Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostComputeEndpoint, error) { - return []hcn.HostComputeEndpoint{}, nil -} +func (f Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostComputeEndpoint, error) { + f.Lock() + defer f.Unlock() + endpoints := make([]hcn.HostComputeEndpoint, 0) + for _, endpoint := range f.cache.endpoints { + if endpoint.HostComputeNetwork == networkId { + endpoints = append(endpoints, endpoint) + } + } + return endpoints, nil +} + +func (f Hnsv2wrapperFake) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { + f.Lock() + defer f.Unlock() + switch requestType { + case hcn.RequestTypeAdd: + for _, newPolicy := range endpointPolicy.Policies { + if newPolicy.Type != hcn.ACL { + continue + } + endpoint.Policies = append(endpoint.Policies, newPolicy) + } + case hcn.RequestTypeRemove: + newtempPolicies := endpoint.Policies + for i, policy := range endpoint.Policies { + for _, newPolicy := range endpointPolicy.Policies { + if policy.Type != newPolicy.Type { + continue + } + if reflect.DeepEqual(policy.Settings, newPolicy.Settings) { + newtempPolicies = append(newtempPolicies[:i], newtempPolicies[i+1:]...) + break + } + } + } + endpoint.Policies = newtempPolicies + case hcn.RequestTypeUpdate: + endpoint.Policies = make([]hcn.EndpointPolicy, 0) + for _, newPolicy := range endpointPolicy.Policies { + if newPolicy.Type != hcn.ACL { + continue + } + endpoint.Policies = append(endpoint.Policies, newPolicy) + } + } -func (Hnsv2wrapperFake) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { return nil } From 8901c9167bda9fab833198ed70a54edb7b97a046 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 28 Oct 2021 17:01:12 -0700 Subject: [PATCH 02/15] adding some test cases for windows DP --- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 39 +++++++++++++++++-- .../ipsets/ipsetmanager_windows_test.go | 38 ++++++++++++++++++ npm/pkg/dataplane/ipsets/testutils.go | 18 ++++++++- npm/pkg/dataplane/policies/policymanager.go | 16 ++++++++ .../policies/policymanager_windows_test.go | 1 + 5 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go create mode 100644 npm/pkg/dataplane/policies/policymanager_windows_test.go diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 50108c4fc5..9954f7ba14 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -19,10 +19,17 @@ const ( testPodIP = "10.0.0.0" ) -var iMgrApplyOnNeedCfg = &IPSetManagerCfg{ - IPSetMode: ApplyOnNeed, - NetworkName: "azure", -} +var ( + iMgrApplyOnNeedCfg = &IPSetManagerCfg{ + IPSetMode: ApplyOnNeed, + NetworkName: "azure", + } + + iMgrApplyAlwaysCfg = &IPSetManagerCfg{ + IPSetMode: ApplyAllIPSets, + NetworkName: "azure", + } +) func TestCreateIPSet(t *testing.T) { iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) @@ -38,6 +45,9 @@ func TestCreateIPSet(t *testing.T) { require.NotNil(t, set) assert.Equal(t, setMetadata.GetPrefixName(), set.Name) assert.Equal(t, util.GetHashedName(setMetadata.GetPrefixName()), set.HashedName) + + err := iMgr.applyIPSets() + require.NoError(t, err) } func TestAddToSet(t *testing.T) { @@ -60,6 +70,9 @@ func TestAddToSet(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{listMetadata}) err = iMgr.AddToSets([]*IPSetMetadata{listMetadata}, testPodIP, testPodKey) require.Error(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestRemoveFromSet(t *testing.T) { @@ -71,6 +84,9 @@ func TestRemoveFromSet(t *testing.T) { require.NoError(t, err) err = iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestRemoveFromSetMissing(t *testing.T) { @@ -78,6 +94,9 @@ func TestRemoveFromSetMissing(t *testing.T) { setMetadata := NewIPSetMetadata(testSetName, Namespace) err := iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestAddToListMissing(t *testing.T) { @@ -86,6 +105,9 @@ func TestAddToListMissing(t *testing.T) { listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNamespace) err := iMgr.AddToLists([]*IPSetMetadata{listMetadata}, []*IPSetMetadata{setMetadata}) require.NoError(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestAddToList(t *testing.T) { @@ -103,6 +125,9 @@ func TestAddToList(t *testing.T) { assert.Equal(t, util.GetHashedName(listMetadata.GetPrefixName()), set.HashedName) assert.Equal(t, 1, len(set.MemberIPSets)) assert.Equal(t, setMetadata.GetPrefixName(), set.MemberIPSets[setMetadata.GetPrefixName()].Name) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestRemoveFromList(t *testing.T) { @@ -127,6 +152,9 @@ func TestRemoveFromList(t *testing.T) { set = iMgr.GetIPSet(listMetadata.GetPrefixName()) assert.NotNil(t, set) assert.Equal(t, 0, len(set.MemberIPSets)) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestRemoveFromListMissing(t *testing.T) { @@ -138,6 +166,9 @@ func TestRemoveFromListMissing(t *testing.T) { err := iMgr.RemoveFromList(listMetadata, []*IPSetMetadata{setMetadata}) require.NoError(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) } func TestDeleteIPSet(t *testing.T) { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go new file mode 100644 index 0000000000..7263b2e133 --- /dev/null +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -0,0 +1,38 @@ +package ipsets + +import ( + "testing" + + "github.com/Azure/azure-container-networking/common" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/stretchr/testify/require" +) + +var ( + hns = GetHNSFake() +) + +func TestAddToSetWindows(t *testing.T) { + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns)) + + setMetadata := NewIPSetMetadata(testSetName, Namespace) + iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) + + err := iMgr.AddToSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) + require.NoError(t, err) + + err = iMgr.AddToSets([]*IPSetMetadata{setMetadata}, "2001:db8:0:0:0:0:2:1", "newpod") + require.NoError(t, err) + + // same IP changed podkey + err = iMgr.AddToSets([]*IPSetMetadata{setMetadata}, testPodIP, "newpod") + require.NoError(t, err) + + listMetadata := NewIPSetMetadata("testipsetlist", KeyLabelOfNamespace) + iMgr.CreateIPSets([]*IPSetMetadata{listMetadata}) + err = iMgr.AddToSets([]*IPSetMetadata{listMetadata}, testPodIP, testPodKey) + require.Error(t, err) + + err = iMgr.applyIPSets() + require.NoError(t, err) +} diff --git a/npm/pkg/dataplane/ipsets/testutils.go b/npm/pkg/dataplane/ipsets/testutils.go index 0224b0c2f2..ab3245a4f6 100644 --- a/npm/pkg/dataplane/ipsets/testutils.go +++ b/npm/pkg/dataplane/ipsets/testutils.go @@ -1,6 +1,10 @@ package ipsets -import "github.com/Azure/azure-container-networking/npm/util" +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/util" + "github.com/Microsoft/hcsshim/hcn" +) type TestSet struct { Metadata *IPSetMetadata @@ -20,6 +24,18 @@ func CreateTestSet(name string, setType SetType) *TestSet { return set } +func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { + hns := hnswrapper.NewHnsv2wrapperFake() + network := &hcn.HostComputeNetwork{ + Id: "1234", + Name: "azure", + } + + hns.CreateNetwork(network) + + return hns +} + var ( TestNSSet = CreateTestSet("test-ns-set", Namespace) TestKeyPodSet = CreateTestSet("test-keyPod-set", KeyLabelOfPod) diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 87ad8c38db..0f1fd39c63 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -8,6 +8,17 @@ import ( "k8s.io/klog" ) +// PolicyManagerMode will be used in windows to decide if +// SetPolicies should be used or not +type PolicyManagerMode string + +const ( + // IPSetPolicyMode will references IPSets in policies + IPSetPolicyMode PolicyManagerMode = "IPSet" + // IPPolicyMode will replace ipset names with their value IPs in policies + IPPolicyMode PolicyManagerMode = "IP" +) + type PolicyMap struct { cache map[string]*NPMNetworkPolicy } @@ -15,6 +26,7 @@ type PolicyMap struct { type PolicyManager struct { policyMap *PolicyMap ioShim *common.IOShim + *PolicyManagerCfg } func NewPolicyManager(ioShim *common.IOShim) *PolicyManager { @@ -33,6 +45,10 @@ func (pMgr *PolicyManager) Initialize() error { return nil } +type PolicyManagerCfg struct { + Mode PolicyManagerMode +} + func (pMgr *PolicyManager) Reset() error { if err := pMgr.reset(); err != nil { return npmerrors.ErrorWrapper(npmerrors.ResetPolicyMgr, false, "failed to reset policy manager", err) diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go new file mode 100644 index 0000000000..a01ffa69c5 --- /dev/null +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -0,0 +1 @@ +package policies From b613dda90369e6243ab11f9932c49a0293c1e5bd Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 28 Oct 2021 17:01:35 -0700 Subject: [PATCH 03/15] adding some test cases for windows DP --- common/ioshim_windows.go | 7 +++++++ network/hnswrapper/hnsv2wrapperfake.go | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/common/ioshim_windows.go b/common/ioshim_windows.go index 323ffff20a..68f841f4b8 100644 --- a/common/ioshim_windows.go +++ b/common/ioshim_windows.go @@ -24,3 +24,10 @@ func NewMockIOShim(calls []testutils.TestCmd) *IOShim { Hns: &hnswrapper.Hnsv2wrapperFake{}, } } + +func NewMockIOShimWithFakeHNS(calls []testutils.TestCmd, hns *hnswrapper.Hnsv2wrapperFake) *IOShim { + return &IOShim{ + Exec: testutils.GetFakeExecWithScripts(calls), + Hns: hns, + } +} diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index f8a72872f4..7dba73be68 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -50,25 +50,25 @@ func (f Hnsv2wrapperFake) ModifyNetworkSettings(network *hcn.HostComputeNetwork, defer f.Unlock() switch request.RequestType { case hcn.RequestTypeAdd: - var setPolSettings []hcn.NetworkPolicy + var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { return err } - for _, setPolSetting := range setPolSettings { + for _, setPolSetting := range setPolSettings.Policies { if setPolSetting.Type == hcn.SetPolicy { network.Policies = append(network.Policies, setPolSetting) } } case hcn.RequestTypeRemove: newtempPolicies := network.Policies - var setPolSettings []hcn.NetworkPolicy + var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { return err } for i, policy := range network.Policies { - for _, newPolicy := range setPolSettings { + for _, newPolicy := range setPolSettings.Policies { if policy.Type != newPolicy.Type { continue } @@ -81,12 +81,12 @@ func (f Hnsv2wrapperFake) ModifyNetworkSettings(network *hcn.HostComputeNetwork, network.Policies = newtempPolicies case hcn.RequestTypeUpdate: network.Policies = []hcn.NetworkPolicy{} - var setPolSettings []hcn.NetworkPolicy + var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { return err } - for _, setPolSetting := range setPolSettings { + for _, setPolSetting := range setPolSettings.Policies { if setPolSetting.Type == hcn.SetPolicy { network.Policies = append(network.Policies, setPolSetting) } From 1185dc27c1cfccb3e29db485c4c5b69678864863 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 11 Nov 2021 16:27:38 -0800 Subject: [PATCH 04/15] Correcting some issues with windows DP --- .../dataplane/ipsets/ipsetmanager_windows.go | 4 +- .../policies/policymanager_windows.go | 28 +++++----- .../policies/policymanager_windows_test.go | 23 ++++++++ test/integration/npm/main.go | 53 +++++++++++++------ 4 files changed, 76 insertions(+), 32 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 19d36ec371..6c71ba767b 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -245,9 +245,9 @@ func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicyS } idx := 0 - policySettingsOrder := []hcn.SetPolicyType{SetPolicyTypeNestedIPSet, hcn.SetPolicyTypeIpSet} + policySettingsOrder := []hcn.SetPolicyType{hcn.SetPolicyTypeIpSet, SetPolicyTypeNestedIPSet} if operation == hcn.RequestTypeRemove { - policySettingsOrder = []hcn.SetPolicyType{hcn.SetPolicyTypeIpSet, SetPolicyTypeNestedIPSet} + policySettingsOrder = []hcn.SetPolicyType{SetPolicyTypeNestedIPSet, hcn.SetPolicyTypeIpSet} } for _, policyType := range policySettingsOrder { for setPol := range setPolicySettings { diff --git a/npm/pkg/dataplane/policies/policymanager_windows.go b/npm/pkg/dataplane/policies/policymanager_windows.go index 6a56e2d6e3..d6a2d334c9 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows.go +++ b/npm/pkg/dataplane/policies/policymanager_windows.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "strings" "github.com/Microsoft/hcsshim/hcn" "k8s.io/klog" @@ -126,6 +125,8 @@ func (pMgr *PolicyManager) removePolicy(policy *NPMNetworkPolicy, endpointList m } epBuilder.compareAndRemovePolicies(rulesToRemove[0].Id, len(rulesToRemove)) + klog.Infof("[DataPlanewindows] Epbuilder ACL policies before removing %+v", epBuilder.aclPolicies) + klog.Infof("[DataPlanewindows] Epbuilder Other policies before removing %+v", epBuilder.otherPolicies) epPolicies, err := epBuilder.getHCNPolicyRequest() if err != nil { aggregateErr = fmt.Errorf("[DataPlanewindows] Skipping removing policies on %s ID Endpoint with %s err\n Previous %w", epID, err.Error(), aggregateErr) @@ -256,15 +257,18 @@ func (epBuilder *endpointPolicyBuilder) compareAndRemovePolicies(ruleIDToRemove // All ACl policies in a given Netpol will have the same ID // starting with "azure-acl-" prefix aclFound := false + toDeleteIndexes := map[int]struct{}{} for i, acl := range epBuilder.aclPolicies { // First check if ID is present and equal, this saves compute cycles to compare both objects if ruleIDToRemove == acl.Id { // Remove the ACL policy from the list - epBuilder.removeACLPolicyAtIndex(i) + klog.Infof("[DataPlane Windows] Found ACL with ID %s and removing it", acl.Id) + toDeleteIndexes[i] = struct{}{} lenOfRulesToRemove-- aclFound = true } } + epBuilder.removeACLPolicyAtIndex(toDeleteIndexes) // If ACl Policies are not found, it means that we might have removed them earlier // or never applied them if !aclFound { @@ -279,19 +283,15 @@ func (epBuilder *endpointPolicyBuilder) compareAndRemovePolicies(ruleIDToRemove } func (epBuilder *endpointPolicyBuilder) resetAllNPMAclPolicies() { - for i, acl := range epBuilder.aclPolicies { - if strings.HasPrefix(acl.Id, "azure-acl-") { - // Remove the ACL policy from the list - epBuilder.removeACLPolicyAtIndex(i) - } - } + epBuilder.aclPolicies = []*NPMACLPolSettings{} } -func (epBuilder *endpointPolicyBuilder) removeACLPolicyAtIndex(i int) { - klog.Infof("[DataPlane Windows] Found ACL with ID %s and removing it", epBuilder.aclPolicies[i].Id) - if i == len(epBuilder.aclPolicies)-1 { - epBuilder.aclPolicies = epBuilder.aclPolicies[:i] - return +func (epBuilder *endpointPolicyBuilder) removeACLPolicyAtIndex(indexes map[int]struct{}) { + tempAclPolicies := []*NPMACLPolSettings{} + for i, acl := range epBuilder.aclPolicies { + if _, ok := indexes[i]; !ok { + tempAclPolicies = append(tempAclPolicies, acl) + } } - epBuilder.aclPolicies = append(epBuilder.aclPolicies[:i], epBuilder.aclPolicies[i+1:]...) + epBuilder.aclPolicies = tempAclPolicies } diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index a01ffa69c5..e3da88db53 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -1 +1,24 @@ package policies + +import "testing" + +func TestCompareAndRemovePolicies(t *testing.T) { + epbuilder := newEndpointPolicyBuilder() + + testPol := &NPMACLPolSettings{ + Id: "test1", + Protocols: string(TCP), + } + testPol2 := &NPMACLPolSettings{ + Id: "test1", + Protocols: string(UDP), + } + + epbuilder.aclPolicies = append(epbuilder.aclPolicies, []*NPMACLPolSettings{testPol, testPol2}...) + + epbuilder.compareAndRemovePolicies("test1", 2) + + if len(epbuilder.aclPolicies) != 0 { + t.Errorf("Expected 0 policies, got %d", len(epbuilder.aclPolicies)) + } +} diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 0f0320ac97..1c81321641 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -39,7 +39,7 @@ var ( Direction: policies.Ingress, }, { - PolicyID: "azure-acl-234", + PolicyID: "azure-acl-123", Target: policies.Allowed, Direction: policies.Ingress, SrcList: []policies.SetInfo{ @@ -62,7 +62,7 @@ var ( func main() { dp, err := dataplane.NewDataPlane(nodeName, common.NewIOShim()) panicOnError(err) - printAndWait() + printAndWait(true) podMetadata := &dataplane.PodMetadata{ PodKey: "a", @@ -80,7 +80,7 @@ func main() { panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadataB)) podMetadataC := &dataplane.PodMetadata{ PodKey: "c", - PodIP: "10.240.0.24", + PodIP: "10.240.0.56", NodeName: nodeName, } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataC)) @@ -90,7 +90,7 @@ func main() { panicOnError(dp.ApplyDataPlane()) - printAndWait() + printAndWait(true) panicOnError(dp.AddToLists([]*ipsets.IPSetMetadata{ipsets.TestKeyNSList.Metadata, ipsets.TestKVNSList.Metadata}, []*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata})) @@ -101,36 +101,55 @@ func main() { dp.DeleteIPSet(ipsets.TestKVPodSet.Metadata) panicOnError(dp.ApplyDataPlane()) - printAndWait() + printAndWait(true) panicOnError(dp.RemoveFromSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadata)) dp.DeleteIPSet(ipsets.TestNSSet.Metadata) panicOnError(dp.ApplyDataPlane()) - printAndWait() + printAndWait(true) panicOnError(dp.AddPolicy(testNetPol)) + printAndWait(true) + + panicOnError(dp.RemovePolicy(testNetPol.Name)) + printAndWait(true) + + panicOnError(dp.AddPolicy(testNetPol)) + printAndWait(true) + + podMetadataD := &dataplane.PodMetadata{ + PodKey: "d", + PodIP: "10.240.0.38", + NodeName: nodeName, + } + panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataD)) + panicOnError(dp.ApplyDataPlane()) + printAndWait(true) + + panicOnError(dp.RemovePolicy(testNetPol.Name)) + + // testPolicyManager() - testPolicyManager() } func testPolicyManager() { pMgr := policies.NewPolicyManager(common.NewIOShim()) panicOnError(pMgr.Reset()) - printAndWait() + printAndWait(false) panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[0], nil)) - printAndWait() + printAndWait(false) panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[1], nil)) - printAndWait() + printAndWait(false) // remove something that doesn't exist panicOnError(pMgr.RemovePolicy(policies.TestNetworkPolicies[2].Name, nil)) - printAndWait() + printAndWait(false) panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[2], nil)) - printAndWait() + printAndWait(false) // remove something that exists panicOnError(pMgr.RemovePolicy(policies.TestNetworkPolicies[1].Name, nil)) @@ -142,10 +161,12 @@ func panicOnError(err error) { } } -func printAndWait() { +func printAndWait(wait bool) { fmt.Printf("#####################\nCompleted running, please check relevant commands, script will resume in %d secs\n#############\n", MaxSleepTime) - for i := 0; i < MaxSleepTime; i++ { - fmt.Print(".") - time.Sleep(time.Second) + if wait { + for i := 0; i < MaxSleepTime; i++ { + fmt.Print(".") + time.Sleep(time.Second) + } } } From 82165c4ac9a91154c81996f99fc3a377961e03ed Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 11 Nov 2021 16:35:54 -0800 Subject: [PATCH 05/15] Splitting each modify call into two, one for 1st level sets and another for nested sets --- .../dataplane/ipsets/ipsetmanager_windows.go | 93 +++++++++++-------- 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 6c71ba767b..9df73f727b 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -172,27 +172,43 @@ func (iMgr *IPSetManager) getHCnNetwork() (*hcn.HostComputeNetwork, error) { func (iMgr *IPSetManager) modifySetPolicies(network *hcn.HostComputeNetwork, operation hcn.RequestType, setPolicies map[string]*hcn.SetPolicySetting) error { klog.Infof("[IPSetManager Windows] %s operation on set policies is called", operation) - policyRequest, err := getPolicyNetworkRequestMarshal(setPolicies, operation) - if err != nil { - klog.Infof("[IPSetManager Windows] Failed to marshal %s operations sets with error %s", operation, err.Error()) - return err + /* + Due to complexities in HNS, we need to do the following: + for (Add) + 1. Add 1st level set policies to HNS + 2. then add nested set policies to HNS + + for (delete) + 1. delete nested set policies from HNS + 2. then delete 1st level set policies from HNS + */ + policySettingsOrder := []hcn.SetPolicyType{hcn.SetPolicyTypeIpSet, SetPolicyTypeNestedIPSet} + if operation == hcn.RequestTypeRemove { + policySettingsOrder = []hcn.SetPolicyType{SetPolicyTypeNestedIPSet, hcn.SetPolicyTypeIpSet} } + for _, policyType := range policySettingsOrder { + policyRequest, err := getPolicyNetworkRequestMarshal(setPolicies, policyType) + if err != nil { + klog.Infof("[IPSetManager Windows] Failed to marshal %s operations sets with error %s", operation, err.Error()) + return err + } - if policyRequest == nil { - klog.Infof("[IPSetManager Windows] No Policies to apply") - return nil - } + if policyRequest == nil { + klog.Infof("[IPSetManager Windows] No Policies to apply") + return nil + } - requestMessage := &hcn.ModifyNetworkSettingRequest{ - ResourceType: hcn.NetworkResourceTypePolicy, - RequestType: operation, - Settings: policyRequest, - } + requestMessage := &hcn.ModifyNetworkSettingRequest{ + ResourceType: hcn.NetworkResourceTypePolicy, + RequestType: operation, + Settings: policyRequest, + } - err = iMgr.ioShim.Hns.ModifyNetworkSettings(network, requestMessage) - if err != nil { - klog.Infof("[IPSetManager Windows] %s operation has failed with error %s", operation, err.Error()) - return err + err = iMgr.ioShim.Hns.ModifyNetworkSettings(network, requestMessage) + if err != nil { + klog.Infof("[IPSetManager Windows] %s operation has failed with error %s", operation, err.Error()) + return err + } } return nil } @@ -234,37 +250,32 @@ func (setPolicyBuilder *networkPolicyBuilder) setNameExists(setName string) bool return ok } -func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicySetting, operation hcn.RequestType) ([]byte, error) { - policyNetworkRequest := &hcn.PolicyNetworkRequest{ - Policies: make([]hcn.NetworkPolicy, len(setPolicySettings)), - } - +func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicySetting, policyType hcn.SetPolicyType) ([]byte, error) { if len(setPolicySettings) == 0 { klog.Info("[Dataplane Windows] no set policies to apply on network") return nil, nil } - - idx := 0 - policySettingsOrder := []hcn.SetPolicyType{hcn.SetPolicyTypeIpSet, SetPolicyTypeNestedIPSet} - if operation == hcn.RequestTypeRemove { - policySettingsOrder = []hcn.SetPolicyType{SetPolicyTypeNestedIPSet, hcn.SetPolicyTypeIpSet} + klog.Info("[Dataplane Windows] marshalling %s type of sets", policyType) + policyNetworkRequest := &hcn.PolicyNetworkRequest{ + Policies: make([]hcn.NetworkPolicy, 0), } - for _, policyType := range policySettingsOrder { - for setPol := range setPolicySettings { - if setPolicySettings[setPol].PolicyType != policyType { - continue - } - klog.Infof("Found set pol %+v", setPolicySettings[setPol]) - rawSettings, err := json.Marshal(setPolicySettings[setPol]) - if err != nil { - return nil, err - } - policyNetworkRequest.Policies[idx] = hcn.NetworkPolicy{ + + for setPol := range setPolicySettings { + if setPolicySettings[setPol].PolicyType != policyType { + continue + } + klog.Infof("Found set pol %+v", setPolicySettings[setPol]) + rawSettings, err := json.Marshal(setPolicySettings[setPol]) + if err != nil { + return nil, err + } + policyNetworkRequest.Policies = append( + policyNetworkRequest.Policies, + hcn.NetworkPolicy{ Type: hcn.SetPolicy, Settings: rawSettings, - } - idx++ - } + }, + ) } policyReqSettings, err := json.Marshal(policyNetworkRequest) From e56d03862d3143d1f7416bea280dccb9389f3f9f Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 11 Nov 2021 16:58:58 -0800 Subject: [PATCH 06/15] Fixing a build issue in linux --- npm/pkg/dataplane/ipsets/testutils.go | 14 -------------- npm/pkg/dataplane/ipsets/testutils_windows.go | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/testutils.go b/npm/pkg/dataplane/ipsets/testutils.go index ab3245a4f6..79d83ec93b 100644 --- a/npm/pkg/dataplane/ipsets/testutils.go +++ b/npm/pkg/dataplane/ipsets/testutils.go @@ -1,9 +1,7 @@ package ipsets import ( - "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/util" - "github.com/Microsoft/hcsshim/hcn" ) type TestSet struct { @@ -24,18 +22,6 @@ func CreateTestSet(name string, setType SetType) *TestSet { return set } -func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { - hns := hnswrapper.NewHnsv2wrapperFake() - network := &hcn.HostComputeNetwork{ - Id: "1234", - Name: "azure", - } - - hns.CreateNetwork(network) - - return hns -} - var ( TestNSSet = CreateTestSet("test-ns-set", Namespace) TestKeyPodSet = CreateTestSet("test-keyPod-set", KeyLabelOfPod) diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index 2a97b83d2f..c7a277ce58 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -1,6 +1,22 @@ package ipsets -import testutils "github.com/Azure/azure-container-networking/test/utils" +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/Microsoft/hcsshim/hcn" +) + +func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { + hns := hnswrapper.NewHnsv2wrapperFake() + network := &hcn.HostComputeNetwork{ + Id: "1234", + Name: "azure", + } + + hns.CreateNetwork(network) + + return hns +} func GetApplyIPSetsTestCalls(_, _ []*IPSetMetadata) []testutils.TestCmd { return []testutils.TestCmd{} From 7bb60883800aca56bf505d4fa17a630b9575e769 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Mon, 15 Nov 2021 22:10:35 -0800 Subject: [PATCH 07/15] Enhancing windows ipset tests --- network/hnswrapper/hnsv2wrapperfake.go | 52 +++-- .../ipsets/ipsetmanager_linux_test.go | 28 +-- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 33 +-- .../dataplane/ipsets/ipsetmanager_windows.go | 2 +- .../ipsets/ipsetmanager_windows_test.go | 205 +++++++++++++++++- npm/pkg/dataplane/ipsets/testutils_linux.go | 4 + npm/pkg/dataplane/ipsets/testutils_windows.go | 5 + test/integration/npm/main.go | 6 +- 8 files changed, 274 insertions(+), 61 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 7dba73be68..4954b671f2 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -14,21 +14,37 @@ import ( "github.com/Microsoft/hcsshim/hcn" ) -type fakeHNSCache struct { - networks map[string]hcn.HostComputeNetwork - endpoints map[string]hcn.HostComputeEndpoint +type FakeHNSCache struct { + networks map[string]*hcn.HostComputeNetwork + endpoints map[string]*hcn.HostComputeEndpoint +} + +func (fCache FakeHNSCache) SetPolicyExists(setId string) bool { + for _, network := range fCache.networks { + for _, policy := range network.Policies { + var setPolSettings hcn.SetPolicySetting + err := json.Unmarshal(policy.Settings, &setPolSettings) + if err != nil { + return false + } + if setPolSettings.Name == setId { + return true + } + } + } + return false } type Hnsv2wrapperFake struct { - cache fakeHNSCache + Cache FakeHNSCache sync.Mutex } func NewHnsv2wrapperFake() *Hnsv2wrapperFake { return &Hnsv2wrapperFake{ - cache: fakeHNSCache{ - networks: map[string]hcn.HostComputeNetwork{}, - endpoints: map[string]hcn.HostComputeEndpoint{}, + Cache: FakeHNSCache{ + networks: map[string]*hcn.HostComputeNetwork{}, + endpoints: map[string]*hcn.HostComputeEndpoint{}, }, } } @@ -37,7 +53,7 @@ func (f Hnsv2wrapperFake) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.H f.Lock() defer f.Unlock() - f.cache.networks[network.Name] = *network + f.Cache.networks[network.Name] = network return network, nil } @@ -107,8 +123,8 @@ func (Hnsv2wrapperFake) RemoveNetworkPolicy(network *hcn.HostComputeNetwork, net func (f Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) { f.Lock() defer f.Unlock() - if network, ok := f.cache.networks[networkName]; ok { - return &network, nil + if network, ok := f.Cache.networks[networkName]; ok { + return network, nil } return &hcn.HostComputeNetwork{}, nil } @@ -116,9 +132,9 @@ func (f Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostCompute func (f Hnsv2wrapperFake) GetNetworkByID(networkID string) (*hcn.HostComputeNetwork, error) { f.Lock() defer f.Unlock() - for _, network := range f.cache.networks { + for _, network := range f.Cache.networks { if network.Id == networkID { - return &network, nil + return network, nil } } return &hcn.HostComputeNetwork{}, nil @@ -127,8 +143,8 @@ func (f Hnsv2wrapperFake) GetNetworkByID(networkID string) (*hcn.HostComputeNetw func (f Hnsv2wrapperFake) GetEndpointByID(endpointID string) (*hcn.HostComputeEndpoint, error) { f.Lock() defer f.Unlock() - if ep, ok := f.cache.endpoints[endpointID]; ok { - return &ep, nil + if ep, ok := f.Cache.endpoints[endpointID]; ok { + return ep, nil } return &hcn.HostComputeEndpoint{}, nil } @@ -136,14 +152,14 @@ func (f Hnsv2wrapperFake) GetEndpointByID(endpointID string) (*hcn.HostComputeEn func (f Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { f.Lock() defer f.Unlock() - f.cache.endpoints[endpoint.Id] = *endpoint + f.Cache.endpoints[endpoint.Id] = endpoint return endpoint, nil } func (f Hnsv2wrapperFake) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error { f.Lock() defer f.Unlock() - delete(f.cache.endpoints, endpoint.Id) + delete(f.Cache.endpoints, endpoint.Id) return nil } @@ -164,9 +180,9 @@ func (f Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostCo f.Lock() defer f.Unlock() endpoints := make([]hcn.HostComputeEndpoint, 0) - for _, endpoint := range f.cache.endpoints { + for _, endpoint := range f.Cache.endpoints { if endpoint.HostComputeNetwork == networkId { - endpoints = append(endpoints, endpoint) + endpoints = append(endpoints, *endpoint) } } return endpoints, nil diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go index dcdb92778e..f91f130e37 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_linux_test.go @@ -14,17 +14,12 @@ import ( ) var ( - iMgrApplyAllCfg = &IPSetManagerCfg{ - IPSetMode: ApplyAllIPSets, - NetworkName: "", - } - ipsetRestoreStringSlice = []string{"ipset", "restore"} ) func TestDestroyNPMIPSets(t *testing.T) { calls := []testutils.TestCmd{} // TODO - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) require.NoError(t, iMgr.resetIPSets()) } @@ -54,7 +49,7 @@ func TestConvertAndDeleteCache(t *testing.T) { // create all possible SetTypes func TestApplyCreationsAndAdds(t *testing.T) { calls := []testutils.TestCmd{fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) lines := []string{ fmt.Sprintf("-N %s -exist nethash", TestNSSet.HashedName), @@ -112,7 +107,7 @@ func TestApplyCreationsAndAdds(t *testing.T) { func TestApplyDeletions(t *testing.T) { calls := []testutils.TestCmd{fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) // Remove members and delete others iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -163,7 +158,7 @@ func TestFailureOnCreation(t *testing.T) { ExitCode: 1, } calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) @@ -206,7 +201,7 @@ func TestFailureOnAddToList(t *testing.T) { ExitCode: 1, } calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) @@ -270,7 +265,7 @@ func TestFailureOnFlush(t *testing.T) { ExitCode: 1, } calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) @@ -311,7 +306,7 @@ func TestFailureOnDeletion(t *testing.T) { ExitCode: 1, } calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand} - iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) @@ -346,15 +341,6 @@ func TestFailureOnDeletion(t *testing.T) { } // TODO if we add file-level error handlers, add tests for them - -func assertEqualContentsTestHelper(t *testing.T, setNames []string, cache map[string]struct{}) { - require.Equal(t, len(setNames), len(cache), "cache is different than list of set names") - for _, setName := range setNames { - _, exists := cache[setName] - require.True(t, exists, "cache is different than list of set names") - } -} - // the order of adds is nondeterministic, so we're sorting them func getSortedLines(set *TestSet, members ...string) []string { result := []string{fmt.Sprintf("-F %s", set.HashedName)} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 9954f7ba14..95b3550109 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -4,7 +4,6 @@ import ( "os" "testing" - "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" testutils "github.com/Azure/azure-container-networking/test/utils" @@ -32,7 +31,7 @@ var ( ) func TestCreateIPSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -51,7 +50,7 @@ func TestCreateIPSet(t *testing.T) { } func TestAddToSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -76,7 +75,7 @@ func TestAddToSet(t *testing.T) { } func TestRemoveFromSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -90,7 +89,7 @@ func TestRemoveFromSet(t *testing.T) { } func TestRemoveFromSetMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) err := iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) @@ -100,7 +99,7 @@ func TestRemoveFromSetMissing(t *testing.T) { } func TestAddToListMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNamespace) err := iMgr.AddToLists([]*IPSetMetadata{listMetadata}, []*IPSetMetadata{setMetadata}) @@ -111,7 +110,7 @@ func TestAddToListMissing(t *testing.T) { } func TestAddToList(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata, listMetadata}) @@ -131,7 +130,7 @@ func TestAddToList(t *testing.T) { } func TestRemoveFromList(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata, listMetadata}) @@ -158,7 +157,7 @@ func TestRemoveFromList(t *testing.T) { } func TestRemoveFromListMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) @@ -172,7 +171,7 @@ func TestRemoveFromListMissing(t *testing.T) { } func TestDeleteIPSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -181,7 +180,7 @@ func TestDeleteIPSet(t *testing.T) { } func TestGetIPsFromSelectorIPSets(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -230,7 +229,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) { } func TestAddDeleteSelectorReferences(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -306,7 +305,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { } func TestAddDeleteNetPolReferences(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -383,3 +382,11 @@ func TestMain(m *testing.M) { os.Exit(exitCode) } + +func assertEqualContentsTestHelper(t *testing.T, setNames []string, cache map[string]struct{}) { + require.Equal(t, len(setNames), len(cache), "cache is different than list of set names") + for _, setName := range setNames { + _, exists := cache[setName] + require.True(t, exists, "cache is different than list of set names") + } +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 9df73f727b..ea601e5002 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -255,7 +255,7 @@ func getPolicyNetworkRequestMarshal(setPolicySettings map[string]*hcn.SetPolicyS klog.Info("[Dataplane Windows] no set policies to apply on network") return nil, nil } - klog.Info("[Dataplane Windows] marshalling %s type of sets", policyType) + klog.Infof("[Dataplane Windows] marshalling %s type of sets", policyType) policyNetworkRequest := &hcn.PolicyNetworkRequest{ Policies: make([]hcn.NetworkPolicy, 0), } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index 7263b2e133..e4f2ceef05 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -8,12 +8,8 @@ import ( "github.com/stretchr/testify/require" ) -var ( - hns = GetHNSFake() -) - func TestAddToSetWindows(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -36,3 +32,202 @@ func TestAddToSetWindows(t *testing.T) { err = iMgr.applyIPSets() require.NoError(t, err) } + +func TestDestroyNPMIPSets(t *testing.T) { + calls := []testutils.TestCmd{} // TODO + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) + require.NoError(t, iMgr.resetIPSets()) +} + +// create all possible SetTypes +func TestApplyCreationsAndAdds(t *testing.T) { + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.5", "c")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) + iMgr.CreateIPSets([]*IPSetMetadata{TestNamedportSet.Metadata}) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) + require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKeyNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata, TestKeyPodSet.Metadata})) + iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) + require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestKVPodSet.Metadata})) + iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) + toAddOrUpdateSetNames := []string{ + TestNSSet.PrefixName, + TestKeyPodSet.PrefixName, + TestKVPodSet.PrefixName, + TestNamedportSet.PrefixName, + TestCIDRSet.PrefixName, + TestKeyNSList.PrefixName, + TestKVNSList.PrefixName, + TestNestedLabelList.PrefixName, + } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + err := iMgr.applyIPSets() + require.NoError(t, err) + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} + +func TestApplyDeletions(t *testing.T) { + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + // Remove members and delete others + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) + require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKeyNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata, TestKeyPodSet.Metadata})) + require.NoError(t, iMgr.RemoveFromSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) + require.NoError(t, iMgr.RemoveFromList(TestKeyNSList.Metadata, []*IPSetMetadata{TestKeyPodSet.Metadata})) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) + iMgr.DeleteIPSet(TestNestedLabelList.PrefixName) + + toDeleteSetNames := []string{TestCIDRSet.PrefixName, TestNestedLabelList.PrefixName} + assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) + toAddOrUpdateSetNames := []string{TestNSSet.PrefixName, TestKeyPodSet.PrefixName, TestKeyNSList.PrefixName} + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + + err := iMgr.applyIPSets() + require.NoError(t, err) + + for _, setName := range toDeleteSetNames { + require.False(t, hns.Cache.SetPolicyExists(setName)) + } + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} + +// TODO test that a reconcile list is updated +func TestFailureOnCreation(t *testing.T) { + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.1", "b")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestKeyPodSet.Metadata}, "10.0.0.5", "c")) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + + toAddOrUpdateSetNames := []string{TestNSSet.PrefixName, TestKeyPodSet.PrefixName} + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + toDeleteSetNames := []string{TestCIDRSet.PrefixName} + assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) + + for _, setName := range toDeleteSetNames { + require.False(t, hns.Cache.SetPolicyExists(setName)) + } + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} + +// TODO test that a reconcile list is updated +func TestFailureOnAddToList(t *testing.T) { + // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyPodSet.Metadata}) + iMgr.CreateIPSets([]*IPSetMetadata{TestKeyNSList.Metadata}) + require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKeyNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata, TestKeyPodSet.Metadata})) + iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) + require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestNSSet.Metadata})) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + + toAddOrUpdateSetNames := []string{ + TestNSSet.PrefixName, + TestKeyPodSet.PrefixName, + TestKeyNSList.PrefixName, + TestKVNSList.PrefixName, + } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + toDeleteSetNames := []string{TestCIDRSet.PrefixName} + assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) + + for _, setName := range toDeleteSetNames { + require.False(t, hns.Cache.SetPolicyExists(setName)) + } + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} + +// TODO test that a reconcile list is updated +func TestFailureOnFlush(t *testing.T) { + // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + + toAddOrUpdateSetNames := []string{TestNSSet.PrefixName} + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} + assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) + + for _, setName := range toDeleteSetNames { + require.False(t, hns.Cache.SetPolicyExists(setName)) + } + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} + +// TODO test that a reconcile list is updated +func TestFailureOnDeletion(t *testing.T) { + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) + + iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) + require.NoError(t, iMgr.AddToSets([]*IPSetMetadata{TestNSSet.Metadata}, "10.0.0.0", "a")) + iMgr.CreateIPSets([]*IPSetMetadata{TestKVPodSet.Metadata}) + iMgr.DeleteIPSet(TestKVPodSet.PrefixName) + iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) + iMgr.DeleteIPSet(TestCIDRSet.PrefixName) + + toAddOrUpdateSetNames := []string{TestNSSet.PrefixName} + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} + assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) + + for _, setName := range toDeleteSetNames { + require.False(t, hns.Cache.SetPolicyExists(setName)) + } + + for _, setName := range toAddOrUpdateSetNames { + require.True(t, hns.Cache.SetPolicyExists(setName)) + } +} diff --git a/npm/pkg/dataplane/ipsets/testutils_linux.go b/npm/pkg/dataplane/ipsets/testutils_linux.go index f1bb187c33..608c469555 100644 --- a/npm/pkg/dataplane/ipsets/testutils_linux.go +++ b/npm/pkg/dataplane/ipsets/testutils_linux.go @@ -8,6 +8,10 @@ var fakeRestoreSuccessCommand = testutils.TestCmd{ ExitCode: 0, } +func getMockIOShim(calls []testutils.TestCmd) *common.IOShim { + return common.NewMockIOShim([]testutils.TestCmd{}) +} + func GetApplyIPSetsTestCalls(toAddOrUpdateIPSets, toDeleteIPSets []*IPSetMetadata) []testutils.TestCmd { // TODO eventually call ipset save if there are toAddOrUpdateIPSets return []testutils.TestCmd{fakeRestoreSuccessCommand} diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index c7a277ce58..be613c8655 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -1,11 +1,16 @@ package ipsets import ( + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/Microsoft/hcsshim/hcn" ) +func getMockIOShim(calls []testutils.TestCmd) *common.IOShim { + return common.NewMockIOShimWithFakeHNS(calls, GetHNSFake()) +} + func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { hns := hnswrapper.NewHnsv2wrapperFake() network := &hcn.HostComputeNetwork{ diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 1c81321641..fecbf7617d 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -80,7 +80,7 @@ func main() { panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadataB)) podMetadataC := &dataplane.PodMetadata{ PodKey: "c", - PodIP: "10.240.0.56", + PodIP: "10.240.0.20", NodeName: nodeName, } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataC)) @@ -119,14 +119,14 @@ func main() { podMetadataD := &dataplane.PodMetadata{ PodKey: "d", - PodIP: "10.240.0.38", + PodIP: "10.240.0.7", NodeName: nodeName, } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataD)) panicOnError(dp.ApplyDataPlane()) printAndWait(true) - panicOnError(dp.RemovePolicy(testNetPol.Name)) + // panicOnError(dp.RemovePolicy(testNetPol.Name)) // testPolicyManager() From bfa9d3b8d27410692ba98fe8af067693c8cf6236 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 16 Nov 2021 14:59:14 -0800 Subject: [PATCH 08/15] Adding ipset mgr windows tests --- common/ioshim_windows.go | 4 +- network/hnswrapper/hnsv2wrapperfake.go | 261 ++++++++++++++---- npm/pkg/dataplane/ipsets/ipsetmanager.go | 1 + .../ipsets/ipsetmanager_windows_test.go | 255 +++++++++++++---- npm/pkg/dataplane/ipsets/testutils_linux.go | 5 +- npm/pkg/dataplane/ipsets/testutils_windows.go | 4 +- .../policies/policymanager_windows_test.go | 36 ++- 7 files changed, 441 insertions(+), 125 deletions(-) diff --git a/common/ioshim_windows.go b/common/ioshim_windows.go index 68f841f4b8..888f9fb129 100644 --- a/common/ioshim_windows.go +++ b/common/ioshim_windows.go @@ -25,9 +25,9 @@ func NewMockIOShim(calls []testutils.TestCmd) *IOShim { } } -func NewMockIOShimWithFakeHNS(calls []testutils.TestCmd, hns *hnswrapper.Hnsv2wrapperFake) *IOShim { +func NewMockIOShimWithFakeHNS(hns *hnswrapper.Hnsv2wrapperFake) *IOShim { return &IOShim{ - Exec: testutils.GetFakeExecWithScripts(calls), + Exec: testutils.GetFakeExecWithScripts([]testutils.TestCmd{}), Hns: hns, } } diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 4954b671f2..302bb53ca1 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -8,43 +8,34 @@ package hnswrapper import ( "encoding/json" + "errors" + "fmt" "reflect" + "strings" "sync" "github.com/Microsoft/hcsshim/hcn" ) -type FakeHNSCache struct { - networks map[string]*hcn.HostComputeNetwork - endpoints map[string]*hcn.HostComputeEndpoint -} +const networkName = "azure" -func (fCache FakeHNSCache) SetPolicyExists(setId string) bool { - for _, network := range fCache.networks { - for _, policy := range network.Policies { - var setPolSettings hcn.SetPolicySetting - err := json.Unmarshal(policy.Settings, &setPolSettings) - if err != nil { - return false - } - if setPolSettings.Name == setId { - return true - } - } - } - return false +var errorFakeHNS = errors.New("errorFakeHNS Error") + +func newErrorFakeHNS(errStr string) error { + return fmt.Errorf("%w : %s", errorFakeHNS, errStr) } type Hnsv2wrapperFake struct { Cache FakeHNSCache - sync.Mutex + *sync.Mutex } func NewHnsv2wrapperFake() *Hnsv2wrapperFake { return &Hnsv2wrapperFake{ + Mutex: &sync.Mutex{}, Cache: FakeHNSCache{ - networks: map[string]*hcn.HostComputeNetwork{}, - endpoints: map[string]*hcn.HostComputeEndpoint{}, + networks: map[string]*FakeHostComputeNetwork{}, + endpoints: map[string]*FakeHostComputeEndpoint{}, }, } } @@ -53,7 +44,7 @@ func (f Hnsv2wrapperFake) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.H f.Lock() defer f.Unlock() - f.Cache.networks[network.Name] = network + f.Cache.networks[network.Name] = NewFakeHostComputeNetwork(network) return network, nil } @@ -64,49 +55,99 @@ func (f Hnsv2wrapperFake) DeleteNetwork(network *hcn.HostComputeNetwork) error { func (f Hnsv2wrapperFake) ModifyNetworkSettings(network *hcn.HostComputeNetwork, request *hcn.ModifyNetworkSettingRequest) error { f.Lock() defer f.Unlock() + networkCache, ok := f.Cache.networks[network.Name] + if !ok { + return nil + } switch request.RequestType { case hcn.RequestTypeAdd: var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { - return err + return newErrorFakeHNS(err.Error()) } for _, setPolSetting := range setPolSettings.Policies { if setPolSetting.Type == hcn.SetPolicy { - network.Policies = append(network.Policies, setPolSetting) + var setpol hcn.SetPolicySetting + err := json.Unmarshal(setPolSetting.Settings, &setpol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + if setpol.PolicyType != hcn.SetPolicyTypeIpSet { + // Check Nested SetPolicy members + members := strings.Split(setpol.Values, ",") + for _, memberID := range members { + _, ok := networkCache.Policies[memberID] + if !ok { + return newErrorFakeHNS(fmt.Sprintf("Member Policy %s not found", memberID)) + } + } + } + networkCache.Policies[setpol.Id] = &setpol } } case hcn.RequestTypeRemove: - newtempPolicies := network.Policies var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { - return err + return newErrorFakeHNS(err.Error()) } - for i, policy := range network.Policies { - for _, newPolicy := range setPolSettings.Policies { - if policy.Type != newPolicy.Type { - continue - } - if reflect.DeepEqual(policy.Settings, newPolicy.Settings) { - newtempPolicies = append(newtempPolicies[:i], newtempPolicies[i+1:]...) - break + for _, newPolicy := range setPolSettings.Policies { + var setpol hcn.SetPolicySetting + err := json.Unmarshal(newPolicy.Settings, &setpol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + if _, ok := networkCache.Policies[setpol.Id]; !ok { + return newErrorFakeHNS(fmt.Sprintf("[FakeHNS] could not find %s ipset", setpol.Name)) + } + if setpol.PolicyType == hcn.SetPolicyTypeIpSet { + // For 1st level sets check if they are being referred by nested sets + for _, cacheSet := range networkCache.Policies { + if cacheSet.PolicyType == hcn.SetPolicyTypeIpSet { + continue + } + if strings.Contains(cacheSet.Values, setpol.Id) { + return newErrorFakeHNS(fmt.Sprintf("Set %s is being referred by another %s set", setpol.Name, cacheSet.Name)) + } } } + delete(networkCache.Policies, setpol.Id) } - network.Policies = newtempPolicies case hcn.RequestTypeUpdate: - network.Policies = []hcn.NetworkPolicy{} var setPolSettings hcn.PolicyNetworkRequest err := json.Unmarshal(request.Settings, &setPolSettings) if err != nil { - return err + return newErrorFakeHNS(err.Error()) } - for _, setPolSetting := range setPolSettings.Policies { - if setPolSetting.Type == hcn.SetPolicy { - network.Policies = append(network.Policies, setPolSetting) + for _, newPolicy := range setPolSettings.Policies { + var setpol hcn.SetPolicySetting + err := json.Unmarshal(newPolicy.Settings, &setpol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + if _, ok := networkCache.Policies[setpol.Id]; !ok { + return newErrorFakeHNS(fmt.Sprintf("[FakeHNS] could not find %s ipset", setpol.Name)) } + _, ok := networkCache.Policies[setpol.Id] + if !ok { + // Replicating HNS behavior, we will not update non-existent set policy + continue + } + if setpol.PolicyType != hcn.SetPolicyTypeIpSet { + // Check Nested SetPolicy members + members := strings.Split(setpol.Values, ",") + for _, memberID := range members { + _, ok := networkCache.Policies[memberID] + if !ok { + return newErrorFakeHNS(fmt.Sprintf("Member Policy %s not found", memberID)) + } + } + } + networkCache.Policies[setpol.Id] = &setpol } + case hcn.RequestTypeRefresh: + return nil } return nil @@ -124,7 +165,7 @@ func (f Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostCompute f.Lock() defer f.Unlock() if network, ok := f.Cache.networks[networkName]; ok { - return network, nil + return network.GetHCNObj(), nil } return &hcn.HostComputeNetwork{}, nil } @@ -133,8 +174,8 @@ func (f Hnsv2wrapperFake) GetNetworkByID(networkID string) (*hcn.HostComputeNetw f.Lock() defer f.Unlock() for _, network := range f.Cache.networks { - if network.Id == networkID { - return network, nil + if network.ID == networkID { + return network.GetHCNObj(), nil } } return &hcn.HostComputeNetwork{}, nil @@ -144,7 +185,7 @@ func (f Hnsv2wrapperFake) GetEndpointByID(endpointID string) (*hcn.HostComputeEn f.Lock() defer f.Unlock() if ep, ok := f.Cache.endpoints[endpointID]; ok { - return ep, nil + return ep.GetHCNObj(), nil } return &hcn.HostComputeEndpoint{}, nil } @@ -152,7 +193,7 @@ func (f Hnsv2wrapperFake) GetEndpointByID(endpointID string) (*hcn.HostComputeEn func (f Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { f.Lock() defer f.Unlock() - f.Cache.endpoints[endpoint.Id] = endpoint + f.Cache.endpoints[endpoint.Id] = NewFakeHostComputeEndpoint(endpoint) return endpoint, nil } @@ -182,7 +223,7 @@ func (f Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostCo endpoints := make([]hcn.HostComputeEndpoint, 0) for _, endpoint := range f.Cache.endpoints { if endpoint.HostComputeNetwork == networkId { - endpoints = append(endpoints, *endpoint) + endpoints = append(endpoints, *endpoint.GetHCNObj()) } } return endpoints, nil @@ -191,37 +232,137 @@ func (f Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostCo func (f Hnsv2wrapperFake) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { f.Lock() defer f.Unlock() + + epCache, ok := f.Cache.endpoints[endpoint.Id] + if !ok { + return newErrorFakeHNS(fmt.Sprintf("[FakeHNS] could not find endpoint %s", endpoint.Id)) + } switch requestType { case hcn.RequestTypeAdd: for _, newPolicy := range endpointPolicy.Policies { if newPolicy.Type != hcn.ACL { continue } - endpoint.Policies = append(endpoint.Policies, newPolicy) + var aclPol FakeEndpointPolicy + err := json.Unmarshal(newPolicy.Settings, &aclPol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + epCache.Policies = append(epCache.Policies, &aclPol) } case hcn.RequestTypeRemove: - newtempPolicies := endpoint.Policies - for i, policy := range endpoint.Policies { - for _, newPolicy := range endpointPolicy.Policies { - if policy.Type != newPolicy.Type { - continue - } - if reflect.DeepEqual(policy.Settings, newPolicy.Settings) { - newtempPolicies = append(newtempPolicies[:i], newtempPolicies[i+1:]...) - break - } + for _, newPolicy := range endpointPolicy.Policies { + if newPolicy.Type != hcn.ACL { + continue + } + var aclPol FakeEndpointPolicy + err := json.Unmarshal(newPolicy.Settings, &aclPol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + err = epCache.RemovePolicy(&aclPol) + if err != nil { + return err } } - endpoint.Policies = newtempPolicies case hcn.RequestTypeUpdate: - endpoint.Policies = make([]hcn.EndpointPolicy, 0) + epCache.Policies = make([]*FakeEndpointPolicy, 0) for _, newPolicy := range endpointPolicy.Policies { if newPolicy.Type != hcn.ACL { continue } - endpoint.Policies = append(endpoint.Policies, newPolicy) + var aclPol FakeEndpointPolicy + err := json.Unmarshal(newPolicy.Settings, &aclPol) + if err != nil { + return newErrorFakeHNS(err.Error()) + } + epCache.Policies = append(epCache.Policies, &aclPol) } + case hcn.RequestTypeRefresh: + return nil } return nil } + +type FakeHNSCache struct { + networks map[string]*FakeHostComputeNetwork + endpoints map[string]*FakeHostComputeEndpoint +} + +func (fCache FakeHNSCache) Policy(setID string) *hcn.SetPolicySetting { + for _, network := range fCache.networks { + for _, policy := range network.Policies { + if policy.Id == setID { + return policy + } + } + } + return nil +} + +type FakeHostComputeNetwork struct { + ID string + Name string + Policies map[string]*hcn.SetPolicySetting +} + +func NewFakeHostComputeNetwork(network *hcn.HostComputeNetwork) *FakeHostComputeNetwork { + return &FakeHostComputeNetwork{ + ID: network.Id, + Name: network.Name, + Policies: make(map[string]*hcn.SetPolicySetting), + } +} + +func (fNetwork *FakeHostComputeNetwork) GetHCNObj() *hcn.HostComputeNetwork { + return &hcn.HostComputeNetwork{ + Id: fNetwork.ID, + Name: fNetwork.Name, + } +} + +type FakeHostComputeEndpoint struct { + ID string + Name string + HostComputeNetwork string + Policies []*FakeEndpointPolicy + IPConfiguration string +} + +func NewFakeHostComputeEndpoint(endpoint *hcn.HostComputeEndpoint) *FakeHostComputeEndpoint { + return &FakeHostComputeEndpoint{ + ID: endpoint.Id, + Name: endpoint.Name, + HostComputeNetwork: endpoint.HostComputeNetwork, + } +} + +func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { + return &hcn.HostComputeEndpoint{ + Id: fEndpoint.ID, + Name: fEndpoint.Name, + HostComputeNetwork: fEndpoint.HostComputeNetwork, + } +} + +func (fEndpoint *FakeHostComputeEndpoint) RemovePolicy(toRemovePol *FakeEndpointPolicy) error { + for i, policy := range fEndpoint.Policies { + if reflect.DeepEqual(policy, toRemovePol) { + fEndpoint.Policies = append(fEndpoint.Policies[:i], fEndpoint.Policies[i+1:]...) + return nil + } + } + return newErrorFakeHNS(fmt.Sprintf("Could not find policy %+v", toRemovePol)) +} + +type FakeEndpointPolicy struct { + ID string `json:",omitempty"` + Protocols string `json:",omitempty"` // EX: 6 (TCP), 17 (UDP), 1 (ICMPv4), 58 (ICMPv6), 2 (IGMP) + Action hcn.ActionType `json:","` + Direction hcn.DirectionType `json:","` + LocalAddresses string `json:",omitempty"` + RemoteAddresses string `json:",omitempty"` + LocalPorts string `json:",omitempty"` + RemotePorts string `json:",omitempty"` +} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 5f477a5b82..5bd4db2398 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -67,6 +67,7 @@ func (iMgr *IPSetManager) CreateIPSets(setMetadatas []*IPSetMetadata) { } func (iMgr *IPSetManager) createIPSet(setMetadata *IPSetMetadata) { + // TODO (vamsi) check for os specific restrictions on ipsets prefixedName := setMetadata.GetPrefixName() if iMgr.exists(prefixedName) { return diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index e4f2ceef05..475262a7f9 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -1,15 +1,20 @@ package ipsets import ( + "fmt" "testing" "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/Microsoft/hcsshim/hcn" "github.com/stretchr/testify/require" ) func TestAddToSetWindows(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim([]testutils.TestCmd{})) + hns := GetHNSFake() + io := common.NewMockIOShimWithFakeHNS(hns) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -42,7 +47,7 @@ func TestDestroyNPMIPSets(t *testing.T) { // create all possible SetTypes func TestApplyCreationsAndAdds(t *testing.T) { hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -58,28 +63,70 @@ func TestApplyCreationsAndAdds(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestKVNSList.Metadata}) require.NoError(t, iMgr.AddToLists([]*IPSetMetadata{TestKVNSList.Metadata}, []*IPSetMetadata{TestKVPodSet.Metadata})) iMgr.CreateIPSets([]*IPSetMetadata{TestNestedLabelList.Metadata}) - toAddOrUpdateSetNames := []string{ - TestNSSet.PrefixName, - TestKeyPodSet.PrefixName, - TestKVPodSet.PrefixName, - TestNamedportSet.PrefixName, - TestCIDRSet.PrefixName, - TestKeyNSList.PrefixName, - TestKVNSList.PrefixName, - TestNestedLabelList.PrefixName, + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0,10.0.0.1", + }, + TestKeyPodSet.PrefixName: { + Id: TestKeyPodSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestKeyPodSet.PrefixName, + Values: "10.0.0.5", + }, + TestKVPodSet.PrefixName: { + Id: TestKVPodSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestKVPodSet.PrefixName, + Values: "", + }, + TestNamedportSet.PrefixName: { + Id: TestNamedportSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNamedportSet.PrefixName, + Values: "", + }, + TestCIDRSet.PrefixName: { + Id: TestCIDRSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestCIDRSet.PrefixName, + Values: "", + }, + TestKeyNSList.PrefixName: { + Id: TestKeyNSList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestKeyNSList.PrefixName, + Values: fmt.Sprintf("%s,%s", TestNSSet.HashedName, TestKeyPodSet.HashedName), + }, + TestKVNSList.PrefixName: { + Id: TestKVNSList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestKVNSList.PrefixName, + Values: TestKVPodSet.HashedName, + }, + TestNestedLabelList.PrefixName: { + Id: TestNestedLabelList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestNestedLabelList.PrefixName, + Values: "", + }, + } + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) err := iMgr.applyIPSets() require.NoError(t, err) - - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) - } + verifyHNSCache(t, toAddOrUpdateSetMap, hns) } func TestApplyDeletions(t *testing.T) { hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) // Remove members and delete others @@ -98,25 +145,42 @@ func TestApplyDeletions(t *testing.T) { toDeleteSetNames := []string{TestCIDRSet.PrefixName, TestNestedLabelList.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - toAddOrUpdateSetNames := []string{TestNSSet.PrefixName, TestKeyPodSet.PrefixName, TestKeyNSList.PrefixName} + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0", + }, + TestKeyPodSet.PrefixName: { + Id: TestKeyPodSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestKeyPodSet.PrefixName, + Values: "", + }, + TestKeyNSList.PrefixName: { + Id: TestKeyNSList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestKeyNSList.PrefixName, + Values: TestNSSet.HashedName, + }, + } + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) + } assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) err := iMgr.applyIPSets() require.NoError(t, err) - - for _, setName := range toDeleteSetNames { - require.False(t, hns.Cache.SetPolicyExists(setName)) - } - - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) - } + verifyHNSCache(t, toAddOrUpdateSetMap, hns) + verifyDeletedHNSCache(t, toDeleteSetNames, hns) } // TODO test that a reconcile list is updated func TestFailureOnCreation(t *testing.T) { hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -127,25 +191,41 @@ func TestFailureOnCreation(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) iMgr.DeleteIPSet(TestCIDRSet.PrefixName) - toAddOrUpdateSetNames := []string{TestNSSet.PrefixName, TestKeyPodSet.PrefixName} - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) toDeleteSetNames := []string{TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - for _, setName := range toDeleteSetNames { - require.False(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0,10.0.0.1", + }, + TestKeyPodSet.PrefixName: { + Id: TestKeyPodSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestKeyPodSet.PrefixName, + Values: "10.0.0.5", + }, } - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + + err := iMgr.applyIPSets() + require.NoError(t, err) + verifyHNSCache(t, toAddOrUpdateSetMap, hns) + verifyDeletedHNSCache(t, toDeleteSetNames, hns) } // TODO test that a reconcile list is updated func TestFailureOnAddToList(t *testing.T) { // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -158,30 +238,52 @@ func TestFailureOnAddToList(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) iMgr.DeleteIPSet(TestCIDRSet.PrefixName) - toAddOrUpdateSetNames := []string{ - TestNSSet.PrefixName, - TestKeyPodSet.PrefixName, - TestKeyNSList.PrefixName, - TestKVNSList.PrefixName, - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) toDeleteSetNames := []string{TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - for _, setName := range toDeleteSetNames { - require.False(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0", + }, + TestKeyPodSet.PrefixName: { + Id: TestKeyPodSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestKeyPodSet.PrefixName, + Values: "", + }, + TestKeyNSList.PrefixName: { + Id: TestKeyNSList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestKeyNSList.PrefixName, + Values: fmt.Sprintf("%s,%s", TestNSSet.HashedName, TestKeyPodSet.HashedName), + }, + TestKVNSList.PrefixName: { + Id: TestKVNSList.HashedName, + PolicyType: SetPolicyTypeNestedIPSet, + Name: TestKVNSList.PrefixName, + Values: TestNSSet.HashedName, + }, } - - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + + err := iMgr.applyIPSets() + require.NoError(t, err) + verifyHNSCache(t, toAddOrUpdateSetMap, hns) + verifyDeletedHNSCache(t, toDeleteSetNames, hns) } // TODO test that a reconcile list is updated func TestFailureOnFlush(t *testing.T) { // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -191,24 +293,34 @@ func TestFailureOnFlush(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) iMgr.DeleteIPSet(TestCIDRSet.PrefixName) - toAddOrUpdateSetNames := []string{TestNSSet.PrefixName} - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - for _, setName := range toDeleteSetNames { - require.False(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0", + }, } - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + + err := iMgr.applyIPSets() + require.NoError(t, err) + verifyHNSCache(t, toAddOrUpdateSetMap, hns) + verifyDeletedHNSCache(t, toDeleteSetNames, hns) } // TODO test that a reconcile list is updated func TestFailureOnDeletion(t *testing.T) { hns := GetHNSFake() - io := common.NewMockIOShimWithFakeHNS([]testutils.TestCmd{}, hns) + io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) iMgr.CreateIPSets([]*IPSetMetadata{TestNSSet.Metadata}) @@ -218,16 +330,41 @@ func TestFailureOnDeletion(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{TestCIDRSet.Metadata}) iMgr.DeleteIPSet(TestCIDRSet.PrefixName) - toAddOrUpdateSetNames := []string{TestNSSet.PrefixName} - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - for _, setName := range toDeleteSetNames { - require.False(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ + TestNSSet.PrefixName: { + Id: TestNSSet.HashedName, + PolicyType: hcn.SetPolicyTypeIpSet, + Name: TestNSSet.PrefixName, + Values: "10.0.0.0", + }, } - for _, setName := range toAddOrUpdateSetNames { - require.True(t, hns.Cache.SetPolicyExists(setName)) + toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) + for setName := range toAddOrUpdateSetMap { + toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) + } + assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) + + err := iMgr.applyIPSets() + require.NoError(t, err) + verifyHNSCache(t, toAddOrUpdateSetMap, hns) + verifyDeletedHNSCache(t, toDeleteSetNames, hns) +} + +func verifyHNSCache(t *testing.T, expected map[string]hcn.SetPolicySetting, hns *hnswrapper.Hnsv2wrapperFake) { + for setName, setObj := range expected { + cacheObj := hns.Cache.Policy(setObj.Id) + require.NotNil(t, cacheObj) + require.Equal(t, setObj, *cacheObj, fmt.Sprintf("%s mismatch in cache", setName)) + } +} + +func verifyDeletedHNSCache(t *testing.T, deleted []string, hns *hnswrapper.Hnsv2wrapperFake) { + for _, setName := range deleted { + cacheObj := hns.Cache.Policy(setName) + require.Nil(t, cacheObj) } } diff --git a/npm/pkg/dataplane/ipsets/testutils_linux.go b/npm/pkg/dataplane/ipsets/testutils_linux.go index 08a572cb5f..2aa147f5db 100644 --- a/npm/pkg/dataplane/ipsets/testutils_linux.go +++ b/npm/pkg/dataplane/ipsets/testutils_linux.go @@ -1,6 +1,9 @@ package ipsets -import testutils "github.com/Azure/azure-container-networking/test/utils" +import ( + "github.com/Azure/azure-container-networking/common" + testutils "github.com/Azure/azure-container-networking/test/utils" +) var ( ipsetRestoreStringSlice = []string{"ipset", "restore"} diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index be613c8655..19a066693e 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -7,8 +7,8 @@ import ( "github.com/Microsoft/hcsshim/hcn" ) -func getMockIOShim(calls []testutils.TestCmd) *common.IOShim { - return common.NewMockIOShimWithFakeHNS(calls, GetHNSFake()) +func getMockIOShim(_ []testutils.TestCmd) *common.IOShim { + return common.NewMockIOShimWithFakeHNS(GetHNSFake()) } func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index e3da88db53..df5b05e8d6 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -1,6 +1,13 @@ package policies -import "testing" +import ( + "testing" + + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Microsoft/hcsshim/hcn" + "github.com/stretchr/testify/require" +) func TestCompareAndRemovePolicies(t *testing.T) { epbuilder := newEndpointPolicyBuilder() @@ -22,3 +29,30 @@ func TestCompareAndRemovePolicies(t *testing.T) { t.Errorf("Expected 0 policies, got %d", len(epbuilder.aclPolicies)) } } + +func TestAddPolicies(t *testing.T) { + hns := ipsets.GetHNSFake() + io := common.NewMockIOShimWithFakeHNS(hns) + pMgr := NewPolicyManager(io) + + endPointIDList := map[string]string{ + "10.0.0.1": "test1", + "10.0.0.2": "test2", + } + for ip, epID := range endPointIDList { + ep := &hcn.HostComputeEndpoint{ + Id: epID, + Name: epID, + IpConfigurations: []hcn.IpConfig{ + { + IpAddress: ip, + }, + }, + } + _, err := hns.CreateEndpoint(ep) + require.NoError(t, err) + } + + err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) + require.NoError(t, err) +} From 81fbf8d2bdec404039adbd5e46b0f58321f65f8b Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Tue, 16 Nov 2021 15:08:20 -0800 Subject: [PATCH 09/15] fixing a build issue --- test/integration/npm/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 4f6d229439..b9cc39cc35 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -123,7 +123,7 @@ func main() { panicOnError(dp.AddPolicy(testNetPol)) printAndWait(true) - podMetadataD := &dataplane.PodMetadata{ + podMetadataD = &dataplane.PodMetadata{ PodKey: "d", PodIP: "10.240.0.7", NodeName: nodeName, @@ -145,7 +145,7 @@ func testPolicyManager() { printAndWait(false) panicOnError(pMgr.Initialize()) - printAndWait() + printAndWait(false) panicOnError(pMgr.AddPolicy(policies.TestNetworkPolicies[0], nil)) printAndWait(false) From 45a3d35c4436afa422511c7102f63d049d05b3e7 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 18 Nov 2021 14:16:01 -0800 Subject: [PATCH 10/15] Adding in ACL verify code --- network/hnswrapper/hnsv2wrapperfake.go | 38 ++++- .../ipsets/ipsetmanager_windows_test.go | 61 +------ npm/pkg/dataplane/ipsets/testutils.go | 4 +- npm/pkg/dataplane/policies/policy_windows.go | 11 +- .../policies/policymanager_windows_test.go | 156 +++++++++++++++++- npm/pkg/dataplane/policies/testutils.go | 6 +- test/integration/npm/main.go | 6 +- 7 files changed, 211 insertions(+), 71 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 302bb53ca1..05233bfd0a 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -290,7 +290,7 @@ type FakeHNSCache struct { endpoints map[string]*FakeHostComputeEndpoint } -func (fCache FakeHNSCache) Policy(setID string) *hcn.SetPolicySetting { +func (fCache FakeHNSCache) SetPolicy(setID string) *hcn.SetPolicySetting { for _, network := range fCache.networks { for _, policy := range network.Policies { if policy.Id == setID { @@ -301,6 +301,36 @@ func (fCache FakeHNSCache) Policy(setID string) *hcn.SetPolicySetting { return nil } +func (fCache FakeHNSCache) ACLPolicies(epList map[string]string, policyID string) (map[string][]*FakeEndpointPolicy, error) { + aclPols := make(map[string][]*FakeEndpointPolicy) + for ip, epID := range epList { + epCache, ok := fCache.endpoints[epID] + if !ok { + return nil, newErrorFakeHNS(fmt.Sprintf("[FakeHNS] could not find endpoint %s", epID)) + } + if epCache.IPConfiguration != ip { + return nil, newErrorFakeHNS(fmt.Sprintf("[FakeHNS] Mismatch in IP addr of endpoint %s Got: %s, Expect %s", + epID, epCache.IPConfiguration, ip)) + } + aclPols[epID] = make([]*FakeEndpointPolicy, 0) + for _, policy := range epCache.Policies { + if policy.ID == policyID { + aclPols[epID] = append(aclPols[epID], policy) + } + } + + } + return aclPols, nil +} + +func (fCache FakeHNSCache) GetAllACLs() map[string][]*FakeEndpointPolicy { + aclPols := make(map[string][]*FakeEndpointPolicy) + for _, ep := range fCache.endpoints { + aclPols[ep.ID] = ep.Policies + } + return aclPols +} + type FakeHostComputeNetwork struct { ID string Name string @@ -331,10 +361,15 @@ type FakeHostComputeEndpoint struct { } func NewFakeHostComputeEndpoint(endpoint *hcn.HostComputeEndpoint) *FakeHostComputeEndpoint { + ip := "" + if endpoint.IpConfigurations != nil { + ip = endpoint.IpConfigurations[0].IpAddress + } return &FakeHostComputeEndpoint{ ID: endpoint.Id, Name: endpoint.Name, HostComputeNetwork: endpoint.HostComputeNetwork, + IPConfiguration: ip, } } @@ -365,4 +400,5 @@ type FakeEndpointPolicy struct { RemoteAddresses string `json:",omitempty"` LocalPorts string `json:",omitempty"` RemotePorts string `json:",omitempty"` + Priority int `json:",omitempty"` } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index 475262a7f9..91a429681c 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -34,7 +34,7 @@ func TestAddToSetWindows(t *testing.T) { err = iMgr.AddToSets([]*IPSetMetadata{listMetadata}, testPodIP, testPodKey) require.Error(t, err) - err = iMgr.applyIPSets() + err = iMgr.ApplyIPSets() require.NoError(t, err) } @@ -113,13 +113,7 @@ func TestApplyCreationsAndAdds(t *testing.T) { Values: "", }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) } @@ -144,7 +138,6 @@ func TestApplyDeletions(t *testing.T) { iMgr.DeleteIPSet(TestNestedLabelList.PrefixName) toDeleteSetNames := []string{TestCIDRSet.PrefixName, TestNestedLabelList.PrefixName} - assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ TestNSSet.PrefixName: { Id: TestNSSet.HashedName, @@ -165,13 +158,8 @@ func TestApplyDeletions(t *testing.T) { Values: TestNSSet.HashedName, }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) verifyDeletedHNSCache(t, toDeleteSetNames, hns) @@ -192,8 +180,6 @@ func TestFailureOnCreation(t *testing.T) { iMgr.DeleteIPSet(TestCIDRSet.PrefixName) toDeleteSetNames := []string{TestCIDRSet.PrefixName} - assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ TestNSSet.PrefixName: { Id: TestNSSet.HashedName, @@ -209,13 +195,7 @@ func TestFailureOnCreation(t *testing.T) { }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) verifyDeletedHNSCache(t, toDeleteSetNames, hns) @@ -239,8 +219,6 @@ func TestFailureOnAddToList(t *testing.T) { iMgr.DeleteIPSet(TestCIDRSet.PrefixName) toDeleteSetNames := []string{TestCIDRSet.PrefixName} - assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ TestNSSet.PrefixName: { Id: TestNSSet.HashedName, @@ -267,13 +245,8 @@ func TestFailureOnAddToList(t *testing.T) { Values: TestNSSet.HashedName, }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) verifyDeletedHNSCache(t, toDeleteSetNames, hns) @@ -294,8 +267,6 @@ func TestFailureOnFlush(t *testing.T) { iMgr.DeleteIPSet(TestCIDRSet.PrefixName) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} - assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ TestNSSet.PrefixName: { Id: TestNSSet.HashedName, @@ -305,13 +276,7 @@ func TestFailureOnFlush(t *testing.T) { }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) verifyDeletedHNSCache(t, toDeleteSetNames, hns) @@ -331,8 +296,6 @@ func TestFailureOnDeletion(t *testing.T) { iMgr.DeleteIPSet(TestCIDRSet.PrefixName) toDeleteSetNames := []string{TestKVPodSet.PrefixName, TestCIDRSet.PrefixName} - assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache) - toAddOrUpdateSetMap := map[string]hcn.SetPolicySetting{ TestNSSet.PrefixName: { Id: TestNSSet.HashedName, @@ -342,13 +305,7 @@ func TestFailureOnDeletion(t *testing.T) { }, } - toAddOrUpdateSetNames := make([]string, 0, len(toAddOrUpdateSetMap)) - for setName := range toAddOrUpdateSetMap { - toAddOrUpdateSetNames = append(toAddOrUpdateSetNames, setName) - } - assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache) - - err := iMgr.applyIPSets() + err := iMgr.ApplyIPSets() require.NoError(t, err) verifyHNSCache(t, toAddOrUpdateSetMap, hns) verifyDeletedHNSCache(t, toDeleteSetNames, hns) @@ -356,7 +313,7 @@ func TestFailureOnDeletion(t *testing.T) { func verifyHNSCache(t *testing.T, expected map[string]hcn.SetPolicySetting, hns *hnswrapper.Hnsv2wrapperFake) { for setName, setObj := range expected { - cacheObj := hns.Cache.Policy(setObj.Id) + cacheObj := hns.Cache.SetPolicy(setObj.Id) require.NotNil(t, cacheObj) require.Equal(t, setObj, *cacheObj, fmt.Sprintf("%s mismatch in cache", setName)) } @@ -364,7 +321,7 @@ func verifyHNSCache(t *testing.T, expected map[string]hcn.SetPolicySetting, hns func verifyDeletedHNSCache(t *testing.T, deleted []string, hns *hnswrapper.Hnsv2wrapperFake) { for _, setName := range deleted { - cacheObj := hns.Cache.Policy(setName) + cacheObj := hns.Cache.SetPolicy(setName) require.Nil(t, cacheObj) } } diff --git a/npm/pkg/dataplane/ipsets/testutils.go b/npm/pkg/dataplane/ipsets/testutils.go index 79d83ec93b..0224b0c2f2 100644 --- a/npm/pkg/dataplane/ipsets/testutils.go +++ b/npm/pkg/dataplane/ipsets/testutils.go @@ -1,8 +1,6 @@ package ipsets -import ( - "github.com/Azure/azure-container-networking/npm/util" -) +import "github.com/Azure/azure-container-networking/npm/util" type TestSet struct { Metadata *IPSetMetadata diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 841c1cb459..59fd161e60 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -8,6 +8,11 @@ import ( "github.com/Microsoft/hcsshim/hcn" ) +const ( + blockRulePriotity = 3000 + allowRulePriotity = 222 +) + var ( protocolNumMap = map[Protocol]string{ TCP: "6", @@ -70,9 +75,9 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { policySettings.Action = getHCNAction(acl.Target) // TODO need to have better priority handling - policySettings.Priority = uint16(222) + policySettings.Priority = uint16(allowRulePriotity) if policySettings.Action == hcn.ActionTypeBlock { - policySettings.Priority = uint16(3000) + policySettings.Priority = uint16(blockRulePriotity) } if acl.Protocol == "" { acl.Protocol = AnyProtocol @@ -101,9 +106,11 @@ func (acl *ACLPolicy) convertToAclSettings() (*NPMACLPolSettings, error) { policySettings.LocalAddresses = srcListStr policySettings.RemoteAddresses = dstListStr policySettings.RemotePorts = dstPortStr + policySettings.LocalPorts = "" if policySettings.Direction == hcn.DirectionTypeOut { policySettings.LocalAddresses = dstListStr policySettings.LocalPorts = dstPortStr + policySettings.RemotePorts = "" policySettings.RemoteAddresses = srcListStr } diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index df5b05e8d6..333c92b420 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -1,14 +1,72 @@ package policies import ( + "fmt" + "reflect" "testing" "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +var ( + expectedACLs = []*hnswrapper.FakeEndpointPolicy{ + { + ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + Protocols: "6", + Direction: "In", + Action: "Block", + LocalAddresses: "azure-npm-3216600258", + RemoteAddresses: "azure-npm-2031808719", + RemotePorts: getPortStr(222, 333), + LocalPorts: "", + Priority: blockRulePriotity, + }, + { + ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + Protocols: "17", + Direction: "In", + Action: "Allow", + LocalAddresses: "azure-npm-3216600258", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: allowRulePriotity, + }, + { + ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + Protocols: "17", + Direction: "Out", + Action: "Block", + LocalAddresses: "", + RemoteAddresses: "azure-npm-3216600258", + LocalPorts: "144", + RemotePorts: "", + Priority: blockRulePriotity, + }, + { + ID: TestNetworkPolicies[0].ACLs[0].PolicyID, + Protocols: "256", + Direction: "Out", + Action: "Allow", + LocalAddresses: "", + RemoteAddresses: "azure-npm-3216600258", + LocalPorts: "", + RemotePorts: "", + Priority: allowRulePriotity, + }, + } + + endPointIDList = map[string]string{ + "10.0.0.1": "test1", + "10.0.0.2": "test2", + } +) + func TestCompareAndRemovePolicies(t *testing.T) { epbuilder := newEndpointPolicyBuilder() @@ -31,14 +89,51 @@ func TestCompareAndRemovePolicies(t *testing.T) { } func TestAddPolicies(t *testing.T) { + pMgr, hns := getPMgr(t) + err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) + require.NoError(t, err) + + aclID := TestNetworkPolicies[0].ACLs[0].PolicyID + + aclPolicies, err := hns.Cache.ACLPolicies(endPointIDList, aclID) + require.NoError(t, err) + for _, id := range endPointIDList { + acls, ok := aclPolicies[id] + if !ok { + t.Errorf("Expected %s to be in ACLs", id) + } + verifyFakeHNSCacheACLs(t, expectedACLs, acls) + } +} + +func TestRemovePolicies(t *testing.T) { + pMgr, hns := getPMgr(t) + err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) + require.NoError(t, err) + + aclID := TestNetworkPolicies[0].ACLs[0].PolicyID + + aclPolicies, err := hns.Cache.ACLPolicies(endPointIDList, aclID) + require.NoError(t, err) + for _, id := range endPointIDList { + acls, ok := aclPolicies[id] + if !ok { + t.Errorf("Expected %s to be in ACLs", id) + } + verifyFakeHNSCacheACLs(t, expectedACLs, acls) + } + + err = pMgr.RemovePolicy(TestNetworkPolicies[0].Name, nil) + require.NoError(t, err) + verifyACLCacheIsCleaned(t, hns, len(endPointIDList)) +} + +// Helper functions for UTS + +func getPMgr(t *testing.T) (*PolicyManager, *hnswrapper.Hnsv2wrapperFake) { hns := ipsets.GetHNSFake() io := common.NewMockIOShimWithFakeHNS(hns) - pMgr := NewPolicyManager(io) - endPointIDList := map[string]string{ - "10.0.0.1": "test1", - "10.0.0.2": "test2", - } for ip, epID := range endPointIDList { ep := &hcn.HostComputeEndpoint{ Id: epID, @@ -52,7 +147,54 @@ func TestAddPolicies(t *testing.T) { _, err := hns.CreateEndpoint(ep) require.NoError(t, err) } + return NewPolicyManager(io), hns +} - err := pMgr.AddPolicy(TestNetworkPolicies[0], endPointIDList) - require.NoError(t, err) +func verifyFakeHNSCacheACLs(t *testing.T, expected, actual []*hnswrapper.FakeEndpointPolicy) bool { + assert.Equal(t, + len(expected), + len(actual), + fmt.Sprintf("Expected %d ACL, got %d", len(TestNetworkPolicies[0].ACLs), len(actual)), + ) + for i, expectedACL := range expected { + foundACL := false + // While printing actual with %+v it only prints the pointers and it is hard to debug. + // So creating this errStr to print the actual values. + errStr := "" + for j, cacheACL := range actual { + assert.Equal(t, + expectedACL.ID, + actual[i].ID, + fmt.Sprintf("Expected %s, got %s", expectedACL.ID, actual[i].ID), + ) + if reflect.DeepEqual(expectedACL, cacheACL) { + foundACL = true + break + } + errStr += fmt.Sprintf("\n%d: %+v", j, cacheACL) + } + require.True(t, foundACL, fmt.Sprintf("Expected %+v to be in ACLs \n Got: %s ", expectedACL, errStr)) + } + return true +} + +func verifyACLCacheIsCleaned(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, lenOfEPs int) { + epACLs := hns.Cache.GetAllACLs() + assert.Equal(t, lenOfEPs, len(epACLs)) + for _, acls := range epACLs { + assert.Equal(t, 0, len(acls)) + } +} + +func getPortStr(start, end int32) string { + portStr := fmt.Sprintf("%d", start) + if start == end || end == 0 { + return portStr + } + + for i := start + 1; i <= end; i++ { + portStr += fmt.Sprintf(",%d", i) + } + + return portStr } diff --git a/npm/pkg/dataplane/policies/testutils.go b/npm/pkg/dataplane/policies/testutils.go index cbb0b63b9e..1e95c0d076 100644 --- a/npm/pkg/dataplane/policies/testutils.go +++ b/npm/pkg/dataplane/policies/testutils.go @@ -56,7 +56,7 @@ var ( Protocol: TCP, }, { - PolicyID: "test2", + PolicyID: "test1", Comment: "comment2", SrcList: []SetInfo{ { @@ -70,7 +70,7 @@ var ( Protocol: UDP, }, { - PolicyID: "test3", + PolicyID: "test1", Comment: "comment3", SrcList: []SetInfo{ { @@ -87,7 +87,7 @@ var ( Protocol: UDP, }, { - PolicyID: "test4", + PolicyID: "test1", Comment: "comment4", SrcList: []SetInfo{ { diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index b9cc39cc35..3f51629841 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -80,7 +80,7 @@ func main() { panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestNSSet.Metadata}, podMetadataB)) podMetadataC := &dataplane.PodMetadata{ PodKey: "c", - PodIP: "10.240.0.20", + PodIP: "10.240.0.28", NodeName: nodeName, } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataC)) @@ -125,14 +125,14 @@ func main() { podMetadataD = &dataplane.PodMetadata{ PodKey: "d", - PodIP: "10.240.0.7", + PodIP: "10.240.0.27", NodeName: nodeName, } panicOnError(dp.AddToSets([]*ipsets.IPSetMetadata{ipsets.TestKeyPodSet.Metadata, ipsets.TestNSSet.Metadata}, podMetadataD)) panicOnError(dp.ApplyDataPlane()) printAndWait(true) - // panicOnError(dp.RemovePolicy(testNetPol.Name)) + panicOnError(dp.RemovePolicy(testNetPol.Name)) // testPolicyManager() From 666b510f880c40d4831964ce1f90fec910974276 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 18 Nov 2021 14:40:03 -0800 Subject: [PATCH 11/15] addressing some lint issues --- common/ioshim_windows.go | 2 +- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 52 ++++++++++++------- .../ipsets/ipsetmanager_windows_test.go | 17 +++--- npm/pkg/dataplane/ipsets/testutils_linux.go | 4 -- npm/pkg/dataplane/ipsets/testutils_windows.go | 13 +++-- .../policies/policymanager_windows_test.go | 2 +- test/integration/npm/main.go | 3 +- 7 files changed, 49 insertions(+), 44 deletions(-) diff --git a/common/ioshim_windows.go b/common/ioshim_windows.go index 888f9fb129..e220135878 100644 --- a/common/ioshim_windows.go +++ b/common/ioshim_windows.go @@ -21,7 +21,7 @@ func NewIOShim() *IOShim { func NewMockIOShim(calls []testutils.TestCmd) *IOShim { return &IOShim{ Exec: testutils.GetFakeExecWithScripts(calls), - Hns: &hnswrapper.Hnsv2wrapperFake{}, + Hns: hnswrapper.NewHnsv2wrapperFake(), } } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 95b3550109..136de43c0f 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -4,6 +4,7 @@ import ( "os" "testing" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" testutils "github.com/Azure/azure-container-networking/test/utils" @@ -31,7 +32,26 @@ var ( ) func TestCreateIPSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) + + setMetadata := NewIPSetMetadata(testSetName, Namespace) + iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) + // creating twice + iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) + + assert.True(t, iMgr.exists(setMetadata.GetPrefixName())) + + set := iMgr.GetIPSet(setMetadata.GetPrefixName()) + require.NotNil(t, set) + assert.Equal(t, setMetadata.GetPrefixName(), set.Name) + assert.Equal(t, util.GetHashedName(setMetadata.GetPrefixName()), set.HashedName) + + err := iMgr.applyIPSets() + require.NoError(t, err) +} + +func TestCreateIPSetApplyAlways(t *testing.T) { + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -50,7 +70,7 @@ func TestCreateIPSet(t *testing.T) { } func TestAddToSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -75,7 +95,7 @@ func TestAddToSet(t *testing.T) { } func TestRemoveFromSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -89,7 +109,7 @@ func TestRemoveFromSet(t *testing.T) { } func TestRemoveFromSetMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) err := iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) @@ -99,7 +119,7 @@ func TestRemoveFromSetMissing(t *testing.T) { } func TestAddToListMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNamespace) err := iMgr.AddToLists([]*IPSetMetadata{listMetadata}, []*IPSetMetadata{setMetadata}) @@ -110,7 +130,7 @@ func TestAddToListMissing(t *testing.T) { } func TestAddToList(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata, listMetadata}) @@ -130,7 +150,7 @@ func TestAddToList(t *testing.T) { } func TestRemoveFromList(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata, listMetadata}) @@ -157,7 +177,7 @@ func TestRemoveFromList(t *testing.T) { } func TestRemoveFromListMissing(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNamespace) @@ -171,7 +191,7 @@ func TestRemoveFromListMissing(t *testing.T) { } func TestDeleteIPSet(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, Namespace) iMgr.CreateIPSets([]*IPSetMetadata{setMetadata}) @@ -180,7 +200,7 @@ func TestDeleteIPSet(t *testing.T) { } func TestGetIPsFromSelectorIPSets(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -229,7 +249,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) { } func TestAddDeleteSelectorReferences(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -305,7 +325,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { } func TestAddDeleteNetPolReferences(t *testing.T) { - iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, getMockIOShim([]testutils.TestCmd{})) + iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -382,11 +402,3 @@ func TestMain(m *testing.M) { os.Exit(exitCode) } - -func assertEqualContentsTestHelper(t *testing.T, setNames []string, cache map[string]struct{}) { - require.Equal(t, len(setNames), len(cache), "cache is different than list of set names") - for _, setName := range setNames { - _, exists := cache[setName] - require.True(t, exists, "cache is different than list of set names") - } -} diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go index 91a429681c..f2377e32af 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows_test.go @@ -12,7 +12,7 @@ import ( ) func TestAddToSetWindows(t *testing.T) { - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -39,14 +39,13 @@ func TestAddToSetWindows(t *testing.T) { } func TestDestroyNPMIPSets(t *testing.T) { - calls := []testutils.TestCmd{} // TODO - iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, getMockIOShim(calls)) + iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, common.NewMockIOShim([]testutils.TestCmd{})) require.NoError(t, iMgr.resetIPSets()) } // create all possible SetTypes func TestApplyCreationsAndAdds(t *testing.T) { - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -119,7 +118,7 @@ func TestApplyCreationsAndAdds(t *testing.T) { } func TestApplyDeletions(t *testing.T) { - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -167,7 +166,7 @@ func TestApplyDeletions(t *testing.T) { // TODO test that a reconcile list is updated func TestFailureOnCreation(t *testing.T) { - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -204,7 +203,7 @@ func TestFailureOnCreation(t *testing.T) { // TODO test that a reconcile list is updated func TestFailureOnAddToList(t *testing.T) { // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -255,7 +254,7 @@ func TestFailureOnAddToList(t *testing.T) { // TODO test that a reconcile list is updated func TestFailureOnFlush(t *testing.T) { // This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel. - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) @@ -284,7 +283,7 @@ func TestFailureOnFlush(t *testing.T) { // TODO test that a reconcile list is updated func TestFailureOnDeletion(t *testing.T) { - hns := GetHNSFake() + hns := GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) iMgr := NewIPSetManager(iMgrApplyAlwaysCfg, io) diff --git a/npm/pkg/dataplane/ipsets/testutils_linux.go b/npm/pkg/dataplane/ipsets/testutils_linux.go index c319ae610f..c31ededb57 100644 --- a/npm/pkg/dataplane/ipsets/testutils_linux.go +++ b/npm/pkg/dataplane/ipsets/testutils_linux.go @@ -16,10 +16,6 @@ var ( } ) -func getMockIOShim(calls []testutils.TestCmd) *common.IOShim { - return common.NewMockIOShim([]testutils.TestCmd{}) -} - func GetApplyIPSetsTestCalls(toAddOrUpdateIPSets, toDeleteIPSets []*IPSetMetadata) []testutils.TestCmd { if len(toAddOrUpdateIPSets) > 0 { return []testutils.TestCmd{ diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index 19a066693e..b354c81a26 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -1,24 +1,23 @@ package ipsets import ( - "github.com/Azure/azure-container-networking/common" + "testing" + "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" + "github.com/Azure/azure-container-networking/vendor/github.com/stretchr/testify/require" "github.com/Microsoft/hcsshim/hcn" ) -func getMockIOShim(_ []testutils.TestCmd) *common.IOShim { - return common.NewMockIOShimWithFakeHNS(GetHNSFake()) -} - -func GetHNSFake() *hnswrapper.Hnsv2wrapperFake { +func GetHNSFake(t *testing.T) *hnswrapper.Hnsv2wrapperFake { hns := hnswrapper.NewHnsv2wrapperFake() network := &hcn.HostComputeNetwork{ Id: "1234", Name: "azure", } - hns.CreateNetwork(network) + _, err := hns.CreateNetwork(network) + require.NoError(t, err) return hns } diff --git a/npm/pkg/dataplane/policies/policymanager_windows_test.go b/npm/pkg/dataplane/policies/policymanager_windows_test.go index 333c92b420..8551c663c5 100644 --- a/npm/pkg/dataplane/policies/policymanager_windows_test.go +++ b/npm/pkg/dataplane/policies/policymanager_windows_test.go @@ -131,7 +131,7 @@ func TestRemovePolicies(t *testing.T) { // Helper functions for UTS func getPMgr(t *testing.T) (*PolicyManager, *hnswrapper.Hnsv2wrapperFake) { - hns := ipsets.GetHNSFake() + hns := ipsets.GetHNSFake(t) io := common.NewMockIOShimWithFakeHNS(hns) for ip, epID := range endPointIDList { diff --git a/test/integration/npm/main.go b/test/integration/npm/main.go index 3f51629841..ab09942e96 100644 --- a/test/integration/npm/main.go +++ b/test/integration/npm/main.go @@ -134,8 +134,7 @@ func main() { panicOnError(dp.RemovePolicy(testNetPol.Name)) - // testPolicyManager() - + testPolicyManager() } func testPolicyManager() { From b3fca1ac29d59fab5833fa94143d926fe4810b23 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 18 Nov 2021 14:42:56 -0800 Subject: [PATCH 12/15] addressing some lint issues --- npm/pkg/dataplane/ipsets/testutils_linux.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/testutils_linux.go b/npm/pkg/dataplane/ipsets/testutils_linux.go index c31ededb57..0b063fb215 100644 --- a/npm/pkg/dataplane/ipsets/testutils_linux.go +++ b/npm/pkg/dataplane/ipsets/testutils_linux.go @@ -1,9 +1,6 @@ package ipsets -import ( - "github.com/Azure/azure-container-networking/common" - testutils "github.com/Azure/azure-container-networking/test/utils" -) +import testutils "github.com/Azure/azure-container-networking/test/utils" var ( ipsetRestoreStringSlice = []string{"ipset", "restore"} From 692e534405d78dde7b5f34d0039357ab0813d5df Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 18 Nov 2021 15:13:26 -0800 Subject: [PATCH 13/15] removing apply ipset in generic ipsetMgr tests --- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 136de43c0f..7b919642de 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -45,9 +45,6 @@ func TestCreateIPSet(t *testing.T) { require.NotNil(t, set) assert.Equal(t, setMetadata.GetPrefixName(), set.Name) assert.Equal(t, util.GetHashedName(setMetadata.GetPrefixName()), set.HashedName) - - err := iMgr.applyIPSets() - require.NoError(t, err) } func TestCreateIPSetApplyAlways(t *testing.T) { @@ -64,9 +61,6 @@ func TestCreateIPSetApplyAlways(t *testing.T) { require.NotNil(t, set) assert.Equal(t, setMetadata.GetPrefixName(), set.Name) assert.Equal(t, util.GetHashedName(setMetadata.GetPrefixName()), set.HashedName) - - err := iMgr.applyIPSets() - require.NoError(t, err) } func TestAddToSet(t *testing.T) { @@ -89,9 +83,6 @@ func TestAddToSet(t *testing.T) { iMgr.CreateIPSets([]*IPSetMetadata{listMetadata}) err = iMgr.AddToSets([]*IPSetMetadata{listMetadata}, testPodIP, testPodKey) require.Error(t, err) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestRemoveFromSet(t *testing.T) { @@ -103,9 +94,6 @@ func TestRemoveFromSet(t *testing.T) { require.NoError(t, err) err = iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestRemoveFromSetMissing(t *testing.T) { @@ -113,9 +101,6 @@ func TestRemoveFromSetMissing(t *testing.T) { setMetadata := NewIPSetMetadata(testSetName, Namespace) err := iMgr.RemoveFromSets([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestAddToListMissing(t *testing.T) { @@ -124,9 +109,6 @@ func TestAddToListMissing(t *testing.T) { listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNamespace) err := iMgr.AddToLists([]*IPSetMetadata{listMetadata}, []*IPSetMetadata{setMetadata}) require.NoError(t, err) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestAddToList(t *testing.T) { @@ -144,9 +126,6 @@ func TestAddToList(t *testing.T) { assert.Equal(t, util.GetHashedName(listMetadata.GetPrefixName()), set.HashedName) assert.Equal(t, 1, len(set.MemberIPSets)) assert.Equal(t, setMetadata.GetPrefixName(), set.MemberIPSets[setMetadata.GetPrefixName()].Name) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestRemoveFromList(t *testing.T) { @@ -171,9 +150,6 @@ func TestRemoveFromList(t *testing.T) { set = iMgr.GetIPSet(listMetadata.GetPrefixName()) assert.NotNil(t, set) assert.Equal(t, 0, len(set.MemberIPSets)) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestRemoveFromListMissing(t *testing.T) { @@ -185,9 +161,6 @@ func TestRemoveFromListMissing(t *testing.T) { err := iMgr.RemoveFromList(listMetadata, []*IPSetMetadata{setMetadata}) require.NoError(t, err) - - err = iMgr.applyIPSets() - require.NoError(t, err) } func TestDeleteIPSet(t *testing.T) { From ca897eb7702428b1e9f501a7cf0729fd91eda86f Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 18 Nov 2021 16:37:35 -0800 Subject: [PATCH 14/15] fixing a build issue with windows --- npm/pkg/dataplane/ipsets/testutils_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index b354c81a26..3421b3ee85 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -5,7 +5,7 @@ import ( "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" - "github.com/Azure/azure-container-networking/vendor/github.com/stretchr/testify/require" + "github.com/stretchr/testify/require" "github.com/Microsoft/hcsshim/hcn" ) From 0a114cab34431071c60b06d3271581343682a295 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 19 Nov 2021 12:10:19 -0800 Subject: [PATCH 15/15] fixing windows build issue --- common/ioshim_windows.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/ioshim_windows.go b/common/ioshim_windows.go index e220135878..6b6e31a21c 100644 --- a/common/ioshim_windows.go +++ b/common/ioshim_windows.go @@ -1,6 +1,8 @@ package common import ( + "testing" + "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" utilexec "k8s.io/utils/exec" @@ -31,3 +33,6 @@ func NewMockIOShimWithFakeHNS(hns *hnswrapper.Hnsv2wrapperFake) *IOShim { Hns: hns, } } + +// VerifyCalls is used for Unit Testing of linux. In windows this is no-op +func (ioshim *IOShim) VerifyCalls(_ *testing.T, _ []testutils.TestCmd) {}