From 2b07c7063ab4e40d06312efdd77cfcaaa0e7e129 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 7 Oct 2021 15:26:18 -0700 Subject: [PATCH 01/11] [NPM] Adding prefixes to IPSets in dataplane --- npm/pkg/dataplane/dataplane.go | 44 +- npm/pkg/dataplane/dataplane_test.go | 146 +++--- npm/pkg/dataplane/ipsets/ipset.go | 61 ++- npm/pkg/dataplane/ipsets/ipsetmanager.go | 85 +-- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 494 ++++++++---------- npm/pkg/dataplane/policies/policy.go | 4 +- npm/util/const.go | 8 +- 7 files changed, 413 insertions(+), 429 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 2f9c877703..28a47f72c1 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -65,19 +65,19 @@ func (dp *DataPlane) ResetDataPlane() error { } // CreateIPSet takes in a set object and updates local cache with this set -func (dp *DataPlane) CreateIPSet(setName string, setType ipsets.SetType) { - dp.ipsetMgr.CreateIPSet(setName, setType) +func (dp *DataPlane) CreateIPSet(setMetaData *ipsets.IPSetMetaData) { + dp.ipsetMgr.CreateIPSet(setMetaData) } // DeleteSet checks for members and references of the given "set" type ipset // if not used then will delete it from cache -func (dp *DataPlane) DeleteIPSet(name string) { - dp.ipsetMgr.DeleteIPSet(name) +func (dp *DataPlane) DeleteIPSet(setMetaData *ipsets.IPSetMetaData) { + dp.ipsetMgr.DeleteIPSet(setMetaData.GetPrefixName()) } // AddToSet takes in a list of IPSet names along with IP member // and then updates it local cache -func (dp *DataPlane) AddToSet(setNames []string, ip, podKey string) error { +func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSetMetaData, ip, podKey string) error { err := dp.ipsetMgr.AddToSet(setNames, ip, podKey) if err != nil { return fmt.Errorf("[DataPlane] error while adding to set: %w", err) @@ -87,7 +87,7 @@ func (dp *DataPlane) AddToSet(setNames []string, ip, podKey string) error { // RemoveFromSet takes in list of setnames from which a given IP member should be // removed and will update the local cache -func (dp *DataPlane) RemoveFromSet(setNames []string, ip, podKey string) error { +func (dp *DataPlane) RemoveFromSet(setNames []*ipsets.IPSetMetaData, ip, podKey string) error { err := dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) if err != nil { return fmt.Errorf("[DataPlane] error while removing from set: %w", err) @@ -97,7 +97,7 @@ func (dp *DataPlane) RemoveFromSet(setNames []string, ip, podKey string) error { // AddToList takes a list name and list of sets which are to be added as members // to given list -func (dp *DataPlane) AddToList(listName string, setNames []string) error { +func (dp *DataPlane) AddToList(listName *ipsets.IPSetMetaData, setNames []*ipsets.IPSetMetaData) error { err := dp.ipsetMgr.AddToList(listName, setNames) if err != nil { return fmt.Errorf("[DataPlane] error while adding to list: %w", err) @@ -107,7 +107,7 @@ func (dp *DataPlane) AddToList(listName string, setNames []string) error { // RemoveFromList takes a list name and list of sets which are to be removed as members // to given list -func (dp *DataPlane) RemoveFromList(listName string, setNames []string) error { +func (dp *DataPlane) RemoveFromList(listName *ipsets.IPSetMetaData, setNames []*ipsets.IPSetMetaData) error { err := dp.ipsetMgr.RemoveFromList(listName, setNames) if err != nil { return fmt.Errorf("[DataPlane] error while removing from list: %w", err) @@ -239,11 +239,11 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { return nil } -func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.IPSet, netpolName string, referenceType ipsets.ReferenceType) error { +func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { // Create IPSets first along with reference updates for _, set := range sets { - dp.ipsetMgr.CreateIPSet(set.Name, set.Type) - err := dp.ipsetMgr.AddReference(set.Name, netpolName, referenceType) + dp.ipsetMgr.CreateIPSet(set.MetaData) + err := dp.ipsetMgr.AddReference(set.MetaData.GetPrefixName(), netpolName, referenceType) if err != nil { npmErrorString := npmerrors.AddSelectorReference if referenceType == ipsets.NetPolType { @@ -259,13 +259,13 @@ func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.IPSet, netpolNam for _, set := range sets { // Check if any 2nd level IPSets are generated by Controller with members // Apply members to the list set - if set.Kind == ipsets.ListSet { + if ipsets.GetSetKind(set.MetaData.Type) == ipsets.ListSet { if len(set.MemberIPSets) > 0 { - memberList := []string{} + memberList := []*ipsets.IPSetMetaData{} for _, memberSet := range set.MemberIPSets { - memberList = append(memberList, memberSet.Name) + memberList = append(memberList, memberSet.MetaData) } - err := dp.ipsetMgr.AddToList(set.Name, memberList) + err := dp.ipsetMgr.AddToList(set.MetaData, memberList) if err != nil { npmErrorString := npmerrors.AddSelectorReference if referenceType == ipsets.NetPolType { @@ -281,11 +281,11 @@ func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.IPSet, netpolNam return nil } -func (dp *DataPlane) deleteIPSetReferences(sets map[string]*ipsets.IPSet, netpolName string, referenceType ipsets.ReferenceType) error { +func (dp *DataPlane) deleteIPSetReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { for _, set := range sets { // TODO ignore set does not exist error // TODO add delete ipset after removing members - err := dp.ipsetMgr.DeleteReference(set.Name, netpolName, referenceType) + err := dp.ipsetMgr.DeleteReference(set.MetaData.GetPrefixName(), netpolName, referenceType) if err != nil { npmErrorString := npmerrors.DeleteSelectorReference if referenceType == ipsets.NetPolType { @@ -302,13 +302,13 @@ func (dp *DataPlane) deleteIPSetReferences(sets map[string]*ipsets.IPSet, netpol // then we should not delete k1:v0:v1 members ( special case for nested ipsets ) for _, set := range sets { // Delete if any 2nd level IPSets are generated by Controller with members - if set.Kind == ipsets.ListSet { + if ipsets.GetSetKind(set.MetaData.Type) == ipsets.ListSet { if len(set.MemberIPSets) > 0 { - memberList := []string{} + memberList := []*ipsets.IPSetMetaData{} for _, memberSet := range set.MemberIPSets { - memberList = append(memberList, memberSet.Name) + memberList = append(memberList, memberSet.MetaData) } - err := dp.ipsetMgr.RemoveFromList(set.Name, memberList) + err := dp.ipsetMgr.RemoveFromList(set.MetaData, memberList) if err != nil { npmErrorString := npmerrors.DeleteSelectorReference if referenceType == ipsets.NetPolType { @@ -320,7 +320,7 @@ func (dp *DataPlane) deleteIPSetReferences(sets map[string]*ipsets.IPSet, netpol } // Try to delete these IPSets - dp.ipsetMgr.DeleteIPSet(set.Name) + dp.ipsetMgr.DeleteIPSet(set.MetaData.GetPrefixName()) } return nil } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 0693ca40e6..fceb45bd3c 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -5,6 +5,8 @@ import ( "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewDataPlane(t *testing.T) { @@ -15,75 +17,68 @@ func TestNewDataPlane(t *testing.T) { t.Error("NewDataPlane() returned nil") } - dp.CreateIPSet("test", ipsets.NameSpace) + setMetadata := ipsets.NewIPSetMetadata("test", ipsets.NameSpace) + dp.CreateIPSet(setMetadata) } func TestInitializeDataPlane(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") - if dp == nil { - t.Error("NewDataPlane() returned nil") - } - + assert.NotNil(t, dp) err := dp.InitializeDataPlane() - if err != nil { - t.Errorf("InitializeDataPlane() returned error %v", err) - } + require.NoError(t, err) } func TestResetDataPlane(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") - if dp == nil { - t.Error("NewDataPlane() returned nil") - } - + assert.NotNil(t, dp) err := dp.InitializeDataPlane() - if err != nil { - t.Errorf("InitializeDataPlane() returned error %v", err) - } + require.NoError(t, err) err = dp.ResetDataPlane() - if err != nil { - t.Errorf("ResetDataPlane() returned error %v", err) - } + require.NoError(t, err) } func TestCreateAndDeleteIpSets(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") - - setsTocreate := map[string]ipsets.SetType{ - "test": ipsets.NameSpace, - "test1": ipsets.NameSpace, + assert.NotNil(t, dp) + setsTocreate := []*ipsets.IPSetMetaData{ + { + Name: "test", + Type: ipsets.NameSpace, + }, + { + Name: "test1", + Type: ipsets.NameSpace, + }, } - for k, v := range setsTocreate { - dp.CreateIPSet(k, v) + for _, v := range setsTocreate { + dp.CreateIPSet(v) } // Creating again to see if duplicates get created - for k, v := range setsTocreate { - dp.CreateIPSet(k, v) + for _, v := range setsTocreate { + dp.CreateIPSet(v) } - for k := range setsTocreate { - set := dp.ipsetMgr.GetIPSet(k) - if set == nil { - t.Errorf("GetIPSet() for %s returned nil", k) - } + for _, v := range setsTocreate { + prefixedName := v.GetPrefixName() + set := dp.ipsetMgr.GetIPSet(prefixedName) + assert.NotNil(t, set) } - for k := range setsTocreate { - dp.DeleteIPSet(k) + for _, v := range setsTocreate { + dp.DeleteIPSet(v) } - for k := range setsTocreate { - set := dp.ipsetMgr.GetIPSet(k) - if set != nil { - t.Errorf("GetIPSet() for %s returned nil", k) - } + for _, v := range setsTocreate { + prefixedName := v.GetPrefixName() + set := dp.ipsetMgr.GetIPSet(prefixedName) + assert.Nil(t, set) } } @@ -91,63 +86,54 @@ func TestAddToSet(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") - setsTocreate := map[string]ipsets.SetType{ - "test": ipsets.NameSpace, - "test1": ipsets.NameSpace, + setsTocreate := []*ipsets.IPSetMetaData{ + { + Name: "test", + Type: ipsets.NameSpace, + }, + { + Name: "test1", + Type: ipsets.NameSpace, + }, } - for k, v := range setsTocreate { - dp.CreateIPSet(k, v) + for _, v := range setsTocreate { + dp.CreateIPSet(v) } - for k := range setsTocreate { - set := dp.ipsetMgr.GetIPSet(k) - if set == nil { - t.Errorf("GetIPSet() for %s returned nil", k) - } - } - setNames := make([]string, len(setsTocreate)) - i := 0 - for k := range setsTocreate { - setNames[i] = k - i++ + for _, v := range setsTocreate { + prefixedName := v.GetPrefixName() + set := dp.ipsetMgr.GetIPSet(prefixedName) + assert.NotNil(t, set) } - err := dp.AddToSet(setNames, "10.0.0.1", "testns/a") - if err != nil { - t.Errorf("AddToSet() returned error %v", err) - } + err := dp.AddToSet(setsTocreate, "10.0.0.1", "testns/a") + require.NoError(t, err) // Test IPV6 addess it should error out - err = dp.AddToSet(setNames, "2001:db8:0:0:0:0:2:1", "testns/a") - if err == nil { - t.Error("AddToSet() ipv6 did not return error") - } + err = dp.AddToSet(setsTocreate, "2001:db8:0:0:0:0:2:1", "testns/a") + require.Error(t, err) - for k := range setsTocreate { - dp.DeleteIPSet(k) + for _, v := range setsTocreate { + dp.DeleteIPSet(v) } - for k := range setsTocreate { - set := dp.ipsetMgr.GetIPSet(k) - if set == nil { - t.Errorf("GetIPSet() for %s returned nil", k) - } + for _, v := range setsTocreate { + prefixedName := v.GetPrefixName() + set := dp.ipsetMgr.GetIPSet(prefixedName) + assert.NotNil(t, set) } - err = dp.RemoveFromSet(setNames, "10.0.0.1", "testns/a") - if err != nil { - t.Errorf("RemoveFromSet() returned error %v", err) - } + err = dp.RemoveFromSet(setsTocreate, "10.0.0.1", "testns/a") + require.NoError(t, err) - for k := range setsTocreate { - dp.DeleteIPSet(k) + for _, v := range setsTocreate { + dp.DeleteIPSet(v) } - for k := range setsTocreate { - set := dp.ipsetMgr.GetIPSet(k) - if set != nil { - t.Errorf("GetIPSet() for %s returned nil", k) - } + for _, v := range setsTocreate { + prefixedName := v.GetPrefixName() + set := dp.ipsetMgr.GetIPSet(prefixedName) + assert.Nil(t, set) } } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index db44fd8dbe..8ea1ec0a28 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -33,6 +33,20 @@ type IPSet struct { kernelReferCount int } +type IPSetMetaData struct { + Name string + Type SetType +} + +type TranslatedIPSet struct { + MetaData *IPSetMetaData + // IpPodKey is used for setMaps to store Ips and ports as keys + // and podKey as value + IPPodKey map[string]string + // This is used for listMaps to store child IP Sets + MemberIPSets map[string]*TranslatedIPSet +} + type SetProperties struct { // Stores type of ip grouping Type SetType @@ -104,13 +118,14 @@ const ( NetPolType ReferenceType = "NetPol" ) -func NewIPSet(name string, setType SetType) *IPSet { +func NewIPSet(setMetaData *IPSetMetaData) *IPSet { + prefixedName := setMetaData.GetPrefixName() set := &IPSet{ - Name: name, - HashedName: util.GetHashedName(name), + Name: prefixedName, + HashedName: util.GetHashedName(prefixedName), SetProperties: SetProperties{ - Type: setType, - Kind: getSetKind(setType), + Type: setMetaData.Type, + Kind: GetSetKind(setMetaData.Type), }, // Map with Key as Network Policy name to to emulate set // and value as struct{} for minimal memory consumption @@ -129,6 +144,40 @@ func NewIPSet(name string, setType SetType) *IPSet { return set } +// NewIPSetMetadata is used for controllers to send in skeleton ipsets to DP +func NewIPSetMetadata(name string, setType SetType) *IPSetMetaData { + set := &IPSetMetaData{ + Name: name, + Type: setType, + } + return set +} + +func (setMetaData *IPSetMetaData) GetPrefixName() string { + switch setMetaData.Type { + case CIDRBlocks: + return fmt.Sprintf("%s%s", setMetaData.Name, util.CIDRPrefix) + case NameSpace: + return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespacePrefix) + case NamedPorts: + return fmt.Sprintf("%s%s", setMetaData.Name, util.NamedPortIPSetPrefix) + case KeyLabelOfPod: + return fmt.Sprintf("%s%s", setMetaData.Name, util.PodLabelPrefix) + case KeyValueLabelOfPod: + return fmt.Sprintf("%s%s", setMetaData.Name, util.PodLabelPrefix) + case KeyLabelOfNameSpace: + return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespaceLabelPrefix) + case KeyValueLabelOfNameSpace: + return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespaceLabelPrefix) + case NestedLabelOfPod: + return fmt.Sprintf("%s%s", setMetaData.Name, util.NestedLabelPrefix) + case Unknown: // adding this to appease golint + return "unknown" + default: + return "unknown" + } +} + func (set *IPSet) GetSetContents() ([]string, error) { switch set.Kind { case HashSet: @@ -199,7 +248,7 @@ func (set *IPSet) Compare(newSet *IPSet) bool { return true } -func getSetKind(setType SetType) SetKind { +func GetSetKind(setType SetType) SetKind { switch setType { case CIDRBlocks: return HashSet diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 4531f8bad2..f67ba32f12 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -48,16 +48,18 @@ func NewIPSetManager(networkName string) *IPSetManager { } } -func (iMgr *IPSetManager) CreateIPSet(setName string, setType SetType) { +func (iMgr *IPSetManager) CreateIPSet(setMetaData *IPSetMetaData) { iMgr.Lock() defer iMgr.Unlock() - if iMgr.exists(setName) { + prefixedName := setMetaData.GetPrefixName() + if iMgr.exists(prefixedName) { return } - iMgr.setMap[setName] = NewIPSet(setName, setType) + iMgr.setMap[prefixedName] = NewIPSet(setMetaData) metrics.IncNumIPSets() } +// DeleteIPSet expects the prefixed ipset name func (iMgr *IPSetManager) DeleteIPSet(name string) { iMgr.Lock() defer iMgr.Unlock() @@ -75,6 +77,7 @@ func (iMgr *IPSetManager) DeleteIPSet(name string) { metrics.DecNumIPSets() } +// GetIPSet needs the prefixed ipset name func (iMgr *IPSetManager) GetIPSet(name string) *IPSet { iMgr.Lock() defer iMgr.Unlock() @@ -84,6 +87,7 @@ func (iMgr *IPSetManager) GetIPSet(name string) *IPSet { return iMgr.setMap[name] } +// AddReference takes in the prefixed setname and adds relevant reference func (iMgr *IPSetManager) AddReference(setName, referenceName string, referenceType ReferenceType) error { iMgr.Lock() defer iMgr.Unlock() @@ -112,6 +116,7 @@ func (iMgr *IPSetManager) AddReference(setName, referenceName string, referenceT return nil } +// DeleteReference takes in the prefixed setname and removes relevant reference func (iMgr *IPSetManager) DeleteReference(setName, referenceName string, referenceType ReferenceType) error { iMgr.Lock() defer iMgr.Unlock() @@ -137,7 +142,7 @@ func (iMgr *IPSetManager) DeleteReference(setName, referenceName string, referen return nil } -func (iMgr *IPSetManager) AddToSet(addToSets []string, ip, podKey string) error { +func (iMgr *IPSetManager) AddToSet(addToSets []*IPSetMetaData, ip, podKey string) error { // check if the IP is IPV4 family if net.ParseIP(ip).To4() == nil { return npmerrors.Errorf(npmerrors.AppendIPSet, false, "IPV6 not supported") @@ -149,8 +154,9 @@ func (iMgr *IPSetManager) AddToSet(addToSets []string, ip, podKey string) error return err } - for _, setName := range addToSets { - set := iMgr.setMap[setName] + for _, setMetaData := range addToSets { + prefixedName := setMetaData.GetPrefixName() + set := iMgr.setMap[prefixedName] cachedPodKey, ok := set.IPPodKey[ip] set.IPPodKey[ip] = podKey if ok && cachedPodKey != podKey { @@ -159,13 +165,13 @@ func (iMgr *IPSetManager) AddToSet(addToSets []string, ip, podKey string) error continue } - iMgr.modifyCacheForKernelMemberUpdate(setName) - metrics.AddEntryToIPSet(setName) + iMgr.modifyCacheForKernelMemberUpdate(prefixedName) + metrics.AddEntryToIPSet(prefixedName) } return nil } -func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey string) error { +func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []*IPSetMetaData, ip, podKey string) error { iMgr.Lock() defer iMgr.Unlock() @@ -173,8 +179,9 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri return err } - for _, setName := range removeFromSets { - set := iMgr.setMap[setName] + for _, setMetaData := range removeFromSets { + prefixedName := setMetaData.GetPrefixName() + set := iMgr.setMap[prefixedName] // in case the IP belongs to a new Pod, then ignore this Delete call as this might be stale cachedPodKey, exists := set.IPPodKey[ip] @@ -183,27 +190,29 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []string, ip, podKey stri } if cachedPodKey != podKey { log.Logf("DeleteFromSet: PodOwner has changed for Ip: %s, setName:%s, Old podKey: %s, new podKey: %s. Ignore the delete as this is stale update", - ip, setName, cachedPodKey, podKey) + ip, prefixedName, cachedPodKey, podKey) continue } // update the IP ownership with podkey delete(set.IPPodKey, ip) - iMgr.modifyCacheForKernelMemberUpdate(setName) - metrics.RemoveEntryFromIPSet(setName) + iMgr.modifyCacheForKernelMemberUpdate(prefixedName) + metrics.RemoveEntryFromIPSet(prefixedName) } return nil } -func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { +func (iMgr *IPSetManager) AddToList(listMetaData *IPSetMetaData, setMetaDatas []*IPSetMetaData) error { iMgr.Lock() defer iMgr.Unlock() - if err := iMgr.checkForListMemberUpdateErrors(listName, setNames, npmerrors.AppendIPSet); err != nil { + if err := iMgr.checkForListMemberUpdateErrors(listMetaData, setMetaDatas, npmerrors.AppendIPSet); err != nil { return err } - for _, setName := range setNames { + listName := listMetaData.GetPrefixName() + for _, setMetaData := range setMetaDatas { + setName := setMetaData.GetPrefixName() iMgr.addMemberIPSet(listName, setName) } iMgr.modifyCacheForKernelMemberUpdate(listName) @@ -211,15 +220,17 @@ func (iMgr *IPSetManager) AddToList(listName string, setNames []string) error { return nil } -func (iMgr *IPSetManager) RemoveFromList(listName string, setNames []string) error { +func (iMgr *IPSetManager) RemoveFromList(listMetaData *IPSetMetaData, setMetaDatas []*IPSetMetaData) error { iMgr.Lock() defer iMgr.Unlock() - if err := iMgr.checkForListMemberUpdateErrors(listName, setNames, npmerrors.DeleteIPSet); err != nil { + if err := iMgr.checkForListMemberUpdateErrors(listMetaData, setMetaDatas, npmerrors.DeleteIPSet); err != nil { return err } - for _, setName := range setNames { + listName := listMetaData.GetPrefixName() + for _, setMetaData := range setMetaDatas { + setName := setMetaData.GetPrefixName() iMgr.removeMemberIPSet(listName, setName) } iMgr.modifyCacheForKernelMemberUpdate(listName) @@ -242,6 +253,7 @@ func (iMgr *IPSetManager) ApplyIPSets(networkID string) error { return nil } +// GetIPsFromSelectorIPSets will take in a map of prefixedSetNames and return an intersection of IPs func (iMgr *IPSetManager) GetIPsFromSelectorIPSets(setList map[string]struct{}) (map[string]struct{}, error) { if len(setList) == 0 { return map[string]struct{}{}, nil @@ -332,15 +344,16 @@ func (iMgr *IPSetManager) decKernelReferCountAndModifyCache(member *IPSet) { } } -func (iMgr *IPSetManager) checkForIPUpdateErrors(setNames []string, npmErrorString string) error { - for _, setName := range setNames { - if !iMgr.exists(setName) { - return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", setName)) +func (iMgr *IPSetManager) checkForIPUpdateErrors(setNames []*IPSetMetaData, npmErrorString string) error { + for _, set := range setNames { + prefixedSetName := set.GetPrefixName() + if !iMgr.exists(prefixedSetName) { + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", prefixedSetName)) } - set := iMgr.setMap[setName] + set := iMgr.setMap[prefixedSetName] if set.Kind != HashSet { - return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s is not a hash set", setName)) + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s is not a hash set", prefixedSetName)) } } return nil @@ -362,19 +375,21 @@ func (iMgr *IPSetManager) modifyCacheForKernelMemberUpdate(setName string) { } } -func (iMgr *IPSetManager) checkForListMemberUpdateErrors(listName string, memberNames []string, npmErrorString string) error { - if !iMgr.exists(listName) { - return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", listName)) +func (iMgr *IPSetManager) checkForListMemberUpdateErrors(listMetaData *IPSetMetaData, memberMetaDatas []*IPSetMetaData, npmErrorString string) error { + prefixedListName := listMetaData.GetPrefixName() + if !iMgr.exists(prefixedListName) { + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", prefixedListName)) } - list := iMgr.setMap[listName] + list := iMgr.setMap[prefixedListName] if list.Kind != ListSet { - return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s is not a list set", listName)) + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s is not a list set", prefixedListName)) } - for _, memberName := range memberNames { - if listName == memberName { - return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s cannot be added to itself", listName)) + for _, memberMetaData := range memberMetaDatas { + memberName := memberMetaData.GetPrefixName() + if prefixedListName == memberName { + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s cannot be added to itself", prefixedListName)) } if !iMgr.exists(memberName) { return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", memberName)) @@ -398,7 +413,7 @@ func (iMgr *IPSetManager) addMemberIPSet(listName, memberName string) { member := iMgr.setMap[memberName] - list.MemberIPSets[member.Name] = member + list.MemberIPSets[memberName] = member member.incIPSetReferCount() metrics.AddEntryToIPSet(list.Name) listIsInKernel := list.shouldBeInKernel() diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index f17f54a284..647008475a 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -2,11 +2,12 @@ package ipsets import ( "os" - "reflect" "testing" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -19,397 +20,326 @@ const ( func TestCreateIPSet(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetaData) // creating twice - iMgr.CreateIPSet(testSetName, NameSpace) + iMgr.CreateIPSet(setMetaData) - if !iMgr.exists(testSetName) { - t.Errorf("CreateIPSet() did not create set") - } + assert.True(t, iMgr.exists(setMetaData.GetPrefixName())) - set := iMgr.GetIPSet(testSetName) - if set == nil { - t.Errorf("CreateIPSet() did not create set") - } else { - if set.Name != testSetName { - t.Errorf("CreateIPSet() did not create set") - } - if set.HashedName != util.GetHashedName(testSetName) { - t.Errorf("CreateIPSet() did not create set") - } - } + 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) } func TestAddToSet(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetaData) - err := iMgr.AddToSet([]string{testSetName}, testPodIP, testPodKey) - if err != nil { - t.Errorf("AddToSet() returned error %s", err.Error()) - } + err := iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + require.NoError(t, err) - err = iMgr.AddToSet([]string{testSetName}, "2001:db8:0:0:0:0:2:1", "newpod") - if err == nil { - t.Error("AddToSet() did not return error") - } + err = iMgr.AddToSet([]*IPSetMetaData{setMetaData}, "2001:db8:0:0:0:0:2:1", "newpod") + require.Error(t, err) // same IP changed podkey - err = iMgr.AddToSet([]string{testSetName}, testPodIP, "newpod") - if err != nil { - t.Errorf("AddToSet() returned error %s", err.Error()) - } + err = iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, "newpod") + require.NoError(t, err) - iMgr.CreateIPSet("testipsetlist", KeyLabelOfNameSpace) - err = iMgr.AddToSet([]string{"testipsetlist"}, testPodIP, testPodKey) - if err == nil { - t.Error("AddToSet() should have returned error while adding member to listset") - } + listMetaData := NewIPSetMetadata("testipsetlist", KeyLabelOfNameSpace) + iMgr.CreateIPSet(listMetaData) + err = iMgr.AddToSet([]*IPSetMetaData{listMetaData}, testPodIP, testPodKey) + require.Error(t, err) } func TestRemoveFromSet(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) - err := iMgr.AddToSet([]string{testSetName}, testPodIP, testPodKey) - if err != nil { - t.Errorf("RemoveFromSet() returned error %s", err.Error()) - } - err = iMgr.RemoveFromSet([]string{testSetName}, testPodIP, testPodKey) - if err != nil { - t.Errorf("RemoveFromSet() returned error %s", err.Error()) - } + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetaData) + err := iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + require.NoError(t, err) + err = iMgr.RemoveFromSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + require.NoError(t, err) } func TestRemoveFromSetMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - err := iMgr.RemoveFromSet([]string{testSetName}, testPodIP, testPodKey) - if err == nil { - t.Errorf("RemoveFromSet() did not return error") - } + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + err := iMgr.RemoveFromSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + require.Error(t, err) } func TestAddToListMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - err := iMgr.AddToList(testPodKey, []string{"newtest"}) - if err == nil { - t.Errorf("AddToList() did not return error") - } + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + listMetaData := NewIPSetMetadata("testlabel", KeyLabelOfNameSpace) + err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + require.Error(t, err) } func TestAddToList(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) - iMgr.CreateIPSet(testListName, KeyLabelOfNameSpace) - - err := iMgr.AddToList(testListName, []string{testSetName}) - if err != nil { - t.Errorf("AddToList() returned error %s", err.Error()) - } - - set := iMgr.GetIPSet(testListName) - if set == nil { - t.Errorf("AddToList() did not create set") - } else { - if set.Name != testListName { - t.Errorf("AddToList() did not create set") - } - if set.HashedName != util.GetHashedName(testListName) { - t.Errorf("AddToList() did not create set") - } - if set.Type != KeyLabelOfNameSpace { - t.Errorf("AddToList() did not create set") - } - if set.MemberIPSets[testSetName].Name != testSetName { - t.Errorf("AddToList() did not add to list") - } - if len(set.MemberIPSets) == 0 { - t.Errorf("AddToList() failed") - } - } + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(setMetaData) + iMgr.CreateIPSet(listMetaData) + + err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + require.NoError(t, err) + + set := iMgr.GetIPSet(listMetaData.GetPrefixName()) + assert.NotNil(t, set) + assert.Equal(t, listMetaData.GetPrefixName(), set.Name) + 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) } func TestRemoveFromList(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) - iMgr.CreateIPSet(testListName, KeyLabelOfNameSpace) - - err := iMgr.AddToList(testListName, []string{testSetName}) - if err != nil { - t.Errorf("AddToList() returned error %s", err.Error()) - } - - set := iMgr.GetIPSet(testListName) - if set == nil { - t.Errorf("AddToList() did not create set") - } else { - if set.Name != testListName { - t.Errorf("AddToList() did not create set") - } - if set.HashedName != util.GetHashedName(testListName) { - t.Errorf("AddToList() did not create set") - } - if set.Type != KeyLabelOfNameSpace { - t.Errorf("AddToList() did not create set") - } - if set.MemberIPSets[testSetName].Name != testSetName { - t.Errorf("AddToList() did not add to list") - } - if len(set.MemberIPSets) == 0 { - t.Errorf("AddToList() failed") - } - } - - err = iMgr.RemoveFromList(testListName, []string{testSetName}) - if err != nil { - t.Errorf("RemoveFromList() returned error %s", err.Error()) - } - set = iMgr.GetIPSet(testListName) - if set == nil { - t.Errorf("RemoveFromList() failed") - } else if len(set.MemberIPSets) != 0 { - t.Errorf("RemoveFromList() failed") - } + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(setMetaData) + iMgr.CreateIPSet(listMetaData) + + err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + require.NoError(t, err) + + set := iMgr.GetIPSet(listMetaData.GetPrefixName()) + assert.NotNil(t, set) + assert.Equal(t, listMetaData.GetPrefixName(), set.Name) + 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.RemoveFromList(listMetaData, []*IPSetMetaData{setMetaData}) + require.NoError(t, err) + + set = iMgr.GetIPSet(listMetaData.GetPrefixName()) + assert.NotNil(t, set) + assert.Equal(t, 0, len(set.MemberIPSets)) } func TestRemoveFromListMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testListName, KeyLabelOfNameSpace) + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(listMetaData) - err := iMgr.RemoveFromList(testListName, []string{testSetName}) - if err == nil { - t.Errorf("RemoveFromList() did not return error") - } + err := iMgr.RemoveFromList(listMetaData, []*IPSetMetaData{setMetaData}) + require.Error(t, err) } func TestDeleteIPSet(t *testing.T) { iMgr := NewIPSetManager("azure") - iMgr.CreateIPSet(testSetName, NameSpace) + setMetaData := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetaData) - iMgr.DeleteIPSet(testSetName) + iMgr.DeleteIPSet(setMetaData.GetPrefixName()) // TODO add cache check } func TestGetIPsFromSelectorIPSets(t *testing.T) { iMgr := NewIPSetManager("azure") - - setsTocreate := map[string]SetType{ - "setNs1": NameSpace, - "setpod1": KeyLabelOfPod, - "setpod2": KeyLabelOfPod, - "setpod3": KeyValueLabelOfPod, - } - - for k, v := range setsTocreate { - iMgr.CreateIPSet(k, v) - } - - err := iMgr.AddToSet([]string{"setNs1", "setpod1", "setpod2", "setpod3"}, "10.0.0.1", "test") - if err != nil { - t.Errorf("AddToSet() returned error %s", err.Error()) - } - - err = iMgr.AddToSet([]string{"setNs1", "setpod1", "setpod2", "setpod3"}, "10.0.0.2", "test1") - if err != nil { - t.Errorf("AddToSet() returned error %s", err.Error()) - } - - err = iMgr.AddToSet([]string{"setNs1", "setpod2", "setpod3"}, "10.0.0.3", "test3") - if err != nil { - t.Errorf("AddToSet() returned error %s", err.Error()) - } - - ipsetList := map[string]struct{}{ - "setNs1": {}, - "setpod1": {}, - "setpod2": {}, - "setpod3": {}, + setsTocreate := []*IPSetMetaData{ + { + Name: "setNs1", + Type: NameSpace, + }, + { + Name: "setpod1", + Type: KeyLabelOfPod, + }, + { + Name: "setpod2", + Type: KeyLabelOfPod, + }, + { + Name: "setpod3", + Type: KeyValueLabelOfPod, + }, + } + + for _, v := range setsTocreate { + iMgr.CreateIPSet(v) + } + + err := iMgr.AddToSet(setsTocreate, "10.0.0.1", "test") + require.NoError(t, err) + + err = iMgr.AddToSet(setsTocreate, "10.0.0.2", "test1") + require.NoError(t, err) + + err = iMgr.AddToSet([]*IPSetMetaData{setsTocreate[0], setsTocreate[2], setsTocreate[3]}, "10.0.0.3", "test3") + require.NoError(t, err) + + ipsetList := map[string]struct{}{} + for _, v := range setsTocreate { + ipsetList[v.GetPrefixName()] = struct{}{} } ips, err := iMgr.GetIPsFromSelectorIPSets(ipsetList) - if err != nil { - t.Errorf("GetIPsFromSelectorIPSets() returned error %s", err.Error()) - } + require.NoError(t, err) - if len(ips) != 2 { - t.Errorf("GetIPsFromSelectorIPSets() returned wrong number of IPs %d", len(ips)) - t.Error(ips) - } + assert.Equal(t, 2, len(ips)) expectedintersection := map[string]struct{}{ "10.0.0.1": {}, "10.0.0.2": {}, } - if reflect.DeepEqual(ips, expectedintersection) == false { - t.Errorf("GetIPsFromSelectorIPSets() returned wrong IPs") - } + assert.Equal(t, ips, expectedintersection) } func TestAddDeleteSelectorReferences(t *testing.T) { iMgr := NewIPSetManager("azure") - - setsTocreate := map[string]SetType{ - "setNs1": NameSpace, - "setpod1": KeyLabelOfPod, - "setpod2": KeyValueLabelOfPod, - "setpod3": NestedLabelOfPod, - "setpod4": KeyLabelOfPod, + setsTocreate := []*IPSetMetaData{ + { + Name: "setNs1", + Type: NameSpace, + }, + { + Name: "setpod1", + Type: KeyLabelOfPod, + }, + { + Name: "setpod2", + Type: KeyLabelOfPod, + }, + { + Name: "setpod3", + Type: NestedLabelOfPod, + }, + { + Name: "setpod4", + Type: KeyLabelOfPod, + }, } networkPolicName := "testNetworkPolicy" - for k := range setsTocreate { - err := iMgr.AddReference(k, networkPolicName, SelectorType) - if err == nil { - t.Errorf("AddReference did not return error") - } + for _, k := range setsTocreate { + err := iMgr.AddReference(k.GetPrefixName(), networkPolicName, SelectorType) + require.Error(t, err) } - for k, v := range setsTocreate { - iMgr.CreateIPSet(k, v) - } - err := iMgr.AddToList("setpod3", []string{"setpod4"}) - if err != nil { - t.Errorf("AddToList failed with error %s", err.Error()) - } - - for k := range setsTocreate { - err = iMgr.AddReference(k, networkPolicName, SelectorType) - if err != nil { - t.Errorf("AddReference failed with error %s", err.Error()) - } + for _, v := range setsTocreate { + iMgr.CreateIPSet(v) } + // Add setpod4 to setpod3 + err := iMgr.AddToList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + require.NoError(t, err) - if len(iMgr.toAddOrUpdateCache) != 5 { - t.Errorf("AddReference did not update toAddOrUpdateCache") + for _, v := range setsTocreate { + err = iMgr.AddReference(v.GetPrefixName(), networkPolicName, SelectorType) + require.NoError(t, err) } - if len(iMgr.toDeleteCache) != 0 { - t.Errorf("AddReference did not update toDeleteCache") - } + assert.Equal(t, 5, len(iMgr.toAddOrUpdateCache)) + assert.Equal(t, 0, len(iMgr.toDeleteCache)) - for k := range setsTocreate { - err = iMgr.DeleteReference(k, networkPolicName, SelectorType) + for _, v := range setsTocreate { + err = iMgr.DeleteReference(v.GetPrefixName(), networkPolicName, SelectorType) if err != nil { t.Errorf("DeleteReference failed with error %s", err.Error()) } } - if len(iMgr.toAddOrUpdateCache) != 0 { - t.Errorf("DeleteReference did not update toAddOrUpdateCache") - } + assert.Equal(t, 0, len(iMgr.toAddOrUpdateCache)) + assert.Equal(t, 5, len(iMgr.toDeleteCache)) - if len(iMgr.toDeleteCache) != 5 { - t.Errorf("DeleteReference did not update toDeleteCache") - } - - for k := range setsTocreate { - iMgr.DeleteIPSet(k) + for _, v := range setsTocreate { + iMgr.DeleteIPSet(v.GetPrefixName()) } // Above delete will not remove setpod3 and setpod4 // because they are referencing each other - if len(iMgr.setMap) != 2 { - t.Errorf("DeleteIPSet did not remove deletable sets") - } + assert.Equal(t, 2, len(iMgr.setMap)) - err = iMgr.RemoveFromList("setpod3", []string{"setpod4"}) - if err != nil { - t.Errorf("RemoveFromList failed with error %s", err.Error()) - } + err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + require.NoError(t, err) - for k := range setsTocreate { - iMgr.DeleteIPSet(k) + for _, v := range setsTocreate { + iMgr.DeleteIPSet(v.GetPrefixName()) } - for k := range setsTocreate { - set := iMgr.GetIPSet(k) - if set != nil { - t.Errorf("DeleteIPSet did not delete %s IPSet", set.Name) - } + for _, v := range setsTocreate { + set := iMgr.GetIPSet(v.GetPrefixName()) + assert.Nil(t, set) } } func TestAddDeleteNetPolReferences(t *testing.T) { iMgr := NewIPSetManager("azure") - - setsTocreate := map[string]SetType{ - "setNs1": NameSpace, - "setpod1": KeyLabelOfPod, - "setpod2": KeyValueLabelOfPod, - "setpod3": NestedLabelOfPod, - "setpod4": KeyLabelOfPod, + setsTocreate := []*IPSetMetaData{ + { + Name: "setNs1", + Type: NameSpace, + }, + { + Name: "setpod1", + Type: KeyLabelOfPod, + }, + { + Name: "setpod2", + Type: KeyLabelOfPod, + }, + { + Name: "setpod3", + Type: NestedLabelOfPod, + }, + { + Name: "setpod4", + Type: KeyLabelOfPod, + }, } networkPolicName := "testNetworkPolicy" - for k, v := range setsTocreate { - iMgr.CreateIPSet(k, v) - } - err := iMgr.AddToList("setpod3", []string{"setpod4"}) - if err != nil { - t.Errorf("AddToList failed with error %s", err.Error()) - } - - for k := range setsTocreate { - err = iMgr.AddReference(k, networkPolicName, NetPolType) - if err != nil { - t.Errorf("AddReference failed with error %s", err.Error()) - } - } - - if len(iMgr.toAddOrUpdateCache) != 5 { - t.Errorf("AddReference did not update toAddOrUpdateCache") + for _, v := range setsTocreate { + iMgr.CreateIPSet(v) } + err := iMgr.AddToList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + require.NoError(t, err) - if len(iMgr.toDeleteCache) != 0 { - t.Errorf("AddReference did not update toDeleteCache") + for _, v := range setsTocreate { + err = iMgr.AddReference(v.GetPrefixName(), networkPolicName, NetPolType) + require.NoError(t, err) } - for k := range setsTocreate { - err = iMgr.DeleteReference(k, networkPolicName, NetPolType) - if err != nil { - t.Errorf("DeleteReference failed with error %s", err.Error()) - } - } - - if len(iMgr.toAddOrUpdateCache) != 0 { - t.Errorf("DeleteReference did not update toAddOrUpdateCache") + assert.Equal(t, 5, len(iMgr.toAddOrUpdateCache)) + assert.Equal(t, 0, len(iMgr.toDeleteCache)) + for _, v := range setsTocreate { + err = iMgr.DeleteReference(v.GetPrefixName(), networkPolicName, NetPolType) + require.NoError(t, err) } - if len(iMgr.toDeleteCache) != 5 { - t.Errorf("DeleteReference did not update toDeleteCache") - } + assert.Equal(t, 0, len(iMgr.toAddOrUpdateCache)) + assert.Equal(t, 5, len(iMgr.toDeleteCache)) - for k := range setsTocreate { - iMgr.DeleteIPSet(k) + for _, v := range setsTocreate { + iMgr.DeleteIPSet(v.GetPrefixName()) } // Above delete will not remove setpod3 and setpod4 // because they are referencing each other - if len(iMgr.setMap) != 2 { - t.Errorf("DeleteIPSet did not remove deletable sets") - } + assert.Equal(t, 2, len(iMgr.setMap)) - err = iMgr.RemoveFromList("setpod3", []string{"setpod4"}) - if err != nil { - t.Errorf("RemoveFromList failed with error %s", err.Error()) - } + err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + require.NoError(t, err) - for k := range setsTocreate { - iMgr.DeleteIPSet(k) + for _, v := range setsTocreate { + iMgr.DeleteIPSet(v.GetPrefixName()) } - for k := range setsTocreate { - set := iMgr.GetIPSet(k) - if set != nil { - t.Errorf("DeleteIPSet did not delete %s IPSet", set.Name) - } + for _, v := range setsTocreate { + set := iMgr.GetIPSet(v.GetPrefixName()) + assert.Nil(t, set) } - for k := range setsTocreate { - err = iMgr.DeleteReference(k, networkPolicName, NetPolType) - if err == nil { - t.Errorf("DeleteReference did not fail with error for ipset %s", k) - } + for _, v := range setsTocreate { + err = iMgr.DeleteReference(v.GetPrefixName(), networkPolicName, NetPolType) + require.Error(t, err) } } diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index b513a22370..711625a4e0 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -8,10 +8,10 @@ import ( type NPMNetworkPolicy struct { Name string // PodSelectorIPSets holds all the IPSets generated from Pod Selector - PodSelectorIPSets map[string]*ipsets.IPSet + PodSelectorIPSets map[string]*ipsets.TranslatedIPSet // RuleIPSets holds all IPSets generated from policy's rules // and not from pod selector IPSets - RuleIPSets map[string]*ipsets.IPSet + RuleIPSets map[string]*ipsets.TranslatedIPSet ACLs []*ACLPolicy // podIP is key and endpoint ID as value // Will be populated by dataplane and policy manager diff --git a/npm/util/const.go b/npm/util/const.go index c177f1c2cd..ebb2453a94 100644 --- a/npm/util/const.go +++ b/npm/util/const.go @@ -142,9 +142,13 @@ const ( // Prefixes for ipsets NamedPortIPSetPrefix string = "namedport:" + NamespacePrefix string = "ns-" + NamespaceLabelPrefix string = "nslabel-" + PodLabelPrefix string = "podlabel-" + CIDRPrefix string = "cidr-" + NestedLabelPrefix string = "nestedlabel-" - NamespacePrefix string = "ns-" - NegationPrefix string = "not-" + NegationPrefix string = "not-" SetPolicyDelimiter string = "," ) From e50f457178f4a50f392e1cd63dabbc35ae164dd3 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 7 Oct 2021 15:42:50 -0700 Subject: [PATCH 02/11] Correcting a linting issue --- npm/pkg/dataplane/dataplane.go | 12 ++++++------ npm/pkg/dataplane/ipsets/ipset.go | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 28a47f72c1..fb58f4c046 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -147,14 +147,14 @@ func (dp *DataPlane) ApplyDataPlane() error { func (dp *DataPlane) AddPolicy(policy *policies.NPMNetworkPolicy) error { klog.Infof("[DataPlane] Add Policy called for %s", policy.Name) // Create and add references for Selector IPSets first - err := dp.addIPSetReferences(policy.PodSelectorIPSets, policy.Name, ipsets.SelectorType) + err := dp.createIPSetsAndReferences(policy.PodSelectorIPSets, policy.Name, ipsets.SelectorType) if err != nil { klog.Infof("[DataPlane] error while adding Selector IPSet references: %s", err.Error()) return fmt.Errorf("[DataPlane] error while adding Selector IPSet references: %w", err) } // Create and add references for Rule IPSets - err = dp.addIPSetReferences(policy.RuleIPSets, policy.Name, ipsets.NetPolType) + err = dp.createIPSetsAndReferences(policy.RuleIPSets, policy.Name, ipsets.NetPolType) if err != nil { klog.Infof("[DataPlane] error while adding Rule IPSet references: %s", err.Error()) return fmt.Errorf("[DataPlane] error while adding Rule IPSet references: %w", err) @@ -194,13 +194,13 @@ func (dp *DataPlane) RemovePolicy(policyName string) error { return fmt.Errorf("[DataPlane] error while removing policy: %w", err) } // Remove references for Rule IPSets first - err = dp.deleteIPSetReferences(policy.RuleIPSets, policy.Name, ipsets.NetPolType) + err = dp.deleteIPSetsAndReferences(policy.RuleIPSets, policy.Name, ipsets.NetPolType) if err != nil { return err } // Remove references for Selector IPSets - err = dp.deleteIPSetReferences(policy.PodSelectorIPSets, policy.Name, ipsets.SelectorType) + err = dp.deleteIPSetsAndReferences(policy.PodSelectorIPSets, policy.Name, ipsets.SelectorType) if err != nil { return err } @@ -239,7 +239,7 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { return nil } -func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { +func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { // Create IPSets first along with reference updates for _, set := range sets { dp.ipsetMgr.CreateIPSet(set.MetaData) @@ -281,7 +281,7 @@ func (dp *DataPlane) addIPSetReferences(sets map[string]*ipsets.TranslatedIPSet, return nil } -func (dp *DataPlane) deleteIPSetReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { +func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { for _, set := range sets { // TODO ignore set does not exist error // TODO add delete ipset after removing members diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 8ea1ec0a28..5eecfae92c 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -58,7 +58,7 @@ type SetType int8 const ( // Unknown SetType - Unknown SetType = 0 + UnknownType SetType = 0 // NameSpace IPSet is created to hold // ips of pods in a given NameSapce NameSpace SetType = 1 @@ -78,11 +78,13 @@ const ( NestedLabelOfPod SetType = 7 // CIDRBlocks holds CIDR blocks CIDRBlocks SetType = 8 + // UnknownString const for unknown string + Unknown string = "unknown" ) var ( setTypeName = map[SetType]string{ - Unknown: "Unknown", + UnknownType: Unknown, NameSpace: "NameSpace", KeyLabelOfNameSpace: "KeyLabelOfNameSpace", KeyValueLabelOfNameSpace: "KeyValueLabelOfNameSpace", @@ -107,6 +109,8 @@ const ( ListSet SetKind = "list" // HashSet is of kind hashset with members as IPs and/or port HashSet SetKind = "set" + // UnknownKind is returned when kind is unknown + UnknownKind SetKind = "unknown" ) // ReferenceType specifies the kind of reference for an IPSet @@ -171,10 +175,10 @@ func (setMetaData *IPSetMetaData) GetPrefixName() string { return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespaceLabelPrefix) case NestedLabelOfPod: return fmt.Sprintf("%s%s", setMetaData.Name, util.NestedLabelPrefix) - case Unknown: // adding this to appease golint - return "unknown" + case UnknownType: // adding this to appease golint + return Unknown default: - return "unknown" + return Unknown } } @@ -266,10 +270,10 @@ func GetSetKind(setType SetType) SetKind { return ListSet case NestedLabelOfPod: return ListSet - case Unknown: // adding this to appease golint - return "unknown" + case UnknownType: // adding this to appease golint + return UnknownKind default: - return "unknown" + return UnknownKind } } From 26ca48e7b67002254319f6c185b41a8e43b1049f Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Thu, 7 Oct 2021 16:46:52 -0700 Subject: [PATCH 03/11] Using the correct case for metadata --- npm/pkg/dataplane/dataplane.go | 40 +++--- npm/pkg/dataplane/dataplane_test.go | 4 +- npm/pkg/dataplane/ipsets/ipset.go | 38 +++--- npm/pkg/dataplane/ipsets/ipsetmanager.go | 48 +++---- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 118 +++++++++--------- .../dataplane/ipsets/ipsetmanager_windows.go | 11 +- 6 files changed, 128 insertions(+), 131 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index fb58f4c046..1336c216ca 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -65,19 +65,19 @@ func (dp *DataPlane) ResetDataPlane() error { } // CreateIPSet takes in a set object and updates local cache with this set -func (dp *DataPlane) CreateIPSet(setMetaData *ipsets.IPSetMetaData) { - dp.ipsetMgr.CreateIPSet(setMetaData) +func (dp *DataPlane) CreateIPSet(setMetadata *ipsets.IPSetMetadata) { + dp.ipsetMgr.CreateIPSet(setMetadata) } // DeleteSet checks for members and references of the given "set" type ipset // if not used then will delete it from cache -func (dp *DataPlane) DeleteIPSet(setMetaData *ipsets.IPSetMetaData) { - dp.ipsetMgr.DeleteIPSet(setMetaData.GetPrefixName()) +func (dp *DataPlane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) { + dp.ipsetMgr.DeleteIPSet(setMetadata.GetPrefixName()) } // AddToSet takes in a list of IPSet names along with IP member // and then updates it local cache -func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSetMetaData, ip, podKey string) error { +func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error { err := dp.ipsetMgr.AddToSet(setNames, ip, podKey) if err != nil { return fmt.Errorf("[DataPlane] error while adding to set: %w", err) @@ -87,7 +87,7 @@ func (dp *DataPlane) AddToSet(setNames []*ipsets.IPSetMetaData, ip, podKey strin // RemoveFromSet takes in list of setnames from which a given IP member should be // removed and will update the local cache -func (dp *DataPlane) RemoveFromSet(setNames []*ipsets.IPSetMetaData, ip, podKey string) error { +func (dp *DataPlane) RemoveFromSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error { err := dp.ipsetMgr.RemoveFromSet(setNames, ip, podKey) if err != nil { return fmt.Errorf("[DataPlane] error while removing from set: %w", err) @@ -97,7 +97,7 @@ func (dp *DataPlane) RemoveFromSet(setNames []*ipsets.IPSetMetaData, ip, podKey // AddToList takes a list name and list of sets which are to be added as members // to given list -func (dp *DataPlane) AddToList(listName *ipsets.IPSetMetaData, setNames []*ipsets.IPSetMetaData) error { +func (dp *DataPlane) AddToList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error { err := dp.ipsetMgr.AddToList(listName, setNames) if err != nil { return fmt.Errorf("[DataPlane] error while adding to list: %w", err) @@ -107,7 +107,7 @@ func (dp *DataPlane) AddToList(listName *ipsets.IPSetMetaData, setNames []*ipset // RemoveFromList takes a list name and list of sets which are to be removed as members // to given list -func (dp *DataPlane) RemoveFromList(listName *ipsets.IPSetMetaData, setNames []*ipsets.IPSetMetaData) error { +func (dp *DataPlane) RemoveFromList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error { err := dp.ipsetMgr.RemoveFromList(listName, setNames) if err != nil { return fmt.Errorf("[DataPlane] error while removing from list: %w", err) @@ -242,8 +242,8 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { // Create IPSets first along with reference updates for _, set := range sets { - dp.ipsetMgr.CreateIPSet(set.MetaData) - err := dp.ipsetMgr.AddReference(set.MetaData.GetPrefixName(), netpolName, referenceType) + dp.ipsetMgr.CreateIPSet(set.Metadata) + err := dp.ipsetMgr.AddReference(set.Metadata.GetPrefixName(), netpolName, referenceType) if err != nil { npmErrorString := npmerrors.AddSelectorReference if referenceType == ipsets.NetPolType { @@ -259,13 +259,13 @@ func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.Translate for _, set := range sets { // Check if any 2nd level IPSets are generated by Controller with members // Apply members to the list set - if ipsets.GetSetKind(set.MetaData.Type) == ipsets.ListSet { + if ipsets.GetSetKind(set.Metadata.Type) == ipsets.ListSet { if len(set.MemberIPSets) > 0 { - memberList := []*ipsets.IPSetMetaData{} + memberList := []*ipsets.IPSetMetadata{} for _, memberSet := range set.MemberIPSets { - memberList = append(memberList, memberSet.MetaData) + memberList = append(memberList, memberSet.Metadata) } - err := dp.ipsetMgr.AddToList(set.MetaData, memberList) + err := dp.ipsetMgr.AddToList(set.Metadata, memberList) if err != nil { npmErrorString := npmerrors.AddSelectorReference if referenceType == ipsets.NetPolType { @@ -285,7 +285,7 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.Translate for _, set := range sets { // TODO ignore set does not exist error // TODO add delete ipset after removing members - err := dp.ipsetMgr.DeleteReference(set.MetaData.GetPrefixName(), netpolName, referenceType) + err := dp.ipsetMgr.DeleteReference(set.Metadata.GetPrefixName(), netpolName, referenceType) if err != nil { npmErrorString := npmerrors.DeleteSelectorReference if referenceType == ipsets.NetPolType { @@ -302,13 +302,13 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.Translate // then we should not delete k1:v0:v1 members ( special case for nested ipsets ) for _, set := range sets { // Delete if any 2nd level IPSets are generated by Controller with members - if ipsets.GetSetKind(set.MetaData.Type) == ipsets.ListSet { + if ipsets.GetSetKind(set.Metadata.Type) == ipsets.ListSet { if len(set.MemberIPSets) > 0 { - memberList := []*ipsets.IPSetMetaData{} + memberList := []*ipsets.IPSetMetadata{} for _, memberSet := range set.MemberIPSets { - memberList = append(memberList, memberSet.MetaData) + memberList = append(memberList, memberSet.Metadata) } - err := dp.ipsetMgr.RemoveFromList(set.MetaData, memberList) + err := dp.ipsetMgr.RemoveFromList(set.Metadata, memberList) if err != nil { npmErrorString := npmerrors.DeleteSelectorReference if referenceType == ipsets.NetPolType { @@ -320,7 +320,7 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.Translate } // Try to delete these IPSets - dp.ipsetMgr.DeleteIPSet(set.MetaData.GetPrefixName()) + dp.ipsetMgr.DeleteIPSet(set.Metadata.GetPrefixName()) } return nil } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index fceb45bd3c..d2d6ce9e4d 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -45,7 +45,7 @@ func TestCreateAndDeleteIpSets(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") assert.NotNil(t, dp) - setsTocreate := []*ipsets.IPSetMetaData{ + setsTocreate := []*ipsets.IPSetMetadata{ { Name: "test", Type: ipsets.NameSpace, @@ -86,7 +86,7 @@ func TestAddToSet(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode") - setsTocreate := []*ipsets.IPSetMetaData{ + setsTocreate := []*ipsets.IPSetMetadata{ { Name: "test", Type: ipsets.NameSpace, diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 5eecfae92c..f903966779 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -33,13 +33,13 @@ type IPSet struct { kernelReferCount int } -type IPSetMetaData struct { +type IPSetMetadata struct { Name string Type SetType } type TranslatedIPSet struct { - MetaData *IPSetMetaData + Metadata *IPSetMetadata // IpPodKey is used for setMaps to store Ips and ports as keys // and podKey as value IPPodKey map[string]string @@ -78,7 +78,7 @@ const ( NestedLabelOfPod SetType = 7 // CIDRBlocks holds CIDR blocks CIDRBlocks SetType = 8 - // UnknownString const for unknown string + // Unknown const for unknown string Unknown string = "unknown" ) @@ -122,14 +122,14 @@ const ( NetPolType ReferenceType = "NetPol" ) -func NewIPSet(setMetaData *IPSetMetaData) *IPSet { - prefixedName := setMetaData.GetPrefixName() +func NewIPSet(setMetadata *IPSetMetadata) *IPSet { + prefixedName := setMetadata.GetPrefixName() set := &IPSet{ Name: prefixedName, HashedName: util.GetHashedName(prefixedName), SetProperties: SetProperties{ - Type: setMetaData.Type, - Kind: GetSetKind(setMetaData.Type), + Type: setMetadata.Type, + Kind: GetSetKind(setMetadata.Type), }, // Map with Key as Network Policy name to to emulate set // and value as struct{} for minimal memory consumption @@ -149,32 +149,32 @@ func NewIPSet(setMetaData *IPSetMetaData) *IPSet { } // NewIPSetMetadata is used for controllers to send in skeleton ipsets to DP -func NewIPSetMetadata(name string, setType SetType) *IPSetMetaData { - set := &IPSetMetaData{ +func NewIPSetMetadata(name string, setType SetType) *IPSetMetadata { + set := &IPSetMetadata{ Name: name, Type: setType, } return set } -func (setMetaData *IPSetMetaData) GetPrefixName() string { - switch setMetaData.Type { +func (setMetadata *IPSetMetadata) GetPrefixName() string { + switch setMetadata.Type { case CIDRBlocks: - return fmt.Sprintf("%s%s", setMetaData.Name, util.CIDRPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.CIDRPrefix) case NameSpace: - return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespacePrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespacePrefix) case NamedPorts: - return fmt.Sprintf("%s%s", setMetaData.Name, util.NamedPortIPSetPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.NamedPortIPSetPrefix) case KeyLabelOfPod: - return fmt.Sprintf("%s%s", setMetaData.Name, util.PodLabelPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.PodLabelPrefix) case KeyValueLabelOfPod: - return fmt.Sprintf("%s%s", setMetaData.Name, util.PodLabelPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.PodLabelPrefix) case KeyLabelOfNameSpace: - return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespaceLabelPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespaceLabelPrefix) case KeyValueLabelOfNameSpace: - return fmt.Sprintf("%s%s", setMetaData.Name, util.NamespaceLabelPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespaceLabelPrefix) case NestedLabelOfPod: - return fmt.Sprintf("%s%s", setMetaData.Name, util.NestedLabelPrefix) + return fmt.Sprintf("%s%s", setMetadata.Name, util.NestedLabelPrefix) case UnknownType: // adding this to appease golint return Unknown default: diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index f67ba32f12..3053281b5c 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -48,14 +48,14 @@ func NewIPSetManager(networkName string) *IPSetManager { } } -func (iMgr *IPSetManager) CreateIPSet(setMetaData *IPSetMetaData) { +func (iMgr *IPSetManager) CreateIPSet(setMetadata *IPSetMetadata) { iMgr.Lock() defer iMgr.Unlock() - prefixedName := setMetaData.GetPrefixName() + prefixedName := setMetadata.GetPrefixName() if iMgr.exists(prefixedName) { return } - iMgr.setMap[prefixedName] = NewIPSet(setMetaData) + iMgr.setMap[prefixedName] = NewIPSet(setMetadata) metrics.IncNumIPSets() } @@ -142,7 +142,7 @@ func (iMgr *IPSetManager) DeleteReference(setName, referenceName string, referen return nil } -func (iMgr *IPSetManager) AddToSet(addToSets []*IPSetMetaData, ip, podKey string) error { +func (iMgr *IPSetManager) AddToSet(addToSets []*IPSetMetadata, ip, podKey string) error { // check if the IP is IPV4 family if net.ParseIP(ip).To4() == nil { return npmerrors.Errorf(npmerrors.AppendIPSet, false, "IPV6 not supported") @@ -154,8 +154,8 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSetMetaData, ip, podKey string return err } - for _, setMetaData := range addToSets { - prefixedName := setMetaData.GetPrefixName() + for _, setMetadata := range addToSets { + prefixedName := setMetadata.GetPrefixName() set := iMgr.setMap[prefixedName] cachedPodKey, ok := set.IPPodKey[ip] set.IPPodKey[ip] = podKey @@ -171,7 +171,7 @@ func (iMgr *IPSetManager) AddToSet(addToSets []*IPSetMetaData, ip, podKey string return nil } -func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []*IPSetMetaData, ip, podKey string) error { +func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []*IPSetMetadata, ip, podKey string) error { iMgr.Lock() defer iMgr.Unlock() @@ -179,8 +179,8 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []*IPSetMetaData, ip, pod return err } - for _, setMetaData := range removeFromSets { - prefixedName := setMetaData.GetPrefixName() + for _, setMetadata := range removeFromSets { + prefixedName := setMetadata.GetPrefixName() set := iMgr.setMap[prefixedName] // in case the IP belongs to a new Pod, then ignore this Delete call as this might be stale @@ -202,17 +202,17 @@ func (iMgr *IPSetManager) RemoveFromSet(removeFromSets []*IPSetMetaData, ip, pod return nil } -func (iMgr *IPSetManager) AddToList(listMetaData *IPSetMetaData, setMetaDatas []*IPSetMetaData) error { +func (iMgr *IPSetManager) AddToList(listMetadata *IPSetMetadata, setMetadatas []*IPSetMetadata) error { iMgr.Lock() defer iMgr.Unlock() - if err := iMgr.checkForListMemberUpdateErrors(listMetaData, setMetaDatas, npmerrors.AppendIPSet); err != nil { + if err := iMgr.checkForListMemberUpdateErrors(listMetadata, setMetadatas, npmerrors.AppendIPSet); err != nil { return err } - listName := listMetaData.GetPrefixName() - for _, setMetaData := range setMetaDatas { - setName := setMetaData.GetPrefixName() + listName := listMetadata.GetPrefixName() + for _, setMetadata := range setMetadatas { + setName := setMetadata.GetPrefixName() iMgr.addMemberIPSet(listName, setName) } iMgr.modifyCacheForKernelMemberUpdate(listName) @@ -220,17 +220,17 @@ func (iMgr *IPSetManager) AddToList(listMetaData *IPSetMetaData, setMetaDatas [] return nil } -func (iMgr *IPSetManager) RemoveFromList(listMetaData *IPSetMetaData, setMetaDatas []*IPSetMetaData) error { +func (iMgr *IPSetManager) RemoveFromList(listMetadata *IPSetMetadata, setMetadatas []*IPSetMetadata) error { iMgr.Lock() defer iMgr.Unlock() - if err := iMgr.checkForListMemberUpdateErrors(listMetaData, setMetaDatas, npmerrors.DeleteIPSet); err != nil { + if err := iMgr.checkForListMemberUpdateErrors(listMetadata, setMetadatas, npmerrors.DeleteIPSet); err != nil { return err } - listName := listMetaData.GetPrefixName() - for _, setMetaData := range setMetaDatas { - setName := setMetaData.GetPrefixName() + listName := listMetadata.GetPrefixName() + for _, setMetadata := range setMetadatas { + setName := setMetadata.GetPrefixName() iMgr.removeMemberIPSet(listName, setName) } iMgr.modifyCacheForKernelMemberUpdate(listName) @@ -344,7 +344,7 @@ func (iMgr *IPSetManager) decKernelReferCountAndModifyCache(member *IPSet) { } } -func (iMgr *IPSetManager) checkForIPUpdateErrors(setNames []*IPSetMetaData, npmErrorString string) error { +func (iMgr *IPSetManager) checkForIPUpdateErrors(setNames []*IPSetMetadata, npmErrorString string) error { for _, set := range setNames { prefixedSetName := set.GetPrefixName() if !iMgr.exists(prefixedSetName) { @@ -375,8 +375,8 @@ func (iMgr *IPSetManager) modifyCacheForKernelMemberUpdate(setName string) { } } -func (iMgr *IPSetManager) checkForListMemberUpdateErrors(listMetaData *IPSetMetaData, memberMetaDatas []*IPSetMetaData, npmErrorString string) error { - prefixedListName := listMetaData.GetPrefixName() +func (iMgr *IPSetManager) checkForListMemberUpdateErrors(listMetadata *IPSetMetadata, memberMetadatas []*IPSetMetadata, npmErrorString string) error { + prefixedListName := listMetadata.GetPrefixName() if !iMgr.exists(prefixedListName) { return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s does not exist", prefixedListName)) } @@ -386,8 +386,8 @@ func (iMgr *IPSetManager) checkForListMemberUpdateErrors(listMetaData *IPSetMeta return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s is not a list set", prefixedListName)) } - for _, memberMetaData := range memberMetaDatas { - memberName := memberMetaData.GetPrefixName() + for _, memberMetadata := range memberMetadatas { + memberName := memberMetadata.GetPrefixName() if prefixedListName == memberName { return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("ipset %s cannot be added to itself", prefixedListName)) } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 647008475a..9d0458f218 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -20,106 +20,106 @@ const ( func TestCreateIPSet(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - iMgr.CreateIPSet(setMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetadata) // creating twice - iMgr.CreateIPSet(setMetaData) + iMgr.CreateIPSet(setMetadata) - assert.True(t, iMgr.exists(setMetaData.GetPrefixName())) + assert.True(t, iMgr.exists(setMetadata.GetPrefixName())) - set := iMgr.GetIPSet(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) + assert.Equal(t, setMetadata.GetPrefixName(), set.Name) + assert.Equal(t, util.GetHashedName(setMetadata.GetPrefixName()), set.HashedName) } func TestAddToSet(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - iMgr.CreateIPSet(setMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetadata) - err := iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + err := iMgr.AddToSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) - err = iMgr.AddToSet([]*IPSetMetaData{setMetaData}, "2001:db8:0:0:0:0:2:1", "newpod") + err = iMgr.AddToSet([]*IPSetMetadata{setMetadata}, "2001:db8:0:0:0:0:2:1", "newpod") require.Error(t, err) // same IP changed podkey - err = iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, "newpod") + err = iMgr.AddToSet([]*IPSetMetadata{setMetadata}, testPodIP, "newpod") require.NoError(t, err) - listMetaData := NewIPSetMetadata("testipsetlist", KeyLabelOfNameSpace) - iMgr.CreateIPSet(listMetaData) - err = iMgr.AddToSet([]*IPSetMetaData{listMetaData}, testPodIP, testPodKey) + listMetadata := NewIPSetMetadata("testipsetlist", KeyLabelOfNameSpace) + iMgr.CreateIPSet(listMetadata) + err = iMgr.AddToSet([]*IPSetMetadata{listMetadata}, testPodIP, testPodKey) require.Error(t, err) } func TestRemoveFromSet(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - iMgr.CreateIPSet(setMetaData) - err := iMgr.AddToSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetadata) + err := iMgr.AddToSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) - err = iMgr.RemoveFromSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + err = iMgr.RemoveFromSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.NoError(t, err) } func TestRemoveFromSetMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - err := iMgr.RemoveFromSet([]*IPSetMetaData{setMetaData}, testPodIP, testPodKey) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + err := iMgr.RemoveFromSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.Error(t, err) } func TestAddToListMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - listMetaData := NewIPSetMetadata("testlabel", KeyLabelOfNameSpace) - err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNameSpace) + err := iMgr.AddToList(listMetadata, []*IPSetMetadata{setMetadata}) require.Error(t, err) } func TestAddToList(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) - iMgr.CreateIPSet(setMetaData) - iMgr.CreateIPSet(listMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(setMetadata) + iMgr.CreateIPSet(listMetadata) - err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + err := iMgr.AddToList(listMetadata, []*IPSetMetadata{setMetadata}) require.NoError(t, err) - set := iMgr.GetIPSet(listMetaData.GetPrefixName()) + set := iMgr.GetIPSet(listMetadata.GetPrefixName()) assert.NotNil(t, set) - assert.Equal(t, listMetaData.GetPrefixName(), set.Name) - assert.Equal(t, util.GetHashedName(listMetaData.GetPrefixName()), set.HashedName) + assert.Equal(t, listMetadata.GetPrefixName(), set.Name) + 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) + assert.Equal(t, setMetadata.GetPrefixName(), set.MemberIPSets[setMetadata.GetPrefixName()].Name) } func TestRemoveFromList(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) - iMgr.CreateIPSet(setMetaData) - iMgr.CreateIPSet(listMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(setMetadata) + iMgr.CreateIPSet(listMetadata) - err := iMgr.AddToList(listMetaData, []*IPSetMetaData{setMetaData}) + err := iMgr.AddToList(listMetadata, []*IPSetMetadata{setMetadata}) require.NoError(t, err) - set := iMgr.GetIPSet(listMetaData.GetPrefixName()) + set := iMgr.GetIPSet(listMetadata.GetPrefixName()) assert.NotNil(t, set) - assert.Equal(t, listMetaData.GetPrefixName(), set.Name) - assert.Equal(t, util.GetHashedName(listMetaData.GetPrefixName()), set.HashedName) + assert.Equal(t, listMetadata.GetPrefixName(), set.Name) + 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) + assert.Equal(t, setMetadata.GetPrefixName(), set.MemberIPSets[setMetadata.GetPrefixName()].Name) - err = iMgr.RemoveFromList(listMetaData, []*IPSetMetaData{setMetaData}) + err = iMgr.RemoveFromList(listMetadata, []*IPSetMetadata{setMetadata}) require.NoError(t, err) - set = iMgr.GetIPSet(listMetaData.GetPrefixName()) + set = iMgr.GetIPSet(listMetadata.GetPrefixName()) assert.NotNil(t, set) assert.Equal(t, 0, len(set.MemberIPSets)) } @@ -127,26 +127,26 @@ func TestRemoveFromList(t *testing.T) { func TestRemoveFromListMissing(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - listMetaData := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) - iMgr.CreateIPSet(listMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) + iMgr.CreateIPSet(listMetadata) - err := iMgr.RemoveFromList(listMetaData, []*IPSetMetaData{setMetaData}) + err := iMgr.RemoveFromList(listMetadata, []*IPSetMetadata{setMetadata}) require.Error(t, err) } func TestDeleteIPSet(t *testing.T) { iMgr := NewIPSetManager("azure") - setMetaData := NewIPSetMetadata(testSetName, NameSpace) - iMgr.CreateIPSet(setMetaData) + setMetadata := NewIPSetMetadata(testSetName, NameSpace) + iMgr.CreateIPSet(setMetadata) - iMgr.DeleteIPSet(setMetaData.GetPrefixName()) + iMgr.DeleteIPSet(setMetadata.GetPrefixName()) // TODO add cache check } func TestGetIPsFromSelectorIPSets(t *testing.T) { iMgr := NewIPSetManager("azure") - setsTocreate := []*IPSetMetaData{ + setsTocreate := []*IPSetMetadata{ { Name: "setNs1", Type: NameSpace, @@ -175,7 +175,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) { err = iMgr.AddToSet(setsTocreate, "10.0.0.2", "test1") require.NoError(t, err) - err = iMgr.AddToSet([]*IPSetMetaData{setsTocreate[0], setsTocreate[2], setsTocreate[3]}, "10.0.0.3", "test3") + err = iMgr.AddToSet([]*IPSetMetadata{setsTocreate[0], setsTocreate[2], setsTocreate[3]}, "10.0.0.3", "test3") require.NoError(t, err) ipsetList := map[string]struct{}{} @@ -197,7 +197,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) { func TestAddDeleteSelectorReferences(t *testing.T) { iMgr := NewIPSetManager("azure") - setsTocreate := []*IPSetMetaData{ + setsTocreate := []*IPSetMetadata{ { Name: "setNs1", Type: NameSpace, @@ -229,7 +229,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { iMgr.CreateIPSet(v) } // Add setpod4 to setpod3 - err := iMgr.AddToList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + err := iMgr.AddToList(setsTocreate[3], []*IPSetMetadata{setsTocreate[4]}) require.NoError(t, err) for _, v := range setsTocreate { @@ -258,7 +258,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { // because they are referencing each other assert.Equal(t, 2, len(iMgr.setMap)) - err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetadata{setsTocreate[4]}) require.NoError(t, err) for _, v := range setsTocreate { @@ -273,7 +273,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { func TestAddDeleteNetPolReferences(t *testing.T) { iMgr := NewIPSetManager("azure") - setsTocreate := []*IPSetMetaData{ + setsTocreate := []*IPSetMetadata{ { Name: "setNs1", Type: NameSpace, @@ -299,7 +299,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { for _, v := range setsTocreate { iMgr.CreateIPSet(v) } - err := iMgr.AddToList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + err := iMgr.AddToList(setsTocreate[3], []*IPSetMetadata{setsTocreate[4]}) require.NoError(t, err) for _, v := range setsTocreate { @@ -325,7 +325,7 @@ func TestAddDeleteNetPolReferences(t *testing.T) { // because they are referencing each other assert.Equal(t, 2, len(iMgr.setMap)) - err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetaData{setsTocreate[4]}) + err = iMgr.RemoveFromList(setsTocreate[3], []*IPSetMetadata{setsTocreate[4]}) require.NoError(t, err) for _, v := range setsTocreate { diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index b11c7a1904..54bf04e976 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -22,10 +22,7 @@ func (iMgr *IPSetManager) applyIPSets() error { return err } - setPolNames, err := getAllSetPolicyNames(network.Policies) - if err != nil { - return err - } + setPolNames := getAllSetPolicyNames(network.Policies) setPolSettings, err := iMgr.calculateNewSetPolicies(setPolNames) if err != nil { @@ -120,7 +117,7 @@ func isValidIPSet(set *IPSet) error { return fmt.Errorf("IPSet " + set.Name + " is missing Name") } - if set.Type == Unknown { + if set.Type == UnknownType { return fmt.Errorf("IPSet " + set.Type.String() + " is missing Type") } @@ -162,7 +159,7 @@ func convertToSetPolicy(set *IPSet) (*hcn.SetPolicySetting, error) { return setPolicy, nil } -func getAllSetPolicyNames(networkPolicies []hcn.NetworkPolicy) ([]string, error) { +func getAllSetPolicyNames(networkPolicies []hcn.NetworkPolicy) []string { setPols := []string{} for _, netpol := range networkPolicies { if netpol.Type == hcn.SetPolicy { @@ -175,5 +172,5 @@ func getAllSetPolicyNames(networkPolicies []hcn.NetworkPolicy) ([]string, error) setPols = append(setPols, set.Name) } } - return setPols, nil + return setPols } From 652280f1fc947aaa6e9589a1eabfb17d73472793 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 10:31:12 -0700 Subject: [PATCH 04/11] Adding IOShim for both windows and linux --- common/ioshim.go | 26 +++++++++++++++++++ network/hnswrapper/hnsv2wrapper.go | 24 ++++++++++++----- network/hnswrapper/hnsv2wrapperfake.go | 21 ++++++++++++--- network/hnswrapper/hnsv2wrapperinterface.go | 8 ++++-- npm/pkg/dataplane/dataplane.go | 9 +++++-- npm/pkg/dataplane/ipsets/ipsetmanager.go | 5 +++- npm/pkg/dataplane/ipsets/ipsetmanager_test.go | 26 ++++++++++--------- npm/pkg/dataplane/policies/policymanager.go | 12 ++++++--- .../dataplane/policies/policymanager_test.go | 13 +++++++--- 9 files changed, 110 insertions(+), 34 deletions(-) create mode 100644 common/ioshim.go diff --git a/common/ioshim.go b/common/ioshim.go new file mode 100644 index 0000000000..323ffff20a --- /dev/null +++ b/common/ioshim.go @@ -0,0 +1,26 @@ +package common + +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + testutils "github.com/Azure/azure-container-networking/test/utils" + utilexec "k8s.io/utils/exec" +) + +type IOShim struct { + Exec utilexec.Interface + Hns hnswrapper.HnsV2WrapperInterface +} + +func NewIOShim() *IOShim { + return &IOShim{ + Exec: utilexec.New(), + Hns: &hnswrapper.Hnsv2wrapper{}, + } +} + +func NewMockIOShim(calls []testutils.TestCmd) *IOShim { + return &IOShim{ + Exec: testutils.GetFakeExecWithScripts(calls), + Hns: &hnswrapper.Hnsv2wrapperFake{}, + } +} diff --git a/network/hnswrapper/hnsv2wrapper.go b/network/hnswrapper/hnsv2wrapper.go index 6144379b3f..c33e3a711b 100644 --- a/network/hnswrapper/hnsv2wrapper.go +++ b/network/hnswrapper/hnsv2wrapper.go @@ -1,6 +1,7 @@ // Copyright 2017 Microsoft. All rights reserved. // MIT License +//go:build windows // +build windows package hnswrapper @@ -10,25 +11,32 @@ import ( ) type Hnsv2wrapper struct { - } -func (Hnsv2wrapper) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { +func (Hnsv2wrapper) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { return endpoint.Create() } -func (Hnsv2wrapper) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error { +func (Hnsv2wrapper) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error { return endpoint.Delete() } -func (Hnsv2wrapper) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.HostComputeNetwork, error) { +func (Hnsv2wrapper) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.HostComputeNetwork, error) { return network.Create() } -func (Hnsv2wrapper) DeleteNetwork(network *hcn.HostComputeNetwork) error { +func (Hnsv2wrapper) DeleteNetwork(network *hcn.HostComputeNetwork) error { return network.Delete() } +func (Hnsv2wrapper) AddNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error { + return network.AddPolicy(networkPolicy) +} + +func (Hnsv2wrapper) RemoveNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error { + return network.RemovePolicy(networkPolicy) +} + func (w Hnsv2wrapper) GetNamespaceByID(netNamespacePath string) (*hcn.HostComputeNamespace, error) { return hcn.GetNamespaceByID(netNamespacePath) } @@ -47,4 +55,8 @@ func (w Hnsv2wrapper) GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, func (f Hnsv2wrapper) GetEndpointByID(endpointId string) (*hcn.HostComputeEndpoint, error) { return hcn.GetEndpointByID(endpointId) -} \ No newline at end of file +} + +func (f Hnsv2wrapper) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { + return endpoint.ApplyPolicy(requestType, endpointPolicy) +} diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index cb17f7bf31..1699528d79 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -1,6 +1,7 @@ // Copyright 2017 Microsoft. All rights reserved. // MIT License +//go:build windows // +build windows package hnswrapper @@ -13,24 +14,32 @@ type Hnsv2wrapperFake struct { } func (f Hnsv2wrapperFake) CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.HostComputeNetwork, error) { - return network,nil + return network, nil } func (f Hnsv2wrapperFake) DeleteNetwork(network *hcn.HostComputeNetwork) error { return nil } +func (Hnsv2wrapperFake) AddNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error { + return nil +} + +func (Hnsv2wrapperFake) RemoveNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error { + return nil +} + func (f Hnsv2wrapperFake) GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, error) { network := &hcn.HostComputeNetwork{Id: "c84257e3-3d60-40c4-8c47-d740a1c260d3"} - return network,nil + return network, nil } func (f Hnsv2wrapperFake) GetEndpointByID(endpointId string) (*hcn.HostComputeEndpoint, error) { endpoint := &hcn.HostComputeEndpoint{Id: "7a2ae98a-0c84-4b35-9684-1c02a2bf7e03"} - return endpoint,nil + return endpoint, nil } -func (Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { +func (Hnsv2wrapperFake) CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) { return endpoint, nil } @@ -50,3 +59,7 @@ func (Hnsv2wrapperFake) AddNamespaceEndpoint(namespaceId string, endpointId stri func (Hnsv2wrapperFake) RemoveNamespaceEndpoint(namespaceId string, endpointId string) error { return nil } + +func (Hnsv2wrapperFake) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { + return nil +} diff --git a/network/hnswrapper/hnsv2wrapperinterface.go b/network/hnswrapper/hnsv2wrapperinterface.go index 3a39e738a6..d06f056138 100644 --- a/network/hnswrapper/hnsv2wrapperinterface.go +++ b/network/hnswrapper/hnsv2wrapperinterface.go @@ -1,6 +1,7 @@ // Copyright 2017 Microsoft. All rights reserved. // MIT License +//go:build windows // +build windows package hnswrapper @@ -11,10 +12,13 @@ type HnsV2WrapperInterface interface { CreateEndpoint(endpoint *hcn.HostComputeEndpoint) (*hcn.HostComputeEndpoint, error) DeleteEndpoint(endpoint *hcn.HostComputeEndpoint) error CreateNetwork(network *hcn.HostComputeNetwork) (*hcn.HostComputeNetwork, error) - DeleteNetwork(network *hcn.HostComputeNetwork) error + DeleteNetwork(network *hcn.HostComputeNetwork) error + AddNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error + RemoveNetworkPolicy(network *hcn.HostComputeNetwork, networkPolicy hcn.PolicyNetworkRequest) error GetNamespaceByID(netNamespacePath string) (*hcn.HostComputeNamespace, error) AddNamespaceEndpoint(namespaceId string, endpointId string) error RemoveNamespaceEndpoint(namespaceId string, endpointId string) error GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, error) GetEndpointByID(endpointId string) (*hcn.HostComputeEndpoint, error) -} \ No newline at end of file + ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error +} diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 1336c216ca..68cf5d5c7a 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -3,8 +3,10 @@ package dataplane import ( "fmt" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + "github.com/Azure/azure-container-networking/npm/util" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" "k8s.io/klog" ) @@ -46,8 +48,8 @@ type UpdateNPMPod struct { func NewDataPlane(nodeName string) *DataPlane { return &DataPlane{ - policyMgr: policies.NewPolicyManager(), - ipsetMgr: ipsets.NewIPSetManager(AzureNetworkName), + policyMgr: policies.NewPolicyManager(common.NewIOShim()), + ipsetMgr: ipsets.NewIPSetManager(AzureNetworkName, common.NewIOShim()), endpointCache: make(map[string]*NPMEndpoint), nodeName: nodeName, } @@ -55,6 +57,9 @@ func NewDataPlane(nodeName string) *DataPlane { // InitializeDataPlane helps in setting up dataplane for NPM func (dp *DataPlane) InitializeDataPlane() error { + // Create Kube-All-NS IPSet + kubeAllSet := ipsets.NewIPSetMetadata(util.KubeAllNamespacesFlag, ipsets.KeyLabelOfNameSpace) + dp.CreateIPSet(kubeAllSet) return dp.initializeDataPlane() } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index 3053281b5c..aee4f4d836 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -5,6 +5,7 @@ import ( "net" "sync" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/metrics" npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" @@ -28,6 +29,7 @@ type IPSetManager struct { toAddOrUpdateCache map[string]struct{} // IPSets referred to in this cache may be in the setMap, but must be deleted from the kernel toDeleteCache map[string]struct{} + ioShim *common.IOShim sync.Mutex } @@ -36,7 +38,7 @@ type ipSetManagerCfg struct { networkName string } -func NewIPSetManager(networkName string) *IPSetManager { +func NewIPSetManager(networkName string, ioShim *common.IOShim) *IPSetManager { return &IPSetManager{ iMgrCfg: &ipSetManagerCfg{ ipSetMode: ApplyOnNeed, @@ -45,6 +47,7 @@ func NewIPSetManager(networkName string) *IPSetManager { setMap: make(map[string]*IPSet), toAddOrUpdateCache: make(map[string]struct{}), toDeleteCache: make(map[string]struct{}), + ioShim: ioShim, } } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 9d0458f218..5cbb997668 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -4,8 +4,10 @@ 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" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +20,7 @@ const ( ) func TestCreateIPSet(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) iMgr.CreateIPSet(setMetadata) @@ -34,7 +36,7 @@ func TestCreateIPSet(t *testing.T) { } func TestAddToSet(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) iMgr.CreateIPSet(setMetadata) @@ -56,7 +58,7 @@ func TestAddToSet(t *testing.T) { } func TestRemoveFromSet(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) iMgr.CreateIPSet(setMetadata) @@ -67,14 +69,14 @@ func TestRemoveFromSet(t *testing.T) { } func TestRemoveFromSetMissing(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) err := iMgr.RemoveFromSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey) require.Error(t, err) } func TestAddToListMissing(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNameSpace) err := iMgr.AddToList(listMetadata, []*IPSetMetadata{setMetadata}) @@ -82,7 +84,7 @@ func TestAddToListMissing(t *testing.T) { } func TestAddToList(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) iMgr.CreateIPSet(setMetadata) @@ -100,7 +102,7 @@ func TestAddToList(t *testing.T) { } func TestRemoveFromList(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) iMgr.CreateIPSet(setMetadata) @@ -125,7 +127,7 @@ func TestRemoveFromList(t *testing.T) { } func TestRemoveFromListMissing(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace) @@ -136,7 +138,7 @@ func TestRemoveFromListMissing(t *testing.T) { } func TestDeleteIPSet(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setMetadata := NewIPSetMetadata(testSetName, NameSpace) iMgr.CreateIPSet(setMetadata) @@ -145,7 +147,7 @@ func TestDeleteIPSet(t *testing.T) { } func TestGetIPsFromSelectorIPSets(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -196,7 +198,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) { } func TestAddDeleteSelectorReferences(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", @@ -272,7 +274,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) { } func TestAddDeleteNetPolReferences(t *testing.T) { - iMgr := NewIPSetManager("azure") + iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{})) setsTocreate := []*IPSetMetadata{ { Name: "setNs1", diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 3335abb357..cd841cad1d 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -1,17 +1,23 @@ package policies +import ( + "github.com/Azure/azure-container-networking/common" +) + type PolicyMap struct { - cache map[string]*NPMNetworkPolicy + cache map[string]*NPMNetworkPolicy + ioShim *common.IOShim } type PolicyManager struct { policyMap *PolicyMap } -func NewPolicyManager() *PolicyManager { +func NewPolicyManager(ioShim *common.IOShim) *PolicyManager { return &PolicyManager{ policyMap: &PolicyMap{ - cache: make(map[string]*NPMNetworkPolicy), + cache: make(map[string]*NPMNetworkPolicy), + ioShim: ioShim, }, } } diff --git a/npm/pkg/dataplane/policies/policymanager_test.go b/npm/pkg/dataplane/policies/policymanager_test.go index 04351cd481..654ef6da5d 100644 --- a/npm/pkg/dataplane/policies/policymanager_test.go +++ b/npm/pkg/dataplane/policies/policymanager_test.go @@ -1,9 +1,14 @@ package policies -import "testing" +import ( + "testing" + + "github.com/Azure/azure-container-networking/common" + testutils "github.com/Azure/azure-container-networking/test/utils" +) func TestAddPolicy(t *testing.T) { - pMgr := NewPolicyManager() + pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) netpol := NPMNetworkPolicy{} @@ -14,7 +19,7 @@ func TestAddPolicy(t *testing.T) { } func TestGetPolicy(t *testing.T) { - pMgr := NewPolicyManager() + pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) netpol := NPMNetworkPolicy{ Name: "test", } @@ -39,7 +44,7 @@ func TestGetPolicy(t *testing.T) { } func TestRemovePolicy(t *testing.T) { - pMgr := NewPolicyManager() + pMgr := NewPolicyManager(common.NewMockIOShim([]testutils.TestCmd{})) err := pMgr.RemovePolicy("test", nil) if err != nil { From c07190e6443366c7a2b31b9566ed90335ef77a0d Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 10:39:22 -0700 Subject: [PATCH 05/11] splitting ioshim for each os --- common/ioshim_linux.go | 23 +++++++++++++++++++++++ common/{ioshim.go => ioshim_windows.go} | 0 2 files changed, 23 insertions(+) create mode 100644 common/ioshim_linux.go rename common/{ioshim.go => ioshim_windows.go} (100%) diff --git a/common/ioshim_linux.go b/common/ioshim_linux.go new file mode 100644 index 0000000000..acf5b8a84b --- /dev/null +++ b/common/ioshim_linux.go @@ -0,0 +1,23 @@ +package common + +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + testutils "github.com/Azure/azure-container-networking/test/utils" + utilexec "k8s.io/utils/exec" +) + +type IOShim struct { + Exec utilexec.Interface +} + +func NewIOShim() *IOShim { + return &IOShim{ + Exec: utilexec.New(), + } +} + +func NewMockIOShim(calls []testutils.TestCmd) *IOShim { + return &IOShim{ + Exec: testutils.GetFakeExecWithScripts(calls), + } +} diff --git a/common/ioshim.go b/common/ioshim_windows.go similarity index 100% rename from common/ioshim.go rename to common/ioshim_windows.go From a83c87ea9632dfe73b994d28ec6e6c56baafe171 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 10:47:35 -0700 Subject: [PATCH 06/11] correcting a import error --- common/ioshim_linux.go | 1 - 1 file changed, 1 deletion(-) diff --git a/common/ioshim_linux.go b/common/ioshim_linux.go index acf5b8a84b..1a5ed94b4c 100644 --- a/common/ioshim_linux.go +++ b/common/ioshim_linux.go @@ -1,7 +1,6 @@ package common import ( - "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" utilexec "k8s.io/utils/exec" ) From dccfe2a26f50fb1b174656b3014e12f8fb282255 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 13:33:47 -0700 Subject: [PATCH 07/11] correcting some mistakes --- npm/pkg/dataplane/dataplane.go | 53 ++++++++++++--------- npm/pkg/dataplane/ipsets/ipset.go | 16 +++---- npm/pkg/dataplane/policies/policymanager.go | 8 ++-- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 68cf5d5c7a..8fdac0ccad 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -246,14 +246,14 @@ func (dp *DataPlane) UpdatePolicy(policy *policies.NPMNetworkPolicy) error { func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { // Create IPSets first along with reference updates + npmErrorString := npmerrors.AddSelectorReference + if referenceType == ipsets.NetPolType { + npmErrorString = npmerrors.AddNetPolReference + } for _, set := range sets { dp.ipsetMgr.CreateIPSet(set.Metadata) err := dp.ipsetMgr.AddReference(set.Metadata.GetPrefixName(), netpolName, referenceType) if err != nil { - npmErrorString := npmerrors.AddSelectorReference - if referenceType == ipsets.NetPolType { - npmErrorString = npmerrors.AddNetPolReference - } return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to add reference with err: %s", err.Error())) } } @@ -262,9 +262,18 @@ func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.Translate // if so this below addition would throw an error because rule ipsets are not created // Check if any list sets are provided with members to add for _, set := range sets { - // Check if any 2nd level IPSets are generated by Controller with members - // Apply members to the list set - if ipsets.GetSetKind(set.Metadata.Type) == ipsets.ListSet { + // Check if any CIDR block IPSets needs to be applied + setType := set.Metadata.Type + if setType == ipsets.CIDRBlocks { + for ip, podKey := range set.IPPodKey { + err := dp.ipsetMgr.AddToSet([]*ipsets.IPSetMetadata{set.Metadata}, ip, podKey) + if err != nil { + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to AddToSet in addIPSetReferences with err: %s", err.Error())) + } + } + } else if ipsets.GetSetKind(setType) == ipsets.ListSet { + // Check if any 2nd level IPSets are generated by Controller with members + // Apply members to the list set if len(set.MemberIPSets) > 0 { memberList := []*ipsets.IPSetMetadata{} for _, memberSet := range set.MemberIPSets { @@ -272,12 +281,7 @@ func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.Translate } err := dp.ipsetMgr.AddToList(set.Metadata, memberList) if err != nil { - npmErrorString := npmerrors.AddSelectorReference - if referenceType == ipsets.NetPolType { - npmErrorString = npmerrors.AddNetPolReference - } return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to AddToList in addIPSetReferences with err: %s", err.Error())) - } } } @@ -287,15 +291,15 @@ func (dp *DataPlane) createIPSetsAndReferences(sets map[string]*ipsets.Translate } func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.TranslatedIPSet, netpolName string, referenceType ipsets.ReferenceType) error { + npmErrorString := npmerrors.DeleteSelectorReference + if referenceType == ipsets.NetPolType { + npmErrorString = npmerrors.DeleteNetPolReference + } for _, set := range sets { // TODO ignore set does not exist error // TODO add delete ipset after removing members err := dp.ipsetMgr.DeleteReference(set.Metadata.GetPrefixName(), netpolName, referenceType) if err != nil { - npmErrorString := npmerrors.DeleteSelectorReference - if referenceType == ipsets.NetPolType { - npmErrorString = npmerrors.DeleteNetPolReference - } return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to deleteIPSetReferences with err: %s", err.Error())) } } @@ -306,8 +310,17 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.Translate // and both have same members // then we should not delete k1:v0:v1 members ( special case for nested ipsets ) for _, set := range sets { - // Delete if any 2nd level IPSets are generated by Controller with members - if ipsets.GetSetKind(set.Metadata.Type) == ipsets.ListSet { + // Check if any CIDR block IPSets needs to be applied + setType := set.Metadata.Type + if setType == ipsets.CIDRBlocks { + for ip, podKey := range set.IPPodKey { + err := dp.ipsetMgr.RemoveFromSet([]*ipsets.IPSetMetadata{set.Metadata}, ip, podKey) + if err != nil { + return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to RemoveFromSet in deleteIPSetReferences with err: %s", err.Error())) + } + } + } else if ipsets.GetSetKind(set.Metadata.Type) == ipsets.ListSet { + // Delete if any 2nd level IPSets are generated by Controller with members if len(set.MemberIPSets) > 0 { memberList := []*ipsets.IPSetMetadata{} for _, memberSet := range set.MemberIPSets { @@ -315,10 +328,6 @@ func (dp *DataPlane) deleteIPSetsAndReferences(sets map[string]*ipsets.Translate } err := dp.ipsetMgr.RemoveFromList(set.Metadata, memberList) if err != nil { - npmErrorString := npmerrors.DeleteSelectorReference - if referenceType == ipsets.NetPolType { - npmErrorString = npmerrors.DeleteNetPolReference - } return npmerrors.Errorf(npmErrorString, false, fmt.Sprintf("[dataplane] failed to RemoveFromList in deleteIPSetReferences with err: %s", err.Error())) } } diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index f903966779..8eb18b0005 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -160,21 +160,21 @@ func NewIPSetMetadata(name string, setType SetType) *IPSetMetadata { func (setMetadata *IPSetMetadata) GetPrefixName() string { switch setMetadata.Type { case CIDRBlocks: - return fmt.Sprintf("%s%s", setMetadata.Name, util.CIDRPrefix) + return fmt.Sprintf("%s%s", util.CIDRPrefix, setMetadata.Name) case NameSpace: - return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespacePrefix) + return fmt.Sprintf("%s%s", util.NamespacePrefix, setMetadata.Name) case NamedPorts: - return fmt.Sprintf("%s%s", setMetadata.Name, util.NamedPortIPSetPrefix) + return fmt.Sprintf("%s%s", util.NamedPortIPSetPrefix, setMetadata.Name) case KeyLabelOfPod: - return fmt.Sprintf("%s%s", setMetadata.Name, util.PodLabelPrefix) + return fmt.Sprintf("%s%s", util.PodLabelPrefix, setMetadata.Name) case KeyValueLabelOfPod: - return fmt.Sprintf("%s%s", setMetadata.Name, util.PodLabelPrefix) + return fmt.Sprintf("%s%s", util.PodLabelPrefix, setMetadata.Name) case KeyLabelOfNameSpace: - return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespaceLabelPrefix) + return fmt.Sprintf("%s%s", util.NamespaceLabelPrefix, setMetadata.Name) case KeyValueLabelOfNameSpace: - return fmt.Sprintf("%s%s", setMetadata.Name, util.NamespaceLabelPrefix) + return fmt.Sprintf("%s%s", util.NamespaceLabelPrefix, setMetadata.Name) case NestedLabelOfPod: - return fmt.Sprintf("%s%s", setMetadata.Name, util.NestedLabelPrefix) + return fmt.Sprintf("%s%s", util.NestedLabelPrefix, setMetadata.Name) case UnknownType: // adding this to appease golint return Unknown default: diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index cd841cad1d..bab567cb47 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -5,20 +5,20 @@ import ( ) type PolicyMap struct { - cache map[string]*NPMNetworkPolicy - ioShim *common.IOShim + cache map[string]*NPMNetworkPolicy } type PolicyManager struct { policyMap *PolicyMap + ioShim *common.IOShim } func NewPolicyManager(ioShim *common.IOShim) *PolicyManager { return &PolicyManager{ policyMap: &PolicyMap{ - cache: make(map[string]*NPMNetworkPolicy), - ioShim: ioShim, + cache: make(map[string]*NPMNetworkPolicy), }, + ioShim: ioShim, } } From e60626c23c6e837205677e95e2fe539e5f42f854 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 14:47:16 -0700 Subject: [PATCH 08/11] Adding tests for policies in Dp --- network/hnswrapper/hnsv2wrapper.go | 8 ++ network/hnswrapper/hnsv2wrapperfake.go | 8 ++ network/hnswrapper/hnsv2wrapperinterface.go | 2 + npm/pkg/dataplane/dataplane.go | 8 +- npm/pkg/dataplane/dataplane_test.go | 100 +++++++++++++++++- npm/pkg/dataplane/dataplane_windows.go | 4 +- .../dataplane/ipsets/ipsetmanager_windows.go | 4 +- 7 files changed, 122 insertions(+), 12 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapper.go b/network/hnswrapper/hnsv2wrapper.go index c33e3a711b..b0ff55de18 100644 --- a/network/hnswrapper/hnsv2wrapper.go +++ b/network/hnswrapper/hnsv2wrapper.go @@ -49,6 +49,10 @@ func (w Hnsv2wrapper) RemoveNamespaceEndpoint(namespaceId string, endpointId str return hcn.RemoveNamespaceEndpoint(namespaceId, endpointId) } +func (w Hnsv2wrapper) GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) { + return hcn.GetNetworkByName(networkName) +} + func (w Hnsv2wrapper) GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, error) { return hcn.GetNetworkByID(networkId) } @@ -57,6 +61,10 @@ func (f Hnsv2wrapper) GetEndpointByID(endpointId string) (*hcn.HostComputeEndpoi return hcn.GetEndpointByID(endpointId) } +func (f Hnsv2wrapper) ListEndpointsOfNetwork(networkId string) ([]hcn.HostComputeEndpoint, error) { + return hcn.ListEndpointsOfNetwork(networkId) +} + func (f Hnsv2wrapper) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { return endpoint.ApplyPolicy(requestType, endpointPolicy) } diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 1699528d79..b761f5197b 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -29,6 +29,10 @@ func (Hnsv2wrapperFake) RemoveNetworkPolicy(network *hcn.HostComputeNetwork, net return nil } +func (Hnsv2wrapperFake) GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) { + return &hcn.HostComputeNetwork{}, nil +} + func (f Hnsv2wrapperFake) GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, error) { network := &hcn.HostComputeNetwork{Id: "c84257e3-3d60-40c4-8c47-d740a1c260d3"} return network, nil @@ -60,6 +64,10 @@ func (Hnsv2wrapperFake) RemoveNamespaceEndpoint(namespaceId string, endpointId s return nil } +func (Hnsv2wrapperFake) ListEndpointsOfNetwork(networkId string) ([]hcn.HostComputeEndpoint, error) { + return []hcn.HostComputeEndpoint{}, nil +} + func (Hnsv2wrapperFake) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error { return nil } diff --git a/network/hnswrapper/hnsv2wrapperinterface.go b/network/hnswrapper/hnsv2wrapperinterface.go index d06f056138..fe3958ad32 100644 --- a/network/hnswrapper/hnsv2wrapperinterface.go +++ b/network/hnswrapper/hnsv2wrapperinterface.go @@ -18,7 +18,9 @@ type HnsV2WrapperInterface interface { GetNamespaceByID(netNamespacePath string) (*hcn.HostComputeNamespace, error) AddNamespaceEndpoint(namespaceId string, endpointId string) error RemoveNamespaceEndpoint(namespaceId string, endpointId string) error + GetNetworkByName(networkName string) (*hcn.HostComputeNetwork, error) GetNetworkByID(networkId string) (*hcn.HostComputeNetwork, error) GetEndpointByID(endpointId string) (*hcn.HostComputeEndpoint, error) + ListEndpointsOfNetwork(networkId string) ([]hcn.HostComputeEndpoint, error) ApplyEndpointPolicy(endpoint *hcn.HostComputeEndpoint, requestType hcn.RequestType, endpointPolicy hcn.PolicyEndpointRequest) error } diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index 8fdac0ccad..75f4f3c9a7 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -23,6 +23,7 @@ type DataPlane struct { nodeName string // key is PodKey endpointCache map[string]*NPMEndpoint + ioShim *common.IOShim } type NPMEndpoint struct { @@ -46,12 +47,13 @@ type UpdateNPMPod struct { IPSetsToRemove []string } -func NewDataPlane(nodeName string) *DataPlane { +func NewDataPlane(nodeName string, ioShim *common.IOShim) *DataPlane { return &DataPlane{ - policyMgr: policies.NewPolicyManager(common.NewIOShim()), - ipsetMgr: ipsets.NewIPSetManager(AzureNetworkName, common.NewIOShim()), + policyMgr: policies.NewPolicyManager(ioShim), + ipsetMgr: ipsets.NewIPSetManager(AzureNetworkName, ioShim), endpointCache: make(map[string]*NPMEndpoint), nodeName: nodeName, + ioShim: ioShim, } } diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index d2d6ce9e4d..398d8b14db 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -3,15 +3,67 @@ package dataplane import ( "testing" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" + testutils "github.com/Azure/azure-container-networking/test/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +var ( + ioShim = common.NewMockIOShim([]testutils.TestCmd{}) + ns1Set = &ipsets.TranslatedIPSet{ + Metadata: ipsets.NewIPSetMetadata("setns1", ipsets.NameSpace), + } + testPolicyobj = &policies.NPMNetworkPolicy{ + Name: "ns1/testpolicy", + PodSelectorIPSets: map[string]*ipsets.TranslatedIPSet{ + "setns1": ns1Set, + "setpodkey1": { + Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod), + }, + "setpodkeyval1": { + Metadata: ipsets.NewIPSetMetadata("setpodkeyval1", ipsets.KeyValueLabelOfPod), + }, + "nestedset1": { + Metadata: ipsets.NewIPSetMetadata("nestedset1", ipsets.NestedLabelOfPod), + MemberIPSets: map[string]*ipsets.TranslatedIPSet{ + "setns1": ns1Set, + }, + }, + }, + RuleIPSets: map[string]*ipsets.TranslatedIPSet{ + "setns2": { + Metadata: ipsets.NewIPSetMetadata("setns2", ipsets.NameSpace), + }, + "setpodkey2": { + Metadata: ipsets.NewIPSetMetadata("setpodkey2", ipsets.KeyLabelOfPod), + }, + "setpodkeyval2": { + Metadata: ipsets.NewIPSetMetadata("setpodkeyval2", ipsets.KeyValueLabelOfPod), + }, + "testcidr1": { + Metadata: ipsets.NewIPSetMetadata("testcidr1", ipsets.CIDRBlocks), + IPPodKey: map[string]string{ + "10.0.0.0": "val1", + }, + }, + }, + ACLs: []*policies.ACLPolicy{ + { + PolicyID: "testpol1", + Target: policies.Dropped, + Direction: policies.Egress, + }, + }, + } +) + func TestNewDataPlane(t *testing.T) { metrics.InitializeAll() - dp := NewDataPlane("testnode") + dp := NewDataPlane("testnode", ioShim) if dp == nil { t.Error("NewDataPlane() returned nil") @@ -23,7 +75,7 @@ func TestNewDataPlane(t *testing.T) { func TestInitializeDataPlane(t *testing.T) { metrics.InitializeAll() - dp := NewDataPlane("testnode") + dp := NewDataPlane("testnode", ioShim) assert.NotNil(t, dp) err := dp.InitializeDataPlane() @@ -32,7 +84,7 @@ func TestInitializeDataPlane(t *testing.T) { func TestResetDataPlane(t *testing.T) { metrics.InitializeAll() - dp := NewDataPlane("testnode") + dp := NewDataPlane("testnode", ioShim) assert.NotNil(t, dp) err := dp.InitializeDataPlane() @@ -43,7 +95,7 @@ func TestResetDataPlane(t *testing.T) { func TestCreateAndDeleteIpSets(t *testing.T) { metrics.InitializeAll() - dp := NewDataPlane("testnode") + dp := NewDataPlane("testnode", ioShim) assert.NotNil(t, dp) setsTocreate := []*ipsets.IPSetMetadata{ { @@ -84,7 +136,7 @@ func TestCreateAndDeleteIpSets(t *testing.T) { func TestAddToSet(t *testing.T) { metrics.InitializeAll() - dp := NewDataPlane("testnode") + dp := NewDataPlane("testnode", ioShim) setsTocreate := []*ipsets.IPSetMetadata{ { @@ -137,3 +189,41 @@ func TestAddToSet(t *testing.T) { assert.Nil(t, set) } } + +func TestApplyPolicy(t *testing.T) { + metrics.InitializeAll() + dp := NewDataPlane("testnode", ioShim) + + err := dp.AddPolicy(testPolicyobj) + require.NoError(t, err) +} + +func TestRemovePolicy(t *testing.T) { + metrics.InitializeAll() + dp := NewDataPlane("testnode", ioShim) + + err := dp.AddPolicy(testPolicyobj) + require.NoError(t, err) + + err = dp.RemovePolicy(testPolicyobj.Name) + require.NoError(t, err) +} + +func TestupdatePolicy(t *testing.T) { + metrics.InitializeAll() + dp := NewDataPlane("testnode", ioShim) + + err := dp.AddPolicy(testPolicyobj) + require.NoError(t, err) + + testPolicyobj.ACLs = []*policies.ACLPolicy{ + { + PolicyID: "testpol1", + Target: policies.Dropped, + Direction: policies.Ingress, + }, + } + + err = dp.UpdatePolicy(testPolicyobj) + require.NoError(t, err) +} diff --git a/npm/pkg/dataplane/dataplane_windows.go b/npm/pkg/dataplane/dataplane_windows.go index 3c5eb25992..7f957f8628 100644 --- a/npm/pkg/dataplane/dataplane_windows.go +++ b/npm/pkg/dataplane/dataplane_windows.go @@ -141,7 +141,7 @@ func (dp *DataPlane) resetDataPlane() error { } func (dp *DataPlane) getAllPodEndpoints() ([]hcn.HostComputeEndpoint, error) { - endpoints, err := hcn.ListEndpointsOfNetwork(dp.networkID) + endpoints, err := dp.ioShim.Hns.ListEndpointsOfNetwork(dp.networkID) if err != nil { return nil, err } @@ -169,7 +169,7 @@ func (dp *DataPlane) refreshAllPodEndpoints() error { func (dp *DataPlane) setNetworkIDByName(networkName string) error { // Get Network ID - network, err := hcn.GetNetworkByName(networkName) + network, err := dp.ioShim.Hns.GetNetworkByName(networkName) if err != nil { return err } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go index 54bf04e976..4677652ab5 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_windows.go @@ -55,7 +55,7 @@ func (iMgr *IPSetManager) applyIPSets() error { ) } - err = network.AddPolicy(policyNetworkRequest) + err = iMgr.ioShim.Hns.AddNetworkPolicy(network, policyNetworkRequest) if err != nil { return err } @@ -105,7 +105,7 @@ func (iMgr *IPSetManager) getHCnNetwork() (*hcn.HostComputeNetwork, error) { if iMgr.iMgrCfg.networkName == "" { iMgr.iMgrCfg.networkName = "azure" } - network, err := hcn.GetNetworkByName("azure") + network, err := iMgr.ioShim.Hns.GetNetworkByName("azure") if err != nil { return nil, err } From 7420d30d9831b0b12ce183263c6d626e36584721 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Fri, 8 Oct 2021 14:52:01 -0700 Subject: [PATCH 09/11] fixing a testname --- npm/pkg/dataplane/dataplane_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index 398d8b14db..af9fdee7f6 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -209,7 +209,7 @@ func TestRemovePolicy(t *testing.T) { require.NoError(t, err) } -func TestupdatePolicy(t *testing.T) { +func TestUpdatePolicy(t *testing.T) { metrics.InitializeAll() dp := NewDataPlane("testnode", ioShim) From c738678ab6eb8d1d5c8aa64842bcf97e2a1ec8c0 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Mon, 11 Oct 2021 10:36:07 -0700 Subject: [PATCH 10/11] Updating the dataplane mock file --- .../mocks/genericdataplane_generated.go | 34 ++++++++----------- npm/pkg/dataplane/types.go | 15 ++++---- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/npm/pkg/dataplane/mocks/genericdataplane_generated.go b/npm/pkg/dataplane/mocks/genericdataplane_generated.go index e3e92b86bc..fbda259900 100644 --- a/npm/pkg/dataplane/mocks/genericdataplane_generated.go +++ b/npm/pkg/dataplane/mocks/genericdataplane_generated.go @@ -2,7 +2,7 @@ // // Code generated by MockGen. DO NOT EDIT. -// Source: /home/matmerr/go/src/github.com/Azure/azure-container-networking/npm/pkg/dataplane/types.go +// Source: /mnt/c/Users/vamsi/Desktop/Microsoft_ws/azure-container-networking/npm/pkg/dataplane/types.go // Package mocks is a generated GoMock package. package mocks @@ -10,7 +10,7 @@ package mocks import ( reflect "reflect" - npm "github.com/Azure/azure-container-networking/npm" + dataplane "github.com/Azure/azure-container-networking/npm/pkg/dataplane" ipsets "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" policies "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" gomock "github.com/golang/mock/gomock" @@ -54,7 +54,7 @@ func (mr *MockGenericDataplaneMockRecorder) AddPolicy(policies interface{}) *gom } // AddToList mocks base method. -func (m *MockGenericDataplane) AddToList(listName string, setNames []string) error { +func (m *MockGenericDataplane) AddToList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddToList", listName, setNames) ret0, _ := ret[0].(error) @@ -68,7 +68,7 @@ func (mr *MockGenericDataplaneMockRecorder) AddToList(listName, setNames interfa } // AddToSet mocks base method. -func (m *MockGenericDataplane) AddToSet(setNames []string, ip, podKey string) error { +func (m *MockGenericDataplane) AddToSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddToSet", setNames, ip, podKey) ret0, _ := ret[0].(error) @@ -96,31 +96,27 @@ func (mr *MockGenericDataplaneMockRecorder) ApplyDataPlane() *gomock.Call { } // CreateIPSet mocks base method. -func (m *MockGenericDataplane) CreateIPSet(setName string, setType ipsets.SetType) error { +func (m *MockGenericDataplane) CreateIPSet(setMetadata *ipsets.IPSetMetadata) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateIPSet", setName, setType) - ret0, _ := ret[0].(error) - return ret0 + m.ctrl.Call(m, "CreateIPSet", setMetadata) } // CreateIPSet indicates an expected call of CreateIPSet. -func (mr *MockGenericDataplaneMockRecorder) CreateIPSet(setName, setType interface{}) *gomock.Call { +func (mr *MockGenericDataplaneMockRecorder) CreateIPSet(setMetadata interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).CreateIPSet), setName, setType) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).CreateIPSet), setMetadata) } // DeleteIPSet mocks base method. -func (m *MockGenericDataplane) DeleteIPSet(name string) error { +func (m *MockGenericDataplane) DeleteIPSet(setMetadata *ipsets.IPSetMetadata) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteIPSet", name) - ret0, _ := ret[0].(error) - return ret0 + m.ctrl.Call(m, "DeleteIPSet", setMetadata) } // DeleteIPSet indicates an expected call of DeleteIPSet. -func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(name interface{}) *gomock.Call { +func (mr *MockGenericDataplaneMockRecorder) DeleteIPSet(setMetadata interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), name) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIPSet", reflect.TypeOf((*MockGenericDataplane)(nil).DeleteIPSet), setMetadata) } // InitializeDataPlane mocks base method. @@ -138,7 +134,7 @@ func (mr *MockGenericDataplaneMockRecorder) InitializeDataPlane() *gomock.Call { } // RemoveFromList mocks base method. -func (m *MockGenericDataplane) RemoveFromList(listName string, setNames []string) error { +func (m *MockGenericDataplane) RemoveFromList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RemoveFromList", listName, setNames) ret0, _ := ret[0].(error) @@ -152,7 +148,7 @@ func (mr *MockGenericDataplaneMockRecorder) RemoveFromList(listName, setNames in } // RemoveFromSet mocks base method. -func (m *MockGenericDataplane) RemoveFromSet(setNames []string, ip, podKey string) error { +func (m *MockGenericDataplane) RemoveFromSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RemoveFromSet", setNames, ip, podKey) ret0, _ := ret[0].(error) @@ -194,7 +190,7 @@ func (mr *MockGenericDataplaneMockRecorder) ResetDataPlane() *gomock.Call { } // UpdatePod mocks base method. -func (m *MockGenericDataplane) UpdatePod(pod *npm.NpmPod) error { +func (m *MockGenericDataplane) UpdatePod(pod *dataplane.UpdateNPMPod) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdatePod", pod) ret0, _ := ret[0].(error) diff --git a/npm/pkg/dataplane/types.go b/npm/pkg/dataplane/types.go index 41cb6f6cb3..d569c38fd3 100644 --- a/npm/pkg/dataplane/types.go +++ b/npm/pkg/dataplane/types.go @@ -1,7 +1,6 @@ package dataplane import ( - "github.com/Azure/azure-container-networking/npm" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" ) @@ -9,13 +8,13 @@ import ( type GenericDataplane interface { InitializeDataPlane() error ResetDataPlane() error - CreateIPSet(setName string, setType ipsets.SetType) error - DeleteIPSet(name string) error - AddToSet(setNames []string, ip, podKey string) error - RemoveFromSet(setNames []string, ip, podKey string) error - AddToList(listName string, setNames []string) error - RemoveFromList(listName string, setNames []string) error - UpdatePod(pod *npm.NpmPod) error + CreateIPSet(setMetadata *ipsets.IPSetMetadata) + DeleteIPSet(setMetadata *ipsets.IPSetMetadata) + AddToSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error + RemoveFromSet(setNames []*ipsets.IPSetMetadata, ip, podKey string) error + AddToList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error + RemoveFromList(listName *ipsets.IPSetMetadata, setNames []*ipsets.IPSetMetadata) error + UpdatePod(pod *UpdateNPMPod) error ApplyDataPlane() error AddPolicy(policies *policies.NPMNetworkPolicy) error RemovePolicy(policyName string) error From 2e1b5c925e6c1d541a3ca7cabc27821cbce62bf6 Mon Sep 17 00:00:00 2001 From: Vamsi Kalapala Date: Mon, 11 Oct 2021 10:47:23 -0700 Subject: [PATCH 11/11] removing dataplane mocks from dataplane tests as their scope is controllers --- npm/pkg/dataplane/dataplane_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_test.go b/npm/pkg/dataplane/dataplane_test.go index f19ea7ebf3..af9fdee7f6 100644 --- a/npm/pkg/dataplane/dataplane_test.go +++ b/npm/pkg/dataplane/dataplane_test.go @@ -6,10 +6,8 @@ import ( "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/npm/metrics" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane/mocks" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" testutils "github.com/Azure/azure-container-networking/test/utils" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -229,13 +227,3 @@ func TestUpdatePolicy(t *testing.T) { err = dp.UpdatePolicy(testPolicyobj) require.NoError(t, err) } - -// gomock sample usage for generated mock dataplane -func TestAddToList(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - m := mocks.NewMockGenericDataplane(ctrl) - m.EXPECT().AddToList("test", []string{"test"}).Return(nil) - require.NoError(t, m.AddToList("test", []string{"test"})) -}