From d28294ff911fc0bc765d6504dff7fd75f3e380bb Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Thu, 20 Jun 2019 15:26:58 -0700 Subject: [PATCH 1/7] read interface configuration from file on azure stack --- ipam/mas.go | 195 +++++++++++++++-------- ipam/mas_test.go | 211 +++++++++++++++++++++++++ ipam/testfiles/masInterfaceConfig.json | 48 ++++++ 3 files changed, 387 insertions(+), 67 deletions(-) create mode 100644 ipam/mas_test.go create mode 100644 ipam/testfiles/masInterfaceConfig.json diff --git a/ipam/mas.go b/ipam/mas.go index ae12ec5aee..aa1e6d93ce 100644 --- a/ipam/mas.go +++ b/ipam/mas.go @@ -5,129 +5,190 @@ package ipam import ( "encoding/json" + "io/ioutil" "net" - "net/http" - "time" + "runtime" + "strings" - "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" ) const ( - // Host URL to query. - masQueryUrl = "http://169.254.169.254:6642/ListNetwork" - - // Minimum time interval between consecutive queries. - masQueryInterval = 10 * time.Second + defaultLinuxFilePath = "/etc/kubernetes/interfaces.json" + defaultWindowsFilePath = `c:\k\interfaces.json` ) // Microsoft Azure Stack IPAM configuration source. type masSource struct { - name string - sink addressConfigSink - queryUrl string - queryInterval time.Duration - lastRefresh time.Time + name string + sink addressConfigSink + fileLoaded bool + filePath string } // MAS host agent JSON object format. -type jsonObject struct { - Isolation string - IPs []struct { - IP string - IsolationId string - Mask string - DefaultGateways []string - DnsServers []string - } +type Interface struct { + MacAddress string + Name string + IsPrimary bool + IPSubnets []IPSubent +} + +type IPSubent struct { + Prefix string + IPAddresses []IPAddress +} + +type IPAddress struct { + Address string + IsPrimary bool } // Creates the MAS source. func newMasSource(options map[string]interface{}) (*masSource, error) { - queryUrl, _ := options[common.OptIpamQueryUrl].(string) - if queryUrl == "" { - queryUrl = masQueryUrl - } - - i, _ := options[common.OptIpamQueryInterval].(int) - queryInterval := time.Duration(i) * time.Second - if queryInterval == 0 { - queryInterval = masQueryInterval + var filePath string + if runtime.GOOS == "windows" { + filePath = defaultWindowsFilePath + } else { + filePath = defaultLinuxFilePath } return &masSource{ - name: "MAS", - queryUrl: queryUrl, - queryInterval: queryInterval, + name: "MAS", + filePath: filePath, }, nil } // Starts the MAS source. -func (s *masSource) start(sink addressConfigSink) error { - s.sink = sink +func (source *masSource) start(sink addressConfigSink) error { + source.sink = sink return nil } // Stops the MAS source. -func (s *masSource) stop() { - s.sink = nil +func (source *masSource) stop() { + source.sink = nil return } // Refreshes configuration. -func (s *masSource) refresh() error { +func (source *masSource) refresh() error { - // Refresh only if enough time has passed since the last query. - if time.Since(s.lastRefresh) < s.queryInterval { + if source.fileLoaded { return nil } - s.lastRefresh = time.Now() - // Configure the local default address space. - local, err := s.sink.newAddressSpace(LocalDefaultAddressSpaceId, LocalScope) + // Query the list of local interfaces. + localInterfaces, err := net.Interfaces() + if err != nil { + return err + } + + // Query the list of Azure Network Interfaces + sdnInterfaces, err := getSDNInterfaces(source.filePath) if err != nil { return err } - // Fetch configuration. - resp, err := http.Get(s.queryUrl) + // Configure the local default address space. + local, err := source.sink.newAddressSpace(LocalDefaultAddressSpaceId, LocalScope) if err != nil { return err } - defer resp.Body.Close() + err = populateAddressSpace(local, sdnInterfaces, localInterfaces) + if err != nil { + return err + } - // Decode JSON object. - var obj jsonObject - decoder := json.NewDecoder(resp.Body) - err = decoder.Decode(&obj) + // Set the local address space as active. + err = source.sink.setAddressSpace(local) if err != nil { return err } - // Add the IP addresses to the local address space. - for _, v := range obj.IPs { - address := net.ParseIP(v.IP) - subnet := net.IPNet{ - IP: net.ParseIP(v.IP), - Mask: net.IPMask(net.ParseIP(v.Mask)), + source.fileLoaded = true + + return nil +} + +func getSDNInterfaces(fileLocation string) ([]Interface, error) { + data, err := ioutil.ReadFile(fileLocation) + if err != nil { + return nil, err + } + + var interfaces []Interface + err = json.Unmarshal(data, &interfaces) + if err != nil { + return nil, err + } + + return interfaces, nil +} + +func populateAddressSpace(localAddressSpace *addressSpace, sdnInterfaces []Interface, localInterfaces []net.Interface) error { + + //Find the interface with matching MacAddress or Name + for _, sdnIf := range sdnInterfaces { + ifName := "" + + for _, localIf := range localInterfaces { + if (sdnIf.MacAddress == "" && sdnIf.Name == localIf.Name) || macAddressesEqual(sdnIf.MacAddress, localIf.HardwareAddr.String()) { + ifName = localIf.Name + break + } } - ap, err := local.newAddressPool("eth0", 0, &subnet) - if err != nil { - log.Printf("[ipam] Failed to create pool:%v err:%v.", subnet, err) + // Skip if interface is not found. + if ifName == "" { + log.Printf("[ipam] Failed to find interface with MAC address:%v or Name:%v.", sdnIf.MacAddress, sdnIf.Name) continue } - _, err = ap.newAddressRecord(&address) - if err != nil { - log.Printf("[ipam] Failed to create address:%v err:%v.", address, err) - continue + // Prioritize secondary interfaces. + priority := 0 + if !sdnIf.IsPrimary { + priority = 1 } - } - // Set the local address space as active. - s.sink.setAddressSpace(local) + for _, subnet := range sdnIf.IPSubnets { + _, network, err := net.ParseCIDR(subnet.Prefix) + if err != nil { + log.Printf("[ipam] Failed to parse subnet:%v err:%v.", subnet.Prefix, err) + continue + } + + addressPool, err := localAddressSpace.newAddressPool(ifName, priority, network) + if err != nil { + log.Printf("[ipam] Failed to create pool:%v ifName:%v err:%v.", subnet, ifName, err) + continue + } + + // Add the IP addresses to the localAddressSpace address space. + for _, ipAddr := range subnet.IPAddresses { + // Primary addresses are reserved for the host. + if ipAddr.IsPrimary { + continue + } + + address := net.ParseIP(ipAddr.Address) + + _, err = addressPool.newAddressRecord(&address) + if err != nil { + log.Printf("[ipam] Failed to create address:%v err:%v.", address, err) + continue + } + } + } + } return nil } + +func macAddressesEqual(macAddress1 string, macAddress2 string) bool { + macAddress1 = strings.ToLower(strings.Replace(macAddress1, ":", "", -1)) + macAddress2 = strings.ToLower(strings.Replace(macAddress2, ":", "", -1)) + + return macAddress1 == macAddress2 +} diff --git a/ipam/mas_test.go b/ipam/mas_test.go new file mode 100644 index 0000000000..992162bf17 --- /dev/null +++ b/ipam/mas_test.go @@ -0,0 +1,211 @@ +package ipam + +import ( + "net" + "reflect" + "runtime" + "testing" +) + +func TestNewMasSource(t *testing.T) { + options := make(map[string]interface{}) + mas, _ := newMasSource(options) + + if runtime.GOOS == "windows" { + if mas.filePath != defaultWindowsFilePath { + t.Errorf("default file path set incorrectly") + } + } else { + if mas.filePath != defaultLinuxFilePath { + t.Errorf("default file path set incorrectly") + } + } + if mas.name != "MAS" { + t.Errorf("mas source Name incorrect") + } +} + +func TestGetSDNInterfaces(t *testing.T) { + const validFileName = "testfiles/masInterfaceConfig.json" + const invalidFileName = "mas_test.go" + const nonexistentFileName = "bad" + + interfaces, err := getSDNInterfaces(validFileName) + if err != nil { + t.Fatalf("failed to get sdn Interfaces from file: %v", err) + } + + correctInterfaces := []Interface{ + { + Name: "eth0", + IsPrimary: false, + IPSubnets: []IPSubent{ + { + Prefix: "10.240.0.0/12", + IPAddresses: []IPAddress{ + {Address: "10.240.0.4", IsPrimary: true}, + {Address: "10.240.0.5", IsPrimary: false}, + }, + }, + }, + }, + { + MacAddress: "000D3A6E1825", + IsPrimary: true, + IPSubnets: []IPSubent{ + { + Prefix: "1.0.0.0/12", + IPAddresses: []IPAddress{ + {Address: "1.0.0.4", IsPrimary: true}, + {Address: "1.0.0.5", IsPrimary: false}, + {Address: "1.0.0.6", IsPrimary: false}, + {Address: "1.0.0.7", IsPrimary: false}, + }, + }, + }, + }, + } + + if !reflect.DeepEqual(interfaces, correctInterfaces) { + t.Fatalf("Interface list did not match expected list. expected: %v, actual: %v", interfaces, correctInterfaces) + } + + interfaces, err = getSDNInterfaces(invalidFileName) + if interfaces != nil || err == nil { + t.Fatal("didn't throw error on invalid file") + } + + interfaces, err = getSDNInterfaces(nonexistentFileName) + if interfaces != nil || err == nil { + t.Fatal("didn't throw error on nonexistent file") + } +} + +func TestPopulateAddressSpace(t *testing.T) { + + hardwareAddress, _ := net.ParseMAC("00:0d:3a:6e:18:25") + localInterfaces := []net.Interface{{HardwareAddr: hardwareAddress, Name: "eth0"}} + + // Interfaces should match based on Mac Address + + local := newLocalAddressSpace() + + sdnInterfaces := []Interface{ + { + MacAddress: "000D3A6E1825", + IsPrimary: true, + IPSubnets: []IPSubent{ + { + Prefix: "1.0.0.0/12", + IPAddresses: []IPAddress{ + {Address: "1.1.1.5", IsPrimary: true}, + {Address: "1.1.1.6", IsPrimary: false}, + {Address: "1.1.1.6", IsPrimary: false}, + {Address: "invalid", IsPrimary: false}, + }, + }, + { + Prefix: "1.0.0.0/12", + }, + { + Prefix: "invalid", + }, + }, + }, + } + + err := populateAddressSpace(local, sdnInterfaces, localInterfaces) + if err != nil { + t.Fatalf("Error populating address space: %v", err) + } + + if len(local.Pools) != 1 { + t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 1, len(local.Pools)) + } + + pool, ok := local.Pools["1.0.0.0/12"] + if !ok { + t.Fatal("Address pool 1.0.0.0/12 missing") + } + + if pool.IfName != localInterfaces[0].Name { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", localInterfaces[0].Name, pool.IfName) + } + + if pool.Priority != 0 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 0, pool.Priority) + } + + if len(pool.Addresses) != 1 { + t.Fatalf("Address list has incorrect length. expected: %d, actual: %d", 1, len(pool.Addresses)) + } + + _, ok = pool.Addresses["1.1.1.6"] + if !ok { + t.Fatal("Address 1.1.1.6 missing") + } + + // Interfaces should match based on Name when no Mac Address Present + + local = newLocalAddressSpace() + + sdnInterfaces = []Interface{ + { + Name: localInterfaces[0].Name, + IsPrimary: false, + IPSubnets: []IPSubent{{Prefix: "2.0.0.0/12"}}, + }, + } + + err = populateAddressSpace(local, sdnInterfaces, localInterfaces) + if err != nil { + t.Fatalf("Error populating address space: %v", err) + } + + if len(local.Pools) != 1 { + t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 1, len(local.Pools)) + } + + pool, ok = local.Pools["2.0.0.0/12"] + if !ok { + t.Fatal("Address pool 2.0.0.0/12 missing") + } + + if pool.IfName != localInterfaces[0].Name { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", localInterfaces[0].Name, pool.IfName) + } + + if pool.Priority != 1 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 1, pool.Priority) + } + + // Interfaces should not match based on Name when Mac Address is Present + + local = newLocalAddressSpace() + + sdnInterfaces = []Interface{ + { + MacAddress: "invalid", + Name: localInterfaces[0].Name, + IsPrimary: false, + IPSubnets: []IPSubent{{Prefix: "2.0.0.0/12"}}, + }, + } + + err = populateAddressSpace(local, sdnInterfaces, localInterfaces) + if err != nil { + t.Fatalf("Error populating address space: %v", err) + } + + if len(local.Pools) != 0 { + t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 0, len(local.Pools)) + } +} + +func newLocalAddressSpace() *addressSpace { + return &addressSpace{ + Id: LocalDefaultAddressSpaceId, + Scope: LocalScope, + Pools: make(map[string]*addressPool), + } +} diff --git a/ipam/testfiles/masInterfaceConfig.json b/ipam/testfiles/masInterfaceConfig.json new file mode 100644 index 0000000000..deafe803cf --- /dev/null +++ b/ipam/testfiles/masInterfaceConfig.json @@ -0,0 +1,48 @@ +[ + { + "Name": "eth0", + "IsPrimary": false, + "IPSubnets": [ + { + "Prefix": "10.240.0.0/12", + "IPAddresses": [ + { + "Address": "10.240.0.4", + "IsPrimary": true + }, + { + "Address": "10.240.0.5", + "IsPrimary": false + } + ] + } + ] + }, + { + "MacAddress": "000D3A6E1825", + "IsPrimary": true, + "IPSubnets": [ + { + "Prefix": "1.0.0.0/12", + "IPAddresses": [ + { + "Address": "1.0.0.4", + "IsPrimary": true + }, + { + "Address": "1.0.0.5", + "IsPrimary": false + }, + { + "Address": "1.0.0.6", + "IsPrimary": false + }, + { + "Address": "1.0.0.7", + "IsPrimary": false + } + ] + } + ] + } +] From 94d744cb680d4d20cea7154bda1ac0d6d9151ae7 Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Mon, 24 Jun 2019 15:51:31 -0700 Subject: [PATCH 2/7] remove name field and wrap interface array --- ipam/mas.go | 19 ++-- ipam/mas_test.go | 151 +++++++------------------ ipam/testfiles/masInterfaceConfig.json | 79 +++++-------- 3 files changed, 82 insertions(+), 167 deletions(-) diff --git a/ipam/mas.go b/ipam/mas.go index aa1e6d93ce..505a94cc28 100644 --- a/ipam/mas.go +++ b/ipam/mas.go @@ -27,9 +27,12 @@ type masSource struct { } // MAS host agent JSON object format. +type NetworkInterfaces struct { + Interfaces []Interface +} + type Interface struct { MacAddress string - Name string IsPrimary bool IPSubnets []IPSubent } @@ -112,14 +115,14 @@ func (source *masSource) refresh() error { return nil } -func getSDNInterfaces(fileLocation string) ([]Interface, error) { +func getSDNInterfaces(fileLocation string) (*NetworkInterfaces, error) { data, err := ioutil.ReadFile(fileLocation) if err != nil { return nil, err } - var interfaces []Interface - err = json.Unmarshal(data, &interfaces) + interfaces := &NetworkInterfaces{} + err = json.Unmarshal(data, interfaces) if err != nil { return nil, err } @@ -127,14 +130,14 @@ func getSDNInterfaces(fileLocation string) ([]Interface, error) { return interfaces, nil } -func populateAddressSpace(localAddressSpace *addressSpace, sdnInterfaces []Interface, localInterfaces []net.Interface) error { +func populateAddressSpace(localAddressSpace *addressSpace, sdnInterfaces *NetworkInterfaces, localInterfaces []net.Interface) error { //Find the interface with matching MacAddress or Name - for _, sdnIf := range sdnInterfaces { + for _, sdnIf := range sdnInterfaces.Interfaces { ifName := "" for _, localIf := range localInterfaces { - if (sdnIf.MacAddress == "" && sdnIf.Name == localIf.Name) || macAddressesEqual(sdnIf.MacAddress, localIf.HardwareAddr.String()) { + if macAddressesEqual(sdnIf.MacAddress, localIf.HardwareAddr.String()) { ifName = localIf.Name break } @@ -142,7 +145,7 @@ func populateAddressSpace(localAddressSpace *addressSpace, sdnInterfaces []Inter // Skip if interface is not found. if ifName == "" { - log.Printf("[ipam] Failed to find interface with MAC address:%v or Name:%v.", sdnIf.MacAddress, sdnIf.Name) + log.Printf("[ipam] Failed to find interface with MAC address:%v", sdnIf.MacAddress) continue } diff --git a/ipam/mas_test.go b/ipam/mas_test.go index 992162bf17..d17262acdd 100644 --- a/ipam/mas_test.go +++ b/ipam/mas_test.go @@ -35,31 +35,20 @@ func TestGetSDNInterfaces(t *testing.T) { t.Fatalf("failed to get sdn Interfaces from file: %v", err) } - correctInterfaces := []Interface{ - { - Name: "eth0", - IsPrimary: false, - IPSubnets: []IPSubent{ - { - Prefix: "10.240.0.0/12", - IPAddresses: []IPAddress{ - {Address: "10.240.0.4", IsPrimary: true}, - {Address: "10.240.0.5", IsPrimary: false}, - }, - }, - }, - }, - { - MacAddress: "000D3A6E1825", - IsPrimary: true, - IPSubnets: []IPSubent{ - { - Prefix: "1.0.0.0/12", - IPAddresses: []IPAddress{ - {Address: "1.0.0.4", IsPrimary: true}, - {Address: "1.0.0.5", IsPrimary: false}, - {Address: "1.0.0.6", IsPrimary: false}, - {Address: "1.0.0.7", IsPrimary: false}, + correctInterfaces := &NetworkInterfaces{ + Interfaces: []Interface{ + { + MacAddress: "000D3A6E1825", + IsPrimary: true, + IPSubnets: []IPSubent{ + { + Prefix: "1.0.0.0/12", + IPAddresses: []IPAddress{ + {Address: "1.0.0.4", IsPrimary: true}, + {Address: "1.0.0.5", IsPrimary: false}, + {Address: "1.0.0.6", IsPrimary: false}, + {Address: "1.0.0.7", IsPrimary: false}, + }, }, }, }, @@ -86,29 +75,33 @@ func TestPopulateAddressSpace(t *testing.T) { hardwareAddress, _ := net.ParseMAC("00:0d:3a:6e:18:25") localInterfaces := []net.Interface{{HardwareAddr: hardwareAddress, Name: "eth0"}} - // Interfaces should match based on Mac Address - - local := newLocalAddressSpace() - - sdnInterfaces := []Interface{ - { - MacAddress: "000D3A6E1825", - IsPrimary: true, - IPSubnets: []IPSubent{ - { - Prefix: "1.0.0.0/12", - IPAddresses: []IPAddress{ - {Address: "1.1.1.5", IsPrimary: true}, - {Address: "1.1.1.6", IsPrimary: false}, - {Address: "1.1.1.6", IsPrimary: false}, - {Address: "invalid", IsPrimary: false}, + local := &addressSpace{ + Id: LocalDefaultAddressSpaceId, + Scope: LocalScope, + Pools: make(map[string]*addressPool), + } + + sdnInterfaces := &NetworkInterfaces{ + Interfaces: []Interface{ + { + MacAddress: "000D3A6E1825", + IsPrimary: true, + IPSubnets: []IPSubent{ + { + Prefix: "1.0.0.0/12", + IPAddresses: []IPAddress{ + {Address: "1.1.1.5", IsPrimary: true}, + {Address: "1.1.1.6", IsPrimary: false}, + {Address: "1.1.1.6", IsPrimary: false}, + {Address: "invalid", IsPrimary: false}, + }, + }, + { + Prefix: "1.0.0.0/12", + }, + { + Prefix: "invalid", }, - }, - { - Prefix: "1.0.0.0/12", - }, - { - Prefix: "invalid", }, }, }, @@ -144,68 +137,4 @@ func TestPopulateAddressSpace(t *testing.T) { if !ok { t.Fatal("Address 1.1.1.6 missing") } - - // Interfaces should match based on Name when no Mac Address Present - - local = newLocalAddressSpace() - - sdnInterfaces = []Interface{ - { - Name: localInterfaces[0].Name, - IsPrimary: false, - IPSubnets: []IPSubent{{Prefix: "2.0.0.0/12"}}, - }, - } - - err = populateAddressSpace(local, sdnInterfaces, localInterfaces) - if err != nil { - t.Fatalf("Error populating address space: %v", err) - } - - if len(local.Pools) != 1 { - t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 1, len(local.Pools)) - } - - pool, ok = local.Pools["2.0.0.0/12"] - if !ok { - t.Fatal("Address pool 2.0.0.0/12 missing") - } - - if pool.IfName != localInterfaces[0].Name { - t.Fatalf("Incorrect interface name. expected: %s, actual %s", localInterfaces[0].Name, pool.IfName) - } - - if pool.Priority != 1 { - t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 1, pool.Priority) - } - - // Interfaces should not match based on Name when Mac Address is Present - - local = newLocalAddressSpace() - - sdnInterfaces = []Interface{ - { - MacAddress: "invalid", - Name: localInterfaces[0].Name, - IsPrimary: false, - IPSubnets: []IPSubent{{Prefix: "2.0.0.0/12"}}, - }, - } - - err = populateAddressSpace(local, sdnInterfaces, localInterfaces) - if err != nil { - t.Fatalf("Error populating address space: %v", err) - } - - if len(local.Pools) != 0 { - t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 0, len(local.Pools)) - } -} - -func newLocalAddressSpace() *addressSpace { - return &addressSpace{ - Id: LocalDefaultAddressSpaceId, - Scope: LocalScope, - Pools: make(map[string]*addressPool), - } } diff --git a/ipam/testfiles/masInterfaceConfig.json b/ipam/testfiles/masInterfaceConfig.json index deafe803cf..6ea9446973 100644 --- a/ipam/testfiles/masInterfaceConfig.json +++ b/ipam/testfiles/masInterfaceConfig.json @@ -1,48 +1,31 @@ -[ - { - "Name": "eth0", - "IsPrimary": false, - "IPSubnets": [ - { - "Prefix": "10.240.0.0/12", - "IPAddresses": [ - { - "Address": "10.240.0.4", - "IsPrimary": true - }, - { - "Address": "10.240.0.5", - "IsPrimary": false - } - ] - } - ] - }, - { - "MacAddress": "000D3A6E1825", - "IsPrimary": true, - "IPSubnets": [ - { - "Prefix": "1.0.0.0/12", - "IPAddresses": [ - { - "Address": "1.0.0.4", - "IsPrimary": true - }, - { - "Address": "1.0.0.5", - "IsPrimary": false - }, - { - "Address": "1.0.0.6", - "IsPrimary": false - }, - { - "Address": "1.0.0.7", - "IsPrimary": false - } - ] - } - ] - } -] +{ + "Interfaces": [ + { + "MacAddress": "000D3A6E1825", + "IsPrimary": true, + "IPSubnets": [ + { + "Prefix": "1.0.0.0/12", + "IPAddresses": [ + { + "Address": "1.0.0.4", + "IsPrimary": true + }, + { + "Address": "1.0.0.5", + "IsPrimary": false + }, + { + "Address": "1.0.0.6", + "IsPrimary": false + }, + { + "Address": "1.0.0.7", + "IsPrimary": false + } + ] + } + ] + } + ] +} From a3178d47fdb122fffeb8e0685e406ef80fead330 Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Wed, 26 Jun 2019 16:31:33 -0700 Subject: [PATCH 3/7] switch from crlf to lf --- ipam/testfiles/masInterfaceConfig.json | 62 +++++++++++++------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/ipam/testfiles/masInterfaceConfig.json b/ipam/testfiles/masInterfaceConfig.json index 6ea9446973..04b697658e 100644 --- a/ipam/testfiles/masInterfaceConfig.json +++ b/ipam/testfiles/masInterfaceConfig.json @@ -1,31 +1,31 @@ -{ - "Interfaces": [ - { - "MacAddress": "000D3A6E1825", - "IsPrimary": true, - "IPSubnets": [ - { - "Prefix": "1.0.0.0/12", - "IPAddresses": [ - { - "Address": "1.0.0.4", - "IsPrimary": true - }, - { - "Address": "1.0.0.5", - "IsPrimary": false - }, - { - "Address": "1.0.0.6", - "IsPrimary": false - }, - { - "Address": "1.0.0.7", - "IsPrimary": false - } - ] - } - ] - } - ] -} +{ + "Interfaces": [ + { + "MacAddress": "000D3A6E1825", + "IsPrimary": true, + "IPSubnets": [ + { + "Prefix": "1.0.0.0/12", + "IPAddresses": [ + { + "Address": "1.0.0.4", + "IsPrimary": true + }, + { + "Address": "1.0.0.5", + "IsPrimary": false + }, + { + "Address": "1.0.0.6", + "IsPrimary": false + }, + { + "Address": "1.0.0.7", + "IsPrimary": false + } + ] + } + ] + } + ] +} From d640750b4e06d700ccf3337263e4740c6485a436 Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Tue, 2 Jul 2019 16:48:04 -0700 Subject: [PATCH 4/7] address comments --- ipam/mas.go | 26 ++++++++++++-------------- ipam/mas_test.go | 22 ++++++++++++++-------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/ipam/mas.go b/ipam/mas.go index 505a94cc28..f8accf83d5 100644 --- a/ipam/mas.go +++ b/ipam/mas.go @@ -14,8 +14,10 @@ import ( ) const ( - defaultLinuxFilePath = "/etc/kubernetes/interfaces.json" + defaultLinuxFilePath = "/etc/kubernetes/interfaces.json" defaultWindowsFilePath = `c:\k\interfaces.json` + windows = "windows" + name = "MAS" ) // Microsoft Azure Stack IPAM configuration source. @@ -34,10 +36,10 @@ type NetworkInterfaces struct { type Interface struct { MacAddress string IsPrimary bool - IPSubnets []IPSubent + IPSubnets []IPSubnet } -type IPSubent struct { +type IPSubnet struct { Prefix string IPAddresses []IPAddress } @@ -50,14 +52,14 @@ type IPAddress struct { // Creates the MAS source. func newMasSource(options map[string]interface{}) (*masSource, error) { var filePath string - if runtime.GOOS == "windows" { + if runtime.GOOS == windows { filePath = defaultWindowsFilePath } else { filePath = defaultLinuxFilePath } return &masSource{ - name: "MAS", + name: name, filePath: filePath, }, nil } @@ -71,12 +73,10 @@ func (source *masSource) start(sink addressConfigSink) error { // Stops the MAS source. func (source *masSource) stop() { source.sink = nil - return } // Refreshes configuration. func (source *masSource) refresh() error { - if source.fileLoaded { return nil } @@ -99,17 +99,17 @@ func (source *masSource) refresh() error { return err } - err = populateAddressSpace(local, sdnInterfaces, localInterfaces) - if err != nil { + if err = populateAddressSpace(local, sdnInterfaces, localInterfaces); err != nil { return err } // Set the local address space as active. - err = source.sink.setAddressSpace(local) - if err != nil { + if err = source.sink.setAddressSpace(local); err != nil { return err } + log.Printf("[ipam] Address space successfully populated from config file") + source.fileLoaded = true return nil @@ -122,8 +122,7 @@ func getSDNInterfaces(fileLocation string) (*NetworkInterfaces, error) { } interfaces := &NetworkInterfaces{} - err = json.Unmarshal(data, interfaces) - if err != nil { + if err = json.Unmarshal(data, interfaces); err != nil { return nil, err } @@ -131,7 +130,6 @@ func getSDNInterfaces(fileLocation string) (*NetworkInterfaces, error) { } func populateAddressSpace(localAddressSpace *addressSpace, sdnInterfaces *NetworkInterfaces, localInterfaces []net.Interface) error { - //Find the interface with matching MacAddress or Name for _, sdnIf := range sdnInterfaces.Interfaces { ifName := "" diff --git a/ipam/mas_test.go b/ipam/mas_test.go index d17262acdd..ab5f27e11f 100644 --- a/ipam/mas_test.go +++ b/ipam/mas_test.go @@ -11,17 +11,17 @@ func TestNewMasSource(t *testing.T) { options := make(map[string]interface{}) mas, _ := newMasSource(options) - if runtime.GOOS == "windows" { + if runtime.GOOS == windows { if mas.filePath != defaultWindowsFilePath { - t.Errorf("default file path set incorrectly") + t.Fatalf("default file path set incorrectly") } } else { if mas.filePath != defaultLinuxFilePath { - t.Errorf("default file path set incorrectly") + t.Fatalf("default file path set incorrectly") } } if mas.name != "MAS" { - t.Errorf("mas source Name incorrect") + t.Fatalf("mas source Name incorrect") } } @@ -40,7 +40,7 @@ func TestGetSDNInterfaces(t *testing.T) { { MacAddress: "000D3A6E1825", IsPrimary: true, - IPSubnets: []IPSubent{ + IPSubnets: []IPSubnet{ { Prefix: "1.0.0.0/12", IPAddresses: []IPAddress{ @@ -86,13 +86,14 @@ func TestPopulateAddressSpace(t *testing.T) { { MacAddress: "000D3A6E1825", IsPrimary: true, - IPSubnets: []IPSubent{ + IPSubnets: []IPSubnet{ { Prefix: "1.0.0.0/12", IPAddresses: []IPAddress{ {Address: "1.1.1.5", IsPrimary: true}, {Address: "1.1.1.6", IsPrimary: false}, {Address: "1.1.1.6", IsPrimary: false}, + {Address: "1.1.1.7", IsPrimary: false}, {Address: "invalid", IsPrimary: false}, }, }, @@ -129,12 +130,17 @@ func TestPopulateAddressSpace(t *testing.T) { t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 0, pool.Priority) } - if len(pool.Addresses) != 1 { - t.Fatalf("Address list has incorrect length. expected: %d, actual: %d", 1, len(pool.Addresses)) + if len(pool.Addresses) != 2 { + t.Fatalf("Address list has incorrect length. expected: %d, actual: %d", 2, len(pool.Addresses)) } _, ok = pool.Addresses["1.1.1.6"] if !ok { t.Fatal("Address 1.1.1.6 missing") } + + _, ok = pool.Addresses["1.1.1.7"] + if !ok { + t.Fatal("Address 1.1.1.7 missing") + } } From 3346e1c9317b113db21e227d08c671cac12a77be Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Wed, 3 Jul 2019 08:58:55 -0700 Subject: [PATCH 5/7] increase unit test coverage --- ipam/mas_test.go | 140 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 130 insertions(+), 10 deletions(-) diff --git a/ipam/mas_test.go b/ipam/mas_test.go index ab5f27e11f..15cfe98051 100644 --- a/ipam/mas_test.go +++ b/ipam/mas_test.go @@ -71,9 +71,15 @@ func TestGetSDNInterfaces(t *testing.T) { } func TestPopulateAddressSpace(t *testing.T) { + hardwareAddress0, _ := net.ParseMAC("00:00:00:00:00:00") + hardwareAddress1, _ := net.ParseMAC("11:11:11:11:11:11") + hardwareAddress2, _ := net.ParseMAC("00:0d:3a:6e:18:25") - hardwareAddress, _ := net.ParseMAC("00:0d:3a:6e:18:25") - localInterfaces := []net.Interface{{HardwareAddr: hardwareAddress, Name: "eth0"}} + localInterfaces := []net.Interface{ + {HardwareAddr: hardwareAddress0, Name: "eth0"}, + {HardwareAddr: hardwareAddress1, Name: "eth1"}, + {HardwareAddr: hardwareAddress2, Name: "eth2"}, + } local := &addressSpace{ Id: LocalDefaultAddressSpaceId, @@ -97,12 +103,6 @@ func TestPopulateAddressSpace(t *testing.T) { {Address: "invalid", IsPrimary: false}, }, }, - { - Prefix: "1.0.0.0/12", - }, - { - Prefix: "invalid", - }, }, }, }, @@ -122,8 +122,8 @@ func TestPopulateAddressSpace(t *testing.T) { t.Fatal("Address pool 1.0.0.0/12 missing") } - if pool.IfName != localInterfaces[0].Name { - t.Fatalf("Incorrect interface name. expected: %s, actual %s", localInterfaces[0].Name, pool.IfName) + if pool.IfName != "eth2" { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", "eth2", pool.IfName) } if pool.Priority != 0 { @@ -144,3 +144,123 @@ func TestPopulateAddressSpace(t *testing.T) { t.Fatal("Address 1.1.1.7 missing") } } + +func TestPopulateAddressSpaceMultipleSDNInterfaces(t *testing.T) { + hardwareAddress0, _ := net.ParseMAC("00:00:00:00:00:00") + hardwareAddress1, _ := net.ParseMAC("11:11:11:11:11:11") + localInterfaces := []net.Interface{ + {HardwareAddr: hardwareAddress0, Name: "eth0"}, + {HardwareAddr: hardwareAddress1, Name: "eth1"}, + } + + local := &addressSpace{ + Id: LocalDefaultAddressSpaceId, + Scope: LocalScope, + Pools: make(map[string]*addressPool), + } + + sdnInterfaces := &NetworkInterfaces{ + Interfaces: []Interface{ + { + MacAddress: "000000000000", + IsPrimary: true, + IPSubnets: []IPSubnet{ + { + Prefix: "0.0.0.0/24", + IPAddresses: []IPAddress{}, + }, + { + Prefix: "0.1.0.0/24", + IPAddresses: []IPAddress{}, + }, + { + Prefix: "0.0.0.0/24", + }, + { + Prefix: "invalid", + }, + }, + }, + { + MacAddress: "111111111111", + IsPrimary: false, + IPSubnets: []IPSubnet{ + { + Prefix: "1.0.0.0/24", + IPAddresses: []IPAddress{}, + }, + { + Prefix: "1.1.0.0/24", + IPAddresses: []IPAddress{}, + }, + }, + }, + { + MacAddress: "222222222222", + IsPrimary: false, + IPSubnets: []IPSubnet{}, + }, + }, + } + + err := populateAddressSpace(local, sdnInterfaces, localInterfaces) + if err != nil { + t.Fatalf("Error populating address space: %v", err) + } + + if len(local.Pools) != 4 { + t.Fatalf("Pool list has incorrect length. expected: %d, actual: %d", 4, len(local.Pools)) + } + + pool, ok := local.Pools["0.0.0.0/24"] + if !ok { + t.Fatal("Address pool 0.0.0.0/24 missing") + } + + if pool.IfName != "eth0" { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", "eth0", pool.IfName) + } + + if pool.Priority != 0 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 0, pool.Priority) + } + + pool, ok = local.Pools["0.1.0.0/24"] + if !ok { + t.Fatal("Address pool 0.1.0.0/24 missing") + } + + if pool.IfName != "eth0" { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", "eth0", pool.IfName) + } + + if pool.Priority != 0 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 0, pool.Priority) + } + + pool, ok = local.Pools["1.0.0.0/24"] + if !ok { + t.Fatal("Address pool 1.0.0.0/24 missing") + } + + if pool.IfName != "eth1" { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", "eth1", pool.IfName) + } + + if pool.Priority != 1 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 1, pool.Priority) + } + + pool, ok = local.Pools["1.1.0.0/24"] + if !ok { + t.Fatal("Address pool 1.1.0.0/24 missing") + } + + if pool.IfName != "eth1" { + t.Fatalf("Incorrect interface name. expected: %s, actual %s", "eth1", pool.IfName) + } + + if pool.Priority != 1 { + t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 1, pool.Priority) + } +} \ No newline at end of file From 48646b8a12ff604ece4859ed18570423c3b8de60 Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Wed, 3 Jul 2019 09:31:19 -0700 Subject: [PATCH 6/7] nil check source on refresh --- ipam/mas.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ipam/mas.go b/ipam/mas.go index f8accf83d5..418e2f0bb2 100644 --- a/ipam/mas.go +++ b/ipam/mas.go @@ -5,6 +5,7 @@ package ipam import ( "encoding/json" + "errors" "io/ioutil" "net" "runtime" @@ -77,6 +78,10 @@ func (source *masSource) stop() { // Refreshes configuration. func (source *masSource) refresh() error { + if source == nil { + return errors.New("masSource is nil") + } + if source.fileLoaded { return nil } From ac60ad66845b4a1cf593f26bd6e8b748d3e5d611 Mon Sep 17 00:00:00 2001 From: Chad Applegate Date: Wed, 3 Jul 2019 14:02:54 -0700 Subject: [PATCH 7/7] remove space --- ipam/mas.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ipam/mas.go b/ipam/mas.go index 418e2f0bb2..45b6ec4fda 100644 --- a/ipam/mas.go +++ b/ipam/mas.go @@ -114,7 +114,6 @@ func (source *masSource) refresh() error { } log.Printf("[ipam] Address space successfully populated from config file") - source.fileLoaded = true return nil