From ccb9c709a9fd8358790255234d7a050b9ccc78e1 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Wed, 12 Oct 2022 16:29:38 -0700 Subject: [PATCH 01/34] wip with StrictlyHasSetPolicies approach --- network/hnswrapper/hnsv2wrapperfake.go | 77 +++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index bc6a9f9775..e3658eb4d8 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -317,10 +317,13 @@ func (f Hnsv2wrapperFake) GetEndpointByName(endpointName string) (*hcn.HostCompu } type FakeHNSCache struct { - networks map[string]*FakeHostComputeNetwork + // networks maps network name to network object + networks map[string]*FakeHostComputeNetwork + // endpoints maps endpoint ID to endpoint object endpoints map[string]*FakeHostComputeEndpoint } +// SetPolicy returns the first SetPolicy found with this ID in any network. func (fCache FakeHNSCache) SetPolicy(setID string) *hcn.SetPolicySetting { for _, network := range fCache.networks { for _, policy := range network.Policies { @@ -332,6 +335,45 @@ func (fCache FakeHNSCache) SetPolicy(setID string) *hcn.SetPolicySetting { return nil } +// StrictlyHasSetPolicies is true if the network exists and has all the SetPolicies and no others. +func (fCache FakeHNSCache) StrictlyHasSetPolicies(networkID string, setIDs []string) bool { + for _, network := range fCache.networks { + if network.ID == networkID { + for _, setID := range setIDs { + hasPolicy := false + for _, policy := range network.Policies { + if policy.Id == setID { + hasPolicy = true + break + } + } + + if !hasPolicy { + return false + } + } + + return len(network.Policies) == len(setIDs) + } + } + return false +} + +func (fCache FakeHNSCache) PrettyString() string { + var networkStrings []string + for _, network := range fCache.networks { + networkStrings = append(networkStrings, fmt.Sprintf("[%+v]", network.PrettyString())) + } + + var endpointStrings []string + for _, endpoint := range fCache.endpoints { + endpointStrings = append(endpointStrings, fmt.Sprintf("[%+v]", endpoint.PrettyString())) + } + + return fmt.Sprintf("networks: [%s]\nendpoints: [%s]", strings.Join(networkStrings, ","), strings.Join(endpointStrings, ",")) +} + +// ACLPolicies returns a map of the inputed Endpoint IDs to Policies with the given policyID func (fCache FakeHNSCache) ACLPolicies(epList map[string]string, policyID string) (map[string][]*FakeEndpointPolicy, error) { aclPols := make(map[string][]*FakeEndpointPolicy) for ip, epID := range epList { @@ -354,6 +396,7 @@ func (fCache FakeHNSCache) ACLPolicies(epList map[string]string, policyID string return aclPols, nil } +// GetAllACLs maps all Endpoint IDs to ACLs func (fCache FakeHNSCache) GetAllACLs() map[string][]*FakeEndpointPolicy { aclPols := make(map[string][]*FakeEndpointPolicy) for _, ep := range fCache.endpoints { @@ -362,9 +405,20 @@ func (fCache FakeHNSCache) GetAllACLs() map[string][]*FakeEndpointPolicy { return aclPols } +// EndpointIP returns the Endpoint's IP or an empty string if the Endpoint doesn't exist. +func (fCache FakeHNSCache) EndpointIP(id string) string { + for _, ep := range fCache.endpoints { + if ep.ID == id { + return ep.IPConfiguration + } + } + return "" +} + type FakeHostComputeNetwork struct { - ID string - Name string + ID string + Name string + // Policies maps SetPolicy ID to SetPolicy object Policies map[string]*hcn.SetPolicySetting } @@ -376,6 +430,14 @@ func NewFakeHostComputeNetwork(network *hcn.HostComputeNetwork) *FakeHostCompute } } +func (fNetwork *FakeHostComputeNetwork) PrettyString() string { + var setPolicyStrings []string + for _, setPolicy := range fNetwork.Policies { + setPolicyStrings = append(setPolicyStrings, fmt.Sprintf("[%+v]", setPolicy)) + } + return fmt.Sprintf("ID: %s, Name: %s, SetPolicies: [%s]", fNetwork.ID, fNetwork.Name, strings.Join(setPolicyStrings, ",")) +} + func (fNetwork *FakeHostComputeNetwork) GetHCNObj() *hcn.HostComputeNetwork { return &hcn.HostComputeNetwork{ Id: fNetwork.ID, @@ -404,6 +466,15 @@ func NewFakeHostComputeEndpoint(endpoint *hcn.HostComputeEndpoint) *FakeHostComp } } +func (fEndpoint *FakeHostComputeEndpoint) PrettyString() string { + var aclStrings []string + for _, acl := range fEndpoint.Policies { + aclStrings = append(aclStrings, fmt.Sprintf("[%+v]", acl)) + } + return fmt.Sprintf("ID: %s, Name: %s, IP: %s, ACLs: [%s]", + fEndpoint.ID, fEndpoint.Name, fEndpoint.IPConfiguration, strings.Join(aclStrings, ",")) +} + func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { // NOTE: not including other policy types like perhaps SetPolicies hcnEndpoint := &hcn.HostComputeEndpoint{ From a092f054cd143c6f17249134cd95c9ece80d3365 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 13 Oct 2022 15:39:37 -0700 Subject: [PATCH 02/34] better approaching of getting all set policies --- network/hnswrapper/hnsv2wrapperfake.go | 63 ++++++++++---------------- 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index e3658eb4d8..4400239a86 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -335,30 +335,6 @@ func (fCache FakeHNSCache) SetPolicy(setID string) *hcn.SetPolicySetting { return nil } -// StrictlyHasSetPolicies is true if the network exists and has all the SetPolicies and no others. -func (fCache FakeHNSCache) StrictlyHasSetPolicies(networkID string, setIDs []string) bool { - for _, network := range fCache.networks { - if network.ID == networkID { - for _, setID := range setIDs { - hasPolicy := false - for _, policy := range network.Policies { - if policy.Id == setID { - hasPolicy = true - break - } - } - - if !hasPolicy { - return false - } - } - - return len(network.Policies) == len(setIDs) - } - } - return false -} - func (fCache FakeHNSCache) PrettyString() string { var networkStrings []string for _, network := range fCache.networks { @@ -373,7 +349,21 @@ func (fCache FakeHNSCache) PrettyString() string { return fmt.Sprintf("networks: [%s]\nendpoints: [%s]", strings.Join(networkStrings, ","), strings.Join(endpointStrings, ",")) } -// ACLPolicies returns a map of the inputed Endpoint IDs to Policies with the given policyID +// AllSetPolicies returns all SetPolicies in a given network as a map of SetPolicy ID to SetPolicy object. +func (fCache FakeHNSCache) AllSetPolicies(networkID string) map[string]*hcn.SetPolicySetting { + setPolicies := make(map[string]*hcn.SetPolicySetting) + for _, network := range fCache.networks { + if network.ID == networkID { + for _, setPolicy := range network.Policies { + setPolicies[setPolicy.Id] = setPolicy + } + break + } + } + return setPolicies +} + +// ACLPolicies returns a map of the inputed Endpoint IDs to Policies with the given policyID. func (fCache FakeHNSCache) ACLPolicies(epList map[string]string, policyID string) (map[string][]*FakeEndpointPolicy, error) { aclPols := make(map[string][]*FakeEndpointPolicy) for ip, epID := range epList { @@ -442,6 +432,7 @@ func (fNetwork *FakeHostComputeNetwork) GetHCNObj() *hcn.HostComputeNetwork { return &hcn.HostComputeNetwork{ Id: fNetwork.ID, Name: fNetwork.Name, + // FIXME need to include SetPolicies } } @@ -481,22 +472,14 @@ func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { Id: fEndpoint.ID, Name: fEndpoint.Name, HostComputeNetwork: fEndpoint.HostComputeNetwork, - Policies: make([]hcn.EndpointPolicy, 0), - } - - for _, fakeEndpointPol := range fEndpoint.Policies { - rawJSON, err := json.Marshal(fakeEndpointPol) - if err != nil { - fmt.Printf("FAILURE marshalling fake endpoint policy: %s\n", err.Error()) - } else { - hcnPolicy := hcn.EndpointPolicy{ - Type: hcn.ACL, - Settings: rawJSON, - } - hcnEndpoint.Policies = append(hcnEndpoint.Policies, hcnPolicy) - } + IpConfigurations: []hcn.IpConfig{ + { + IpAddress: fEndpoint.IPConfiguration, + }, + }, + // FIXME transfer policies with JSON marshalling + Policies: []hcn.EndpointPolicy{}, } - return hcnEndpoint } From dea282fdaec71744797d0d096c37183b6758bcd8 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 13 Oct 2022 15:42:01 -0700 Subject: [PATCH 03/34] wip for rigorous win dp UTs --- npm/pkg/dataplane/dataplane_windows_test.go | 419 ++++++++++++++++++++ 1 file changed, 419 insertions(+) create mode 100644 npm/pkg/dataplane/dataplane_windows_test.go diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go new file mode 100644 index 0000000000..9ef611f520 --- /dev/null +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -0,0 +1,419 @@ +package dataplane + +import ( + "fmt" + "reflect" + "sort" + "strings" + "testing" + "time" + + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Microsoft/hcsshim/hcn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/klog" +) + +type dpEvent func(*testing.T, *DataPlane, *hnswrapper.Hnsv2wrapperFake) + +// FIXME move this into the common code path along with the verification functions below +const azureNetworkID = "1234" + +const ( + thisNode = "this-node" + otherNode = "other-node" + + podKey1 = "pod1" + podKey2 = "pod2" + + ip1 = "10.0.0.1" + ip2 = "10.0.0.2" + + endpoint1 = "test1" + endpoint2 = "test2" + + defaultHNSLatency = time.Duration(0) +) + +var ( + podLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) + podLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) + podLabel2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) + podLabelVal2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) + + podLabelSets1 = []*ipsets.IPSetMetadata{podLabel1Set, podLabelVal1Set} + podLabelSets2 = []*ipsets.IPSetMetadata{podLabel2Set, podLabelVal2Set} + + // emptySet is a member of a list if enabled in the dp Config + // in Windows, this Config option is actually forced to be enabled in NewDataPlane() + emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) + allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) + ns1Set = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) + ns2Set = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) + + nsLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsLabel2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsLabelVal2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) +) + +// podCreateEvent models a Pod CREATE in the PodController +func podCreateEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // PodController might not call this if the namespace already existed + require.Nil(t, dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet})) + require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{nsIPSet}, pod)) + // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) + require.Nil(t, dp.AddToSets(labelIPSets, pod)) + + require.Nil(t, dp.ApplyDataPlane()) + } +} + +// podUpdateEvent models a Pod UPDATE in the PodController +func podUpdateEvent(oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // think it's impossible for this to be called on an UPDATE + // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) + + for _, toRemoveSet := range toRemoveLabelSets { + require.Nil(t, dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, oldPod)) + } + + for _, toAddSet := range toAddLabelSets { + require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, newPod)) + } + + require.Nil(t, dp.ApplyDataPlane()) + } +} + +// podUpdateEvent models a Pod UPDATE in the PodController where the Pod does not change IP/node. +func podUpdateEventSameIP(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { + return podUpdateEvent(pod, pod, nsIPSet, toRemoveLabelSets, toAddLabelSets) +} + +// podDeleteEvent models a Pod DELETE in the PodController +func podDeleteEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + require.Nil(t, dp.RemoveFromSets([]*ipsets.IPSetMetadata{nsIPSet}, pod)) + // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) + require.Nil(t, dp.RemoveFromSets(labelIPSets, pod)) + + require.Nil(t, dp.ApplyDataPlane()) + } +} + +// nsCreateEvent models a Namespace CREATE in the NamespaceController +func nsCreateEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // TODO + } +} + +// nsUpdateEvent models a Namespace UPDATE in the NamespaceController +func nsUpdateEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // TODO + } +} + +// nsDeleteEvent models a Namespace DELETE in the NamespaceController +func nsDeleteEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // TODO + } +} + +// policyUpdateEvent models a Network Policy CREATE/UPDATE in the PolicyController +// FIXME have NPMNetworkPolicy as input +func policyUpdateEvent(policyKey string) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // TODO + } +} + +// policyDeleteEvent models a Network Policy DELETE in the PolicyController +func policyDeleteEvent(policyKey string) dpEvent { + return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + // TODO + } +} + +// endpointCreateEvent models an Endpoint being created within HNS +// With this Event, Endpoints can be created in between calls to dp +func endpointCreateEvent(epID, ip string) dpEvent { + return func(t *testing.T, _ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { + ep := &hcn.HostComputeEndpoint{ + Id: epID, + Name: epID, + HostComputeNetwork: azureNetworkID, + IpConfigurations: []hcn.IpConfig{ + { + IpAddress: ip, + }, + }, + } + _, err := hns.CreateEndpoint(ep) + require.Nil(t, err, "failed to create hns endpoint in mock: %+v", ep) + } +} + +// endpointDeleteEvent models an Endpoint being deleted within HNS +// With this Event, Endpoints can be deleted in between calls to dp +func endpointDeleteEvent(epID string) dpEvent { + return func(t *testing.T, _ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { + ep := &hcn.HostComputeEndpoint{ + Id: epID, + } + err := hns.DeleteEndpoint(ep) + require.Nil(t, err, "failed to create hns endpoint in mock: %+v", ep) + } +} + +// backgroundEvent will run the dpEvents in the background when called +func backgroundEvent(event1 dpEvent, otherEvents ...dpEvent) dpEvent { + allEvents := make([]dpEvent, 1) + allEvents[0] = event1 + allEvents = append(allEvents, otherEvents...) + + return func(t *testing.T, dp *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { + go func() { + // delay would impact other threads as well + for _, event := range allEvents { + event(t, dp, hns) + } + }() + } +} + +// TestAllEventSequences can test any config with a sequence of events. +// TODO double check HNS mock is working as planned +func TestAllEventSequences(t *testing.T) { + tests := []struct { + name string + cfg *Config + ipEndpoints map[string]string + events []dpEvent + expectedSetPolicies []*hcn.SetPolicySetting + expectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy + }{ + { + name: "add set for pod on node", + cfg: dpCfg, + ipEndpoints: map[string]string{ + ip1: endpoint1, + }, + events: []dpEvent{ + // custom dpEvent + func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + pod1 := NewPodMetadata(podKey1, ip1, thisNode) + require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{ns1Set, podLabel1Set}, pod1)) + require.Nil(t, dp.ApplyDataPlane()) + }, + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + { + name: "pod created then deleted", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + // mix of pre-defined dpEvents and a custom one + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + endpointDeleteEvent(endpoint1), + podDeleteEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + dp.ipsetMgr.Reconcile() + require.Nil(t, dp.ApplyDataPlane()) + }, + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set), + // should be garbage collected: podLabel1Set and podLabel1Set + }, + expectedEnpdointACLs: nil, + }, + { + name: "pod created then updated, with policy add in background", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + // pre-defined dpEvents + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + backgroundEvent(policyUpdateEvent("x/base")), + podUpdateEventSameIP(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel2Set, ip1), + setPolicy(podLabelVal2Set, ip1), + // the rest are not garbage collected yet + setPolicy(podLabel1Set), + setPolicy(podLabel1Set), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + // { + // ID: "", + // Protocols: "", + // Action: "", + // Direction: "", + // LocalAddresses: "", + // RemoteAddresses: "", + // LocalPorts: "", + // RemotePorts: "", + // Priority: 0, + // }, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + hns := ipsets.GetHNSFake(t) + hns.Delay = defaultHNSLatency + io := common.NewMockIOShimWithFakeHNS(hns) + for ip, epID := range tt.ipEndpoints { + event := endpointCreateEvent(epID, ip) + event(t, nil, hns) + } + + dp, err := NewDataPlane(thisNode, io, tt.cfg, nil) + require.NoError(t, err, "failed to initialize dp") + + for _, event := range tt.events { + event(t, dp, hns) + } + + verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) + + // uncomment to see output even for succeeding test cases + // require.Fail(t, "DEBUGME") + }) + } + + // I1013 15:07:01.118147 12452 ipsetmanager_windows.go:226] [IPSetManager Windows] Done applying IPSets. + // I1013 15:07:01.118147 12452 dataplane_windows.go:279] Getting all endpoints for Network ID 1234 + // I1013 15:07:01.118147 12452 dataplane_windows.go:318] Endpoint ID test1 has no IPAddreses + // I1013 15:07:01.118147 12452 dataplane_windows.go:105] [DataPlane] updatePod called for Pod Key pod1 + // W1013 15:07:01.118147 12452 dataplane_windows.go:121] [DataPlane] did not find endpoint with IPaddress 10.0.0.1 +} + +func setPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { + pType := hcn.SetPolicyType("") + switch setMetadata.GetSetKind() { + case ipsets.ListSet: + pType = hcn.SetPolicyTypeNestedIpSet + case ipsets.HashSet: + pType = hcn.SetPolicyTypeIpSet + } + + sort.Strings(members) + + return &hcn.SetPolicySetting{ + Id: setMetadata.GetHashedName(), + Name: setMetadata.GetPrefixName(), + PolicyType: pType, + Values: strings.Join(members, ","), + } +} + +// verifyHNSCache asserts that HNS has the correct state. +// TODO: move all these functions to common location used by windows test files in pkg ipsets and policies. +func verifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) { + // we want to evaluate both verify functions even if one fails, so don't write as verifySetPolicies() && verifyACLs() in case of short-circuiting + success := verifySetPolicies(t, hns, expectedSetPolicies) + success = success && verifyACLs(t, hns, expectedEndpointACLs) + + printGetAllOutput(hns) + if !success { + require.FailNow(t, fmt.Sprintf("hns cache had unexpected state. printing hns cache...\n%s", hns.Cache.PrettyString())) + } +} + +// verifySetPolicies is true if HNS strictly has the expected SetPolicies. +func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting) bool { + cachedSetPolicies := hns.Cache.AllSetPolicies(azureNetworkID) + + success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") + for _, expectedSetPolicy := range expectedSetPolicies { + cachedSetPolicy, ok := cachedSetPolicies[expectedSetPolicy.Id] + success = success && assert.True(t, ok, fmt.Sprintf("expected SetPolicy not found. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) + if !ok { + continue + } + + members := strings.Split(cachedSetPolicy.Values, ",") + sort.Strings(members) + copyOfCachedSetPolicy := *cachedSetPolicy + copyOfCachedSetPolicy.Values = strings.Join(members, ",") + + success = success && assert.Equal(t, expectedSetPolicy, ©OfCachedSetPolicy, fmt.Sprintf("SetPolicy has unexpected contents. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) + } + + return success +} + +// verifyACLs is true if HNS strictly has the expected Endpoints and ACLs. +func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) bool { + cachedEndpointACLs := hns.Cache.GetAllACLs() + + success := assert.Equal(t, len(expectedEndpointACLs), len(cachedEndpointACLs), "unexpected number of Endpoints") + for epID, expectedACLs := range expectedEndpointACLs { + cachedACLs, ok := cachedEndpointACLs[epID] + success = success && assert.True(t, ok, fmt.Sprintf("expected ACL not found for endpoint %s", epID)) + if !ok { + continue + } + + for _, expectedACL := range expectedACLs { + success = success && assert.Equal(t, len(expectedACLs), len(cachedACLs), fmt.Sprintf("unexpected number of ACLs for Endpoint with ID: %s", epID)) + + foundACL := false + for _, cacheACL := range cachedACLs { + if expectedACL.ID == cacheACL.ID { + if reflect.DeepEqual(expectedACL, cacheACL) { + foundACL = true + break + } + } + } + success = success && assert.True(t, foundACL, fmt.Sprintf("missing expected ACL. ID: %s, full ACL: %+v", expectedACL.ID, expectedACL)) + } + } + return success +} + +// helpful for debugging if there's a discrepancy between GetAll functions and the HNS PrettyString +func printGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { + klog.Info("SETPOLICIES...") + for _, setPol := range hns.Cache.AllSetPolicies(azureNetworkID) { + klog.Infof("%+v\n", setPol) + } + klog.Info("Endpoint ACLs...") + for id, acls := range hns.Cache.GetAllACLs() { + a := make([]string, len(acls)) + for k, v := range acls { + a[k] = fmt.Sprintf("%+v", v) + } + klog.Infof("%s:\n%s\n", id, strings.Join(a, "\n")) + } +} From bf03f80cadb5813edcf2da7573600d20f220218f Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 13 Oct 2022 18:30:15 -0700 Subject: [PATCH 04/34] marshal setpolicies in hns mock and dont short circuit in UTs --- network/hnswrapper/hnsv2wrapperfake.go | 22 +++++++++++++++++---- npm/pkg/dataplane/dataplane_windows_test.go | 21 +++++++------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 4400239a86..6b50d318ea 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -346,7 +346,7 @@ func (fCache FakeHNSCache) PrettyString() string { endpointStrings = append(endpointStrings, fmt.Sprintf("[%+v]", endpoint.PrettyString())) } - return fmt.Sprintf("networks: [%s]\nendpoints: [%s]", strings.Join(networkStrings, ","), strings.Join(endpointStrings, ",")) + return fmt.Sprintf("networks: %s\nendpoints: %s", strings.Join(networkStrings, ","), strings.Join(endpointStrings, ",")) } // AllSetPolicies returns all SetPolicies in a given network as a map of SetPolicy ID to SetPolicy object. @@ -429,10 +429,24 @@ func (fNetwork *FakeHostComputeNetwork) PrettyString() string { } func (fNetwork *FakeHostComputeNetwork) GetHCNObj() *hcn.HostComputeNetwork { + setPolicies := make([]hcn.NetworkPolicy, 0) + for _, setPolicy := range fNetwork.Policies { + rawSettings, err := json.Marshal(setPolicy) + if err != nil { + fmt.Printf("FakeHostComputeNetwork: error marshalling SetPolicy: %+v. err: %s\n", setPolicy, err.Error()) + continue + } + policy := hcn.NetworkPolicy{ + Type: hcn.SetPolicy, + Settings: rawSettings, + } + setPolicies = append(setPolicies, policy) + } + return &hcn.HostComputeNetwork{ - Id: fNetwork.ID, - Name: fNetwork.Name, - // FIXME need to include SetPolicies + Id: fNetwork.ID, + Name: fNetwork.Name, + Policies: setPolicies, } } diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 9ef611f520..88feeee5b1 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -306,15 +306,9 @@ func TestAllEventSequences(t *testing.T) { verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) // uncomment to see output even for succeeding test cases - // require.Fail(t, "DEBUGME") + require.Fail(t, "DEBUGME: successful") }) } - - // I1013 15:07:01.118147 12452 ipsetmanager_windows.go:226] [IPSetManager Windows] Done applying IPSets. - // I1013 15:07:01.118147 12452 dataplane_windows.go:279] Getting all endpoints for Network ID 1234 - // I1013 15:07:01.118147 12452 dataplane_windows.go:318] Endpoint ID test1 has no IPAddreses - // I1013 15:07:01.118147 12452 dataplane_windows.go:105] [DataPlane] updatePod called for Pod Key pod1 - // W1013 15:07:01.118147 12452 dataplane_windows.go:121] [DataPlane] did not find endpoint with IPaddress 10.0.0.1 } func setPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { @@ -341,7 +335,7 @@ func setPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPol func verifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) { // we want to evaluate both verify functions even if one fails, so don't write as verifySetPolicies() && verifyACLs() in case of short-circuiting success := verifySetPolicies(t, hns, expectedSetPolicies) - success = success && verifyACLs(t, hns, expectedEndpointACLs) + success = verifyACLs(t, hns, expectedEndpointACLs) && success printGetAllOutput(hns) if !success { @@ -356,7 +350,7 @@ func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedS success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") for _, expectedSetPolicy := range expectedSetPolicies { cachedSetPolicy, ok := cachedSetPolicies[expectedSetPolicy.Id] - success = success && assert.True(t, ok, fmt.Sprintf("expected SetPolicy not found. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) + success = assert.True(t, ok, fmt.Sprintf("expected SetPolicy not found. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success if !ok { continue } @@ -366,7 +360,7 @@ func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedS copyOfCachedSetPolicy := *cachedSetPolicy copyOfCachedSetPolicy.Values = strings.Join(members, ",") - success = success && assert.Equal(t, expectedSetPolicy, ©OfCachedSetPolicy, fmt.Sprintf("SetPolicy has unexpected contents. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) + success = assert.Equal(t, expectedSetPolicy, ©OfCachedSetPolicy, fmt.Sprintf("SetPolicy has unexpected contents. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success } return success @@ -379,14 +373,13 @@ func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpoint success := assert.Equal(t, len(expectedEndpointACLs), len(cachedEndpointACLs), "unexpected number of Endpoints") for epID, expectedACLs := range expectedEndpointACLs { cachedACLs, ok := cachedEndpointACLs[epID] - success = success && assert.True(t, ok, fmt.Sprintf("expected ACL not found for endpoint %s", epID)) + success = assert.True(t, ok, fmt.Sprintf("expected ACL not found for endpoint %s", epID)) && success if !ok { continue } + success = assert.Equal(t, len(expectedACLs), len(cachedACLs), fmt.Sprintf("unexpected number of ACLs for Endpoint with ID: %s", epID)) && success for _, expectedACL := range expectedACLs { - success = success && assert.Equal(t, len(expectedACLs), len(cachedACLs), fmt.Sprintf("unexpected number of ACLs for Endpoint with ID: %s", epID)) - foundACL := false for _, cacheACL := range cachedACLs { if expectedACL.ID == cacheACL.ID { @@ -396,7 +389,7 @@ func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpoint } } } - success = success && assert.True(t, foundACL, fmt.Sprintf("missing expected ACL. ID: %s, full ACL: %+v", expectedACL.ID, expectedACL)) + success = assert.True(t, foundACL, fmt.Sprintf("missing expected ACL. ID: %s, full ACL: %+v", expectedACL.ID, expectedACL)) && success } } return success From 8700de4b11e2c0fc57a9ed43c8b83d871c1e7fb2 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 13 Oct 2022 18:30:47 -0700 Subject: [PATCH 05/34] policy stuff and update test cases --- npm/pkg/dataplane/dataplane_windows_test.go | 184 +++++++++++++++++--- 1 file changed, 164 insertions(+), 20 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 88feeee5b1..e8d9ddb556 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -10,10 +10,13 @@ import ( "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" ) @@ -38,6 +41,7 @@ const ( defaultHNSLatency = time.Duration(0) ) +// IPSet constants var ( podLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) podLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) @@ -60,6 +64,37 @@ var ( nsLabelVal2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) ) +// POLICIES +// see translatePolicy_test.go for example rules + +func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labelPair1-allow-all", + Namespace: ns1Set.Name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k1": "v1", + }, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {}, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + {}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, + } +} + +// DP EVENTS + // podCreateEvent models a Pod CREATE in the PodController func podCreateEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { @@ -129,17 +164,18 @@ func nsDeleteEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMe } // policyUpdateEvent models a Network Policy CREATE/UPDATE in the PolicyController -// FIXME have NPMNetworkPolicy as input -func policyUpdateEvent(policyKey string) dpEvent { +func policyUpdateEvent(policy *networkingv1.NetworkPolicy) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { - // TODO + npmNetPol, err := translation.TranslatePolicy(policy) + require.Nil(t, err, "failed to translate policy") + require.Nil(t, dp.UpdatePolicy(npmNetPol), "failed to update policy") } } // policyDeleteEvent models a Network Policy DELETE in the PolicyController func policyDeleteEvent(policyKey string) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { - // TODO + require.Nil(t, dp.RemovePolicy(policyKey), "failed to delete policy") } } @@ -224,7 +260,125 @@ func TestAllEventSequences(t *testing.T) { }, }, { - name: "pod created then deleted", + name: "policy created with no pods", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + // pre-defined dpEvent + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + // will not be an all-namespaces IPSet unless there's a Pod/Namespace event + setPolicy(ns1Set), + // Policies do not create the KeyLabelOfPod type IPSet if the selector has a key-value requirement + setPolicy(podLabelVal1Set), + }, + expectedEnpdointACLs: nil, + }, + { + name: "pod created on node -> policy created and applied to it", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + { + // FIXME: failing because need to fix hnsv2 wrapper fake to marshal Endpoint ACLs + name: "pod created on node -> policy created and applied to it -> policy deleted", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + { + // NOTE: this fails right now. we incorrectly add a policy + name: "pod created off node -> relevant policy created but not applied (even though there's a local Endpoint with the same IP)", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + { + name: "pod created off node -> relevant policy created but not applied (no local Endpoint)", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + podCreateEvent(NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: nil, + }, + { + name: "pod created -> pod deleted", cfg: dpCfg, ipEndpoints: nil, events: []dpEvent{ @@ -233,6 +387,7 @@ func TestAllEventSequences(t *testing.T) { podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), endpointDeleteEvent(endpoint1), podDeleteEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + // garbage collect IPSets func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { dp.ipsetMgr.Reconcile() require.Nil(t, dp.ApplyDataPlane()) @@ -247,14 +402,15 @@ func TestAllEventSequences(t *testing.T) { expectedEnpdointACLs: nil, }, { - name: "pod created then updated, with policy add in background", + // FIXME: may fail because need to fix hnsv2 wrapper fake to marshal Endpoint ACLs + name: "pod created on node -> relevant policy created in background -> pod updated so policy no longer relevant", cfg: dpCfg, ipEndpoints: nil, events: []dpEvent{ // pre-defined dpEvents endpointCreateEvent(endpoint1, ip1), podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - backgroundEvent(policyUpdateEvent("x/base")), + backgroundEvent(policyUpdateEvent(policyNs1LabelPair1AllowAll())), podUpdateEventSameIP(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), }, expectedSetPolicies: []*hcn.SetPolicySetting{ @@ -268,19 +424,7 @@ func TestAllEventSequences(t *testing.T) { setPolicy(podLabel1Set), }, expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: { - // { - // ID: "", - // Protocols: "", - // Action: "", - // Direction: "", - // LocalAddresses: "", - // RemoteAddresses: "", - // LocalPorts: "", - // RemotePorts: "", - // Priority: 0, - // }, - }, + endpoint1: {}, }, }, } From f69c7a877e1be96c1fd2560f12f62b10c2484056 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 14 Oct 2022 13:14:27 -0700 Subject: [PATCH 06/34] marshal ACLs in hns mock --- network/hnswrapper/hnsv2wrapperfake.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 6b50d318ea..27b4e77445 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -481,8 +481,21 @@ func (fEndpoint *FakeHostComputeEndpoint) PrettyString() string { } func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { - // NOTE: not including other policy types like perhaps SetPolicies - hcnEndpoint := &hcn.HostComputeEndpoint{ + acls := make([]hcn.EndpointPolicy, 0) + for _, acl := range fEndpoint.Policies { + rawSettings, err := json.Marshal(acl) + if err != nil { + fmt.Printf("FakeHostComputeEndpoint: error marshalling ACL: %+v. err: %s\n", acl, err.Error()) + continue + } + policy := hcn.EndpointPolicy{ + Type: hcn.ACL, + Settings: rawSettings, + } + acls = append(acls, policy) + } + + return &hcn.HostComputeEndpoint{ Id: fEndpoint.ID, Name: fEndpoint.Name, HostComputeNetwork: fEndpoint.HostComputeNetwork, @@ -491,8 +504,7 @@ func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { IpAddress: fEndpoint.IPConfiguration, }, }, - // FIXME transfer policies with JSON marshalling - Policies: []hcn.EndpointPolicy{}, + Policies: acls, } return hcnEndpoint } From 2d0de59a37669dee9c72bec203d2be44246eaa31 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 14 Oct 2022 13:14:54 -0700 Subject: [PATCH 07/34] more UTs and minor refinements --- npm/pkg/dataplane/dataplane_windows_test.go | 76 +++++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index e8d9ddb556..c5157121b6 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -71,7 +71,7 @@ func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { return &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: "labelPair1-allow-all", - Namespace: ns1Set.Name, + Namespace: "ns1", }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ @@ -319,7 +319,6 @@ func TestAllEventSequences(t *testing.T) { }, }, { - // FIXME: failing because need to fix hnsv2 wrapper fake to marshal Endpoint ACLs name: "pod created on node -> policy created and applied to it -> policy deleted", cfg: dpCfg, ipEndpoints: nil, @@ -327,6 +326,7 @@ func TestAllEventSequences(t *testing.T) { endpointCreateEvent(endpoint1, ip1), podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), policyUpdateEvent(policyNs1LabelPair1AllowAll()), + policyDeleteEvent("ns1/labelPair1-allow-all"), }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(emptySet), @@ -402,7 +402,71 @@ func TestAllEventSequences(t *testing.T) { expectedEnpdointACLs: nil, }, { - // FIXME: may fail because need to fix hnsv2 wrapper fake to marshal Endpoint ACLs + name: "policy created -> pod created which satisfies the policy pod selector", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + { + // FIXME: debug why this case fails + name: "policy created -> pod created which satisfies the policy pod selector -> pod relabeled so policy removed", + cfg: dpCfg, + ipEndpoints: nil, + events: []dpEvent{ + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podUpdateEventSameIP(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + }, + expectedSetPolicies: []*hcn.SetPolicySetting{ + setPolicy(emptySet), + setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + setPolicy(ns1Set, ip1), + setPolicy(podLabel1Set, ip1), + setPolicy(podLabelVal1Set, ip1), + }, + expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + { name: "pod created on node -> relevant policy created in background -> pod updated so policy no longer relevant", cfg: dpCfg, ipEndpoints: nil, @@ -450,7 +514,7 @@ func TestAllEventSequences(t *testing.T) { verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) // uncomment to see output even for succeeding test cases - require.Fail(t, "DEBUGME: successful") + // require.Fail(t, "DEBUGME: successful") }) } } @@ -543,7 +607,7 @@ func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpoint func printGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { klog.Info("SETPOLICIES...") for _, setPol := range hns.Cache.AllSetPolicies(azureNetworkID) { - klog.Infof("%+v\n", setPol) + klog.Infof("%+v", setPol) } klog.Info("Endpoint ACLs...") for id, acls := range hns.Cache.GetAllACLs() { @@ -551,6 +615,6 @@ func printGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { for k, v := range acls { a[k] = fmt.Sprintf("%+v", v) } - klog.Infof("%s:\n%s\n", id, strings.Join(a, "\n")) + klog.Infof("%s: %s", id, strings.Join(a, ",")) } } From ce0b406c3ed996a154fc8a362bbba297b6fce9d1 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 17 Oct 2022 10:58:30 -0700 Subject: [PATCH 08/34] option to apply dp or not --- npm/pkg/dataplane/dataplane_windows_test.go | 53 ++++++++++++--------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index c5157121b6..516d7d7a06 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -26,6 +26,9 @@ type dpEvent func(*testing.T, *DataPlane, *hnswrapper.Hnsv2wrapperFake) const azureNetworkID = "1234" const ( + applyDP bool = true + doNotApplyDP bool = false + thisNode = "this-node" otherNode = "other-node" @@ -96,7 +99,7 @@ func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { // DP EVENTS // podCreateEvent models a Pod CREATE in the PodController -func podCreateEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { +func podCreateEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { // PodController might not call this if the namespace already existed require.Nil(t, dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet})) @@ -104,12 +107,14 @@ func podCreateEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) require.Nil(t, dp.AddToSets(labelIPSets, pod)) - require.Nil(t, dp.ApplyDataPlane()) + if shouldApply { + require.Nil(t, dp.ApplyDataPlane()) + } } } // podUpdateEvent models a Pod UPDATE in the PodController -func podUpdateEvent(oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { +func podUpdateEvent(shouldApply bool, oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { // think it's impossible for this to be called on an UPDATE // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) @@ -122,42 +127,46 @@ func podUpdateEvent(oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, newPod)) } - require.Nil(t, dp.ApplyDataPlane()) + if shouldApply { + require.Nil(t, dp.ApplyDataPlane()) + } } } // podUpdateEvent models a Pod UPDATE in the PodController where the Pod does not change IP/node. -func podUpdateEventSameIP(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { - return podUpdateEvent(pod, pod, nsIPSet, toRemoveLabelSets, toAddLabelSets) +func podUpdateEventSameIP(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { + return podUpdateEvent(shouldApply, pod, pod, nsIPSet, toRemoveLabelSets, toAddLabelSets) } // podDeleteEvent models a Pod DELETE in the PodController -func podDeleteEvent(pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { +func podDeleteEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { require.Nil(t, dp.RemoveFromSets([]*ipsets.IPSetMetadata{nsIPSet}, pod)) // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) require.Nil(t, dp.RemoveFromSets(labelIPSets, pod)) - require.Nil(t, dp.ApplyDataPlane()) + if shouldApply { + require.Nil(t, dp.ApplyDataPlane()) + } } } // nsCreateEvent models a Namespace CREATE in the NamespaceController -func nsCreateEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { +func nsCreateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { // TODO } } // nsUpdateEvent models a Namespace UPDATE in the NamespaceController -func nsUpdateEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { +func nsUpdateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { // TODO } } // nsDeleteEvent models a Namespace DELETE in the NamespaceController -func nsDeleteEvent(nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { +func nsDeleteEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { // TODO } @@ -281,7 +290,7 @@ func TestAllEventSequences(t *testing.T) { ipEndpoints: nil, events: []dpEvent{ endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), policyUpdateEvent(policyNs1LabelPair1AllowAll()), }, expectedSetPolicies: []*hcn.SetPolicySetting{ @@ -324,7 +333,7 @@ func TestAllEventSequences(t *testing.T) { ipEndpoints: nil, events: []dpEvent{ endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), policyUpdateEvent(policyNs1LabelPair1AllowAll()), policyDeleteEvent("ns1/labelPair1-allow-all"), }, @@ -346,7 +355,7 @@ func TestAllEventSequences(t *testing.T) { ipEndpoints: nil, events: []dpEvent{ endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), policyUpdateEvent(policyNs1LabelPair1AllowAll()), }, expectedSetPolicies: []*hcn.SetPolicySetting{ @@ -365,7 +374,7 @@ func TestAllEventSequences(t *testing.T) { cfg: dpCfg, ipEndpoints: nil, events: []dpEvent{ - podCreateEvent(NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), policyUpdateEvent(policyNs1LabelPair1AllowAll()), }, expectedSetPolicies: []*hcn.SetPolicySetting{ @@ -384,9 +393,9 @@ func TestAllEventSequences(t *testing.T) { events: []dpEvent{ // mix of pre-defined dpEvents and a custom one endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), endpointDeleteEvent(endpoint1), - podDeleteEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podDeleteEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), // garbage collect IPSets func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { dp.ipsetMgr.Reconcile() @@ -408,7 +417,7 @@ func TestAllEventSequences(t *testing.T) { events: []dpEvent{ policyUpdateEvent(policyNs1LabelPair1AllowAll()), endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(emptySet), @@ -452,8 +461,8 @@ func TestAllEventSequences(t *testing.T) { events: []dpEvent{ policyUpdateEvent(policyNs1LabelPair1AllowAll()), endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - podUpdateEventSameIP(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(emptySet), @@ -473,9 +482,9 @@ func TestAllEventSequences(t *testing.T) { events: []dpEvent{ // pre-defined dpEvents endpointCreateEvent(endpoint1, ip1), - podCreateEvent(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), backgroundEvent(policyUpdateEvent(policyNs1LabelPair1AllowAll())), - podUpdateEventSameIP(NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(emptySet), From 4c903f4e5a51f983b09488b703b06042483c6589 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 17 Oct 2022 16:39:11 -0700 Subject: [PATCH 09/34] address cmp.Equal and t.Helper comments --- npm/pkg/dataplane/dataplane_windows_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 516d7d7a06..10887b853a 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -2,7 +2,6 @@ package dataplane import ( "fmt" - "reflect" "sort" "strings" "testing" @@ -13,6 +12,8 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" networkingv1 "k8s.io/api/networking/v1" @@ -550,11 +551,14 @@ func setPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPol // verifyHNSCache asserts that HNS has the correct state. // TODO: move all these functions to common location used by windows test files in pkg ipsets and policies. func verifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) { + t.Helper() + + printGetAllOutput(hns) + // we want to evaluate both verify functions even if one fails, so don't write as verifySetPolicies() && verifyACLs() in case of short-circuiting success := verifySetPolicies(t, hns, expectedSetPolicies) success = verifyACLs(t, hns, expectedEndpointACLs) && success - printGetAllOutput(hns) if !success { require.FailNow(t, fmt.Sprintf("hns cache had unexpected state. printing hns cache...\n%s", hns.Cache.PrettyString())) } @@ -562,6 +566,8 @@ func verifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetP // verifySetPolicies is true if HNS strictly has the expected SetPolicies. func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting) bool { + t.Helper() + cachedSetPolicies := hns.Cache.AllSetPolicies(azureNetworkID) success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") @@ -585,6 +591,8 @@ func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedS // verifyACLs is true if HNS strictly has the expected Endpoints and ACLs. func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) bool { + t.Helper() + cachedEndpointACLs := hns.Cache.GetAllACLs() success := assert.Equal(t, len(expectedEndpointACLs), len(cachedEndpointACLs), "unexpected number of Endpoints") @@ -600,7 +608,7 @@ func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpoint foundACL := false for _, cacheACL := range cachedACLs { if expectedACL.ID == cacheACL.ID { - if reflect.DeepEqual(expectedACL, cacheACL) { + if cmp.Equal(expectedACL, cacheACL) { foundACL = true break } From af2da1bc8c32c042077ad0d00534e5e512e2aa84 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 18 Oct 2022 10:41:47 -0700 Subject: [PATCH 10/34] dpEvent returns error and better defined concurrency --- npm/pkg/dataplane/dataplane_windows_test.go | 252 +++++++++++++++----- 1 file changed, 192 insertions(+), 60 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 10887b853a..78eb118f32 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -4,6 +4,7 @@ import ( "fmt" "sort" "strings" + "sync" "testing" "time" @@ -21,7 +22,9 @@ import ( "k8s.io/klog" ) -type dpEvent func(*testing.T, *DataPlane, *hnswrapper.Hnsv2wrapperFake) +type dpEvent func(*DataPlane, *hnswrapper.Hnsv2wrapperFake) error +type thread []dpEvent +type concurrentSession []thread // FIXME move this into the common code path along with the verification functions below const azureNetworkID = "1234" @@ -101,36 +104,59 @@ func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { // podCreateEvent models a Pod CREATE in the PodController func podCreateEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { + context := fmt.Sprintf("create context: [pod: %+v. ns: %s. label sets: %+v]", pod, nsIPSet.Name, prefixNames(labelIPSets)) + // PodController might not call this if the namespace already existed - require.Nil(t, dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet})) - require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{nsIPSet}, pod)) + if err := dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add ns set to all namespaces list. %s", context) + } + + if err := dp.AddToSets([]*ipsets.IPSetMetadata{nsIPSet}, pod); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to ns set. %s", context) + } + // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) - require.Nil(t, dp.AddToSets(labelIPSets, pod)) + if err := dp.AddToSets(labelIPSets, pod); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to label sets. %s", context) + } if shouldApply { - require.Nil(t, dp.ApplyDataPlane()) + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to apply. %s", context) + } } + return nil } } // podUpdateEvent models a Pod UPDATE in the PodController func podUpdateEvent(shouldApply bool, oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { + context := fmt.Sprintf("update context: [old pod: %+v. new pod: %+v. ns: %s. label sets to remove: %+v. label sets to add: %+v]", + oldPod, newPod, nsIPSet.Name, prefixNames(toRemoveLabelSets), prefixNames(toAddLabelSets)) + // think it's impossible for this to be called on an UPDATE // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) for _, toRemoveSet := range toRemoveLabelSets { - require.Nil(t, dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, oldPod)) + if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, oldPod); err != nil { + return errors.Wrapf(err, "[podUpdateEvent] failed to remove old pod ip from set %s. %s", toRemoveSet.GetPrefixName(), context) + } } for _, toAddSet := range toAddLabelSets { - require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, newPod)) + if err := dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, newPod); err != nil { + return errors.Wrapf(err, "[podUpdateEvent] failed to add new pod ip to set %s. %s", toAddSet.GetPrefixName(), context) + } } if shouldApply { - require.Nil(t, dp.ApplyDataPlane()) + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[podUpdateEvent] failed to apply. %s", context) + } } + return nil } } @@ -141,58 +167,80 @@ func podUpdateEventSameIP(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IP // podDeleteEvent models a Pod DELETE in the PodController func podDeleteEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { - require.Nil(t, dp.RemoveFromSets([]*ipsets.IPSetMetadata{nsIPSet}, pod)) + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { + context := fmt.Sprintf("delete context: [pod: %+v. ns: %s. label sets: %+v]", pod, nsIPSet.Name, prefixNames(labelIPSets)) + + if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{nsIPSet}, pod); err != nil { + return errors.Wrapf(err, "[podDeleteEvent] failed to remove pod ip from ns set. %s", context) + } + // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) - require.Nil(t, dp.RemoveFromSets(labelIPSets, pod)) + if err := dp.RemoveFromSets(labelIPSets, pod); err != nil { + return errors.Wrapf(err, "[podDeleteEvent] failed to remove pod ip from label sets. %s", context) + } if shouldApply { - require.Nil(t, dp.ApplyDataPlane()) + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[podDeleteEvent] failed to apply. %s", context) + } } + return nil } } // nsCreateEvent models a Namespace CREATE in the NamespaceController func nsCreateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { // TODO + return nil } } // nsUpdateEvent models a Namespace UPDATE in the NamespaceController func nsUpdateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { // TODO + return nil } } // nsDeleteEvent models a Namespace DELETE in the NamespaceController func nsDeleteEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { // TODO + return nil } } // policyUpdateEvent models a Network Policy CREATE/UPDATE in the PolicyController func policyUpdateEvent(policy *networkingv1.NetworkPolicy) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { npmNetPol, err := translation.TranslatePolicy(policy) - require.Nil(t, err, "failed to translate policy") - require.Nil(t, dp.UpdatePolicy(npmNetPol), "failed to update policy") + if err != nil { + return errors.Wrapf(err, "[policyUpdateEvent] failed to translate policy with key %s/%s", policy.Namespace, policy.Name) + } + + if err := dp.UpdatePolicy(npmNetPol); err != nil { + return errors.Wrapf(err, "[policyUpdateEvent] failed to update policy with key %s/%s", policy.Namespace, policy.Name) + } + return nil } } // policyDeleteEvent models a Network Policy DELETE in the PolicyController func policyDeleteEvent(policyKey string) dpEvent { - return func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { - require.Nil(t, dp.RemovePolicy(policyKey), "failed to delete policy") + return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { + if err := dp.RemovePolicy(policyKey); err != nil { + return errors.Wrapf(err, "[policyDeleteEvent] failed to remove policy with key %s", policyKey) + } + return nil } } // endpointCreateEvent models an Endpoint being created within HNS // With this Event, Endpoints can be created in between calls to dp func endpointCreateEvent(epID, ip string) dpEvent { - return func(t *testing.T, _ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { + return func(_ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) error { ep := &hcn.HostComputeEndpoint{ Id: epID, Name: epID, @@ -204,40 +252,28 @@ func endpointCreateEvent(epID, ip string) dpEvent { }, } _, err := hns.CreateEndpoint(ep) - require.Nil(t, err, "failed to create hns endpoint in mock: %+v", ep) + if err != nil { + return errors.Wrapf(err, "[endpointCreateEvent] failed to create hns endpoint. ep: %+v", ep) + } + return nil } } // endpointDeleteEvent models an Endpoint being deleted within HNS // With this Event, Endpoints can be deleted in between calls to dp func endpointDeleteEvent(epID string) dpEvent { - return func(t *testing.T, _ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { + return func(_ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) error { ep := &hcn.HostComputeEndpoint{ Id: epID, } - err := hns.DeleteEndpoint(ep) - require.Nil(t, err, "failed to create hns endpoint in mock: %+v", ep) - } -} - -// backgroundEvent will run the dpEvents in the background when called -func backgroundEvent(event1 dpEvent, otherEvents ...dpEvent) dpEvent { - allEvents := make([]dpEvent, 1) - allEvents[0] = event1 - allEvents = append(allEvents, otherEvents...) - - return func(t *testing.T, dp *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) { - go func() { - // delay would impact other threads as well - for _, event := range allEvents { - event(t, dp, hns) - } - }() + if err := hns.DeleteEndpoint(ep); err != nil { + return errors.Wrapf(err, "[endpointDeleteEvent] failed to delete hns endpoint. ep: %+v", ep) + } + return nil } } // TestAllEventSequences can test any config with a sequence of events. -// TODO double check HNS mock is working as planned func TestAllEventSequences(t *testing.T) { tests := []struct { name string @@ -255,15 +291,21 @@ func TestAllEventSequences(t *testing.T) { }, events: []dpEvent{ // custom dpEvent - func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { pod1 := NewPodMetadata(podKey1, ip1, thisNode) - require.Nil(t, dp.AddToSets([]*ipsets.IPSetMetadata{ns1Set, podLabel1Set}, pod1)) - require.Nil(t, dp.ApplyDataPlane()) + if err := dp.AddToSets([]*ipsets.IPSetMetadata{ns1Set, podLabel1Set}, pod1); err != nil { + return errors.Wrap(err, "[custom-add-apply] failed to add set") + } + + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[custom-add-apply] failed to apply dp") + } + return nil }, }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), + // setPolicy(podLabel1Set, ip1), }, expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ endpoint1: {}, @@ -398,9 +440,12 @@ func TestAllEventSequences(t *testing.T) { endpointDeleteEvent(endpoint1), podDeleteEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), // garbage collect IPSets - func(t *testing.T, dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) { + func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { dp.ipsetMgr.Reconcile() - require.Nil(t, dp.ApplyDataPlane()) + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrap(err, "[custom-reconcile] failed to apply dp") + } + return nil }, }, expectedSetPolicies: []*hcn.SetPolicySetting{ @@ -476,16 +521,62 @@ func TestAllEventSequences(t *testing.T) { endpoint1: {}, }, }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + hns := ipsets.GetHNSFake(t) + hns.Delay = defaultHNSLatency + io := common.NewMockIOShimWithFakeHNS(hns) + for ip, epID := range tt.ipEndpoints { + event := endpointCreateEvent(epID, ip) + require.Nil(t, event(nil, hns), "failed to create endpoint. ep ID: %s. ip: %s", epID, ip) + } + + dp, err := NewDataPlane(thisNode, io, tt.cfg, nil) + require.NoError(t, err, "failed to initialize dp") + + for k, event := range tt.events { + require.Nil(t, event(dp, hns), "failed while running event number %d", k) + } + + verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) + + // uncomment to see output even for succeeding test cases + // require.Fail(t, "DEBUGME: successful") + }) + } +} + +func TestConcurrentEvents(t *testing.T) { + tests := []struct { + name string + cfg *Config + ipEndpoints map[string]string + sessions []concurrentSession + expectedSetPolicies []*hcn.SetPolicySetting + expectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy + }{ { - name: "pod created on node -> relevant policy created in background -> pod updated so policy no longer relevant", + name: "pod created on node -> concurrent: 1) creating relevant policy <=> 2) updating pod so policy no longer relevant", cfg: dpCfg, ipEndpoints: nil, - events: []dpEvent{ - // pre-defined dpEvents - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - backgroundEvent(policyUpdateEvent(policyNs1LabelPair1AllowAll())), - podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + sessions: []concurrentSession{ + { + thread{ + endpointCreateEvent(endpoint1, ip1), + podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), + }, + }, + { + thread{ + policyUpdateEvent(policyNs1LabelPair1AllowAll()), + }, + thread{ + podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), + }, + }, }, expectedSetPolicies: []*hcn.SetPolicySetting{ setPolicy(emptySet), @@ -511,14 +602,47 @@ func TestAllEventSequences(t *testing.T) { io := common.NewMockIOShimWithFakeHNS(hns) for ip, epID := range tt.ipEndpoints { event := endpointCreateEvent(epID, ip) - event(t, nil, hns) + require.Nil(t, event(nil, hns), "failed to create endpoint. ep ID: %s. ip: %s", epID, ip) } dp, err := NewDataPlane(thisNode, io, tt.cfg, nil) require.NoError(t, err, "failed to initialize dp") - for _, event := range tt.events { - event(t, dp, hns) + for i, session := range tt.sessions { + if len(session) == 0 { + t.Fatal("sequence should have >= 1 threads") + } + + if len(session) == 1 { + // single-thread case: no need to run go routines + th := session[0] + for j, event := range th { + require.Nil(t, event(dp, hns), "failed while running single thread for sequence %d, event %d", i, j) + } + } else { + // concurrent case with multiple go routines + wg := new(sync.WaitGroup) + wg.Add(len(session)) + threadErrors := make(chan error, len(session)) + for j, th := range session { + go func(threadNum int, th thread) { + defer wg.Done() + for k, event := range th { + if err := event(dp, hns); err != nil { + threadErrors <- errors.Wrapf(err, "failed to run thread %d, event %d", threadNum, k) + return + } + } + threadErrors <- nil + }(j, th) + } + + wg.Wait() + close(threadErrors) + for err := range threadErrors { + assert.Nil(t, err, "failed during concurrency for sequence %d", i) + } + } } verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) @@ -635,3 +759,11 @@ func printGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { klog.Infof("%s: %s", id, strings.Join(a, ",")) } } + +func prefixNames(sets []*ipsets.IPSetMetadata) []string { + a := make([]string, len(sets)) + for k, s := range sets { + a[k] = s.GetPrefixName() + } + return a +} From ee91dfd5930b489ca09dd7e4e8db38e5274fb66c Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Tue, 18 Oct 2022 16:46:29 -0700 Subject: [PATCH 11/34] remove unnecessary logic in concurrent test code --- npm/pkg/dataplane/dataplane_windows_test.go | 50 ++++++++------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 78eb118f32..4f6bfffb2e 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -609,39 +609,27 @@ func TestConcurrentEvents(t *testing.T) { require.NoError(t, err, "failed to initialize dp") for i, session := range tt.sessions { - if len(session) == 0 { - t.Fatal("sequence should have >= 1 threads") - } - - if len(session) == 1 { - // single-thread case: no need to run go routines - th := session[0] - for j, event := range th { - require.Nil(t, event(dp, hns), "failed while running single thread for sequence %d, event %d", i, j) - } - } else { - // concurrent case with multiple go routines - wg := new(sync.WaitGroup) - wg.Add(len(session)) - threadErrors := make(chan error, len(session)) - for j, th := range session { - go func(threadNum int, th thread) { - defer wg.Done() - for k, event := range th { - if err := event(dp, hns); err != nil { - threadErrors <- errors.Wrapf(err, "failed to run thread %d, event %d", threadNum, k) - return - } + // concurrent case with multiple go routines + wg := new(sync.WaitGroup) + wg.Add(len(session)) + threadErrors := make(chan error, len(session)) + for j, th := range session { + go func(threadNum int, th thread) { + defer wg.Done() + for k, event := range th { + if err := event(dp, hns); err != nil { + threadErrors <- errors.Wrapf(err, "failed to run thread %d, event %d", threadNum, k) + return } - threadErrors <- nil - }(j, th) - } + } + threadErrors <- nil + }(j, th) + } - wg.Wait() - close(threadErrors) - for err := range threadErrors { - assert.Nil(t, err, "failed during concurrency for sequence %d", i) - } + wg.Wait() + close(threadErrors) + for err := range threadErrors { + assert.Nil(t, err, "failed during concurrency for sequence %d", i) } } From eecb47debceec075fc732c4cee18b898da302ab7 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Thu, 20 Oct 2022 10:15:51 -0700 Subject: [PATCH 12/34] approach #3 emulating cyclonus --- npm/pkg/dataplane/e2es/e2e_windows_test.go | 74 ++++++++ .../dataplane/e2es/npm-types_windows_test.go | 107 +++++++++++ .../dataplane/e2es/test-cases_windows_test.go | 176 ++++++++++++++++++ npm/pkg/dataplane/e2es/types_windows_test.go | 120 ++++++++++++ npm/pkg/dataplane/testutils/utils_windows.go | 148 +++++++++++++++ 5 files changed, 625 insertions(+) create mode 100644 npm/pkg/dataplane/e2es/e2e_windows_test.go create mode 100644 npm/pkg/dataplane/e2es/npm-types_windows_test.go create mode 100644 npm/pkg/dataplane/e2es/test-cases_windows_test.go create mode 100644 npm/pkg/dataplane/e2es/types_windows_test.go create mode 100644 npm/pkg/dataplane/testutils/utils_windows.go diff --git a/npm/pkg/dataplane/e2es/e2e_windows_test.go b/npm/pkg/dataplane/e2es/e2e_windows_test.go new file mode 100644 index 0000000000..5897e4b0b3 --- /dev/null +++ b/npm/pkg/dataplane/e2es/e2e_windows_test.go @@ -0,0 +1,74 @@ +package e2e + +import ( + "sync" + "testing" + "time" + + "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/pkg/errors" + + "github.com/Azure/azure-container-networking/npm/pkg/dataplane" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const defaultHNSLatency = time.Duration(0) + +func TestAll(t *testing.T) { + tests := getAllTests() + for i, tt := range tests { + i := i + tt := tt + t.Run(tt.Description, func(t *testing.T) { + t.Logf("beginning test #%d. Description: [%s]. Tags: %+v", i, tt.Description, tt.Tags) + + hns := ipsets.GetHNSFake(t) + hns.Delay = defaultHNSLatency + io := common.NewMockIOShimWithFakeHNS(hns) + for _, ep := range tt.InitialEndpoints { + _, err := hns.CreateEndpoint(ep) + require.Nil(t, err, "failed to create initial endpoint %+v", ep) + } + + // the dp is necessary for NPM tests + dp, err := dataplane.NewDataPlane(thisNode, io, tt.DpCfg, nil) + require.NoError(t, err, "failed to initialize dp") + + backgroundErrors := make(chan error) + for _, s := range tt.Steps { + s.SetHNS(hns) + // necessary for NPM tests + s.SetDP(dp) + + if s.InBackground { + wg := new(sync.WaitGroup) + wg.Add(1) + tt.AddStepWaitGroup(s.ID, wg) + go func() { + defer wg.Done() + tt.WaitToRun(s.ID) + if err := s.Do(); err != nil { + backgroundErrors <- errors.Wrapf(err, "failed to run step in background: %s", s.ID) + } + }() + } else { + if !assert.Nil(t, s.Do(), "failed to run step in foreground: %s", s.ID) { + // stop processing steps on a foreground failure + break + } + } + } + + tt.WaitForAll() + close(backgroundErrors) + for err := range backgroundErrors { + assert.Nil(t, err, "failed during concurrency") + } + + dptestutils.VerifyHNSCache(t, hns, tt.ExpectedSetPolicies, tt.ExpectedEnpdointACLs) + }) + } +} diff --git a/npm/pkg/dataplane/e2es/npm-types_windows_test.go b/npm/pkg/dataplane/e2es/npm-types_windows_test.go new file mode 100644 index 0000000000..1e26bc1b49 --- /dev/null +++ b/npm/pkg/dataplane/e2es/npm-types_windows_test.go @@ -0,0 +1,107 @@ +package e2e + +import ( + "fmt" + + "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/pkg/errors" + + networkingv1 "k8s.io/api/networking/v1" +) + +type PodCreateAction struct { + *DPAction + // TODO Pod name and node instead of podmetadata object (infer podkey from name and namespace) + Pod *dataplane.PodMetadata + Namespace string + Labels map[string]string +} + +func (p *PodCreateAction) Do() error { + if p.dp == nil { + return errors.New("no DP") // FIXME make constant type for lint + } + + context := fmt.Sprintf("create context: [pod: %+v. ns: %s. labels: %+v]", p.Pod, p.Namespace, p.Labels) + + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Namespace, ipsets.Namespace)} + // PodController technically wouldn't call this if the namespace already existed + if err := p.dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, nsIPSet); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add ns set to all namespaces list. %s", context) + } + + if err := p.dp.AddToSets(nsIPSet, p.Pod); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to ns set. %s", context) + } + + for key, val := range p.Labels { + keyVal := fmt.Sprintf("%s:%s", key, val) + labelIPSets := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(key, ipsets.KeyLabelOfPod), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), + } + + if err := p.dp.AddToSets(labelIPSets, p.Pod); err != nil { + return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to label sets. %s", context) + } + + } + + return nil +} + +type ApplyDPAction struct { + *DPAction +} + +func (a *ApplyDPAction) Do() error { + if a.dp == nil { + return errors.New("no DP") // FIXME make constant type for lint + } + + if err := a.dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[ApplyDPAction] failed to apply") + } + return nil +} + +type PolicyUpdateAction struct { + *DPAction + Policy *networkingv1.NetworkPolicy +} + +func (p *PolicyUpdateAction) Do() error { + if p.dp == nil { + return errors.New("no DP") // FIXME make constant type for lint + } + + npmNetPol, err := translation.TranslatePolicy(p.Policy) + if err != nil { + return errors.Wrapf(err, "[PolicyUpdateAction] failed to translate policy with key %s/%s", p.Policy.Namespace, p.Policy.Name) + } + + if err := p.dp.UpdatePolicy(npmNetPol); err != nil { + return errors.Wrapf(err, "[PolicyUpdateAction] failed to update policy with key %s/%s", p.Policy.Namespace, p.Policy.Name) + } + return nil +} + +type PolicyDeleteAction struct { + *DPAction + Namespace string + Name string +} + +func (p *PolicyDeleteAction) Do() error { + if p.dp == nil { + return errors.New("no DP") // FIXME make constant type for lint + } + + policyKey := fmt.Sprintf("%s/%s", p.Namespace, p.Name) + if err := p.dp.RemovePolicy(policyKey); err != nil { + return errors.Wrapf(err, "[PolicyDeleteAction] failed to update policy with key %s", policyKey) + } + return nil +} diff --git a/npm/pkg/dataplane/e2es/test-cases_windows_test.go b/npm/pkg/dataplane/e2es/test-cases_windows_test.go new file mode 100644 index 0000000000..d7db05ad9c --- /dev/null +++ b/npm/pkg/dataplane/e2es/test-cases_windows_test.go @@ -0,0 +1,176 @@ +package e2e + +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/Microsoft/hcsshim/hcn" + + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// tags +const ( + podCrudTag Tag = "pod-crud" + nsCrudTag Tag = "namespace-crud" + policyCrudTag Tag = "policy-crud" + backgroundTag Tag = "has-background-steps" +) + +const ( + applyDP bool = true + doNotApplyDP bool = false + + thisNode = "this-node" + otherNode = "other-node" + + podKey1 = "pod1" + podKey2 = "pod2" + + ip1 = "10.0.0.1" + ip2 = "10.0.0.2" + + endpoint1 = "test1" + endpoint2 = "test2" +) + +// IPSet constants +var ( + podLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) + podLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) + podLabel2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) + podLabelVal2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) + + // emptySet is a member of a list if enabled in the dp Config + // in Windows, this Config option is actually forced to be enabled in NewDataPlane() + emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) + allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) + ns1Set = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) + ns2Set = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) + + nsLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsLabel2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsLabelVal2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) +) + +func getAllTests() []*TestCaseMetadata { + return []*TestCaseMetadata{ + { + Description: "test case 1", + Tags: []Tag{ + podCrudTag, + backgroundTag, + }, + InitialEndpoints: []*hcn.HostComputeEndpoint{ + dptestutils.Endpoint(endpoint1, ip1), + }, + TestCase: NewTestCase([]*TestStep{ + { + ID: "pod controller 1", + InBackground: false, + Action: &PodCreateAction{ + Pod: dataplane.NewPodMetadata(podKey1, ip1, thisNode), + Namespace: "ns1", + Labels: map[string]string{ + "k1": "v1", + }, + }, + }, + { + ID: "policy controller 1", + InBackground: true, + Action: &PolicyUpdateAction{ + Policy: policyNs1LabelPair1AllowAll(), + }, + }, + { + ID: "pod controller 2", + InBackground: true, + Action: &PodCreateAction{ + Pod: dataplane.NewPodMetadata(podKey1, ip1, thisNode), + Namespace: "ns1", + Labels: map[string]string{ + "k1": "v1", + }, + }, + }, + { + ID: "policy controller 2", + InBackground: false, + Action: &PolicyDeleteAction{ + Namespace: policyNs1LabelPair1AllowAll().Namespace, + Name: policyNs1LabelPair1AllowAll().Name, + }, + }, + }, map[string][]string{ + // "policy controller 2" action won't run until both "policy controller 1" and "pod controller 2" are done + "policy controller 2": { + "policy controller 1", + "pod controller 2", + }, + }), + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(ns1Set, ip1), + dptestutils.SetPolicy(podLabel1Set, ip1), + dptestutils.SetPolicy(podLabelVal1Set, ip1), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + } +} + +func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labelPair1-allow-all", + Namespace: "ns1", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k1": "v1", + }, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {}, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + {}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, + } +} diff --git a/npm/pkg/dataplane/e2es/types_windows_test.go b/npm/pkg/dataplane/e2es/types_windows_test.go new file mode 100644 index 0000000000..9da69877f7 --- /dev/null +++ b/npm/pkg/dataplane/e2es/types_windows_test.go @@ -0,0 +1,120 @@ +package e2e + +import ( + "sync" + + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane" + "github.com/Microsoft/hcsshim/hcn" +) + +type Tag string + +type TestCaseMetadata struct { + Description string + Tags []Tag + InitialEndpoints []*hcn.HostComputeEndpoint + // DpCfg needs to be set for NPM tests only + DpCfg *dataplane.Config + *TestCase + ExpectedSetPolicies []*hcn.SetPolicySetting + ExpectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy +} + +type TestCase struct { + Steps []*TestStep + // waitToComplete maps a Step to the Steps it should wait to complete before it runs. + waitToComplete map[string][]string + // waitGroups maps a Step to its WaitGroup so that we can see when a Step has completed. + waitGroups map[string]*sync.WaitGroup + // lock is held while accessing the above maps + lock sync.Mutex +} + +func NewTestCase(steps []*TestStep, waitToComplete map[string][]string) *TestCase { + if waitToComplete == nil { + waitToComplete = make(map[string][]string) + } + + return &TestCase{ + Steps: steps, + waitToComplete: waitToComplete, + waitGroups: make(map[string]*sync.WaitGroup), + } +} + +// AddStepWaitGroup tracks the wait group for the step. +// The caller is expected to call wg.Done(). +func (tt *TestCase) AddStepWaitGroup(step string, wg *sync.WaitGroup) { + tt.lock.Lock() + defer tt.lock.Unlock() + tt.waitGroups[step] = wg +} + +func (tt *TestCase) WaitToRun(step string) { + tt.lock.Lock() + stepsToWaitOn, ok := tt.waitToComplete[step] + tt.lock.Unlock() + if !ok { + return + } + + for _, s := range stepsToWaitOn { + tt.lock.Lock() + wg, ok := tt.waitGroups[s] + tt.lock.Unlock() + if !ok { + continue + } + wg.Wait() + } +} + +func (tt *TestCase) WaitForAll() { + tt.lock.Lock() + defer tt.lock.Unlock() + for _, wg := range tt.waitGroups { + wg.Wait() + } +} + +type TestStep struct { + ID string + InBackground bool + Action +} + +type Action interface { + Do() error + SetHNS(hns *hnswrapper.Hnsv2wrapperFake) + // SetDP needs to be implemented for NPM tests only + SetDP(dp *dataplane.DataPlane) +} + +// HNSAction is meant to be embedded in other Actions so that they don't have to implement SetHNS and SetDP. +// HNSAction does not implement Do(). +type HNSAction struct { + hns *hnswrapper.Hnsv2wrapperFake +} + +func (h *HNSAction) SetHNS(hns *hnswrapper.Hnsv2wrapperFake) { + h.hns = hns +} + +func (h *HNSAction) SetDP(_ *dataplane.DataPlane) { + // purposely not implemented +} + +// DPAction is meant to be embedded in other Actions so that they don't have to implement SetHNS and SetDP. +// DPAction does not implement Do(). +type DPAction struct { + dp *dataplane.DataPlane +} + +func (d *DPAction) SetHNS(_ *hnswrapper.Hnsv2wrapperFake) { + // purposely not implemented +} + +func (d *DPAction) SetDP(dp *dataplane.DataPlane) { + d.dp = dp +} diff --git a/npm/pkg/dataplane/testutils/utils_windows.go b/npm/pkg/dataplane/testutils/utils_windows.go new file mode 100644 index 0000000000..408d5491b1 --- /dev/null +++ b/npm/pkg/dataplane/testutils/utils_windows.go @@ -0,0 +1,148 @@ +package dptestutils + +import ( + "fmt" + "sort" + "strings" + "testing" + + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Microsoft/hcsshim/hcn" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/klog" +) + +// NOTE: depends on fake ioshim input +const azureNetworkID = "1234" + +func PrefixNames(sets []*ipsets.IPSetMetadata) []string { + a := make([]string, len(sets)) + for k, s := range sets { + a[k] = s.GetPrefixName() + } + return a +} + +func Endpoint(epID, ip string) *hcn.HostComputeEndpoint { + return &hcn.HostComputeEndpoint{ + Id: epID, + Name: epID, + HostComputeNetwork: azureNetworkID, + IpConfigurations: []hcn.IpConfig{ + { + IpAddress: ip, + }, + }, + } +} + +func SetPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { + pType := hcn.SetPolicyType("") + switch setMetadata.GetSetKind() { + case ipsets.ListSet: + pType = hcn.SetPolicyTypeNestedIpSet + case ipsets.HashSet: + pType = hcn.SetPolicyTypeIpSet + } + + // sort for easier comparison + sort.Strings(members) + + return &hcn.SetPolicySetting{ + Id: setMetadata.GetHashedName(), + Name: setMetadata.GetPrefixName(), + PolicyType: pType, + Values: strings.Join(members, ","), + } +} + +// VerifyHNSCache asserts that HNS has the correct state. +func VerifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) { + t.Helper() + + PrintGetAllOutput(hns) + + // we want to evaluate both verify functions even if one fails, so don't write as verifySetPolicies() && verifyACLs() in case of short-circuiting + success := VerifySetPolicies(t, hns, expectedSetPolicies) + success = VerifyACLs(t, hns, expectedEndpointACLs) && success + + if !success { + require.FailNow(t, fmt.Sprintf("hns cache had unexpected state. printing hns cache...\n%s", hns.Cache.PrettyString())) + } +} + +// VerifySetPolicies is true if HNS strictly has the expected SetPolicies. +func VerifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting) bool { + t.Helper() + + cachedSetPolicies := hns.Cache.AllSetPolicies(azureNetworkID) + + success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") + for _, expectedSetPolicy := range expectedSetPolicies { + cachedSetPolicy, ok := cachedSetPolicies[expectedSetPolicy.Id] + success = assert.True(t, ok, fmt.Sprintf("expected SetPolicy not found. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success + if !ok { + continue + } + + members := strings.Split(cachedSetPolicy.Values, ",") + sort.Strings(members) + copyOfCachedSetPolicy := *cachedSetPolicy + copyOfCachedSetPolicy.Values = strings.Join(members, ",") + + // required that the expectedSetPolicy already has sorted members + success = assert.Equal(t, expectedSetPolicy, ©OfCachedSetPolicy, fmt.Sprintf("SetPolicy has unexpected contents. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success + } + + return success +} + +// verifyACLs is true if HNS strictly has the expected Endpoints and ACLs. +func VerifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) bool { + t.Helper() + + cachedEndpointACLs := hns.Cache.GetAllACLs() + + success := assert.Equal(t, len(expectedEndpointACLs), len(cachedEndpointACLs), "unexpected number of Endpoints") + for epID, expectedACLs := range expectedEndpointACLs { + cachedACLs, ok := cachedEndpointACLs[epID] + success = assert.True(t, ok, fmt.Sprintf("expected ACL not found for endpoint %s", epID)) && success + if !ok { + continue + } + + success = assert.Equal(t, len(expectedACLs), len(cachedACLs), fmt.Sprintf("unexpected number of ACLs for Endpoint with ID: %s", epID)) && success + for _, expectedACL := range expectedACLs { + foundACL := false + for _, cacheACL := range cachedACLs { + if expectedACL.ID == cacheACL.ID { + if cmp.Equal(expectedACL, cacheACL) { + foundACL = true + break + } + } + } + success = assert.True(t, foundACL, fmt.Sprintf("missing expected ACL. ID: %s, full ACL: %+v", expectedACL.ID, expectedACL)) && success + } + } + return success +} + +// helpful for debugging if there's a discrepancy between GetAll functions and the HNS PrettyString +func PrintGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { + klog.Info("SETPOLICIES...") + for _, setPol := range hns.Cache.AllSetPolicies(azureNetworkID) { + klog.Infof("%+v", setPol) + } + klog.Info("Endpoint ACLs...") + for id, acls := range hns.Cache.GetAllACLs() { + a := make([]string, len(acls)) + for k, v := range acls { + a[k] = fmt.Sprintf("%+v", v) + } + klog.Infof("%s: %s", id, strings.Join(a, ",")) + } +} From 03ee49750b4bf2fe64b8b773fc2846241504040d Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 10:45:54 -0700 Subject: [PATCH 13/34] namespace method for podmetadata --- npm/pkg/dataplane/types.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/npm/pkg/dataplane/types.go b/npm/pkg/dataplane/types.go index 2df765ddfa..d8fd26afc3 100644 --- a/npm/pkg/dataplane/types.go +++ b/npm/pkg/dataplane/types.go @@ -1,6 +1,8 @@ package dataplane import ( + "strings" + "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" @@ -51,6 +53,10 @@ func NewPodMetadata(podKey, podIP, nodeName string) *PodMetadata { } } +func (p *PodMetadata) Namespace() string { + return strings.Split(p.PodKey, "/")[0] +} + func newUpdateNPMPod(podMetadata *PodMetadata) *updateNPMPod { return &updateNPMPod{ PodMetadata: podMetadata, From c0cdbc8bd0e11a88049f5b32983434bbb3f09af7 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 10:46:54 -0700 Subject: [PATCH 14/34] refactor Action structure and TestCase wait group behavior --- npm/pkg/dataplane/e2es/e2e_windows_test.go | 40 ++++---- .../dataplane/e2es/npm-types_windows_test.go | 77 +++++++++------ .../dataplane/e2es/test-cases_windows_test.go | 93 +++++++++---------- npm/pkg/dataplane/e2es/types_windows_test.go | 65 ++++++------- 4 files changed, 140 insertions(+), 135 deletions(-) diff --git a/npm/pkg/dataplane/e2es/e2e_windows_test.go b/npm/pkg/dataplane/e2es/e2e_windows_test.go index 5897e4b0b3..2ccb7448d6 100644 --- a/npm/pkg/dataplane/e2es/e2e_windows_test.go +++ b/npm/pkg/dataplane/e2es/e2e_windows_test.go @@ -1,7 +1,6 @@ package e2e import ( - "sync" "testing" "time" @@ -39,30 +38,35 @@ func TestAll(t *testing.T) { backgroundErrors := make(chan error) for _, s := range tt.Steps { - s.SetHNS(hns) - // necessary for NPM tests - s.SetDP(dp) - if s.InBackground { - wg := new(sync.WaitGroup) - wg.Add(1) - tt.AddStepWaitGroup(s.ID, wg) - go func() { - defer wg.Done() - tt.WaitToRun(s.ID) - if err := s.Do(); err != nil { - backgroundErrors <- errors.Wrapf(err, "failed to run step in background: %s", s.ID) + tt.MarkStepRunningInBackground(s.ID) + go func(s *TestStep) { + defer tt.MarkStepComplete(s.ID) + + var err error + if s.HNSAction != nil { + err = s.HNSAction.Do(hns) + } else if s.DPAction != nil { + err = s.DPAction.Do(dp) } - }() + + if err != nil { + backgroundErrors <- errors.Wrapf(err, "failed to run action for step in background: %s", s.ID) + } + }(s) } else { - if !assert.Nil(t, s.Do(), "failed to run step in foreground: %s", s.ID) { - // stop processing steps on a foreground failure - break + var err error + if s.HNSAction != nil { + err = s.HNSAction.Do(hns) + } else if s.DPAction != nil { + err = s.DPAction.Do(dp) } + + assert.Nil(t, err, "failed to run step in foreground: %s", s.ID) } } - tt.WaitForAll() + tt.WaitForAllStepsToComplete() close(backgroundErrors) for err := range backgroundErrors { assert.Nil(t, err, "failed during concurrency") diff --git a/npm/pkg/dataplane/e2es/npm-types_windows_test.go b/npm/pkg/dataplane/e2es/npm-types_windows_test.go index 1e26bc1b49..28d4dca503 100644 --- a/npm/pkg/dataplane/e2es/npm-types_windows_test.go +++ b/npm/pkg/dataplane/e2es/npm-types_windows_test.go @@ -12,27 +12,29 @@ import ( ) type PodCreateAction struct { - *DPAction - // TODO Pod name and node instead of podmetadata object (infer podkey from name and namespace) - Pod *dataplane.PodMetadata - Namespace string - Labels map[string]string + Pod *dataplane.PodMetadata + Labels map[string]string } -func (p *PodCreateAction) Do() error { - if p.dp == nil { - return errors.New("no DP") // FIXME make constant type for lint +func CreatePod(namespace, name, ip, node string, labels map[string]string) *Action { + return &Action{ + DPAction: &PodCreateAction{ + Pod: dataplane.NewPodMetadata(fmt.Sprintf("%s/%s", namespace, name), ip, node), + Labels: labels, + }, } +} - context := fmt.Sprintf("create context: [pod: %+v. ns: %s. labels: %+v]", p.Pod, p.Namespace, p.Labels) +func (p *PodCreateAction) Do(dp *dataplane.DataPlane) error { + context := fmt.Sprintf("create context: [pod: %+v. labels: %+v]", p.Pod, p.Labels) - nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Namespace, ipsets.Namespace)} + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Pod.Namespace(), ipsets.Namespace)} // PodController technically wouldn't call this if the namespace already existed - if err := p.dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, nsIPSet); err != nil { + if err := dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, nsIPSet); err != nil { return errors.Wrapf(err, "[podCreateEvent] failed to add ns set to all namespaces list. %s", context) } - if err := p.dp.AddToSets(nsIPSet, p.Pod); err != nil { + if err := dp.AddToSets(nsIPSet, p.Pod); err != nil { return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to ns set. %s", context) } @@ -43,7 +45,7 @@ func (p *PodCreateAction) Do() error { ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), } - if err := p.dp.AddToSets(labelIPSets, p.Pod); err != nil { + if err := dp.AddToSets(labelIPSets, p.Pod); err != nil { return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to label sets. %s", context) } @@ -52,55 +54,70 @@ func (p *PodCreateAction) Do() error { return nil } -type ApplyDPAction struct { - *DPAction -} +type ApplyDPAction struct{} -func (a *ApplyDPAction) Do() error { - if a.dp == nil { - return errors.New("no DP") // FIXME make constant type for lint +func ApplyDP() *Action { + return &Action{ + DPAction: &ApplyDPAction{}, } +} - if err := a.dp.ApplyDataPlane(); err != nil { +func (*ApplyDPAction) Do(dp *dataplane.DataPlane) error { + if err := dp.ApplyDataPlane(); err != nil { return errors.Wrapf(err, "[ApplyDPAction] failed to apply") } return nil } +// TODO PodUpdateAction and PodDeleteAction + +// TODO namespace actions + type PolicyUpdateAction struct { - *DPAction Policy *networkingv1.NetworkPolicy } -func (p *PolicyUpdateAction) Do() error { - if p.dp == nil { - return errors.New("no DP") // FIXME make constant type for lint +func UpdatePolicy(policy *networkingv1.NetworkPolicy) *Action { + return &Action{ + DPAction: &PolicyUpdateAction{ + Policy: policy, + }, } +} +func (p *PolicyUpdateAction) Do(dp *dataplane.DataPlane) error { npmNetPol, err := translation.TranslatePolicy(p.Policy) if err != nil { return errors.Wrapf(err, "[PolicyUpdateAction] failed to translate policy with key %s/%s", p.Policy.Namespace, p.Policy.Name) } - if err := p.dp.UpdatePolicy(npmNetPol); err != nil { + if err := dp.UpdatePolicy(npmNetPol); err != nil { return errors.Wrapf(err, "[PolicyUpdateAction] failed to update policy with key %s/%s", p.Policy.Namespace, p.Policy.Name) } return nil } type PolicyDeleteAction struct { - *DPAction Namespace string Name string } -func (p *PolicyDeleteAction) Do() error { - if p.dp == nil { - return errors.New("no DP") // FIXME make constant type for lint +func DeletePolicy(namespace, name string) *Action { + return &Action{ + DPAction: &PolicyDeleteAction{ + Namespace: namespace, + Name: name, + }, } +} + +func DeletePolicyByObject(policy *networkingv1.NetworkPolicy) *Action { + return DeletePolicy(policy.Namespace, policy.Name) +} +func (p *PolicyDeleteAction) Do(dp *dataplane.DataPlane) error { policyKey := fmt.Sprintf("%s/%s", p.Namespace, p.Name) - if err := p.dp.RemovePolicy(policyKey); err != nil { + if err := dp.RemovePolicy(policyKey); err != nil { return errors.Wrapf(err, "[PolicyDeleteAction] failed to update policy with key %s", policyKey) } return nil diff --git a/npm/pkg/dataplane/e2es/test-cases_windows_test.go b/npm/pkg/dataplane/e2es/test-cases_windows_test.go index d7db05ad9c..b2838e65d4 100644 --- a/npm/pkg/dataplane/e2es/test-cases_windows_test.go +++ b/npm/pkg/dataplane/e2es/test-cases_windows_test.go @@ -2,7 +2,6 @@ package e2e import ( "github.com/Azure/azure-container-networking/network/hnswrapper" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" "github.com/Microsoft/hcsshim/hcn" @@ -15,20 +14,14 @@ import ( const ( podCrudTag Tag = "pod-crud" nsCrudTag Tag = "namespace-crud" - policyCrudTag Tag = "policy-crud" + netpolCrudTag Tag = "netpol-crud" backgroundTag Tag = "has-background-steps" ) const ( - applyDP bool = true - doNotApplyDP bool = false - thisNode = "this-node" otherNode = "other-node" - podKey1 = "pod1" - podKey2 = "pod2" - ip1 = "10.0.0.1" ip2 = "10.0.0.2" @@ -38,22 +31,22 @@ const ( // IPSet constants var ( - podLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) - podLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) - podLabel2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) - podLabelVal2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) + podK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) + podK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) + podK2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) + podK2V2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) // emptySet is a member of a list if enabled in the dp Config // in Windows, this Config option is actually forced to be enabled in NewDataPlane() emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) - ns1Set = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) - ns2Set = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) + nsXSet = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) + nsYSet = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) - nsLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) - nsLabel2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsLabelVal2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsK2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsK2V2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) ) func getAllTests() []*TestCaseMetadata { @@ -62,6 +55,7 @@ func getAllTests() []*TestCaseMetadata { Description: "test case 1", Tags: []Tag{ podCrudTag, + netpolCrudTag, backgroundTag, }, InitialEndpoints: []*hcn.HostComputeEndpoint{ @@ -69,55 +63,52 @@ func getAllTests() []*TestCaseMetadata { }, TestCase: NewTestCase([]*TestStep{ { - ID: "pod controller 1", + ID: "pod1", InBackground: false, - Action: &PodCreateAction{ - Pod: dataplane.NewPodMetadata(podKey1, ip1, thisNode), - Namespace: "ns1", - Labels: map[string]string{ - "k1": "v1", - }, - }, + Action: CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), }, { - ID: "policy controller 1", + ID: "netpol1", InBackground: true, - Action: &PolicyUpdateAction{ - Policy: policyNs1LabelPair1AllowAll(), - }, + Action: UpdatePolicy(policyNs1LabelPair1AllowAll()), }, { - ID: "pod controller 2", + ID: "netpol2", InBackground: true, - Action: &PodCreateAction{ - Pod: dataplane.NewPodMetadata(podKey1, ip1, thisNode), - Namespace: "ns1", - Labels: map[string]string{ - "k1": "v1", - }, - }, + Action: UpdatePolicy(policyNs1LabelPair1AllowAll()), + }, + { + ID: "pod2", + InBackground: true, + Action: CreatePod("x", "b", ip2, otherNode, map[string]string{"k2": "v2"}), + }, + { + ID: "pod3", + InBackground: true, + Action: CreatePod("y", "a", ip2, otherNode, map[string]string{"k1": "v1"}), }, { - ID: "policy controller 2", + ID: "netpol3", InBackground: false, - Action: &PolicyDeleteAction{ - Namespace: policyNs1LabelPair1AllowAll().Namespace, - Name: policyNs1LabelPair1AllowAll().Name, - }, + Action: DeletePolicyByObject(policyNs1LabelPair1AllowAll()), }, }, map[string][]string{ - // "policy controller 2" action won't run until both "policy controller 1" and "pod controller 2" are done - "policy controller 2": { - "policy controller 1", - "pod controller 2", + // netpol2 won't run until netpol1 is complete + "netpol2": {"netpol1"}, + // pod3 won't run until pod2 is complete + "pod3": {"pod2"}, + // netpol3 won't run until all background threads have complete + "netpol3": { + "netpol2", + "pod3", }, }), ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - dptestutils.SetPolicy(ns1Set, ip1), - dptestutils.SetPolicy(podLabel1Set, ip1), - dptestutils.SetPolicy(podLabelVal1Set, ip1), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), }, ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ endpoint1: { diff --git a/npm/pkg/dataplane/e2es/types_windows_test.go b/npm/pkg/dataplane/e2es/types_windows_test.go index 9da69877f7..80c082f484 100644 --- a/npm/pkg/dataplane/e2es/types_windows_test.go +++ b/npm/pkg/dataplane/e2es/types_windows_test.go @@ -43,15 +43,30 @@ func NewTestCase(steps []*TestStep, waitToComplete map[string][]string) *TestCas } } -// AddStepWaitGroup tracks the wait group for the step. -// The caller is expected to call wg.Done(). -func (tt *TestCase) AddStepWaitGroup(step string, wg *sync.WaitGroup) { +// MarkStepRunningInBackground should be called if a step will run in the background. +// MarkStepComplete must be called afterwards. +func (tt *TestCase) MarkStepRunningInBackground(stepID string) { tt.lock.Lock() defer tt.lock.Unlock() - tt.waitGroups[step] = wg + + wg := new(sync.WaitGroup) + wg.Add(1) + tt.waitGroups[stepID] = wg +} + +func (tt *TestCase) MarkStepComplete(stepID string) { + tt.lock.Lock() + defer tt.lock.Unlock() + + wg, ok := tt.waitGroups[stepID] + if ok { + wg.Done() + } } -func (tt *TestCase) WaitToRun(step string) { +func (tt *TestCase) WaitToRunStep(step string) { + // don't keep tt locked while waiting on a single wait group + tt.lock.Lock() stepsToWaitOn, ok := tt.waitToComplete[step] tt.lock.Unlock() @@ -70,7 +85,7 @@ func (tt *TestCase) WaitToRun(step string) { } } -func (tt *TestCase) WaitForAll() { +func (tt *TestCase) WaitForAllStepsToComplete() { tt.lock.Lock() defer tt.lock.Unlock() for _, wg := range tt.waitGroups { @@ -81,40 +96,18 @@ func (tt *TestCase) WaitForAll() { type TestStep struct { ID string InBackground bool - Action -} - -type Action interface { - Do() error - SetHNS(hns *hnswrapper.Hnsv2wrapperFake) - // SetDP needs to be implemented for NPM tests only - SetDP(dp *dataplane.DataPlane) -} - -// HNSAction is meant to be embedded in other Actions so that they don't have to implement SetHNS and SetDP. -// HNSAction does not implement Do(). -type HNSAction struct { - hns *hnswrapper.Hnsv2wrapperFake -} - -func (h *HNSAction) SetHNS(hns *hnswrapper.Hnsv2wrapperFake) { - h.hns = hns -} - -func (h *HNSAction) SetDP(_ *dataplane.DataPlane) { - // purposely not implemented + *Action } -// DPAction is meant to be embedded in other Actions so that they don't have to implement SetHNS and SetDP. -// DPAction does not implement Do(). -type DPAction struct { - dp *dataplane.DataPlane +type Action struct { + HNSAction + DPAction } -func (d *DPAction) SetHNS(_ *hnswrapper.Hnsv2wrapperFake) { - // purposely not implemented +type HNSAction interface { + Do(hns *hnswrapper.Hnsv2wrapperFake) error } -func (d *DPAction) SetDP(dp *dataplane.DataPlane) { - d.dp = dp +type DPAction interface { + Do(dp *dataplane.DataPlane) error } From c27f3b4fb1c6b40882d884653a4a4f5400c0114f Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 11:01:40 -0700 Subject: [PATCH 15/34] hnsactions and renaming a file --- ...dows_test.go => dpactions_windows_test.go} | 6 +-- .../dataplane/e2es/hnsactions_windows_test.go | 53 +++++++++++++++++++ .../dataplane/e2es/test-cases_windows_test.go | 17 +++--- 3 files changed, 66 insertions(+), 10 deletions(-) rename npm/pkg/dataplane/e2es/{npm-types_windows_test.go => dpactions_windows_test.go} (91%) create mode 100644 npm/pkg/dataplane/e2es/hnsactions_windows_test.go diff --git a/npm/pkg/dataplane/e2es/npm-types_windows_test.go b/npm/pkg/dataplane/e2es/dpactions_windows_test.go similarity index 91% rename from npm/pkg/dataplane/e2es/npm-types_windows_test.go rename to npm/pkg/dataplane/e2es/dpactions_windows_test.go index 28d4dca503..188a055765 100644 --- a/npm/pkg/dataplane/e2es/npm-types_windows_test.go +++ b/npm/pkg/dataplane/e2es/dpactions_windows_test.go @@ -31,11 +31,11 @@ func (p *PodCreateAction) Do(dp *dataplane.DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Pod.Namespace(), ipsets.Namespace)} // PodController technically wouldn't call this if the namespace already existed if err := dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, nsIPSet); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add ns set to all namespaces list. %s", context) + return errors.Wrapf(err, "[PodCreateAction] failed to add ns set to all namespaces list. %s", context) } if err := dp.AddToSets(nsIPSet, p.Pod); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to ns set. %s", context) + return errors.Wrapf(err, "[PodCreateAction] failed to add pod ip to ns set. %s", context) } for key, val := range p.Labels { @@ -46,7 +46,7 @@ func (p *PodCreateAction) Do(dp *dataplane.DataPlane) error { } if err := dp.AddToSets(labelIPSets, p.Pod); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to label sets. %s", context) + return errors.Wrapf(err, "[PodCreateAction] failed to add pod ip to label sets. %s", context) } } diff --git a/npm/pkg/dataplane/e2es/hnsactions_windows_test.go b/npm/pkg/dataplane/e2es/hnsactions_windows_test.go new file mode 100644 index 0000000000..557dbdf1d3 --- /dev/null +++ b/npm/pkg/dataplane/e2es/hnsactions_windows_test.go @@ -0,0 +1,53 @@ +package e2e + +import ( + "github.com/Azure/azure-container-networking/network/hnswrapper" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/Microsoft/hcsshim/hcn" + "github.com/pkg/errors" +) + +type EndpointCreateAction struct { + ID string + IP string +} + +func CreateEndpoint(id, ip string) *Action { + return &Action{ + HNSAction: &EndpointCreateAction{ + ID: id, + IP: ip, + }, + } +} + +func (e *EndpointCreateAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { + ep := dptestutils.Endpoint(e.ID, e.IP) + _, err := hns.CreateEndpoint(ep) + if err != nil { + return errors.Wrapf(err, "[EndpointCreateAction] failed to create endpoint. ep: [%+v]", ep) + } + return nil +} + +type EndpointDeleteAction struct { + ID string +} + +func DeleteEndpoint(id string) *Action { + return &Action{ + HNSAction: &EndpointDeleteAction{ + ID: id, + }, + } +} + +func (e *EndpointDeleteAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { + ep := &hcn.HostComputeEndpoint{ + Id: e.ID, + } + if err := hns.DeleteEndpoint(ep); err != nil { + return errors.Wrapf(err, "[EndpointDeleteAction] failed to delete endpoint. ep: [%+v]", ep) + } + return nil +} diff --git a/npm/pkg/dataplane/e2es/test-cases_windows_test.go b/npm/pkg/dataplane/e2es/test-cases_windows_test.go index b2838e65d4..2cb4e8efd5 100644 --- a/npm/pkg/dataplane/e2es/test-cases_windows_test.go +++ b/npm/pkg/dataplane/e2es/test-cases_windows_test.go @@ -52,7 +52,7 @@ var ( func getAllTests() []*TestCaseMetadata { return []*TestCaseMetadata{ { - Description: "test case 1", + Description: "pod x/a created, then two netpols added in the background while pods are creating, then netpol deleted", Tags: []Tag{ podCrudTag, netpolCrudTag, @@ -63,9 +63,12 @@ func getAllTests() []*TestCaseMetadata { }, TestCase: NewTestCase([]*TestStep{ { - ID: "pod1", - InBackground: false, - Action: CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + ID: "ep1", + Action: CreateEndpoint(endpoint1, ip1), + }, + { + ID: "pod1", + Action: CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), }, { ID: "netpol1", @@ -83,14 +86,14 @@ func getAllTests() []*TestCaseMetadata { Action: CreatePod("x", "b", ip2, otherNode, map[string]string{"k2": "v2"}), }, { + // for demo purposes (technically an impossible pod event) ID: "pod3", InBackground: true, Action: CreatePod("y", "a", ip2, otherNode, map[string]string{"k1": "v1"}), }, { - ID: "netpol3", - InBackground: false, - Action: DeletePolicyByObject(policyNs1LabelPair1AllowAll()), + ID: "netpol3", + Action: DeletePolicyByObject(policyNs1LabelPair1AllowAll()), }, }, map[string][]string{ // netpol2 won't run until netpol1 is complete From 477d66cec4808e7b561c4f3e11459876252c3e2e Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 16:29:32 -0700 Subject: [PATCH 16/34] refactor to Serial and ThreadedTestCase structs, and move files to dp pkg --- .../dataplane-test-cases_windows_test.go | 155 ++++ npm/pkg/dataplane/dataplane_windows_test.go | 776 ++---------------- npm/pkg/dataplane/e2es/e2e_windows_test.go | 78 -- .../dataplane/e2es/hnsactions_windows_test.go | 53 -- .../dataplane/e2es/test-cases_windows_test.go | 170 ---- npm/pkg/dataplane/e2es/types_windows_test.go | 113 --- ..._windows_test.go => types_windows_test.go} | 103 ++- 7 files changed, 313 insertions(+), 1135 deletions(-) create mode 100644 npm/pkg/dataplane/dataplane-test-cases_windows_test.go delete mode 100644 npm/pkg/dataplane/e2es/e2e_windows_test.go delete mode 100644 npm/pkg/dataplane/e2es/hnsactions_windows_test.go delete mode 100644 npm/pkg/dataplane/e2es/test-cases_windows_test.go delete mode 100644 npm/pkg/dataplane/e2es/types_windows_test.go rename npm/pkg/dataplane/{e2es/dpactions_windows_test.go => types_windows_test.go} (57%) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go new file mode 100644 index 0000000000..5263231dcc --- /dev/null +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -0,0 +1,155 @@ +package dataplane + +import ( + "time" + + "github.com/Azure/azure-container-networking/network/hnswrapper" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Microsoft/hcsshim/hcn" + + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// tags +const ( + podCrudTag Tag = "pod-crud" + nsCrudTag Tag = "namespace-crud" + netpolCrudTag Tag = "netpol-crud" + backgroundTag Tag = "has-background-steps" +) + +const ( + thisNode = "this-node" + otherNode = "other-node" + + ip1 = "10.0.0.1" + ip2 = "10.0.0.2" + + endpoint1 = "test1" + endpoint2 = "test2" +) + +// IPSet constants +var ( + podK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) + podK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) + podK2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) + podK2V2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) + + // emptySet is a member of a list if enabled in the dp Config + // in Windows, this Config option is actually forced to be enabled in NewDataPlane() + emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) + allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) + nsXSet = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) + nsYSet = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) + + nsK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsK2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) + nsK2V2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) +) + +func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "labelPair1-allow-all", + Namespace: "ns1", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "k1": "v1", + }, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {}, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + {}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, + } +} + +func getAllSerialTests() []*SerialTestCase { + return []*SerialTestCase{ + { + Description: "pod x/a created, then relevant network policy created", + Actions: []*Action{ + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + UpdatePolicy(policyNs1LabelPair1AllowAll()), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + backgroundTag, + }, + InitialEndpoints: []*hcn.HostComputeEndpoint{ + dptestutils.Endpoint(endpoint1, ip1), + }, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-ns1-labelPair1-allow-all", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + }, + } +} + +func getAllThreadedTests() []*ThreadedTestCase { + return []*ThreadedTestCase{ + { + Description: "pod x/a created, then relevant network policy created", + HNSLatency: time.Duration(1 * time.Second), + Threads: map[string][]*Action{ + "pod_controller": { + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + CreatePod("y", "a", ip2, otherNode, map[string]string{"k2": "v2"}), + }, + "policy_controller": { + UpdatePolicy(policyNs1LabelPair1AllowAll()), + }, + "namespace_controller": {}, + }, + // would fill metadata out for actual test case + TestCaseMetadata: nil, + }, + } +} diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 4f6bfffb2e..a968c50933 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -2,756 +2,108 @@ package dataplane import ( "fmt" - "sort" "strings" "sync" "testing" "time" "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/network/hnswrapper" - "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" - "github.com/Microsoft/hcsshim/hcn" - "github.com/google/go-cmp/cmp" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog" -) - -type dpEvent func(*DataPlane, *hnswrapper.Hnsv2wrapperFake) error -type thread []dpEvent -type concurrentSession []thread - -// FIXME move this into the common code path along with the verification functions below -const azureNetworkID = "1234" - -const ( - applyDP bool = true - doNotApplyDP bool = false - - thisNode = "this-node" - otherNode = "other-node" - - podKey1 = "pod1" - podKey2 = "pod2" - - ip1 = "10.0.0.1" - ip2 = "10.0.0.2" - - endpoint1 = "test1" - endpoint2 = "test2" - - defaultHNSLatency = time.Duration(0) -) - -// IPSet constants -var ( - podLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) - podLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) - podLabel2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) - podLabelVal2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) - - podLabelSets1 = []*ipsets.IPSetMetadata{podLabel1Set, podLabelVal1Set} - podLabelSets2 = []*ipsets.IPSetMetadata{podLabel2Set, podLabelVal2Set} - - // emptySet is a member of a list if enabled in the dp Config - // in Windows, this Config option is actually forced to be enabled in NewDataPlane() - emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) - allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) - ns1Set = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) - ns2Set = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) - - nsLabel1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsLabelVal1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) - nsLabel2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsLabelVal2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) ) -// POLICIES -// see translatePolicy_test.go for example rules - -func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { - return &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "labelPair1-allow-all", - Namespace: "ns1", - }, - Spec: networkingv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "k1": "v1", - }, - }, - Ingress: []networkingv1.NetworkPolicyIngressRule{ - {}, - }, - Egress: []networkingv1.NetworkPolicyEgressRule{ - {}, - }, - PolicyTypes: []networkingv1.PolicyType{ - networkingv1.PolicyTypeIngress, - networkingv1.PolicyTypeEgress, - }, - }, - } -} - -// DP EVENTS - -// podCreateEvent models a Pod CREATE in the PodController -func podCreateEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - context := fmt.Sprintf("create context: [pod: %+v. ns: %s. label sets: %+v]", pod, nsIPSet.Name, prefixNames(labelIPSets)) - - // PodController might not call this if the namespace already existed - if err := dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add ns set to all namespaces list. %s", context) - } - - if err := dp.AddToSets([]*ipsets.IPSetMetadata{nsIPSet}, pod); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to ns set. %s", context) - } - - // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) - if err := dp.AddToSets(labelIPSets, pod); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to add pod ip to label sets. %s", context) - } - - if shouldApply { - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrapf(err, "[podCreateEvent] failed to apply. %s", context) - } - } - return nil - } -} - -// podUpdateEvent models a Pod UPDATE in the PodController -func podUpdateEvent(shouldApply bool, oldPod, newPod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - context := fmt.Sprintf("update context: [old pod: %+v. new pod: %+v. ns: %s. label sets to remove: %+v. label sets to add: %+v]", - oldPod, newPod, nsIPSet.Name, prefixNames(toRemoveLabelSets), prefixNames(toAddLabelSets)) - - // think it's impossible for this to be called on an UPDATE - // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) - - for _, toRemoveSet := range toRemoveLabelSets { - if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, oldPod); err != nil { - return errors.Wrapf(err, "[podUpdateEvent] failed to remove old pod ip from set %s. %s", toRemoveSet.GetPrefixName(), context) - } - } - - for _, toAddSet := range toAddLabelSets { - if err := dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, newPod); err != nil { - return errors.Wrapf(err, "[podUpdateEvent] failed to add new pod ip to set %s. %s", toAddSet.GetPrefixName(), context) - } - } - - if shouldApply { - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrapf(err, "[podUpdateEvent] failed to apply. %s", context) - } - } - return nil - } -} - -// podUpdateEvent models a Pod UPDATE in the PodController where the Pod does not change IP/node. -func podUpdateEventSameIP(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, toRemoveLabelSets, toAddLabelSets []*ipsets.IPSetMetadata) dpEvent { - return podUpdateEvent(shouldApply, pod, pod, nsIPSet, toRemoveLabelSets, toAddLabelSets) -} - -// podDeleteEvent models a Pod DELETE in the PodController -func podDeleteEvent(shouldApply bool, pod *PodMetadata, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - context := fmt.Sprintf("delete context: [pod: %+v. ns: %s. label sets: %+v]", pod, nsIPSet.Name, prefixNames(labelIPSets)) - - if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{nsIPSet}, pod); err != nil { - return errors.Wrapf(err, "[podDeleteEvent] failed to remove pod ip from ns set. %s", context) - } - - // technically, the Pod Controller would make this call two sets at a time (for each key-val pair) - if err := dp.RemoveFromSets(labelIPSets, pod); err != nil { - return errors.Wrapf(err, "[podDeleteEvent] failed to remove pod ip from label sets. %s", context) - } - - if shouldApply { - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrapf(err, "[podDeleteEvent] failed to apply. %s", context) - } - } - return nil - } -} - -// nsCreateEvent models a Namespace CREATE in the NamespaceController -func nsCreateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - // TODO - return nil - } -} - -// nsUpdateEvent models a Namespace UPDATE in the NamespaceController -func nsUpdateEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - // TODO - return nil - } -} - -// nsDeleteEvent models a Namespace DELETE in the NamespaceController -func nsDeleteEvent(shouldApply bool, nsIPSet *ipsets.IPSetMetadata, labelIPSets ...*ipsets.IPSetMetadata) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - // TODO - return nil - } -} - -// policyUpdateEvent models a Network Policy CREATE/UPDATE in the PolicyController -func policyUpdateEvent(policy *networkingv1.NetworkPolicy) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - npmNetPol, err := translation.TranslatePolicy(policy) - if err != nil { - return errors.Wrapf(err, "[policyUpdateEvent] failed to translate policy with key %s/%s", policy.Namespace, policy.Name) - } - - if err := dp.UpdatePolicy(npmNetPol); err != nil { - return errors.Wrapf(err, "[policyUpdateEvent] failed to update policy with key %s/%s", policy.Namespace, policy.Name) - } - return nil - } -} - -// policyDeleteEvent models a Network Policy DELETE in the PolicyController -func policyDeleteEvent(policyKey string) dpEvent { - return func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - if err := dp.RemovePolicy(policyKey); err != nil { - return errors.Wrapf(err, "[policyDeleteEvent] failed to remove policy with key %s", policyKey) - } - return nil - } -} - -// endpointCreateEvent models an Endpoint being created within HNS -// With this Event, Endpoints can be created in between calls to dp -func endpointCreateEvent(epID, ip string) dpEvent { - return func(_ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) error { - ep := &hcn.HostComputeEndpoint{ - Id: epID, - Name: epID, - HostComputeNetwork: azureNetworkID, - IpConfigurations: []hcn.IpConfig{ - { - IpAddress: ip, - }, - }, - } - _, err := hns.CreateEndpoint(ep) - if err != nil { - return errors.Wrapf(err, "[endpointCreateEvent] failed to create hns endpoint. ep: %+v", ep) - } - return nil - } -} - -// endpointDeleteEvent models an Endpoint being deleted within HNS -// With this Event, Endpoints can be deleted in between calls to dp -func endpointDeleteEvent(epID string) dpEvent { - return func(_ *DataPlane, hns *hnswrapper.Hnsv2wrapperFake) error { - ep := &hcn.HostComputeEndpoint{ - Id: epID, - } - if err := hns.DeleteEndpoint(ep); err != nil { - return errors.Wrapf(err, "[endpointDeleteEvent] failed to delete hns endpoint. ep: %+v", ep) - } - return nil - } -} - -// TestAllEventSequences can test any config with a sequence of events. -func TestAllEventSequences(t *testing.T) { - tests := []struct { - name string - cfg *Config - ipEndpoints map[string]string - events []dpEvent - expectedSetPolicies []*hcn.SetPolicySetting - expectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy - }{ - { - name: "add set for pod on node", - cfg: dpCfg, - ipEndpoints: map[string]string{ - ip1: endpoint1, - }, - events: []dpEvent{ - // custom dpEvent - func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - pod1 := NewPodMetadata(podKey1, ip1, thisNode) - if err := dp.AddToSets([]*ipsets.IPSetMetadata{ns1Set, podLabel1Set}, pod1); err != nil { - return errors.Wrap(err, "[custom-add-apply] failed to add set") - } - - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrapf(err, "[custom-add-apply] failed to apply dp") - } - return nil - }, - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(ns1Set, ip1), - // setPolicy(podLabel1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: {}, - }, - }, - { - name: "policy created with no pods", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - // pre-defined dpEvent - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - // will not be an all-namespaces IPSet unless there's a Pod/Namespace event - setPolicy(ns1Set), - // Policies do not create the KeyLabelOfPod type IPSet if the selector has a key-value requirement - setPolicy(podLabelVal1Set), - }, - expectedEnpdointACLs: nil, - }, - { - name: "pod created on node -> policy created and applied to it", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: { - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "In", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "Out", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - }, - }, - }, - { - name: "pod created on node -> policy created and applied to it -> policy deleted", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - policyDeleteEvent("ns1/labelPair1-allow-all"), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: {}, - }, - }, - { - // NOTE: this fails right now. we incorrectly add a policy - name: "pod created off node -> relevant policy created but not applied (even though there's a local Endpoint with the same IP)", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: {}, - }, - }, - { - name: "pod created off node -> relevant policy created but not applied (no local Endpoint)", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, otherNode), ns1Set, podLabelSets1...), - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: nil, - }, - { - name: "pod created -> pod deleted", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - // mix of pre-defined dpEvents and a custom one - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - endpointDeleteEvent(endpoint1), - podDeleteEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - // garbage collect IPSets - func(dp *DataPlane, _ *hnswrapper.Hnsv2wrapperFake) error { - dp.ipsetMgr.Reconcile() - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrap(err, "[custom-reconcile] failed to apply dp") - } - return nil - }, - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set), - // should be garbage collected: podLabel1Set and podLabel1Set - }, - expectedEnpdointACLs: nil, - }, - { - name: "policy created -> pod created which satisfies the policy pod selector", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: { - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "In", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "Out", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - }, - }, - }, - { - // FIXME: debug why this case fails - name: "policy created -> pod created which satisfies the policy pod selector -> pod relabeled so policy removed", - cfg: dpCfg, - ipEndpoints: nil, - events: []dpEvent{ - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel1Set, ip1), - setPolicy(podLabelVal1Set, ip1), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: {}, - }, - }, - } +const defaultHNSLatency = time.Duration(0) - for _, tt := range tests { +func TestAllSerialCases(t *testing.T) { + tests := getAllSerialTests() + for i, tt := range tests { + i := i tt := tt - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.Description, func(t *testing.T) { + t.Logf("beginning test #%d. Description: [%s]. Tags: %+v", i, tt.Description, tt.Tags) + hns := ipsets.GetHNSFake(t) hns.Delay = defaultHNSLatency io := common.NewMockIOShimWithFakeHNS(hns) - for ip, epID := range tt.ipEndpoints { - event := endpointCreateEvent(epID, ip) - require.Nil(t, event(nil, hns), "failed to create endpoint. ep ID: %s. ip: %s", epID, ip) + for _, ep := range tt.InitialEndpoints { + _, err := hns.CreateEndpoint(ep) + require.Nil(t, err, "failed to create initial endpoint %+v", ep) } - dp, err := NewDataPlane(thisNode, io, tt.cfg, nil) + dp, err := NewDataPlane(thisNode, io, tt.DpCfg, nil) require.NoError(t, err, "failed to initialize dp") - for k, event := range tt.events { - require.Nil(t, event(dp, hns), "failed while running event number %d", k) - } + for j, a := range tt.Actions { + var err error + if a.HNSAction != nil { + err = a.HNSAction.Do(hns) + } else if a.DPAction != nil { + err = a.DPAction.Do(dp) + } - verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) + require.Nil(t, err, "failed to run action %d", j) + } - // uncomment to see output even for succeeding test cases - // require.Fail(t, "DEBUGME: successful") + dptestutils.VerifyHNSCache(t, hns, tt.ExpectedSetPolicies, tt.ExpectedEnpdointACLs) }) } } -func TestConcurrentEvents(t *testing.T) { - tests := []struct { - name string - cfg *Config - ipEndpoints map[string]string - sessions []concurrentSession - expectedSetPolicies []*hcn.SetPolicySetting - expectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy - }{ - { - name: "pod created on node -> concurrent: 1) creating relevant policy <=> 2) updating pod so policy no longer relevant", - cfg: dpCfg, - ipEndpoints: nil, - sessions: []concurrentSession{ - { - thread{ - endpointCreateEvent(endpoint1, ip1), - podCreateEvent(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1...), - }, - }, - { - thread{ - policyUpdateEvent(policyNs1LabelPair1AllowAll()), - }, - thread{ - podUpdateEventSameIP(applyDP, NewPodMetadata(podKey1, ip1, thisNode), ns1Set, podLabelSets1, podLabelSets2), - }, - }, - }, - expectedSetPolicies: []*hcn.SetPolicySetting{ - setPolicy(emptySet), - setPolicy(allNamespaces, ns1Set.GetHashedName(), emptySet.GetHashedName()), - setPolicy(ns1Set, ip1), - setPolicy(podLabel2Set, ip1), - setPolicy(podLabelVal2Set, ip1), - // the rest are not garbage collected yet - setPolicy(podLabel1Set), - setPolicy(podLabel1Set), - }, - expectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: {}, - }, - }, - } - - for _, tt := range tests { +func TestAllThreadedCases(t *testing.T) { + tests := getAllThreadedTests() + for i, tt := range tests { + i := i tt := tt - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.Description, func(t *testing.T) { + t.Logf("beginning test #%d. Description: [%s]. Tags: %+v", i, tt.Description, tt.Tags) + hns := ipsets.GetHNSFake(t) - hns.Delay = defaultHNSLatency + hns.Delay = tt.HNSLatency io := common.NewMockIOShimWithFakeHNS(hns) - for ip, epID := range tt.ipEndpoints { - event := endpointCreateEvent(epID, ip) - require.Nil(t, event(nil, hns), "failed to create endpoint. ep ID: %s. ip: %s", epID, ip) + for _, ep := range tt.InitialEndpoints { + _, err := hns.CreateEndpoint(ep) + require.Nil(t, err, "failed to create initial endpoint %+v", ep) } - dp, err := NewDataPlane(thisNode, io, tt.cfg, nil) + // the dp is necessary for NPM tests + dp, err := NewDataPlane(thisNode, io, tt.DpCfg, nil) require.NoError(t, err, "failed to initialize dp") - for i, session := range tt.sessions { - // concurrent case with multiple go routines - wg := new(sync.WaitGroup) - wg.Add(len(session)) - threadErrors := make(chan error, len(session)) - for j, th := range session { - go func(threadNum int, th thread) { - defer wg.Done() - for k, event := range th { - if err := event(dp, hns); err != nil { - threadErrors <- errors.Wrapf(err, "failed to run thread %d, event %d", threadNum, k) - return - } + wg := new(sync.WaitGroup) + wg.Add(len(tt.Threads)) + backgroundErrors := make(chan error, len(tt.Threads)) + for thName, th := range tt.Threads { + thName := thName + th := th + go func() { + defer wg.Done() + for k, a := range th { + var err error + if a.HNSAction != nil { + err = a.HNSAction.Do(hns) + } else if a.DPAction != nil { + err = a.DPAction.Do(dp) } - threadErrors <- nil - }(j, th) - } - - wg.Wait() - close(threadErrors) - for err := range threadErrors { - assert.Nil(t, err, "failed during concurrency for sequence %d", i) - } - } - - verifyHNSCache(t, hns, tt.expectedSetPolicies, tt.expectedEnpdointACLs) - - // uncomment to see output even for succeeding test cases - // require.Fail(t, "DEBUGME: successful") - }) - } -} - -func setPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { - pType := hcn.SetPolicyType("") - switch setMetadata.GetSetKind() { - case ipsets.ListSet: - pType = hcn.SetPolicyTypeNestedIpSet - case ipsets.HashSet: - pType = hcn.SetPolicyTypeIpSet - } - sort.Strings(members) - - return &hcn.SetPolicySetting{ - Id: setMetadata.GetHashedName(), - Name: setMetadata.GetPrefixName(), - PolicyType: pType, - Values: strings.Join(members, ","), - } -} - -// verifyHNSCache asserts that HNS has the correct state. -// TODO: move all these functions to common location used by windows test files in pkg ipsets and policies. -func verifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) { - t.Helper() - - printGetAllOutput(hns) - - // we want to evaluate both verify functions even if one fails, so don't write as verifySetPolicies() && verifyACLs() in case of short-circuiting - success := verifySetPolicies(t, hns, expectedSetPolicies) - success = verifyACLs(t, hns, expectedEndpointACLs) && success - - if !success { - require.FailNow(t, fmt.Sprintf("hns cache had unexpected state. printing hns cache...\n%s", hns.Cache.PrettyString())) - } -} - -// verifySetPolicies is true if HNS strictly has the expected SetPolicies. -func verifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting) bool { - t.Helper() - - cachedSetPolicies := hns.Cache.AllSetPolicies(azureNetworkID) - - success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") - for _, expectedSetPolicy := range expectedSetPolicies { - cachedSetPolicy, ok := cachedSetPolicies[expectedSetPolicy.Id] - success = assert.True(t, ok, fmt.Sprintf("expected SetPolicy not found. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success - if !ok { - continue - } - - members := strings.Split(cachedSetPolicy.Values, ",") - sort.Strings(members) - copyOfCachedSetPolicy := *cachedSetPolicy - copyOfCachedSetPolicy.Values = strings.Join(members, ",") - - success = assert.Equal(t, expectedSetPolicy, ©OfCachedSetPolicy, fmt.Sprintf("SetPolicy has unexpected contents. ID %s, name: %s", expectedSetPolicy.Id, expectedSetPolicy.Name)) && success - } - - return success -} - -// verifyACLs is true if HNS strictly has the expected Endpoints and ACLs. -func verifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpointACLs map[string][]*hnswrapper.FakeEndpointPolicy) bool { - t.Helper() - - cachedEndpointACLs := hns.Cache.GetAllACLs() - - success := assert.Equal(t, len(expectedEndpointACLs), len(cachedEndpointACLs), "unexpected number of Endpoints") - for epID, expectedACLs := range expectedEndpointACLs { - cachedACLs, ok := cachedEndpointACLs[epID] - success = assert.True(t, ok, fmt.Sprintf("expected ACL not found for endpoint %s", epID)) && success - if !ok { - continue - } - - success = assert.Equal(t, len(expectedACLs), len(cachedACLs), fmt.Sprintf("unexpected number of ACLs for Endpoint with ID: %s", epID)) && success - for _, expectedACL := range expectedACLs { - foundACL := false - for _, cacheACL := range cachedACLs { - if expectedACL.ID == cacheACL.ID { - if cmp.Equal(expectedACL, cacheACL) { - foundACL = true - break + if err != nil { + backgroundErrors <- errors.Wrapf(err, "failed to run action %d in thread %s", k, thName) + break + } } - } + }() } - success = assert.True(t, foundACL, fmt.Sprintf("missing expected ACL. ID: %s, full ACL: %+v", expectedACL.ID, expectedACL)) && success - } - } - return success -} -// helpful for debugging if there's a discrepancy between GetAll functions and the HNS PrettyString -func printGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { - klog.Info("SETPOLICIES...") - for _, setPol := range hns.Cache.AllSetPolicies(azureNetworkID) { - klog.Infof("%+v", setPol) - } - klog.Info("Endpoint ACLs...") - for id, acls := range hns.Cache.GetAllACLs() { - a := make([]string, len(acls)) - for k, v := range acls { - a[k] = fmt.Sprintf("%+v", v) - } - klog.Infof("%s: %s", id, strings.Join(a, ",")) - } -} - -func prefixNames(sets []*ipsets.IPSetMetadata) []string { - a := make([]string, len(sets)) - for k, s := range sets { - a[k] = s.GetPrefixName() + wg.Wait() + close(backgroundErrors) + errStrings := make([]string, len(backgroundErrors)) + for err := range backgroundErrors { + errStrings = append(errStrings, fmt.Sprintf("[%s]", err.Error())) + } + assert.Empty(t, backgroundErrors, "encountered errors in threaded test: %s", strings.Join(errStrings, ",")) + }) } - return a } diff --git a/npm/pkg/dataplane/e2es/e2e_windows_test.go b/npm/pkg/dataplane/e2es/e2e_windows_test.go deleted file mode 100644 index 2ccb7448d6..0000000000 --- a/npm/pkg/dataplane/e2es/e2e_windows_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package e2e - -import ( - "testing" - "time" - - "github.com/Azure/azure-container-networking/common" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" - dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" - "github.com/pkg/errors" - - "github.com/Azure/azure-container-networking/npm/pkg/dataplane" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -const defaultHNSLatency = time.Duration(0) - -func TestAll(t *testing.T) { - tests := getAllTests() - for i, tt := range tests { - i := i - tt := tt - t.Run(tt.Description, func(t *testing.T) { - t.Logf("beginning test #%d. Description: [%s]. Tags: %+v", i, tt.Description, tt.Tags) - - hns := ipsets.GetHNSFake(t) - hns.Delay = defaultHNSLatency - io := common.NewMockIOShimWithFakeHNS(hns) - for _, ep := range tt.InitialEndpoints { - _, err := hns.CreateEndpoint(ep) - require.Nil(t, err, "failed to create initial endpoint %+v", ep) - } - - // the dp is necessary for NPM tests - dp, err := dataplane.NewDataPlane(thisNode, io, tt.DpCfg, nil) - require.NoError(t, err, "failed to initialize dp") - - backgroundErrors := make(chan error) - for _, s := range tt.Steps { - if s.InBackground { - tt.MarkStepRunningInBackground(s.ID) - go func(s *TestStep) { - defer tt.MarkStepComplete(s.ID) - - var err error - if s.HNSAction != nil { - err = s.HNSAction.Do(hns) - } else if s.DPAction != nil { - err = s.DPAction.Do(dp) - } - - if err != nil { - backgroundErrors <- errors.Wrapf(err, "failed to run action for step in background: %s", s.ID) - } - }(s) - } else { - var err error - if s.HNSAction != nil { - err = s.HNSAction.Do(hns) - } else if s.DPAction != nil { - err = s.DPAction.Do(dp) - } - - assert.Nil(t, err, "failed to run step in foreground: %s", s.ID) - } - } - - tt.WaitForAllStepsToComplete() - close(backgroundErrors) - for err := range backgroundErrors { - assert.Nil(t, err, "failed during concurrency") - } - - dptestutils.VerifyHNSCache(t, hns, tt.ExpectedSetPolicies, tt.ExpectedEnpdointACLs) - }) - } -} diff --git a/npm/pkg/dataplane/e2es/hnsactions_windows_test.go b/npm/pkg/dataplane/e2es/hnsactions_windows_test.go deleted file mode 100644 index 557dbdf1d3..0000000000 --- a/npm/pkg/dataplane/e2es/hnsactions_windows_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package e2e - -import ( - "github.com/Azure/azure-container-networking/network/hnswrapper" - dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" - "github.com/Microsoft/hcsshim/hcn" - "github.com/pkg/errors" -) - -type EndpointCreateAction struct { - ID string - IP string -} - -func CreateEndpoint(id, ip string) *Action { - return &Action{ - HNSAction: &EndpointCreateAction{ - ID: id, - IP: ip, - }, - } -} - -func (e *EndpointCreateAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { - ep := dptestutils.Endpoint(e.ID, e.IP) - _, err := hns.CreateEndpoint(ep) - if err != nil { - return errors.Wrapf(err, "[EndpointCreateAction] failed to create endpoint. ep: [%+v]", ep) - } - return nil -} - -type EndpointDeleteAction struct { - ID string -} - -func DeleteEndpoint(id string) *Action { - return &Action{ - HNSAction: &EndpointDeleteAction{ - ID: id, - }, - } -} - -func (e *EndpointDeleteAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { - ep := &hcn.HostComputeEndpoint{ - Id: e.ID, - } - if err := hns.DeleteEndpoint(ep); err != nil { - return errors.Wrapf(err, "[EndpointDeleteAction] failed to delete endpoint. ep: [%+v]", ep) - } - return nil -} diff --git a/npm/pkg/dataplane/e2es/test-cases_windows_test.go b/npm/pkg/dataplane/e2es/test-cases_windows_test.go deleted file mode 100644 index 2cb4e8efd5..0000000000 --- a/npm/pkg/dataplane/e2es/test-cases_windows_test.go +++ /dev/null @@ -1,170 +0,0 @@ -package e2e - -import ( - "github.com/Azure/azure-container-networking/network/hnswrapper" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" - dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" - "github.com/Microsoft/hcsshim/hcn" - - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// tags -const ( - podCrudTag Tag = "pod-crud" - nsCrudTag Tag = "namespace-crud" - netpolCrudTag Tag = "netpol-crud" - backgroundTag Tag = "has-background-steps" -) - -const ( - thisNode = "this-node" - otherNode = "other-node" - - ip1 = "10.0.0.1" - ip2 = "10.0.0.2" - - endpoint1 = "test1" - endpoint2 = "test2" -) - -// IPSet constants -var ( - podK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfPod) - podK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfPod) - podK2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfPod) - podK2V2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfPod) - - // emptySet is a member of a list if enabled in the dp Config - // in Windows, this Config option is actually forced to be enabled in NewDataPlane() - emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) - allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) - nsXSet = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) - nsYSet = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) - - nsK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) - nsK2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsK2V2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) -) - -func getAllTests() []*TestCaseMetadata { - return []*TestCaseMetadata{ - { - Description: "pod x/a created, then two netpols added in the background while pods are creating, then netpol deleted", - Tags: []Tag{ - podCrudTag, - netpolCrudTag, - backgroundTag, - }, - InitialEndpoints: []*hcn.HostComputeEndpoint{ - dptestutils.Endpoint(endpoint1, ip1), - }, - TestCase: NewTestCase([]*TestStep{ - { - ID: "ep1", - Action: CreateEndpoint(endpoint1, ip1), - }, - { - ID: "pod1", - Action: CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), - }, - { - ID: "netpol1", - InBackground: true, - Action: UpdatePolicy(policyNs1LabelPair1AllowAll()), - }, - { - ID: "netpol2", - InBackground: true, - Action: UpdatePolicy(policyNs1LabelPair1AllowAll()), - }, - { - ID: "pod2", - InBackground: true, - Action: CreatePod("x", "b", ip2, otherNode, map[string]string{"k2": "v2"}), - }, - { - // for demo purposes (technically an impossible pod event) - ID: "pod3", - InBackground: true, - Action: CreatePod("y", "a", ip2, otherNode, map[string]string{"k1": "v1"}), - }, - { - ID: "netpol3", - Action: DeletePolicyByObject(policyNs1LabelPair1AllowAll()), - }, - }, map[string][]string{ - // netpol2 won't run until netpol1 is complete - "netpol2": {"netpol1"}, - // pod3 won't run until pod2 is complete - "pod3": {"pod2"}, - // netpol3 won't run until all background threads have complete - "netpol3": { - "netpol2", - "pod3", - }, - }), - ExpectedSetPolicies: []*hcn.SetPolicySetting{ - dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), - dptestutils.SetPolicy(nsXSet, ip1), - dptestutils.SetPolicy(podK1Set, ip1), - dptestutils.SetPolicy(podK1V1Set, ip1), - }, - ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ - endpoint1: { - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "In", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - { - ID: "azure-acl-ns1-labelPair1-allow-all", - Protocols: "", - Action: "Allow", - Direction: "Out", - LocalAddresses: "", - RemoteAddresses: "", - LocalPorts: "", - RemotePorts: "", - Priority: 222, - }, - }, - }, - }, - } -} - -func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { - return &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "labelPair1-allow-all", - Namespace: "ns1", - }, - Spec: networkingv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "k1": "v1", - }, - }, - Ingress: []networkingv1.NetworkPolicyIngressRule{ - {}, - }, - Egress: []networkingv1.NetworkPolicyEgressRule{ - {}, - }, - PolicyTypes: []networkingv1.PolicyType{ - networkingv1.PolicyTypeIngress, - networkingv1.PolicyTypeEgress, - }, - }, - } -} diff --git a/npm/pkg/dataplane/e2es/types_windows_test.go b/npm/pkg/dataplane/e2es/types_windows_test.go deleted file mode 100644 index 80c082f484..0000000000 --- a/npm/pkg/dataplane/e2es/types_windows_test.go +++ /dev/null @@ -1,113 +0,0 @@ -package e2e - -import ( - "sync" - - "github.com/Azure/azure-container-networking/network/hnswrapper" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane" - "github.com/Microsoft/hcsshim/hcn" -) - -type Tag string - -type TestCaseMetadata struct { - Description string - Tags []Tag - InitialEndpoints []*hcn.HostComputeEndpoint - // DpCfg needs to be set for NPM tests only - DpCfg *dataplane.Config - *TestCase - ExpectedSetPolicies []*hcn.SetPolicySetting - ExpectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy -} - -type TestCase struct { - Steps []*TestStep - // waitToComplete maps a Step to the Steps it should wait to complete before it runs. - waitToComplete map[string][]string - // waitGroups maps a Step to its WaitGroup so that we can see when a Step has completed. - waitGroups map[string]*sync.WaitGroup - // lock is held while accessing the above maps - lock sync.Mutex -} - -func NewTestCase(steps []*TestStep, waitToComplete map[string][]string) *TestCase { - if waitToComplete == nil { - waitToComplete = make(map[string][]string) - } - - return &TestCase{ - Steps: steps, - waitToComplete: waitToComplete, - waitGroups: make(map[string]*sync.WaitGroup), - } -} - -// MarkStepRunningInBackground should be called if a step will run in the background. -// MarkStepComplete must be called afterwards. -func (tt *TestCase) MarkStepRunningInBackground(stepID string) { - tt.lock.Lock() - defer tt.lock.Unlock() - - wg := new(sync.WaitGroup) - wg.Add(1) - tt.waitGroups[stepID] = wg -} - -func (tt *TestCase) MarkStepComplete(stepID string) { - tt.lock.Lock() - defer tt.lock.Unlock() - - wg, ok := tt.waitGroups[stepID] - if ok { - wg.Done() - } -} - -func (tt *TestCase) WaitToRunStep(step string) { - // don't keep tt locked while waiting on a single wait group - - tt.lock.Lock() - stepsToWaitOn, ok := tt.waitToComplete[step] - tt.lock.Unlock() - if !ok { - return - } - - for _, s := range stepsToWaitOn { - tt.lock.Lock() - wg, ok := tt.waitGroups[s] - tt.lock.Unlock() - if !ok { - continue - } - wg.Wait() - } -} - -func (tt *TestCase) WaitForAllStepsToComplete() { - tt.lock.Lock() - defer tt.lock.Unlock() - for _, wg := range tt.waitGroups { - wg.Wait() - } -} - -type TestStep struct { - ID string - InBackground bool - *Action -} - -type Action struct { - HNSAction - DPAction -} - -type HNSAction interface { - Do(hns *hnswrapper.Hnsv2wrapperFake) error -} - -type DPAction interface { - Do(dp *dataplane.DataPlane) error -} diff --git a/npm/pkg/dataplane/e2es/dpactions_windows_test.go b/npm/pkg/dataplane/types_windows_test.go similarity index 57% rename from npm/pkg/dataplane/e2es/dpactions_windows_test.go rename to npm/pkg/dataplane/types_windows_test.go index 188a055765..7ca4cea419 100644 --- a/npm/pkg/dataplane/e2es/dpactions_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -1,31 +1,116 @@ -package e2e +package dataplane import ( "fmt" + "time" + "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" - "github.com/Azure/azure-container-networking/npm/pkg/dataplane" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" + "github.com/Microsoft/hcsshim/hcn" "github.com/pkg/errors" - networkingv1 "k8s.io/api/networking/v1" ) +type Tag string + +type SerialTestCase struct { + Description string + Actions []*Action + *TestCaseMetadata +} + +type ThreadedTestCase struct { + Description string + HNSLatency time.Duration + Threads map[string][]*Action + *TestCaseMetadata +} + +type TestCaseMetadata struct { + Tags []Tag + InitialEndpoints []*hcn.HostComputeEndpoint + DpCfg *Config + ExpectedSetPolicies []*hcn.SetPolicySetting + ExpectedEnpdointACLs map[string][]*hnswrapper.FakeEndpointPolicy +} + +// Action represents a single action (either an HNSAction or a DPAction). +// Exactly one of HNSAction or DPAction should be non-nil. +type Action struct { + HNSAction + DPAction +} + +type HNSAction interface { + Do(hns *hnswrapper.Hnsv2wrapperFake) error +} + +type EndpointCreateAction struct { + ID string + IP string +} + +func CreateEndpoint(id, ip string) *Action { + return &Action{ + HNSAction: &EndpointCreateAction{ + ID: id, + IP: ip, + }, + } +} + +func (e *EndpointCreateAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { + ep := dptestutils.Endpoint(e.ID, e.IP) + _, err := hns.CreateEndpoint(ep) + if err != nil { + return errors.Wrapf(err, "[EndpointCreateAction] failed to create endpoint. ep: [%+v]", ep) + } + return nil +} + +type EndpointDeleteAction struct { + ID string +} + +func DeleteEndpoint(id string) *Action { + return &Action{ + HNSAction: &EndpointDeleteAction{ + ID: id, + }, + } +} + +func (e *EndpointDeleteAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { + ep := &hcn.HostComputeEndpoint{ + Id: e.ID, + } + if err := hns.DeleteEndpoint(ep); err != nil { + return errors.Wrapf(err, "[EndpointDeleteAction] failed to delete endpoint. ep: [%+v]", ep) + } + return nil +} + +type DPAction interface { + Do(dp *DataPlane) error +} + type PodCreateAction struct { - Pod *dataplane.PodMetadata + Pod *PodMetadata Labels map[string]string } func CreatePod(namespace, name, ip, node string, labels map[string]string) *Action { return &Action{ DPAction: &PodCreateAction{ - Pod: dataplane.NewPodMetadata(fmt.Sprintf("%s/%s", namespace, name), ip, node), + Pod: NewPodMetadata(fmt.Sprintf("%s/%s", namespace, name), ip, node), Labels: labels, }, } } -func (p *PodCreateAction) Do(dp *dataplane.DataPlane) error { +func (p *PodCreateAction) Do(dp *DataPlane) error { context := fmt.Sprintf("create context: [pod: %+v. labels: %+v]", p.Pod, p.Labels) nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Pod.Namespace(), ipsets.Namespace)} @@ -62,7 +147,7 @@ func ApplyDP() *Action { } } -func (*ApplyDPAction) Do(dp *dataplane.DataPlane) error { +func (*ApplyDPAction) Do(dp *DataPlane) error { if err := dp.ApplyDataPlane(); err != nil { return errors.Wrapf(err, "[ApplyDPAction] failed to apply") } @@ -85,7 +170,7 @@ func UpdatePolicy(policy *networkingv1.NetworkPolicy) *Action { } } -func (p *PolicyUpdateAction) Do(dp *dataplane.DataPlane) error { +func (p *PolicyUpdateAction) Do(dp *DataPlane) error { npmNetPol, err := translation.TranslatePolicy(p.Policy) if err != nil { return errors.Wrapf(err, "[PolicyUpdateAction] failed to translate policy with key %s/%s", p.Policy.Namespace, p.Policy.Name) @@ -115,7 +200,7 @@ func DeletePolicyByObject(policy *networkingv1.NetworkPolicy) *Action { return DeletePolicy(policy.Namespace, policy.Name) } -func (p *PolicyDeleteAction) Do(dp *dataplane.DataPlane) error { +func (p *PolicyDeleteAction) Do(dp *DataPlane) error { policyKey := fmt.Sprintf("%s/%s", p.Namespace, p.Name) if err := dp.RemovePolicy(policyKey); err != nil { return errors.Wrapf(err, "[PolicyDeleteAction] failed to update policy with key %s", policyKey) From ba0cf5b6362c7f8af7338ca7a3ab4a93811bafe3 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 16:35:29 -0700 Subject: [PATCH 17/34] hns latency hard coded to be the same for all threaded test cases --- npm/pkg/dataplane/dataplane-test-cases_windows_test.go | 3 --- npm/pkg/dataplane/dataplane_windows_test.go | 7 +++++-- npm/pkg/dataplane/types_windows_test.go | 2 -- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index 5263231dcc..1213c23b45 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -1,8 +1,6 @@ package dataplane import ( - "time" - "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" @@ -137,7 +135,6 @@ func getAllThreadedTests() []*ThreadedTestCase { return []*ThreadedTestCase{ { Description: "pod x/a created, then relevant network policy created", - HNSLatency: time.Duration(1 * time.Second), Threads: map[string][]*Action{ "pod_controller": { CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index a968c50933..78621a8787 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -16,7 +16,10 @@ import ( "github.com/stretchr/testify/require" ) -const defaultHNSLatency = time.Duration(0) +const ( + defaultHNSLatency = time.Duration(0) + threadedHNSLatency = time.Duration(1 * time.Second) +) func TestAllSerialCases(t *testing.T) { tests := getAllSerialTests() @@ -62,7 +65,7 @@ func TestAllThreadedCases(t *testing.T) { t.Logf("beginning test #%d. Description: [%s]. Tags: %+v", i, tt.Description, tt.Tags) hns := ipsets.GetHNSFake(t) - hns.Delay = tt.HNSLatency + hns.Delay = threadedHNSLatency io := common.NewMockIOShimWithFakeHNS(hns) for _, ep := range tt.InitialEndpoints { _, err := hns.CreateEndpoint(ep) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index 7ca4cea419..bc555cce1b 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -2,7 +2,6 @@ package dataplane import ( "fmt" - "time" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/controlplane/translation" @@ -23,7 +22,6 @@ type SerialTestCase struct { type ThreadedTestCase struct { Description string - HNSLatency time.Duration Threads map[string][]*Action *TestCaseMetadata } From 60928b3bd9657d5180fabb83a09a09a01efd0c55 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 16:48:47 -0700 Subject: [PATCH 18/34] fix build error after rebasing --- network/hnswrapper/hnsv2wrapperfake.go | 1 - 1 file changed, 1 deletion(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 27b4e77445..8355308511 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -506,7 +506,6 @@ func (fEndpoint *FakeHostComputeEndpoint) GetHCNObj() *hcn.HostComputeEndpoint { }, Policies: acls, } - return hcnEndpoint } func (fEndpoint *FakeHostComputeEndpoint) RemovePolicy(toRemovePol *FakeEndpointPolicy) error { From 32cd5ab09e05176449f7e69b72afb79db8028a74 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Fri, 21 Oct 2022 16:49:35 -0700 Subject: [PATCH 19/34] export fake hns network id --- common/ioshim_windows.go | 4 +++- npm/pkg/dataplane/ipsets/testutils_windows.go | 5 +++-- npm/pkg/dataplane/testutils/utils_windows.go | 10 ++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/common/ioshim_windows.go b/common/ioshim_windows.go index 9dd5ceb82c..56878a2005 100644 --- a/common/ioshim_windows.go +++ b/common/ioshim_windows.go @@ -9,6 +9,8 @@ import ( utilexec "k8s.io/utils/exec" ) +const FakeHNSNetworkID = "1234" + type IOShim struct { Exec utilexec.Interface Hns hnswrapper.HnsV2WrapperInterface @@ -24,7 +26,7 @@ func NewIOShim() *IOShim { func NewMockIOShim(calls []testutils.TestCmd) *IOShim { hns := hnswrapper.NewHnsv2wrapperFake() network := &hcn.HostComputeNetwork{ - Id: "1234", + Id: FakeHNSNetworkID, Name: "azure", } diff --git a/npm/pkg/dataplane/ipsets/testutils_windows.go b/npm/pkg/dataplane/ipsets/testutils_windows.go index 3421b3ee85..f5c780f981 100644 --- a/npm/pkg/dataplane/ipsets/testutils_windows.go +++ b/npm/pkg/dataplane/ipsets/testutils_windows.go @@ -3,16 +3,17 @@ package ipsets import ( "testing" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/hnswrapper" testutils "github.com/Azure/azure-container-networking/test/utils" - "github.com/stretchr/testify/require" "github.com/Microsoft/hcsshim/hcn" + "github.com/stretchr/testify/require" ) func GetHNSFake(t *testing.T) *hnswrapper.Hnsv2wrapperFake { hns := hnswrapper.NewHnsv2wrapperFake() network := &hcn.HostComputeNetwork{ - Id: "1234", + Id: common.FakeHNSNetworkID, Name: "azure", } diff --git a/npm/pkg/dataplane/testutils/utils_windows.go b/npm/pkg/dataplane/testutils/utils_windows.go index 408d5491b1..d78a870729 100644 --- a/npm/pkg/dataplane/testutils/utils_windows.go +++ b/npm/pkg/dataplane/testutils/utils_windows.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" @@ -15,9 +16,6 @@ import ( "k8s.io/klog" ) -// NOTE: depends on fake ioshim input -const azureNetworkID = "1234" - func PrefixNames(sets []*ipsets.IPSetMetadata) []string { a := make([]string, len(sets)) for k, s := range sets { @@ -30,7 +28,7 @@ func Endpoint(epID, ip string) *hcn.HostComputeEndpoint { return &hcn.HostComputeEndpoint{ Id: epID, Name: epID, - HostComputeNetwork: azureNetworkID, + HostComputeNetwork: common.FakeHNSNetworkID, IpConfigurations: []hcn.IpConfig{ { IpAddress: ip, @@ -78,7 +76,7 @@ func VerifyHNSCache(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetP func VerifySetPolicies(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedSetPolicies []*hcn.SetPolicySetting) bool { t.Helper() - cachedSetPolicies := hns.Cache.AllSetPolicies(azureNetworkID) + cachedSetPolicies := hns.Cache.AllSetPolicies(common.FakeHNSNetworkID) success := assert.Equal(t, len(expectedSetPolicies), len(cachedSetPolicies), "unexpected number of SetPolicies") for _, expectedSetPolicy := range expectedSetPolicies { @@ -134,7 +132,7 @@ func VerifyACLs(t *testing.T, hns *hnswrapper.Hnsv2wrapperFake, expectedEndpoint // helpful for debugging if there's a discrepancy between GetAll functions and the HNS PrettyString func PrintGetAllOutput(hns *hnswrapper.Hnsv2wrapperFake) { klog.Info("SETPOLICIES...") - for _, setPol := range hns.Cache.AllSetPolicies(azureNetworkID) { + for _, setPol := range hns.Cache.AllSetPolicies(common.FakeHNSNetworkID) { klog.Infof("%+v", setPol) } klog.Info("Endpoint ACLs...") From cfc00995902b77681a1826393ad120fdad4cd217 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 11:28:50 -0700 Subject: [PATCH 20/34] address comments on multierr and terminology --- .../dataplane-test-cases_windows_test.go | 21 ++++++++++++-- npm/pkg/dataplane/dataplane_windows_test.go | 29 ++++++++----------- npm/pkg/dataplane/types_windows_test.go | 4 +-- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index 1213c23b45..2d56c8a702 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -3,6 +3,7 @@ package dataplane import ( "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" + "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" "github.com/Microsoft/hcsshim/hcn" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" @@ -50,6 +51,19 @@ var ( nsK2V2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) ) +// DP Configs +var ( + defaultWindowsDPCfg = &Config{ + IPSetManagerCfg: &ipsets.IPSetManagerCfg{ + IPSetMode: ipsets.ApplyAllIPSets, + AddEmptySetToLists: true, + }, + PolicyManagerCfg: &policies.PolicyManagerCfg{ + PolicyMode: policies.IPSetPolicyMode, + }, + } +) + func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { return &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -90,6 +104,7 @@ func getAllSerialTests() []*SerialTestCase { netpolCrudTag, backgroundTag, }, + DpCfg: defaultWindowsDPCfg, InitialEndpoints: []*hcn.HostComputeEndpoint{ dptestutils.Endpoint(endpoint1, ip1), }, @@ -131,11 +146,11 @@ func getAllSerialTests() []*SerialTestCase { } } -func getAllThreadedTests() []*ThreadedTestCase { - return []*ThreadedTestCase{ +func getAllMultiRoutineTests() []*MultiRoutineTestCase { + return []*MultiRoutineTestCase{ { Description: "pod x/a created, then relevant network policy created", - Threads: map[string][]*Action{ + Routines: map[string][]*Action{ "pod_controller": { CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), CreatePod("y", "a", ip2, otherNode, map[string]string{"k2": "v2"}), diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 78621a8787..f771f51e9b 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -1,8 +1,6 @@ package dataplane import ( - "fmt" - "strings" "sync" "testing" "time" @@ -14,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "go.uber.org/multierr" ) const ( @@ -56,8 +56,8 @@ func TestAllSerialCases(t *testing.T) { } } -func TestAllThreadedCases(t *testing.T) { - tests := getAllThreadedTests() +func TestAllMultiRoutineCases(t *testing.T) { + tests := getAllMultiRoutineTests() for i, tt := range tests { i := i tt := tt @@ -76,15 +76,15 @@ func TestAllThreadedCases(t *testing.T) { dp, err := NewDataPlane(thisNode, io, tt.DpCfg, nil) require.NoError(t, err, "failed to initialize dp") + var errMulti error wg := new(sync.WaitGroup) - wg.Add(len(tt.Threads)) - backgroundErrors := make(chan error, len(tt.Threads)) - for thName, th := range tt.Threads { - thName := thName - th := th + wg.Add(len(tt.Routines)) + for rName, r := range tt.Routines { + rName := rName + r := r go func() { defer wg.Done() - for k, a := range th { + for k, a := range r { var err error if a.HNSAction != nil { err = a.HNSAction.Do(hns) @@ -93,7 +93,7 @@ func TestAllThreadedCases(t *testing.T) { } if err != nil { - backgroundErrors <- errors.Wrapf(err, "failed to run action %d in thread %s", k, thName) + errMulti = multierr.Append(errMulti, errors.Wrapf(err, "failed to run action %d in routine %s", k, rName)) break } } @@ -101,12 +101,7 @@ func TestAllThreadedCases(t *testing.T) { } wg.Wait() - close(backgroundErrors) - errStrings := make([]string, len(backgroundErrors)) - for err := range backgroundErrors { - errStrings = append(errStrings, fmt.Sprintf("[%s]", err.Error())) - } - assert.Empty(t, backgroundErrors, "encountered errors in threaded test: %s", strings.Join(errStrings, ",")) + assert.Nil(t, errMulti, "encountered errors in multi-routine test") }) } } diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index bc555cce1b..1f760b05b2 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -20,9 +20,9 @@ type SerialTestCase struct { *TestCaseMetadata } -type ThreadedTestCase struct { +type MultiRoutineTestCase struct { Description string - Threads map[string][]*Action + Routines map[string][]*Action *TestCaseMetadata } From c1bad9dd0651fbcc99e4096fdee8c99f1a86ef4e Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 11:29:23 -0700 Subject: [PATCH 21/34] add comment about pod metadata in controller --- npm/pkg/controlplane/controllers/v2/podController.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/npm/pkg/controlplane/controllers/v2/podController.go b/npm/pkg/controlplane/controllers/v2/podController.go index b9a84e0b7b..83877c3cd7 100644 --- a/npm/pkg/controlplane/controllers/v2/podController.go +++ b/npm/pkg/controlplane/controllers/v2/podController.go @@ -573,6 +573,8 @@ func (c *PodController) cleanUpDeletedPod(cachedNpmPodKey string) error { func (c *PodController) manageNamedPortIpsets(portList []corev1.ContainerPort, podKey, podIP, nodeName string, namedPortOperation NamedPortOperation) error { if util.IsWindowsDP() { + // NOTE: if we support namedport operations, need to be careful of implications of including the node name in the pod metadata below + // since we say the node name is "" in cleanUpDeletedPod klog.Warningf("Windows Dataplane does not support NamedPort operations. Operation: %s portList is %+v", namedPortOperation, portList) return nil } From e99fba5a5b94a33cf9aa552e2ac4ea8cc65dff7f Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 11:29:48 -0700 Subject: [PATCH 22/34] pod update and delete actions --- npm/pkg/dataplane/types_windows_test.go | 103 ++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 5 deletions(-) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index 1f760b05b2..ac2de69280 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -100,9 +100,10 @@ type PodCreateAction struct { } func CreatePod(namespace, name, ip, node string, labels map[string]string) *Action { + podKey := fmt.Sprintf("%s/%s", namespace, name) return &Action{ DPAction: &PodCreateAction{ - Pod: NewPodMetadata(fmt.Sprintf("%s/%s", namespace, name), ip, node), + Pod: NewPodMetadata(podKey, ip, node), Labels: labels, }, } @@ -125,13 +126,107 @@ func (p *PodCreateAction) Do(dp *DataPlane) error { keyVal := fmt.Sprintf("%s:%s", key, val) labelIPSets := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(key, ipsets.KeyLabelOfPod), - ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfPod), } if err := dp.AddToSets(labelIPSets, p.Pod); err != nil { - return errors.Wrapf(err, "[PodCreateAction] failed to add pod ip to label sets. %s", context) + return errors.Wrapf(err, "[PodCreateAction] failed to add pod ip to label sets %+v. %s", labelIPSets, context) } + } + + return nil +} + +type PodUpdateAction struct { + OldPod *PodMetadata + NewPod *PodMetadata + OldLabels map[string]string + NewLabels map[string]string +} + +func UpdatePod(namespace, name, oldIP, oldNode string, oldLabels map[string]string, newIP, newNode string, newLabels map[string]string) *Action { + podKey := fmt.Sprintf("%s/%s", namespace, name) + return &Action{ + DPAction: &PodUpdateAction{ + OldPod: NewPodMetadata(podKey, oldIP, oldNode), + NewPod: NewPodMetadata(podKey, newIP, newNode), + OldLabels: oldLabels, + NewLabels: newLabels, + }, + } +} + +func UpdatePodLabels(namespace, name, ip, node string, oldLabels, newLabels map[string]string) *Action { + return UpdatePod(namespace, name, ip, node, oldLabels, ip, node, newLabels) +} + +func (p *PodUpdateAction) Do(dp *DataPlane) error { + context := fmt.Sprintf("update context: [old pod: %+v. current IP: %+v. old labels: %+v. new labels: %+v]", p.OldPod, p.NewPod.PodIP, p.OldLabels, p.NewLabels) + + // think it's impossible for this to be called on an UPDATE + // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) + + for k, v := range p.OldLabels { + sets := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), + } + for _, toRemoveSet := range sets { + if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, p.OldPod); err != nil { + return errors.Wrapf(err, "[PodUpdateAction] failed to remove old pod ip from set %s. %s", toRemoveSet.GetPrefixName(), context) + } + } + } + + for k, v := range p.NewLabels { + sets := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), + } + for _, toAddSet := range sets { + if err := dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, p.NewPod); err != nil { + return errors.Wrapf(err, "[PodUpdateAction] failed to add new pod ip to set %s. %s", toAddSet.GetPrefixName(), context) + } + } + } + + return nil +} + +type PodDeleteAction struct { + Pod *PodMetadata + Labels map[string]string +} + +func DeletePod(namespace, name, ip string, labels map[string]string) *Action { + podKey := fmt.Sprintf("%s/%s", namespace, name) + return &Action{ + DPAction: &PodDeleteAction{ + // currently, the Pod Controller doesn't share the node name + Pod: NewPodMetadata(podKey, ip, ""), + Labels: labels, + }, + } +} +func (p *PodDeleteAction) Do(dp *DataPlane) error { + context := fmt.Sprintf("delete context: [pod: %+v. labels: %+v]", p.Pod, p.Labels) + + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Pod.Namespace(), ipsets.Namespace)} + if err := dp.RemoveFromSets(nsIPSet, p.Pod); err != nil { + return errors.Wrapf(err, "[PodDeleteAction] failed to remove pod ip from ns set. %s", context) + } + + for key, val := range p.Labels { + keyVal := fmt.Sprintf("%s:%s", key, val) + labelIPSets := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(key, ipsets.KeyLabelOfPod), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfPod), + } + + if err := dp.RemoveFromSets(labelIPSets, p.Pod); err != nil { + return errors.Wrapf(err, "[PodDeleteAction] failed to remove pod ip from label set %+v. %s", labelIPSets, context) + } } return nil @@ -152,8 +247,6 @@ func (*ApplyDPAction) Do(dp *DataPlane) error { return nil } -// TODO PodUpdateAction and PodDeleteAction - // TODO namespace actions type PolicyUpdateAction struct { From 67ba42c68bcabfdef5d7800fbeda8c8cdbbd2825 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 11:30:56 -0700 Subject: [PATCH 23/34] move ApplyDPAction to top --- npm/pkg/dataplane/types_windows_test.go | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index ac2de69280..fa9005477e 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -94,6 +94,21 @@ type DPAction interface { Do(dp *DataPlane) error } +type ApplyDPAction struct{} + +func ApplyDP() *Action { + return &Action{ + DPAction: &ApplyDPAction{}, + } +} + +func (*ApplyDPAction) Do(dp *DataPlane) error { + if err := dp.ApplyDataPlane(); err != nil { + return errors.Wrapf(err, "[ApplyDPAction] failed to apply") + } + return nil +} + type PodCreateAction struct { Pod *PodMetadata Labels map[string]string @@ -232,21 +247,6 @@ func (p *PodDeleteAction) Do(dp *DataPlane) error { return nil } -type ApplyDPAction struct{} - -func ApplyDP() *Action { - return &Action{ - DPAction: &ApplyDPAction{}, - } -} - -func (*ApplyDPAction) Do(dp *DataPlane) error { - if err := dp.ApplyDataPlane(); err != nil { - return errors.Wrapf(err, "[ApplyDPAction] failed to apply") - } - return nil -} - // TODO namespace actions type PolicyUpdateAction struct { From 12d0dcfa4bdfe7913ad654b53c4e2528c59e132c Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 12:06:16 -0700 Subject: [PATCH 24/34] namespace actions and rename some fields of UpdatePod --- npm/pkg/dataplane/types_windows_test.go | 143 +++++++++++++++++++++--- 1 file changed, 127 insertions(+), 16 deletions(-) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index fa9005477e..eaeb564579 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -130,7 +130,7 @@ func (p *PodCreateAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(p.Pod.Namespace(), ipsets.Namespace)} // PodController technically wouldn't call this if the namespace already existed if err := dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, nsIPSet); err != nil { - return errors.Wrapf(err, "[PodCreateAction] failed to add ns set to all namespaces list. %s", context) + return errors.Wrapf(err, "[PodCreateAction] failed to add ns set to all-namespaces list. %s", context) } if err := dp.AddToSets(nsIPSet, p.Pod); err != nil { @@ -153,35 +153,35 @@ func (p *PodCreateAction) Do(dp *DataPlane) error { } type PodUpdateAction struct { - OldPod *PodMetadata - NewPod *PodMetadata - OldLabels map[string]string - NewLabels map[string]string + OldPod *PodMetadata + NewPod *PodMetadata + LabelsToRemove map[string]string + LabelsToAdd map[string]string } -func UpdatePod(namespace, name, oldIP, oldNode string, oldLabels map[string]string, newIP, newNode string, newLabels map[string]string) *Action { +func UpdatePod(namespace, name, oldIP, oldNode, newIP, newNode string, labelsToRemove, labelsToAdd map[string]string) *Action { podKey := fmt.Sprintf("%s/%s", namespace, name) return &Action{ DPAction: &PodUpdateAction{ - OldPod: NewPodMetadata(podKey, oldIP, oldNode), - NewPod: NewPodMetadata(podKey, newIP, newNode), - OldLabels: oldLabels, - NewLabels: newLabels, + OldPod: NewPodMetadata(podKey, oldIP, oldNode), + NewPod: NewPodMetadata(podKey, newIP, newNode), + LabelsToRemove: labelsToRemove, + LabelsToAdd: labelsToAdd, }, } } -func UpdatePodLabels(namespace, name, ip, node string, oldLabels, newLabels map[string]string) *Action { - return UpdatePod(namespace, name, ip, node, oldLabels, ip, node, newLabels) +func UpdatePodLabels(namespace, name, ip, node string, labelsToRemove, labelsToAdd map[string]string) *Action { + return UpdatePod(namespace, name, ip, node, ip, node, labelsToRemove, labelsToAdd) } func (p *PodUpdateAction) Do(dp *DataPlane) error { - context := fmt.Sprintf("update context: [old pod: %+v. current IP: %+v. old labels: %+v. new labels: %+v]", p.OldPod, p.NewPod.PodIP, p.OldLabels, p.NewLabels) + context := fmt.Sprintf("update context: [old pod: %+v. current IP: %+v. old labels: %+v. new labels: %+v]", p.OldPod, p.NewPod.PodIP, p.LabelsToRemove, p.LabelsToAdd) // think it's impossible for this to be called on an UPDATE // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) - for k, v := range p.OldLabels { + for k, v := range p.LabelsToRemove { sets := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), @@ -193,7 +193,7 @@ func (p *PodUpdateAction) Do(dp *DataPlane) error { } } - for k, v := range p.NewLabels { + for k, v := range p.LabelsToAdd { sets := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), @@ -247,7 +247,118 @@ func (p *PodDeleteAction) Do(dp *DataPlane) error { return nil } -// TODO namespace actions +type NamespaceCreateAction struct { + NS string + Labels map[string]string +} + +func CreateNamespace(ns string, labels map[string]string) *Action { + return &Action{ + DPAction: &NamespaceCreateAction{ + NS: ns, + Labels: labels, + }, + } +} + +func (n *NamespaceCreateAction) Do(dp *DataPlane) error { + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} + + listsToAddTo := []*ipsets.IPSetMetadata{allNamespaces} + for k, v := range n.Labels { + listsToAddTo = append(listsToAddTo, + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace)) + } + + if err := dp.AddToLists(listsToAddTo, nsIPSet); err != nil { + return errors.Wrapf(err, "[NamespaceCreateAction] failed to add ns ipset to all lists. Action: %+v", n) + } + + return nil +} + +type NamespaceUpdateAction struct { + NS string + LabelsToRemove map[string]string + LabelsToAdd map[string]string +} + +func UpdateNamespace(ns string, labelsToRemove, labelsToAdd map[string]string) *Action { + return &Action{ + DPAction: &NamespaceUpdateAction{ + NS: ns, + LabelsToRemove: labelsToRemove, + LabelsToAdd: labelsToAdd, + }, + } +} + +func (n *NamespaceUpdateAction) Do(dp *DataPlane) error { + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} + + for k, v := range n.LabelsToRemove { + lists := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + } + for _, listToRemoveFrom := range lists { + if err := dp.RemoveFromList(listToRemoveFrom, nsIPSet); err != nil { + return errors.Wrapf(err, "[NamespaceUpdateAction] failed to remove ns ipset from list %s. Action: %+v", listToRemoveFrom.GetPrefixName(), n) + } + } + } + + for k, v := range n.LabelsToAdd { + lists := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + } + for _, listToAddTo := range lists { + if err := dp.RemoveFromList(listToAddTo, nsIPSet); err != nil { + return errors.Wrapf(err, "[NamespaceUpdateAction] failed to add ns ipset to list %s. Action: %+v", listToAddTo.GetPrefixName(), n) + } + } + } + + return nil +} + +type NamespaceDeleteAction struct { + NS string + Labels map[string]string +} + +func DeleteNamespace(ns string, labels map[string]string) *Action { + return &Action{ + DPAction: &NamespaceDeleteAction{ + NS: ns, + Labels: labels, + }, + } +} + +func (n *NamespaceDeleteAction) Do(dp *DataPlane) error { + nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} + + for k, v := range n.Labels { + lists := []*ipsets.IPSetMetadata{ + ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), + ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + } + for _, listToRemoveFrom := range lists { + if err := dp.RemoveFromList(listToRemoveFrom, nsIPSet); err != nil { + return errors.Wrapf(err, "[NamespaceDeleteAction] failed to remove ns ipset from list %s. Action: %+v", listToRemoveFrom.GetPrefixName(), n) + } + } + } + + if err := dp.RemoveFromList(allNamespaces, nsIPSet); err != nil { + return errors.Wrapf(err, "[NamespaceDeleteAction] failed to remove ns ipset from all-namespaces list. Action: %+v", n) + } + + return nil +} type PolicyUpdateAction struct { Policy *networkingv1.NetworkPolicy From 76f040c0eea82f9235a18343762fddce76dc607f Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 12:13:54 -0700 Subject: [PATCH 25/34] adding code comments --- npm/pkg/dataplane/types_windows_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index eaeb564579..2dc5aff971 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -42,6 +42,7 @@ type Action struct { } type HNSAction interface { + // Do models events in HNS Do(hns *hnswrapper.Hnsv2wrapperFake) error } @@ -59,6 +60,7 @@ func CreateEndpoint(id, ip string) *Action { } } +// Do models endpoint creation in HNS func (e *EndpointCreateAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { ep := dptestutils.Endpoint(e.ID, e.IP) _, err := hns.CreateEndpoint(ep) @@ -80,6 +82,7 @@ func DeleteEndpoint(id string) *Action { } } +// Do models endpoint deletion in HNS func (e *EndpointDeleteAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { ep := &hcn.HostComputeEndpoint{ Id: e.ID, @@ -91,6 +94,7 @@ func (e *EndpointDeleteAction) Do(hns *hnswrapper.Hnsv2wrapperFake) error { } type DPAction interface { + // Do models interactions with the DataPlane Do(dp *DataPlane) error } @@ -102,6 +106,7 @@ func ApplyDP() *Action { } } +// Do applies the dataplane func (*ApplyDPAction) Do(dp *DataPlane) error { if err := dp.ApplyDataPlane(); err != nil { return errors.Wrapf(err, "[ApplyDPAction] failed to apply") @@ -124,6 +129,7 @@ func CreatePod(namespace, name, ip, node string, labels map[string]string) *Acti } } +// Do models pod creation in the PodController func (p *PodCreateAction) Do(dp *DataPlane) error { context := fmt.Sprintf("create context: [pod: %+v. labels: %+v]", p.Pod, p.Labels) @@ -175,6 +181,7 @@ func UpdatePodLabels(namespace, name, ip, node string, labelsToRemove, labelsToA return UpdatePod(namespace, name, ip, node, ip, node, labelsToRemove, labelsToAdd) } +// Do models pod updates in the PodController func (p *PodUpdateAction) Do(dp *DataPlane) error { context := fmt.Sprintf("update context: [old pod: %+v. current IP: %+v. old labels: %+v. new labels: %+v]", p.OldPod, p.NewPod.PodIP, p.LabelsToRemove, p.LabelsToAdd) @@ -217,13 +224,14 @@ func DeletePod(namespace, name, ip string, labels map[string]string) *Action { podKey := fmt.Sprintf("%s/%s", namespace, name) return &Action{ DPAction: &PodDeleteAction{ - // currently, the Pod Controller doesn't share the node name + // currently, the PodController doesn't share the node name Pod: NewPodMetadata(podKey, ip, ""), Labels: labels, }, } } +// Do models pod deletion in the PodController func (p *PodDeleteAction) Do(dp *DataPlane) error { context := fmt.Sprintf("delete context: [pod: %+v. labels: %+v]", p.Pod, p.Labels) @@ -261,6 +269,7 @@ func CreateNamespace(ns string, labels map[string]string) *Action { } } +// Do models namespace creation in the NamespaceController func (n *NamespaceCreateAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} @@ -294,6 +303,7 @@ func UpdateNamespace(ns string, labelsToRemove, labelsToAdd map[string]string) * } } +// Do models namespace updates in the NamespaceController func (n *NamespaceUpdateAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} @@ -338,6 +348,7 @@ func DeleteNamespace(ns string, labels map[string]string) *Action { } } +// Do models namespace deletion in the NamespaceController func (n *NamespaceDeleteAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} @@ -372,6 +383,7 @@ func UpdatePolicy(policy *networkingv1.NetworkPolicy) *Action { } } +// Do models policy updates in the NetworkPolicyController func (p *PolicyUpdateAction) Do(dp *DataPlane) error { npmNetPol, err := translation.TranslatePolicy(p.Policy) if err != nil { @@ -402,6 +414,7 @@ func DeletePolicyByObject(policy *networkingv1.NetworkPolicy) *Action { return DeletePolicy(policy.Namespace, policy.Name) } +// Do models policy deletion in the NetworkPolicyController func (p *PolicyDeleteAction) Do(dp *DataPlane) error { policyKey := fmt.Sprintf("%s/%s", p.Namespace, p.Name) if err := dp.RemovePolicy(policyKey); err != nil { From 1f304f6c23f5dbc526dd216d3e1f3d6015dc6305 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 12:19:25 -0700 Subject: [PATCH 26/34] reconcile action --- npm/pkg/dataplane/types_windows_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index 2dc5aff971..20bfd4143f 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -114,6 +114,22 @@ func (*ApplyDPAction) Do(dp *DataPlane) error { return nil } +type ReconcileDPAction struct{} + +func ReconcileDP() *Action { + return &Action{ + DPAction: &ReconcileDPAction{}, + } +} + +// Do reconciles the IPSetManager and PolicyManager +func (*ReconcileDPAction) Do(dp *DataPlane) error { + dp.ipsetMgr.Reconcile() + // currently does nothing in windows + dp.policyMgr.Reconcile() + return nil +} + type PodCreateAction struct { Pod *PodMetadata Labels map[string]string From 9740ccdd8ff08c6347b91591532ac6d56916c0e1 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 13:17:29 -0700 Subject: [PATCH 27/34] fix bug in key-val ipsets --- npm/pkg/dataplane/types_windows_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index 20bfd4143f..21398501f8 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -205,9 +205,10 @@ func (p *PodUpdateAction) Do(dp *DataPlane) error { // dp.AddToLists([]*ipsets.IPSetMetadata{allNamespaces}, []*ipsets.IPSetMetadata{nsIPSet}) for k, v := range p.LabelsToRemove { + keyVal := fmt.Sprintf("%s:%s", k, v) sets := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfPod), } for _, toRemoveSet := range sets { if err := dp.RemoveFromSets([]*ipsets.IPSetMetadata{toRemoveSet}, p.OldPod); err != nil { @@ -217,9 +218,10 @@ func (p *PodUpdateAction) Do(dp *DataPlane) error { } for k, v := range p.LabelsToAdd { + keyVal := fmt.Sprintf("%s:%s", k, v) sets := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfPod), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfPod), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfPod), } for _, toAddSet := range sets { if err := dp.AddToSets([]*ipsets.IPSetMetadata{toAddSet}, p.NewPod); err != nil { @@ -291,9 +293,10 @@ func (n *NamespaceCreateAction) Do(dp *DataPlane) error { listsToAddTo := []*ipsets.IPSetMetadata{allNamespaces} for k, v := range n.Labels { + keyVal := fmt.Sprintf("%s:%s", k, v) listsToAddTo = append(listsToAddTo, ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace)) + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace)) } if err := dp.AddToLists(listsToAddTo, nsIPSet); err != nil { @@ -324,9 +327,10 @@ func (n *NamespaceUpdateAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} for k, v := range n.LabelsToRemove { + keyVal := fmt.Sprintf("%s:%s", k, v) lists := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), } for _, listToRemoveFrom := range lists { if err := dp.RemoveFromList(listToRemoveFrom, nsIPSet); err != nil { @@ -336,9 +340,10 @@ func (n *NamespaceUpdateAction) Do(dp *DataPlane) error { } for k, v := range n.LabelsToAdd { + keyVal := fmt.Sprintf("%s:%s", k, v) lists := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), } for _, listToAddTo := range lists { if err := dp.RemoveFromList(listToAddTo, nsIPSet); err != nil { @@ -369,9 +374,10 @@ func (n *NamespaceDeleteAction) Do(dp *DataPlane) error { nsIPSet := []*ipsets.IPSetMetadata{ipsets.NewIPSetMetadata(n.NS, ipsets.Namespace)} for k, v := range n.Labels { + keyVal := fmt.Sprintf("%s:%s", k, v) lists := []*ipsets.IPSetMetadata{ ipsets.NewIPSetMetadata(k, ipsets.KeyLabelOfNamespace), - ipsets.NewIPSetMetadata(v, ipsets.KeyValueLabelOfNamespace), + ipsets.NewIPSetMetadata(keyVal, ipsets.KeyValueLabelOfNamespace), } for _, listToRemoveFrom := range lists { if err := dp.RemoveFromList(listToRemoveFrom, nsIPSet); err != nil { From de6baafc819d08eae85bf6b59fc2c984445a6bd3 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 13:17:48 -0700 Subject: [PATCH 28/34] implement all previous test cases --- .../dataplane-test-cases_windows_test.go | 287 ++++++++++++++++-- 1 file changed, 258 insertions(+), 29 deletions(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index 2d56c8a702..6b9f25db86 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -17,7 +17,6 @@ const ( podCrudTag Tag = "pod-crud" nsCrudTag Tag = "namespace-crud" netpolCrudTag Tag = "netpol-crud" - backgroundTag Tag = "has-background-steps" ) const ( @@ -42,8 +41,8 @@ var ( // in Windows, this Config option is actually forced to be enabled in NewDataPlane() emptySet = ipsets.NewIPSetMetadata("emptyhashset", ipsets.EmptyHashSet) allNamespaces = ipsets.NewIPSetMetadata("all-namespaces", ipsets.KeyLabelOfNamespace) - nsXSet = ipsets.NewIPSetMetadata("ns1", ipsets.Namespace) - nsYSet = ipsets.NewIPSetMetadata("ns2", ipsets.Namespace) + nsXSet = ipsets.NewIPSetMetadata("x", ipsets.Namespace) + nsYSet = ipsets.NewIPSetMetadata("y", ipsets.Namespace) nsK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) nsK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) @@ -64,11 +63,11 @@ var ( } ) -func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { +func policyXBaseOnK1V1() *networkingv1.NetworkPolicy { return &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: "labelPair1-allow-all", - Namespace: "ns1", + Name: "base", + Namespace: "x", }, Spec: networkingv1.NetworkPolicySpec{ PodSelector: metav1.LabelSelector{ @@ -93,21 +92,116 @@ func policyNs1LabelPair1AllowAll() *networkingv1.NetworkPolicy { func getAllSerialTests() []*SerialTestCase { return []*SerialTestCase{ { - Description: "pod x/a created, then relevant network policy created", + Description: "pod created", Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), - UpdatePolicy(policyNs1LabelPair1AllowAll()), + ApplyDP(), }, TestCaseMetadata: &TestCaseMetadata{ Tags: []Tag{ podCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + }, + { + Description: "pod created, then pod deleted", + Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + ApplyDP(), + DeleteEndpoint(endpoint1), + DeletePod("x", "a", ip1, map[string]string{"k1": "v1"}), + ApplyDP(), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet), + dptestutils.SetPolicy(podK1Set), + dptestutils.SetPolicy(podK1V1Set), + }, + ExpectedEnpdointACLs: nil, + }, + }, + { + Description: "pod created, then pod deleted, then ipsets garbage collected", + Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + ApplyDP(), + DeleteEndpoint(endpoint1), + DeletePod("x", "a", ip1, map[string]string{"k1": "v1"}), + ApplyDP(), + ReconcileDP(), + ApplyDP(), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet), + }, + ExpectedEnpdointACLs: nil, + }, + }, + { + Description: "policy created with no pods", + Actions: []*Action{ + UpdatePolicy(policyXBaseOnK1V1()), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ netpolCrudTag, - backgroundTag, }, - DpCfg: defaultWindowsDPCfg, - InitialEndpoints: []*hcn.HostComputeEndpoint{ - dptestutils.Endpoint(endpoint1, ip1), + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + // will not be an all-namespaces IPSet unless there's a Pod/Namespace event + dptestutils.SetPolicy(nsXSet), + // Policies do not create the KeyLabelOfPod type IPSet if the selector has a key-value requirement + dptestutils.SetPolicy(podK1V1Set), + }, + }, + }, + { + Description: "pod created on node, then relevant policy created", + Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + // will apply dirty ipsets from CreatePod + UpdatePolicy(policyXBaseOnK1V1()), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), @@ -118,7 +212,7 @@ func getAllSerialTests() []*SerialTestCase { ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ endpoint1: { { - ID: "azure-acl-ns1-labelPair1-allow-all", + ID: "azure-acl-x-base", Protocols: "", Action: "Allow", Direction: "In", @@ -129,7 +223,7 @@ func getAllSerialTests() []*SerialTestCase { Priority: 222, }, { - ID: "azure-acl-ns1-labelPair1-allow-all", + ID: "azure-acl-x-base", Protocols: "", Action: "Allow", Direction: "Out", @@ -143,25 +237,160 @@ func getAllSerialTests() []*SerialTestCase { }, }, }, - } -} - -func getAllMultiRoutineTests() []*MultiRoutineTestCase { - return []*MultiRoutineTestCase{ { - Description: "pod x/a created, then relevant network policy created", - Routines: map[string][]*Action{ - "pod_controller": { - CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), - CreatePod("y", "a", ip2, otherNode, map[string]string{"k2": "v2"}), + Description: "pod created on node, then relevant policy created, then policy deleted", + Actions: []*Action{ + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + // will apply dirty ipsets from CreatePod + UpdatePolicy(policyXBaseOnK1V1()), + DeletePolicyByObject(policyXBaseOnK1V1()), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), }, - "policy_controller": { - UpdatePolicy(policyNs1LabelPair1AllowAll()), + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, }, - "namespace_controller": {}, }, - // would fill metadata out for actual test case - TestCaseMetadata: nil, }, + { + Description: "pod created off node (no local endpoint), then relevant policy created", + Actions: []*Action{ + CreatePod("x", "a", ip1, otherNode, map[string]string{"k1": "v1"}), + // will apply dirty ipsets from CreatePod + UpdatePolicy(policyXBaseOnK1V1()), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), + }, + ExpectedEnpdointACLs: nil, + }, + }, + { + Description: "policy created, then pod created which satisfies policy", + Actions: []*Action{ + UpdatePolicy(policyXBaseOnK1V1()), + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + ApplyDP(), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(podK1Set, ip1), + dptestutils.SetPolicy(podK1V1Set, ip1), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + }, + }, + }, + { + Description: "policy created, then pod created which satisfies policy, then pod relabeled and no longer satisfies policy", + Actions: []*Action{ + UpdatePolicy(policyXBaseOnK1V1()), + CreateEndpoint(endpoint1, ip1), + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + ApplyDP(), + UpdatePodLabels("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}, map[string]string{"k2": "v2"}), + ApplyDP(), + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: nil, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + // old labels (not yet garbage collected) + dptestutils.SetPolicy(podK1Set), + dptestutils.SetPolicy(podK1V1Set), + // new labels + dptestutils.SetPolicy(podK2Set, ip1), + dptestutils.SetPolicy(podK2V2Set, ip1), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: {}, + }, + }, + }, + } +} + +func getAllMultiRoutineTests() []*MultiRoutineTestCase { + return []*MultiRoutineTestCase{ + // { + // Description: "pod x/a created, then relevant network policy created", + // Routines: map[string][]*Action{ + // "pod_controller": { + // CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + // CreatePod("y", "a", ip2, otherNode, map[string]string{"k2": "v2"}), + // }, + // "policy_controller": { + // UpdatePolicy(policyNs1LabelPair1AllowAll()), + // }, + // "namespace_controller": {}, + // }, + // // would fill metadata out for actual test case + // TestCaseMetadata: nil, + // }, } } From 9bde4b206c485abc0e226910cd46feea3b35b689 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 13:19:47 -0700 Subject: [PATCH 29/34] fix incorrect error wrapping in dataplane.go --- npm/pkg/dataplane/dataplane.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/pkg/dataplane/dataplane.go b/npm/pkg/dataplane/dataplane.go index a2514c2847..b967adeef5 100644 --- a/npm/pkg/dataplane/dataplane.go +++ b/npm/pkg/dataplane/dataplane.go @@ -240,7 +240,7 @@ func (dp *DataPlane) ApplyDataPlane() error { delete(dp.updatePodCache.cache, podKey) } if aggregateErr != nil { - return fmt.Errorf("[DataPlane] error while updating pods: %w", err) + return fmt.Errorf("[DataPlane] error while updating pods: %w", aggregateErr) } } return nil From 0aaba053b53bb6797cff95226ee8abba88df0e3e Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 14:29:41 -0700 Subject: [PATCH 30/34] multi-job tests are working. updated terminology from routine to job --- .../dataplane-test-cases_windows_test.go | 108 +++++++++++++----- npm/pkg/dataplane/dataplane_windows_test.go | 20 ++-- npm/pkg/dataplane/types_windows_test.go | 4 +- 3 files changed, 94 insertions(+), 38 deletions(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index 6b9f25db86..cfb4da98fc 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -46,8 +46,8 @@ var ( nsK1Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) nsK1V1Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) - nsK2Set = ipsets.NewIPSetMetadata("k1", ipsets.KeyLabelOfNamespace) - nsK2V2Set = ipsets.NewIPSetMetadata("k1:v1", ipsets.KeyValueLabelOfNamespace) + nsK2Set = ipsets.NewIPSetMetadata("k2", ipsets.KeyLabelOfNamespace) + nsK2V2Set = ipsets.NewIPSetMetadata("k2:v2", ipsets.KeyValueLabelOfNamespace) ) // DP Configs @@ -106,7 +106,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), dptestutils.SetPolicy(podK1Set, ip1), dptestutils.SetPolicy(podK1V1Set, ip1), @@ -134,7 +134,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet), dptestutils.SetPolicy(podK1Set), dptestutils.SetPolicy(podK1V1Set), @@ -162,7 +162,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet), }, ExpectedEnpdointACLs: nil, @@ -204,7 +204,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), dptestutils.SetPolicy(podK1Set, ip1), dptestutils.SetPolicy(podK1V1Set, ip1), @@ -255,7 +255,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), dptestutils.SetPolicy(podK1Set, ip1), dptestutils.SetPolicy(podK1V1Set, ip1), @@ -281,7 +281,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), dptestutils.SetPolicy(podK1Set, ip1), dptestutils.SetPolicy(podK1V1Set, ip1), @@ -306,7 +306,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), dptestutils.SetPolicy(podK1Set, ip1), dptestutils.SetPolicy(podK1V1Set, ip1), @@ -358,7 +358,7 @@ func getAllSerialTests() []*SerialTestCase { InitialEndpoints: nil, ExpectedSetPolicies: []*hcn.SetPolicySetting{ dptestutils.SetPolicy(emptySet), - dptestutils.SetPolicy(allNamespaces, nsXSet.GetHashedName(), emptySet.GetHashedName()), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName()), dptestutils.SetPolicy(nsXSet, ip1), // old labels (not yet garbage collected) dptestutils.SetPolicy(podK1Set), @@ -375,22 +375,76 @@ func getAllSerialTests() []*SerialTestCase { } } -func getAllMultiRoutineTests() []*MultiRoutineTestCase { - return []*MultiRoutineTestCase{ - // { - // Description: "pod x/a created, then relevant network policy created", - // Routines: map[string][]*Action{ - // "pod_controller": { - // CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), - // CreatePod("y", "a", ip2, otherNode, map[string]string{"k2": "v2"}), - // }, - // "policy_controller": { - // UpdatePolicy(policyNs1LabelPair1AllowAll()), - // }, - // "namespace_controller": {}, - // }, - // // would fill metadata out for actual test case - // TestCaseMetadata: nil, - // }, +func getAllMultiJobTests() []*MultiJobTestCase { + return []*MultiJobTestCase{ + { + Description: "create namespaces, pods, and a policy which applies to a pod", + Jobs: map[string][]*Action{ + "namespace_controller": { + CreateNamespace("x", map[string]string{"k1": "v1"}), + CreateNamespace("y", map[string]string{"k2": "v2"}), + ApplyDP(), + }, + "pod_controller": { + CreatePod("x", "a", ip1, thisNode, map[string]string{"k1": "v1"}), + CreatePod("y", "a", ip2, otherNode, map[string]string{"k1": "v1"}), + ApplyDP(), + }, + "policy_controller": { + UpdatePolicy(policyXBaseOnK1V1()), + }, + }, + TestCaseMetadata: &TestCaseMetadata{ + Tags: []Tag{ + nsCrudTag, + podCrudTag, + netpolCrudTag, + }, + DpCfg: defaultWindowsDPCfg, + InitialEndpoints: []*hcn.HostComputeEndpoint{ + dptestutils.Endpoint(endpoint1, ip1), + dptestutils.Endpoint(endpoint2, ip2), + }, + ExpectedSetPolicies: []*hcn.SetPolicySetting{ + dptestutils.SetPolicy(emptySet), + dptestutils.SetPolicy(allNamespaces, emptySet.GetHashedName(), nsXSet.GetHashedName(), nsYSet.GetHashedName()), + dptestutils.SetPolicy(nsXSet, ip1), + dptestutils.SetPolicy(nsYSet, ip2), + dptestutils.SetPolicy(nsK1Set, emptySet.GetHashedName(), nsXSet.GetHashedName()), + dptestutils.SetPolicy(nsK1V1Set, emptySet.GetHashedName(), nsXSet.GetHashedName()), + dptestutils.SetPolicy(nsK2Set, emptySet.GetHashedName(), nsYSet.GetHashedName()), + dptestutils.SetPolicy(nsK2V2Set, emptySet.GetHashedName(), nsYSet.GetHashedName()), + dptestutils.SetPolicy(podK1Set, ip1, ip2), + dptestutils.SetPolicy(podK1V1Set, ip1, ip2), + }, + ExpectedEnpdointACLs: map[string][]*hnswrapper.FakeEndpointPolicy{ + endpoint1: { + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "In", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + { + ID: "azure-acl-x-base", + Protocols: "", + Action: "Allow", + Direction: "Out", + LocalAddresses: "", + RemoteAddresses: "", + LocalPorts: "", + RemotePorts: "", + Priority: 222, + }, + }, + endpoint2: {}, + }, + }, + }, } } diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index f771f51e9b..5d1c514017 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -56,8 +56,8 @@ func TestAllSerialCases(t *testing.T) { } } -func TestAllMultiRoutineCases(t *testing.T) { - tests := getAllMultiRoutineTests() +func TestAllMultiJobCases(t *testing.T) { + tests := getAllMultiJobTests() for i, tt := range tests { i := i tt := tt @@ -78,13 +78,13 @@ func TestAllMultiRoutineCases(t *testing.T) { var errMulti error wg := new(sync.WaitGroup) - wg.Add(len(tt.Routines)) - for rName, r := range tt.Routines { - rName := rName - r := r + wg.Add(len(tt.Jobs)) + for jobName, job := range tt.Jobs { + jobName := jobName + job := job go func() { defer wg.Done() - for k, a := range r { + for k, a := range job { var err error if a.HNSAction != nil { err = a.HNSAction.Do(hns) @@ -93,7 +93,7 @@ func TestAllMultiRoutineCases(t *testing.T) { } if err != nil { - errMulti = multierr.Append(errMulti, errors.Wrapf(err, "failed to run action %d in routine %s", k, rName)) + errMulti = multierr.Append(errMulti, errors.Wrapf(err, "failed to run action %d in job %s", k, jobName)) break } } @@ -101,7 +101,9 @@ func TestAllMultiRoutineCases(t *testing.T) { } wg.Wait() - assert.Nil(t, errMulti, "encountered errors in multi-routine test") + assert.Nil(t, errMulti, "encountered errors in multi-job test") + + dptestutils.VerifyHNSCache(t, hns, tt.ExpectedSetPolicies, tt.ExpectedEnpdointACLs) }) } } diff --git a/npm/pkg/dataplane/types_windows_test.go b/npm/pkg/dataplane/types_windows_test.go index 21398501f8..7902341186 100644 --- a/npm/pkg/dataplane/types_windows_test.go +++ b/npm/pkg/dataplane/types_windows_test.go @@ -20,9 +20,9 @@ type SerialTestCase struct { *TestCaseMetadata } -type MultiRoutineTestCase struct { +type MultiJobTestCase struct { Description string - Routines map[string][]*Action + Jobs map[string][]*Action *TestCaseMetadata } From 67d951801b37f5f8a32afe5cf3486c526b48a884 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 14:40:28 -0700 Subject: [PATCH 31/34] MultiErrManager instead of dependency for multierr --- npm/pkg/dataplane/dataplane_windows_test.go | 23 ++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 5d1c514017..2748150bc3 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -12,8 +12,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "go.uber.org/multierr" ) const ( @@ -21,6 +19,24 @@ const ( threadedHNSLatency = time.Duration(1 * time.Second) ) +type MultiErrManager struct { + sync.Mutex +} + +func (m *MultiErrManager) Append(left, right error) error { + m.Lock() + defer m.Unlock() + if left == nil { + return right + } + + if right == nil { + return left + } + + return errors.Wrap(left, right.Error()) +} + func TestAllSerialCases(t *testing.T) { tests := getAllSerialTests() for i, tt := range tests { @@ -77,6 +93,7 @@ func TestAllMultiJobCases(t *testing.T) { require.NoError(t, err, "failed to initialize dp") var errMulti error + m := new(MultiErrManager) wg := new(sync.WaitGroup) wg.Add(len(tt.Jobs)) for jobName, job := range tt.Jobs { @@ -93,7 +110,7 @@ func TestAllMultiJobCases(t *testing.T) { } if err != nil { - errMulti = multierr.Append(errMulti, errors.Wrapf(err, "failed to run action %d in job %s", k, jobName)) + errMulti = m.Append(errMulti, errors.Wrapf(err, "failed to run action %d in job %s", k, jobName)) break } } From 036a6c84e3d385d6008284d58aa0b70728cbe9d6 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 14:57:47 -0700 Subject: [PATCH 32/34] return to the channel approach for multierr, now using FailNow instead of asserting on channel length --- npm/pkg/dataplane/dataplane_windows_test.go | 34 +++++++-------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 2748150bc3..726ad6e58e 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -1,6 +1,7 @@ package dataplane import ( + "fmt" "sync" "testing" "time" @@ -10,7 +11,6 @@ import ( dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" "github.com/pkg/errors" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -19,24 +19,6 @@ const ( threadedHNSLatency = time.Duration(1 * time.Second) ) -type MultiErrManager struct { - sync.Mutex -} - -func (m *MultiErrManager) Append(left, right error) error { - m.Lock() - defer m.Unlock() - if left == nil { - return right - } - - if right == nil { - return left - } - - return errors.Wrap(left, right.Error()) -} - func TestAllSerialCases(t *testing.T) { tests := getAllSerialTests() for i, tt := range tests { @@ -92,8 +74,7 @@ func TestAllMultiJobCases(t *testing.T) { dp, err := NewDataPlane(thisNode, io, tt.DpCfg, nil) require.NoError(t, err, "failed to initialize dp") - var errMulti error - m := new(MultiErrManager) + backgroundErrors := make(chan error, len(tt.Jobs)) wg := new(sync.WaitGroup) wg.Add(len(tt.Jobs)) for jobName, job := range tt.Jobs { @@ -110,7 +91,7 @@ func TestAllMultiJobCases(t *testing.T) { } if err != nil { - errMulti = m.Append(errMulti, errors.Wrapf(err, "failed to run action %d in job %s", k, jobName)) + backgroundErrors <- errors.Wrapf(err, "failed to run action %d in job %s", k, jobName) break } } @@ -118,7 +99,14 @@ func TestAllMultiJobCases(t *testing.T) { } wg.Wait() - assert.Nil(t, errMulti, "encountered errors in multi-job test") + close(backgroundErrors) + if len(backgroundErrors) > 0 { + errStrings := make([]string, 0) + for err := range backgroundErrors { + errStrings = append(errStrings, fmt.Sprintf("[%s]", err.Error())) + } + require.FailNow(t, "encountered errors in multi-job test: %+v", errStrings) + } dptestutils.VerifyHNSCache(t, hns, tt.ExpectedSetPolicies, tt.ExpectedEnpdointACLs) }) From 8cf6b59da28a61d89c8c5c5ad0e01c184189f17a Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 15:24:22 -0700 Subject: [PATCH 33/34] fix some lints --- network/hnswrapper/hnsv2wrapperfake.go | 8 ++++---- npm/pkg/dataplane/testutils/utils_windows.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/network/hnswrapper/hnsv2wrapperfake.go b/network/hnswrapper/hnsv2wrapperfake.go index 8355308511..50f392c884 100644 --- a/network/hnswrapper/hnsv2wrapperfake.go +++ b/network/hnswrapper/hnsv2wrapperfake.go @@ -336,12 +336,12 @@ func (fCache FakeHNSCache) SetPolicy(setID string) *hcn.SetPolicySetting { } func (fCache FakeHNSCache) PrettyString() string { - var networkStrings []string + networkStrings := make([]string, 0, len(fCache.networks)) for _, network := range fCache.networks { networkStrings = append(networkStrings, fmt.Sprintf("[%+v]", network.PrettyString())) } - var endpointStrings []string + endpointStrings := make([]string, 0, len(fCache.endpoints)) for _, endpoint := range fCache.endpoints { endpointStrings = append(endpointStrings, fmt.Sprintf("[%+v]", endpoint.PrettyString())) } @@ -421,7 +421,7 @@ func NewFakeHostComputeNetwork(network *hcn.HostComputeNetwork) *FakeHostCompute } func (fNetwork *FakeHostComputeNetwork) PrettyString() string { - var setPolicyStrings []string + setPolicyStrings := make([]string, 0, len(fNetwork.Policies)) for _, setPolicy := range fNetwork.Policies { setPolicyStrings = append(setPolicyStrings, fmt.Sprintf("[%+v]", setPolicy)) } @@ -472,7 +472,7 @@ func NewFakeHostComputeEndpoint(endpoint *hcn.HostComputeEndpoint) *FakeHostComp } func (fEndpoint *FakeHostComputeEndpoint) PrettyString() string { - var aclStrings []string + aclStrings := make([]string, 0, len(fEndpoint.Policies)) for _, acl := range fEndpoint.Policies { aclStrings = append(aclStrings, fmt.Sprintf("[%+v]", acl)) } diff --git a/npm/pkg/dataplane/testutils/utils_windows.go b/npm/pkg/dataplane/testutils/utils_windows.go index d78a870729..1f19eb532a 100644 --- a/npm/pkg/dataplane/testutils/utils_windows.go +++ b/npm/pkg/dataplane/testutils/utils_windows.go @@ -38,12 +38,14 @@ func Endpoint(epID, ip string) *hcn.HostComputeEndpoint { } func SetPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { - pType := hcn.SetPolicyType("") + var pType hcn.SetPolicyType switch setMetadata.GetSetKind() { case ipsets.ListSet: pType = hcn.SetPolicyTypeNestedIpSet case ipsets.HashSet: pType = hcn.SetPolicyTypeIpSet + default: + pType = hcn.SetPolicyType("") } // sort for easier comparison From 39894d2ca647157154c7b2408854398487c13036 Mon Sep 17 00:00:00 2001 From: Hunter Gregory Date: Mon, 24 Oct 2022 15:43:08 -0700 Subject: [PATCH 34/34] fix more lints --- npm/pkg/dataplane/dataplane-test-cases_windows_test.go | 4 +--- npm/pkg/dataplane/dataplane_windows_test.go | 1 - npm/pkg/dataplane/testutils/utils_windows.go | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go index cfb4da98fc..0812682fba 100644 --- a/npm/pkg/dataplane/dataplane-test-cases_windows_test.go +++ b/npm/pkg/dataplane/dataplane-test-cases_windows_test.go @@ -4,10 +4,8 @@ import ( "github.com/Azure/azure-container-networking/network/hnswrapper" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies" - "github.com/Microsoft/hcsshim/hcn" - dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" - + "github.com/Microsoft/hcsshim/hcn" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) diff --git a/npm/pkg/dataplane/dataplane_windows_test.go b/npm/pkg/dataplane/dataplane_windows_test.go index 726ad6e58e..1eec813e44 100644 --- a/npm/pkg/dataplane/dataplane_windows_test.go +++ b/npm/pkg/dataplane/dataplane_windows_test.go @@ -10,7 +10,6 @@ import ( "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" dptestutils "github.com/Azure/azure-container-networking/npm/pkg/dataplane/testutils" "github.com/pkg/errors" - "github.com/stretchr/testify/require" ) diff --git a/npm/pkg/dataplane/testutils/utils_windows.go b/npm/pkg/dataplane/testutils/utils_windows.go index 1f19eb532a..a2da51a62c 100644 --- a/npm/pkg/dataplane/testutils/utils_windows.go +++ b/npm/pkg/dataplane/testutils/utils_windows.go @@ -38,13 +38,13 @@ func Endpoint(epID, ip string) *hcn.HostComputeEndpoint { } func SetPolicy(setMetadata *ipsets.IPSetMetadata, members ...string) *hcn.SetPolicySetting { - var pType hcn.SetPolicyType + pType := hcn.SetPolicyType("") switch setMetadata.GetSetKind() { case ipsets.ListSet: pType = hcn.SetPolicyTypeNestedIpSet case ipsets.HashSet: pType = hcn.SetPolicyTypeIpSet - default: + case ipsets.UnknownKind: pType = hcn.SetPolicyType("") }