From 9ea2f70ec3242e3229bb0ce269ce33ce7b3e9e9a Mon Sep 17 00:00:00 2001 From: Levente Kale Date: Fri, 19 Jul 2019 22:28:23 +0200 Subject: [PATCH] Added new UTs to thoroughlly condition-cover the new scenarios. Found a bug: during CNI DEL the IP coming from the DanmEp is in CIDR format, and shall be decoded accordingly. Error found by introducing reserve checks to cnidel UTs too, and also checking how many times DanmNet Update was called. The latter check is introduced to ipam UTs as well, which made me realize we can optimize API write both during Reserve and Free. We only need to push back changes if anything was changed at all. Write need not be called for pur IPv6, and erroneous cases! --- cmd/cnitest/cnitest.go | 14 ++-- pkg/cnidel/cnidel.go | 3 + pkg/ipam/ipam.go | 10 +++ test/stubs/danm/client_stub.go | 6 +- test/stubs/danm/clientset_stub.go | 8 +- test/stubs/danm/netclient_stub.go | 22 ++--- test/utils/utils.go | 13 ++- test/uts/cnidel_test/cnidel_test.go | 101 ++++++++++++++++++++--- test/uts/ipam_test/ipam_test.go | 121 +++++++++++++++------------- 9 files changed, 208 insertions(+), 90 deletions(-) diff --git a/cmd/cnitest/cnitest.go b/cmd/cnitest/cnitest.go index 027f075a..7ac3335b 100644 --- a/cmd/cnitest/cnitest.go +++ b/cmd/cnitest/cnitest.go @@ -123,17 +123,17 @@ func validateMacvlanConfig(receivedCniConfig, expectedCniConfig []byte, tcConf T var recMacvlanConf cnidel.MacvlanNet err := json.Unmarshal(receivedCniConfig, &recMacvlanConf) if err != nil { - return errors.New("Received MACVLAN config could not be unmarshalled, because:" + err.Error()) + return errors.New("Received CNI config could not be unmarshalled, because:" + err.Error()) } - log.Printf("Received MACVLAN config:%v",recMacvlanConf) + log.Printf("Received CNI config:%v",recMacvlanConf) var expMacvlanConf MacvlanCniTestConfig err = json.Unmarshal(expectedCniConfig, &expMacvlanConf) if err != nil { - return errors.New("Expected MACVLAN config could not be unmarshalled, because:" + err.Error()) + return errors.New("Expected CNI config could not be unmarshalled, because:" + err.Error()) } if tcConf.CniExpectations.Ip6 != "" { if recMacvlanConf.Ipam.Ips == nil { - return errors.New("Received MACVLAN CNI config does not contain IPv6 address under ipam section, but it shall!") + return errors.New("Received CNI config does not contain IPv6 address under ipam section, but it shall!") } newIpamConfig := datastructs.IpamConfig{Type: "fakeipam"} for _,ip := range recMacvlanConf.Ipam.Ips { @@ -142,11 +142,11 @@ func validateMacvlanConfig(receivedCniConfig, expectedCniConfig []byte, tcConf T } } recMacvlanConf.Ipam = newIpamConfig - log.Printf("Received MACVLAN config after IPv6 adjustment:%v",recMacvlanConf) + log.Printf("Received config after IPv6 adjustment:%v",recMacvlanConf) } - log.Printf("Expected MACVLAN config:%v",expMacvlanConf.CniConf) + log.Printf("Expected config:%v",expMacvlanConf.CniConf) if !reflect.DeepEqual(recMacvlanConf, expMacvlanConf.CniConf) { - return errors.New("Received MACVLAN delegate configuration does not match with expected!") + return errors.New("Received delegate configuration does not match with expected!") } return nil } diff --git a/pkg/cnidel/cnidel.go b/pkg/cnidel/cnidel.go index 4d8849d0..87fcb829 100644 --- a/pkg/cnidel/cnidel.go +++ b/pkg/cnidel/cnidel.go @@ -217,6 +217,9 @@ func freeDelegatedIp(danmClient danmclientset.Interface, netInfo *danmtypes.Danm _, v4Subnet, _ := net.ParseCIDR(netInfo.Spec.Options.Cidr) _, v6Subnet, _ := net.ParseCIDR(netInfo.Spec.Options.Net6) parsedIp := net.ParseIP(ip) + if parsedIp == nil { + parsedIp,_,_ = net.ParseCIDR(ip) + } //We only need to Free an IP if it was allocated by DANM IPAM, and it was allocated by DANM only if it falls into any of the defined subnets if parsedIp != nil && ((v4Subnet != nil && v4Subnet.Contains(parsedIp)) || (v6Subnet != nil && v6Subnet.Contains(parsedIp))) { err := ipam.Free(danmClient, *netInfo, ip) diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index 23e55622..458da81e 100644 --- a/pkg/ipam/ipam.go +++ b/pkg/ipam/ipam.go @@ -21,12 +21,17 @@ import ( // The reserved IP address is represented by setting a bit in the network's BitArray type allocation matrix // The refreshed network object is modified in the K8s API server at the end func Reserve(danmClient danmclientset.Interface, netInfo danmtypes.DanmNet, req4, req6 string) (string, string, string, error) { + origAlloc := netInfo.Spec.Options.Alloc tempNetSpec := netInfo for { ip4, ip6, macAddr, err := allocateIP(&tempNetSpec, req4, req6) if err != nil { return "", "", "", errors.New("failed to allocate IP address for network:" + netInfo.ObjectMeta.Name + " with error:" + err.Error()) } + //Right now we only store IPv4 allocations in the API. If this bitmask is unchanged, there is nothing to update in the API server + if tempNetSpec.Spec.Options.Alloc == origAlloc { + return ip4, ip6, macAddr, nil + } retryNeeded, err, newNetSpec := updateIpAllocation(danmClient, tempNetSpec) if err != nil { return "", "", "", err @@ -47,9 +52,14 @@ func Free(danmClient danmclientset.Interface, netInfo danmtypes.DanmNet, ip stri // Nothing to return here: either network, or the interface is an L2 return nil } + origAlloc := netInfo.Spec.Options.Alloc tempNetSpec := netInfo for { resetIP(&tempNetSpec, ip) + //Right now we only store IPv4 allocations in the API. If this bitmask is unchanged, there is nothing to update in the API server + if tempNetSpec.Spec.Options.Alloc == origAlloc { + return nil + } retryNeeded, err, newNetSpec := updateIpAllocation(danmClient, tempNetSpec) if err != nil { return err diff --git a/test/stubs/danm/client_stub.go b/test/stubs/danm/client_stub.go index b5ec0eab..e6b10453 100644 --- a/test/stubs/danm/client_stub.go +++ b/test/stubs/danm/client_stub.go @@ -25,10 +25,14 @@ type Reservation struct { type ClientStub struct { Objects TestArtifacts + NetClient *NetClientStub } func (client *ClientStub) DanmNets(namespace string) client.DanmNetInterface { - return newNetClientStub(client.Objects.TestNets, client.Objects.ReservedIps) + if client.NetClient == nil { + client.NetClient = newNetClientStub(client.Objects.TestNets, client.Objects.ReservedIps) + } + return client.NetClient } func (client *ClientStub) DanmEps(namespace string) client.DanmEpInterface { diff --git a/test/stubs/danm/clientset_stub.go b/test/stubs/danm/clientset_stub.go index e2ef0445..8e60cc1b 100644 --- a/test/stubs/danm/clientset_stub.go +++ b/test/stubs/danm/clientset_stub.go @@ -6,15 +6,15 @@ import ( ) type ClientSetStub struct { - danmClient *ClientStub + DanmClient *ClientStub } func (c *ClientSetStub) DanmV1() danmv1.DanmV1Interface { - return c.danmClient + return c.DanmClient } func (c *ClientSetStub) Danm() danmv1.DanmV1Interface { - return c.danmClient + return c.DanmClient } func (c *ClientSetStub) Discovery() discovery.DiscoveryInterface { @@ -23,6 +23,6 @@ func (c *ClientSetStub) Discovery() discovery.DiscoveryInterface { func NewClientSetStub(objects TestArtifacts) *ClientSetStub { var clientSet ClientSetStub - clientSet.danmClient = newClientStub(objects) + clientSet.DanmClient = newClientStub(objects) return &clientSet } \ No newline at end of file diff --git a/test/stubs/danm/netclient_stub.go b/test/stubs/danm/netclient_stub.go index beb1463f..7dd7ec05 100644 --- a/test/stubs/danm/netclient_stub.go +++ b/test/stubs/danm/netclient_stub.go @@ -17,12 +17,13 @@ const ( ) type NetClientStub struct{ - testNets []danmtypes.DanmNet - reservedIpsList []ReservedIpsList + TestNets []danmtypes.DanmNet + ReservedIpsList []ReservedIpsList + TimesUpdateWasCalled int } func newNetClientStub(nets []danmtypes.DanmNet, ips []ReservedIpsList) *NetClientStub { - return &NetClientStub{testNets: nets, reservedIpsList: ips} + return &NetClientStub{TestNets: nets, ReservedIpsList: ips} } func (netClient *NetClientStub) Create(obj *danmtypes.DanmNet) (*danmtypes.DanmNet, error) { @@ -30,7 +31,8 @@ func (netClient *NetClientStub) Create(obj *danmtypes.DanmNet) (*danmtypes.DanmN } func (netClient *NetClientStub) Update(obj *danmtypes.DanmNet) (*danmtypes.DanmNet, error) { - for _, netReservation := range netClient.reservedIpsList { + netClient.TimesUpdateWasCalled++ + for _, netReservation := range netClient.ReservedIpsList { if obj.Spec.NetworkID == netReservation.NetworkId { ba := bitarray.NewBitArrayFromBase64(obj.Spec.Options.Alloc) _, ipnet, _ := net.ParseCIDR(obj.Spec.Options.Cidr) @@ -45,18 +47,18 @@ func (netClient *NetClientStub) Update(obj *danmtypes.DanmNet) (*danmtypes.DanmN continue } if !ba.Get(uint32(ipInInt)) && reservation.Set { - return nil, errors.New("Reservation failure, IP:" + reservation.Ip + " must be reserved in DanmNet:" + obj.Spec.NetworkID) + return nil, errors.New("Reservation failure, IP:" + reservation.Ip + " should have been reserved in DanmNet:" + obj.Spec.NetworkID) } if ba.Get(uint32(ipInInt)) && !reservation.Set { - return nil, errors.New("Reservation failure, IP:" + reservation.Ip + " must be free in DanmNet:" + obj.Spec.NetworkID) + return nil, errors.New("Reservation failure, IP:" + reservation.Ip + " should have been free in DanmNet:" + obj.Spec.NetworkID) } } } } if strings.Contains(obj.Spec.NetworkID, "conflict") && obj.ObjectMeta.ResourceVersion != magicVersion { - for index, net := range netClient.testNets { + for index, net := range netClient.TestNets { if net.Spec.NetworkID == obj.Spec.NetworkID { - netClient.testNets[index].ObjectMeta.ResourceVersion = magicVersion + netClient.TestNets[index].ObjectMeta.ResourceVersion = magicVersion } } return nil, errors.New(datastructs.OptimisticLockErrorMsg) @@ -79,7 +81,7 @@ func (netClient *NetClientStub) Get(netName string, options meta_v1.GetOptions) if strings.Contains(netName, "error") { return nil, errors.New("fatal error, don't retry") } - for _, testNet := range netClient.testNets { + for _, testNet := range netClient.TestNets { if testNet.Spec.NetworkID == netName { return &testNet, nil } @@ -101,5 +103,5 @@ func (netClient *NetClientStub) Patch(name string, pt types.PatchType, data []by } func (netClient *NetClientStub) AddReservedIpsList(reservedIps []ReservedIpsList) { - netClient.reservedIpsList = reservedIps + netClient.ReservedIpsList = reservedIps } diff --git a/test/utils/utils.go b/test/utils/utils.go index 9488e28f..30a36ce7 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -7,6 +7,7 @@ import ( "github.com/nokia/danm/pkg/bitarray" "github.com/nokia/danm/pkg/ipam" "github.com/nokia/danm/pkg/admit" + stubs "github.com/nokia/danm/test/stubs/danm" ) const ( @@ -37,7 +38,7 @@ func SetupAllocationPools(nets []danmtypes.DanmNet) error { if dnet.Spec.Options.Pool.End == "" { dnet.Spec.Options.Pool.End = (ipam.Int2ip(ipam.Ip2int(admit.GetBroadcastAddress(ipnet)) - 1)).String() } - if strings.HasPrefix(dnet.Spec.NetworkID, "full") { + if strings.HasPrefix(dnet.ObjectMeta.Name, "full") { exhaustNetwork(&dnet) } nets[index].Spec = dnet.Spec @@ -55,6 +56,16 @@ func GetTestNet(netId string, testNets []danmtypes.DanmNet) *danmtypes.DanmNet { return nil } +func CreateExpectedAllocationsList(ip string, isExpectedToBeSet bool, networkId string) []stubs.ReservedIpsList { + var ips []stubs.ReservedIpsList + if ip != "" { + reservation := stubs.Reservation {Ip: ip, Set: isExpectedToBeSet,} + expectedAllocation := stubs.ReservedIpsList{NetworkId: networkId, Reservations: []stubs.Reservation {reservation,},} + ips = append(ips, expectedAllocation) + } + return ips +} + func exhaustNetwork(netInfo *danmtypes.DanmNet) { ba := bitarray.NewBitArrayFromBase64(netInfo.Spec.Options.Alloc) _, ipnet, _ := net.ParseCIDR(netInfo.Spec.Options.Cidr) diff --git a/test/uts/cnidel_test/cnidel_test.go b/test/uts/cnidel_test/cnidel_test.go index d2d92292..98ad5bfc 100644 --- a/test/uts/cnidel_test/cnidel_test.go +++ b/test/uts/cnidel_test/cnidel_test.go @@ -94,6 +94,38 @@ var testNets = []danmtypes.DanmNet { ObjectMeta: meta_v1.ObjectMeta {Name: "full-macvlan"}, Spec: danmtypes.DanmNetSpec{NetworkType: "macvlan", NetworkID: "full", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26", Device: "ens1f0"}}, }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-ipam-ipv4"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l3", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-ipam-l2"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l2", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-invalid"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_invalid", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-noipam"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l3"}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-noipam-l2"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l2"}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-ipam-ipv6"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l3", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/64"}}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "bridge-ipam-ds"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l3", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26", Net6: "2a00:8a00:a000:1193::/64"}}, + }, + danmtypes.DanmNet { + ObjectMeta: meta_v1.ObjectMeta {Name: "full-bridge"}, + Spec: danmtypes.DanmNetSpec{NetworkType: "bridge", NetworkID: "bridge_l2", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}, + }, } var expectedCniConfigs = []CniConf { @@ -108,10 +140,20 @@ var expectedCniConfigs = []CniConf { {"sriov-l2", []byte(`{"cniexp":{"cnitype":"sriov","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"cniVersion":"0.3.1","name":"sriov-test","type":"sriov","master":"enp175s0f1","l2enable":true,"vlan":500,"deviceID":"0000:af:06.0"}}`)}, {"deleteflannel", []byte(`{"cniexp":{"cnitype":"flannel","env":{"CNI_COMMAND":"DEL","CNI_IFNAME":"eth0"}},"cniconf":{"name":"cbr0","type":"flannel","delegate":{"hairpinMode":true,"isDefaultGateway":true}}}`)}, {"deletemacvlan", []byte(`{"cniexp":{"cnitype":"macvlan","env":{"CNI_COMMAND":"DEL","CNI_IFNAME":"ens1f0"}},"cniconf":{"cniVersion":"0.3.1","master":"ens1f0","mode":"bridge","mtu":1500}}`)}, + {"bridge-l3-ip4", []byte(`{"cniexp":{"cnitype":"macvlan","ip":"192.168.1.65/26","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0","isDefaultGateway": true,"forceAddress": false,"ipMasq": true,"hairpinMode": true,"ipam": {"type": "fakeipam","ips":[{"ipcidr":"192.168.1.65/26","version":4}]}}}`)}, + {"bridge-l2-ip4", []byte(`{"cniexp":{"cnitype":"macvlan","ip":"192.168.1.65/26","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0","ipam": {"type": "fakeipam","ips":[{"ipcidr":"192.168.1.65/26","version":4}]}}}`)}, + {"bridge-l3-orig", []byte(`{"cniexp":{"cnitype":"macvlan","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0","isDefaultGateway": true,"forceAddress": false,"ipMasq": true,"hairpinMode": true,"ipam": {"type": "host-local","subnet": "10.10.0.0/16"}}}`)}, + {"bridge-l2-orig", []byte(`{"cniexp":{"cnitype":"macvlan","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0"}}`)}, + {"bridge-l3-ip6", []byte(`{"cniexp":{"cnitype":"macvlan","ip6":"2a00:8a00:a000:1193::/64","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0","isDefaultGateway": true,"forceAddress": false,"ipMasq": true,"hairpinMode": true,"ipam": {"type": "fakeipam"}}}`)}, + {"bridge-l3-ds", []byte(`{"cniexp":{"cnitype":"macvlan","ip":"192.168.1.65/26","ip6":"2a00:8a00:a000:1193::/64","env":{"CNI_COMMAND":"ADD","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0","isDefaultGateway": true,"forceAddress": false,"ipMasq": true,"hairpinMode": true,"ipam": {"type": "fakeipam","ips":[{"ipcidr":"192.168.1.65/26","version":4}]}}}`)}, + {"deletebridge", []byte(`{"cniexp":{"cnitype":"macvlan","env":{"CNI_COMMAND":"DEL","CNI_IFNAME":"eth0"}},"cniconf":{"name": "mynet","type": "bridge","bridge": "mynet0"}}`)}, } var testCniConfFiles = []CniConf { {"flannel_conf.conf", []byte(`{"name":"cbr0","type":"flannel","delegate":{"hairpinMode":true,"isDefaultGateway":true}}`)}, + {"bridge_l3.conf", []byte(`{"name": "mynet","type": "bridge","bridge": "mynet0","isDefaultGateway": true,"forceAddress": false,"ipMasq": true,"hairpinMode": true,"ipam": {"type": "host-local","subnet": "10.10.0.0/16"}}`)}, + {"bridge_l2.conf", []byte(`{"name": "mynet","type": "bridge","bridge": "mynet0"}`)}, + {"bridge_invalid.conf", []byte(`{"name": "mynet","type": "bridge","bridge": "myne`)}, } var testEps = []danmtypes.DanmEp { @@ -144,7 +186,27 @@ var testEps = []danmtypes.DanmEp { }, danmtypes.DanmEp{ ObjectMeta: meta_v1.ObjectMeta {Name: "withAddress"}, - Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"ens1f0", Address: "192.168.1.65",},}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"ens1f0", Address: "192.168.1.65/26",},}, + }, + danmtypes.DanmEp{ + ObjectMeta: meta_v1.ObjectMeta {Name: "simpleIpv4"}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"eth0", Address: "dynamic",},}, + }, + danmtypes.DanmEp{ + ObjectMeta: meta_v1.ObjectMeta {Name: "simpleIpv6"}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"eth0", AddressIPv6: "dynamic",},}, + }, + danmtypes.DanmEp{ + ObjectMeta: meta_v1.ObjectMeta {Name: "simpleDs"}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"eth0", Address: "dynamic", AddressIPv6: "dynamic",},}, + }, + danmtypes.DanmEp{ + ObjectMeta: meta_v1.ObjectMeta {Name: "withAddressSimple"}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"eth0", Address: "192.168.1.65/26",},}, + }, + danmtypes.DanmEp{ + ObjectMeta: meta_v1.ObjectMeta {Name: "withForeignAddressSimple"}, + Spec: danmtypes.DanmEpSpec {Iface: danmtypes.DanmEpIface{Name:"eth0", Address: "10.244.1.10/24",},}, }, } @@ -192,6 +254,14 @@ var delSetupTcs = []struct { {"dynamicSriovNoDeviceId", "sriov-test", "dynamicIpv4", "", "", "", true}, {"dynamicSriovL3", "sriov-test", "dynamicIpv4WithDeviceId", "sriov-l3", "", "", false}, {"dynamicSriovL2", "sriov-test", "noneWithDeviceId", "sriov-l2", "", "", false}, + {"bridgeWithV4Overwrite", "bridge-ipam-ipv4", "simpleIpv4", "bridge-l3-ip4", "", "", false}, + {"bridgeWithV4Add", "bridge-ipam-l2", "simpleIpv4", "bridge-l2-ip4", "", "", false}, + {"bridgeWithInvalidAdd", "bridge-invalid", "simpleIpv4", "", "", "", false}, + {"bridgeL3OriginalNoCidr", "bridge-noipam", "simpleIpv4", "bridge-l3-orig", "", "", false}, + {"bridgeL3OriginalNoIp", "bridge-ipam-ipv4", "noIps", "bridge-l3-orig", "", "", false}, + {"bridgeL2OriginalNoCidr", "bridge-noipam-l2", "simpleIpv4", "bridge-l2-orig", "", "", false}, + {"bridgeWithV6Overwrite", "bridge-ipam-ipv6", "simpleIpv6", "bridge-l3-ip6", "", "", false}, + {"bridgeWithDsOverwrite", "bridge-ipam-ds", "simpleDs", "bridge-l3-ds", "", "", false}, } var delDeleteTcs = []struct { @@ -200,9 +270,12 @@ var delDeleteTcs = []struct { epName string cniConfName string isErrorExpected bool + timesUpdateShouldBeCalled int }{ - {"flannel", "flannel-test", "deleteFlannel", "deleteflannel", false}, - {"macvlan", "full-macvlan", "withAddress", "deletemacvlan", false}, + {"flannel", "flannel-test", "deleteFlannel", "deleteflannel", false, 0}, + {"macvlan", "full-macvlan", "withAddress", "deletemacvlan", false, 1}, + {"bridgeWithDanmIpam", "full-bridge", "withAddressSimple", "deletebridge", false, 1}, + {"bridgeWithExternalIpam", "full-bridge", "withForeignAddressSimple", "deletebridge", false, 0}, } func TestIsDelegationRequired(t *testing.T) { @@ -268,12 +341,12 @@ func TestCalculateIfaceName(t *testing.T) { } func TestDelegateInterfaceSetup(t *testing.T) { - testArtifacts := stubs.TestArtifacts{TestNets: testNets} - netClientStub := stubs.NewClientSetStub(testArtifacts) err := setupDelTest("ADD") if err != nil { t.Errorf("Test suite could not be set-up because:%s", err.Error()) } + testArtifacts := stubs.TestArtifacts{TestNets: testNets} + netClientStub := stubs.NewClientSetStub(testArtifacts) for _, tc := range delSetupTcs { t.Run(tc.tcName, func(t *testing.T) { err = setupDelTestTc(tc.cniConfName) @@ -315,20 +388,21 @@ func TestDelegateInterfaceSetup(t *testing.T) { } func TestDelegateInterfaceDelete(t *testing.T) { - testArtifacts := stubs.TestArtifacts{TestNets: testNets} - netClientStub := stubs.NewClientSetStub(testArtifacts) err := setupDelTest("DEL") if err != nil { t.Errorf("Test suite could not be set-up because:%s", err.Error()) } for _, tc := range delDeleteTcs { t.Run(tc.tcName, func(t *testing.T) { + testEp := getTestEp(tc.epName) + testNet := utils.GetTestNet(tc.netName, testNets) + ips := utils.CreateExpectedAllocationsList(testEp.Spec.Iface.Address,false,testNet.Spec.NetworkID) + testArtifacts := stubs.TestArtifacts{TestNets: testNets, ReservedIps: ips} + netClientStub := stubs.NewClientSetStub(testArtifacts) err = setupDelTestTc(tc.cniConfName) if err != nil { t.Errorf("TC could not be set-up because:%s", err.Error()) } - testNet := utils.GetTestNet(tc.netName, testNets) - testEp := getTestEp(tc.epName) if testNet.Spec.NetworkType == "flannel" && testEp.Spec.Iface.Address != "" { var dataDir = filepath.Join(defaultDataDir, flannelBridge) err = os.MkdirAll(dataDir, os.ModePerm) @@ -355,6 +429,13 @@ func TestDelegateInterfaceDelete(t *testing.T) { t.Errorf("IP file:" + ipFile + " was not cleaned-up by Flannel IP exhaustion protection code!") } } + var timesUpdateWasCalled int + if netClientStub.DanmClient.NetClient != nil { + timesUpdateWasCalled = netClientStub.DanmClient.NetClient.TimesUpdateWasCalled + } + if tc.timesUpdateShouldBeCalled != timesUpdateWasCalled { + t.Errorf("Network manifest should have been updated:" + strconv.Itoa(tc.timesUpdateShouldBeCalled) + " times, but it happened:" + strconv.Itoa(timesUpdateWasCalled) + " times instead") + } }) } } @@ -381,7 +462,7 @@ func setupDelTest(opType string) error { if err != nil { return err } - testPlugins := [3]string{"flannel","macvlan","sriov"} + testPlugins := [4]string{"flannel","macvlan","sriov","bridge"} for _, plugin := range testPlugins { os.RemoveAll(filepath.Join(cniTesterDir, plugin)) input, err := ioutil.ReadFile(filepath.Join(os.Getenv("GOPATH"),"bin","cnitest")) diff --git a/test/uts/ipam_test/ipam_test.go b/test/uts/ipam_test/ipam_test.go index 6a3bf1bd..e66d54c6 100644 --- a/test/uts/ipam_test/ipam_test.go +++ b/test/uts/ipam_test/ipam_test.go @@ -2,6 +2,7 @@ package ipam_test import ( "os" + "strconv" "strings" "testing" danmtypes "github.com/nokia/danm/crd/apis/danm/v1" @@ -17,10 +18,11 @@ var testNets = []danmtypes.DanmNet { danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "fullIpv4"},Spec: danmtypes.DanmNetSpec{NetworkID: "fullIpv4", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.0/30"}}}, danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "net6"},Spec: danmtypes.DanmNetSpec{NetworkID: "net6", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/64", Cidr: "192.168.1.64/26",}}}, danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "smallNet6"},Spec: danmtypes.DanmNetSpec{NetworkID: "smallNet6", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/69"}}}, - danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflict"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflict", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/64"}}}, - danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflicterror"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflicterror", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/64"}}}, - danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflictFree"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflictFree", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, - danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflicterrorFree"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflicterrorFree", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, + danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflict"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflict", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, + danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "conflicterror"},Spec: danmtypes.DanmNetSpec{NetworkID: "conflicterror", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, + danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "fullconflictFree"},Spec: danmtypes.DanmNetSpec{NetworkID: "fullconflictFree", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, + danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "fullconflicterrorFree"},Spec: danmtypes.DanmNetSpec{NetworkID: "fullconflicterrorFree", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, + danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "fullerror"},Spec: danmtypes.DanmNetSpec{NetworkID: "error", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, danmtypes.DanmNet {ObjectMeta: meta_v1.ObjectMeta {Name: "error"},Spec: danmtypes.DanmNetSpec{NetworkID: "error", Options: danmtypes.DanmNetOption{Cidr: "192.168.1.64/26"}}}, } @@ -33,36 +35,37 @@ var reserveTcs = []struct { expectedIp6 string isErrorExpected bool isMacExpected bool + timesUpdateShouldBeCalled int }{ - {"noIpsRequested", 0, "", "", "", "", false, true}, - {"noneIPv4", 0, "none", "", "", "", false, true}, - {"noneIPv6", 0, "", "none", "", "", false, true}, - {"noneDualStack", 0, "none", "none", "", "", false, true}, - {"dynamicErrorIPv4", 0, "dynamic", "", "", "", true, false}, - {"dynamicErrorIPv6", 0, "", "dynamic", "", "", true, false}, - {"dynamicErrorDualStack", 0, "dynamic", "dynamic", "", "", true, false}, - {"dynamicIPv4Success", 1, "dynamic", "", "192.168.1.65/26", "", false, true}, - {"dynamicIPv4Exhausted", 2, "dynamic", "", "", "", true, false}, - {"staticInvalidIPv4", 2, "hululululu", "", "", "", true, false}, - {"staticInvalidNoCidrIPv4", 2, "192.168.1.1", "", "", "", true, false}, - {"staticL2IPv4", 0, "192.168.1.1/26", "", "", "", true, false}, - {"staticNetmaskMismatchIPv4", 2, "192.168.1.1/32", "", "", "", true, false}, - {"staticAlreadyUsedIPv4", 2, "192.168.1.2/30", "", "", "", true, false}, - {"staticSuccessLastIPv4", 1, "192.168.1.126/26", "", "192.168.1.126/26", "", false, true}, - {"staticSuccessFirstIPv4", 1, "192.168.1.65/26", "", "192.168.1.65/26", "", false, true}, - {"staticFailAfterLastIPv4", 1, "192.168.1.127/26", "", "", "", true, false}, - {"staticFailBeforeFirstIPv4", 1, "192.168.1.64/26", "", "", "", true, false}, - {"dynamicIPv6Success", 3, "", "dynamic", "", "2a00:8a00:a000:1193", false, true}, - {"dynamicNotSupportedCidrSizeIPv6", 4, "", "dynamic", "", "", true, false}, //basically anything smaller than /64. Restriction must be fixed some day! - {"staticL2IPv6", 2, "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "", "", true, false}, - {"staticInvalidIPv6", 3, "", "2a00:8a00:a000:1193:hulu:lulu:lulu:lulu/64", "", "", true, false}, - {"staticNetmaskMismatchIPv6", 3, "", "2a00:8a00:a000:2193:f816:3eff:fe24:e348/64", "", "", true, false}, - {"staticIPv6Success", 3, "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false, false}, - {"dynamicDualStackSuccess", 3, "dynamic", "dynamic", "192.168.1.65/26", "2a00:8a00:a000:1193", false, true}, - {"staticDualStackSuccess", 3, "192.168.1.115/26", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "192.168.1.115/26", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false, true}, - {"resolvedConflictDuringUpdate", 5, "", "dynamic", "", "2a00:8a00:a000:1193", false, true}, - {"unresolvedConflictDuringUpdate", 6, "", "dynamic", "", "", true, false}, - {"errorUpdate", 9, "", "dynamic", "", "", true, false}, + {"noIpsRequested", 0, "", "", "", "", false, true, 0}, + {"noneIPv4", 0, "none", "", "", "", false, true, 0}, + {"noneIPv6", 0, "", "none", "", "", false, true, 0}, + {"noneDualStack", 0, "none", "none", "", "", false, true, 0}, + {"dynamicErrorIPv4", 0, "dynamic", "", "", "", true, false, 0}, + {"dynamicErrorIPv6", 0, "", "dynamic", "", "", true, false, 0}, + {"dynamicErrorDualStack", 0, "dynamic", "dynamic", "", "", true, false, 0}, + {"dynamicIPv4Success", 1, "dynamic", "", "192.168.1.65/26", "", false, true, 1}, + {"dynamicIPv4Exhausted", 2, "dynamic", "", "", "", true, false, 0}, + {"staticInvalidIPv4", 2, "hululululu", "", "", "", true, false, 0}, + {"staticInvalidNoCidrIPv4", 2, "192.168.1.1", "", "", "", true, false, 0}, + {"staticL2IPv4", 0, "192.168.1.1/26", "", "", "", true, false, 0}, + {"staticNetmaskMismatchIPv4", 2, "192.168.1.1/32", "", "", "", true, false, 0}, + {"staticAlreadyUsedIPv4", 2, "192.168.1.2/30", "", "", "", true, false, 0}, + {"staticSuccessLastIPv4", 1, "192.168.1.126/26", "", "192.168.1.126/26", "", false, true, 1}, + {"staticSuccessFirstIPv4", 1, "192.168.1.65/26", "", "192.168.1.65/26", "", false, true, 1}, + {"staticFailAfterLastIPv4", 1, "192.168.1.127/26", "", "", "", true, false, 0}, + {"staticFailBeforeFirstIPv4", 1, "192.168.1.64/26", "", "", "", true, false, 0}, + {"dynamicIPv6Success", 3, "", "dynamic", "", "2a00:8a00:a000:1193", false, true, 0}, + {"dynamicNotSupportedCidrSizeIPv6", 4, "", "dynamic", "", "", true, false, 0}, //basically anything smaller than /64. Restriction must be fixed some day! + {"staticL2IPv6", 2, "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "", "", true, false, 0}, + {"staticInvalidIPv6", 3, "", "2a00:8a00:a000:1193:hulu:lulu:lulu:lulu/64", "", "", true, false, 0}, + {"staticNetmaskMismatchIPv6", 3, "", "2a00:8a00:a000:2193:f816:3eff:fe24:e348/64", "", "", true, false, 0}, + {"staticIPv6Success", 3, "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false, false, 0}, + {"dynamicDualStackSuccess", 3, "dynamic", "dynamic", "192.168.1.65/26", "2a00:8a00:a000:1193", false, true, 1}, + {"staticDualStackSuccess", 3, "192.168.1.115/26", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", "192.168.1.115/26", "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false, true, 1}, + {"resolvedConflictDuringUpdate", 5, "dynamic", "", "192.168.1.65/26", "", false, true, 2}, + {"unresolvedConflictAfterUpdate", 6, "dynamic", "", "", "", true, false, 1}, + {"errorUpdate", 10, "dynamic", "", "", "", true, false, 1}, } var freeTcs = []struct { @@ -70,17 +73,18 @@ var freeTcs = []struct { netIndex int allocatedIp string isErrorExpected bool + timesUpdateShouldBeCalled int }{ - {"l2Network", 0, "192.168.1.126/26", false}, - {"noAssignedIp", 1, "", false}, - {"successfulFree", 2, "192.168.1.2/30", false}, - {"noNetmask", 2, "192.168.1.2", false}, - {"outOfRange", 2, "192.168.1.10/30", false}, - {"invalidIp", 2, "192.168.hululu/30", false}, - {"ipv6", 2, "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false}, - {"resolvedConflictDuringUpdate", 7, "192.168.1.69/26", false}, - {"unresolvedConflictDuringUpdate", 8, "192.168.1.69/26", true}, - {"errorUpdate", 9, "192.168.1.69/26", true}, + {"l2Network", 0, "192.168.1.126/26", false, 0}, + {"noAssignedIp", 1, "", false, 0}, + {"successfulFree", 2, "192.168.1.2/30", false, 1}, + {"noNetmask", 2, "192.168.1.2", false, 0}, + {"outOfRange", 2, "192.168.1.10/30", false, 0}, + {"invalidIp", 2, "192.168.hululu/30", false, 0}, + {"ipv6", 2, "2a00:8a00:a000:1193:f816:3eff:fe24:e348/64", false, 0}, + {"resolvedConflictDuringUpdate", 7, "192.168.1.69/26", false, 2}, + {"unresolvedConflictAfterUpdate", 8, "192.168.1.69/26", true, 1}, + {"errorUpdate", 9, "192.168.1.69/26", true, 1}, } var gcTcs = []struct { @@ -101,7 +105,7 @@ func TestReserve(t *testing.T) { } for _, tc := range reserveTcs { t.Run(tc.netName, func(t *testing.T) { - ips := createExpectedAllocationsList(tc.expectedIp4,true,tc.netIndex) + ips := utils.CreateExpectedAllocationsList(tc.expectedIp4,true,testNets[tc.netIndex].Spec.NetworkID) testArtifacts := stubs.TestArtifacts{TestNets: testNets, ReservedIps: ips} netClientStub := stubs.NewClientSetStub(testArtifacts) ip4, ip6, mac, err := ipam.Reserve(netClientStub, testNets[tc.netIndex], tc.requestedIp4, tc.requestedIp6) @@ -120,6 +124,13 @@ func TestReserve(t *testing.T) { if !strings.HasPrefix(ip6,tc.expectedIp6) { t.Errorf("Allocated IP6 address:%s does not prefixed with the expected CIDR:%s", ip6, tc.expectedIp6) } + var timesUpdateWasCalled int + if netClientStub.DanmClient.NetClient != nil { + timesUpdateWasCalled = netClientStub.DanmClient.NetClient.TimesUpdateWasCalled + } + if tc.timesUpdateShouldBeCalled != timesUpdateWasCalled { + t.Errorf("Network manifest should have been updated:" + strconv.Itoa(tc.timesUpdateShouldBeCalled) + " times, but it happened:" + strconv.Itoa(timesUpdateWasCalled) + " times instead") + } }) } } @@ -131,7 +142,7 @@ func TestFree(t *testing.T) { } for _, tc := range freeTcs { t.Run(tc.netName, func(t *testing.T) { - ips := createExpectedAllocationsList(tc.allocatedIp,false,tc.netIndex) + ips := utils.CreateExpectedAllocationsList(tc.allocatedIp,false,testNets[tc.netIndex].Spec.NetworkID) testArtifacts := stubs.TestArtifacts{TestNets: testNets, ReservedIps: ips} netClientStub := stubs.NewClientSetStub(testArtifacts) err := ipam.Free(netClientStub, testNets[tc.netIndex], tc.allocatedIp) @@ -139,6 +150,13 @@ func TestFree(t *testing.T) { t.Errorf("Received error:%v does not match with expectation", err) return } + var timesUpdateWasCalled int + if netClientStub.DanmClient.NetClient != nil { + timesUpdateWasCalled = netClientStub.DanmClient.NetClient.TimesUpdateWasCalled + } + if tc.timesUpdateShouldBeCalled != timesUpdateWasCalled { + t.Errorf("Network manifest should have been updated:" + strconv.Itoa(tc.timesUpdateShouldBeCalled) + " times, but it happened:" + strconv.Itoa(timesUpdateWasCalled) + " times instead") + } }) } } @@ -150,7 +168,7 @@ func TestGarbageCollectIps(t *testing.T) { } for _, tc := range gcTcs { t.Run(tc.netName, func(t *testing.T) { - ips := createExpectedAllocationsList(tc.allocatedIp4,false,tc.netIndex) + ips := utils.CreateExpectedAllocationsList(tc.allocatedIp4,false,testNets[tc.netIndex].Spec.NetworkID) testArtifacts := stubs.TestArtifacts{TestNets: testNets, ReservedIps: ips} netClientStub := stubs.NewClientSetStub(testArtifacts) ipam.GarbageCollectIps(netClientStub, &testNets[tc.netIndex], tc.allocatedIp4, tc.allocatedIp6) @@ -158,17 +176,6 @@ func TestGarbageCollectIps(t *testing.T) { } } -func createExpectedAllocationsList(ip string, isExpectedToBeSet bool, index int) []stubs.ReservedIpsList { - var ips []stubs.ReservedIpsList - if ip != "" { - strippedIp := strings.Split(ip, "/") - reservation := stubs.Reservation {Ip: strippedIp[0], Set: isExpectedToBeSet,} - expectedAllocation := stubs.ReservedIpsList{NetworkId: testNets[index].Spec.NetworkID, Reservations: []stubs.Reservation {reservation,},} - ips = append(ips, expectedAllocation) - } - return ips -} - func TestMain(m *testing.M) { code := m.Run() os.Exit(code)