From bccc019752213efc9b68867e011383390c1690e7 Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Thu, 23 Jan 2020 18:56:11 +0530 Subject: [PATCH 1/6] adds environment flag fileIPAM --- cnm/plugin/main.go | 5 ++-- cns/service/main.go | 5 ++-- common/config.go | 9 ++++--- ipam/{mas.go => fileIpam.go} | 35 +++++++++++++++++--------- ipam/{mas_test.go => fileIpam_test.go} | 28 ++++++++++++++++++--- ipam/manager.go | 5 +++- 6 files changed, 63 insertions(+), 24 deletions(-) rename ipam/{mas.go => fileIpam.go} (84%) rename ipam/{mas_test.go => fileIpam_test.go} (89%) diff --git a/cnm/plugin/main.go b/cnm/plugin/main.go index 995b957545..9b3923b7a2 100644 --- a/cnm/plugin/main.go +++ b/cnm/plugin/main.go @@ -34,8 +34,9 @@ var args = common.ArgumentList{ Type: "string", DefaultValue: common.OptEnvironmentAzure, ValueMap: map[string]interface{}{ - common.OptEnvironmentAzure: 0, - common.OptEnvironmentMAS: 0, + common.OptEnvironmentAzure: 0, + common.OptEnvironmentMAS: 0, + common.OptEnvironmentFileIPAM: 0, }, }, { diff --git a/cns/service/main.go b/cns/service/main.go index 3609aba856..786e48f11c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -45,8 +45,9 @@ var args = acn.ArgumentList{ Type: "string", DefaultValue: acn.OptEnvironmentAzure, ValueMap: map[string]interface{}{ - acn.OptEnvironmentAzure: 0, - acn.OptEnvironmentMAS: 0, + acn.OptEnvironmentAzure: 0, + acn.OptEnvironmentMAS: 0, + acn.OptEnvironmentFileIPAM: 0, }, }, diff --git a/common/config.go b/common/config.go index 7a272dc73e..4a87d10ca7 100644 --- a/common/config.go +++ b/common/config.go @@ -6,10 +6,11 @@ package common // Command line options. const ( // Operating environment. - OptEnvironment = "environment" - OptEnvironmentAlias = "e" - OptEnvironmentAzure = "azure" - OptEnvironmentMAS = "mas" + OptEnvironment = "environment" + OptEnvironmentAlias = "e" + OptEnvironmentAzure = "azure" + OptEnvironmentMAS = "mas" + OptEnvironmentFileIPAM = "fileIpam" // API server URL. OptAPIServerURL = "api-url" diff --git a/ipam/mas.go b/ipam/fileIpam.go similarity index 84% rename from ipam/mas.go rename to ipam/fileIpam.go index 45b6ec4fda..ba42375f6a 100644 --- a/ipam/mas.go +++ b/ipam/fileIpam.go @@ -11,6 +11,7 @@ import ( "runtime" "strings" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" ) @@ -18,11 +19,12 @@ const ( defaultLinuxFilePath = "/etc/kubernetes/interfaces.json" defaultWindowsFilePath = `c:\k\interfaces.json` windows = "windows" - name = "MAS" + masName = "MAS" + fileIpamName = "fileIpam" ) // Microsoft Azure Stack IPAM configuration source. -type masSource struct { +type fileIpamSource struct { name string sink addressConfigSink fileLoaded bool @@ -50,8 +52,8 @@ type IPAddress struct { IsPrimary bool } -// Creates the MAS source. -func newMasSource(options map[string]interface{}) (*masSource, error) { +// Creates the MAS/fileIPAM source. +func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) { var filePath string if runtime.GOOS == windows { filePath = defaultWindowsFilePath @@ -59,27 +61,36 @@ func newMasSource(options map[string]interface{}) (*masSource, error) { filePath = defaultLinuxFilePath } - return &masSource{ - name: name, - filePath: filePath, - }, nil + environment, _ := options[common.OptEnvironment].(string) + + if environment == common.OptEnvironmentMAS { + return &fileIpamSource{ + name: masName, + filePath: filePath, + }, nil + } else { + return &fileIpamSource{ + name: fileIpamName, + filePath: filePath, + }, nil + } } // Starts the MAS source. -func (source *masSource) start(sink addressConfigSink) error { +func (source *fileIpamSource) start(sink addressConfigSink) error { source.sink = sink return nil } // Stops the MAS source. -func (source *masSource) stop() { +func (source *fileIpamSource) stop() { source.sink = nil } // Refreshes configuration. -func (source *masSource) refresh() error { +func (source *fileIpamSource) refresh() error { if source == nil { - return errors.New("masSource is nil") + return errors.New("fileIpamSource is nil") } if source.fileLoaded { diff --git a/ipam/mas_test.go b/ipam/fileIpam_test.go similarity index 89% rename from ipam/mas_test.go rename to ipam/fileIpam_test.go index 15cfe98051..b9ac2b06e2 100644 --- a/ipam/mas_test.go +++ b/ipam/fileIpam_test.go @@ -5,11 +5,14 @@ import ( "reflect" "runtime" "testing" + + "github.com/Azure/azure-container-networking/common" ) func TestNewMasSource(t *testing.T) { options := make(map[string]interface{}) - mas, _ := newMasSource(options) + options[common.OptEnvironment] = common.OptEnvironmentMAS + mas, _ := newFileIpamSource(options) if runtime.GOOS == windows { if mas.filePath != defaultWindowsFilePath { @@ -25,9 +28,28 @@ func TestNewMasSource(t *testing.T) { } } +func TestNewFileIpamSource(t *testing.T) { + options := make(map[string]interface{}) + options[common.OptEnvironment] = common.OptEnvironmentFileIPAM + fileIpam, _ := newFileIpamSource(options) + + if runtime.GOOS == windows { + if fileIpam.filePath != defaultWindowsFilePath { + t.Fatalf("default file path set incorrectly") + } + } else { + if fileIpam.filePath != defaultLinuxFilePath { + t.Fatalf("default file path set incorrectly") + } + } + if fileIpam.name != "fileIpam" { + t.Fatalf("fileIpam source Name incorrect") + } +} + func TestGetSDNInterfaces(t *testing.T) { const validFileName = "testfiles/masInterfaceConfig.json" - const invalidFileName = "mas_test.go" + const invalidFileName = "fileIpam_test.go" const nonexistentFileName = "bad" interfaces, err := getSDNInterfaces(validFileName) @@ -263,4 +285,4 @@ func TestPopulateAddressSpaceMultipleSDNInterfaces(t *testing.T) { if pool.Priority != 1 { t.Fatalf("Incorrect interface priority. expected: %d, actual %d", 1, pool.Priority) } -} \ No newline at end of file +} diff --git a/ipam/manager.go b/ipam/manager.go index 4cbd34f790..fa414995e4 100644 --- a/ipam/manager.go +++ b/ipam/manager.go @@ -190,7 +190,10 @@ func (am *addressManager) StartSource(options map[string]interface{}) error { am.source, err = newAzureSource(options) case common.OptEnvironmentMAS: - am.source, err = newMasSource(options) + am.source, err = newFileIpamSource(options) + + case common.OptEnvironmentFileIPAM : + am.source, err = newFileIpamSource(options) case "null": am.source, err = newNullSource() From 741c4104f099bb3ab0e4cc4274fa7bf020d584ca Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Fri, 24 Jan 2020 11:13:12 +0530 Subject: [PATCH 2/6] "creating fileIpamSource once" --- ipam/fileIpam.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/ipam/fileIpam.go b/ipam/fileIpam.go index ba42375f6a..43980978e8 100644 --- a/ipam/fileIpam.go +++ b/ipam/fileIpam.go @@ -55,6 +55,7 @@ type IPAddress struct { // Creates the MAS/fileIPAM source. func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) { var filePath string + var name string if runtime.GOOS == windows { filePath = defaultWindowsFilePath } else { @@ -62,18 +63,16 @@ func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) } environment, _ := options[common.OptEnvironment].(string) - if environment == common.OptEnvironmentMAS { - return &fileIpamSource{ - name: masName, - filePath: filePath, - }, nil + name = masName } else { - return &fileIpamSource{ - name: fileIpamName, - filePath: filePath, - }, nil + name = fileIpamName } + + return &fileIpamSource{ + name: name, + filePath: filePath, + }, nil } // Starts the MAS source. From a807ea5d7bf90c67d83bb062ea70fd6321a81f25 Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Tue, 4 Feb 2020 11:43:23 +0530 Subject: [PATCH 3/6] Update fileIpam.go well spaced if-1 --- ipam/fileIpam.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ipam/fileIpam.go b/ipam/fileIpam.go index 43980978e8..5ca737a66d 100644 --- a/ipam/fileIpam.go +++ b/ipam/fileIpam.go @@ -56,6 +56,7 @@ type IPAddress struct { func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) { var filePath string var name string + if runtime.GOOS == windows { filePath = defaultWindowsFilePath } else { From f75a0fa655b90805147aff3404d3e8d2425fb9b8 Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Tue, 4 Feb 2020 11:46:34 +0530 Subject: [PATCH 4/6] Update fileIpam_test.go well spaced if-2 --- ipam/fileIpam_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ipam/fileIpam_test.go b/ipam/fileIpam_test.go index b9ac2b06e2..34f2e788e7 100644 --- a/ipam/fileIpam_test.go +++ b/ipam/fileIpam_test.go @@ -42,6 +42,7 @@ func TestNewFileIpamSource(t *testing.T) { t.Fatalf("default file path set incorrectly") } } + if fileIpam.name != "fileIpam" { t.Fatalf("fileIpam source Name incorrect") } From d5374c48f7c5c4ed81dae0d97c9cc97b924ab655 Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Thu, 6 Feb 2020 10:12:25 +0530 Subject: [PATCH 5/6] comment improved --- ipam/fileIpam.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipam/fileIpam.go b/ipam/fileIpam.go index 43980978e8..42d2722433 100644 --- a/ipam/fileIpam.go +++ b/ipam/fileIpam.go @@ -52,7 +52,7 @@ type IPAddress struct { IsPrimary bool } -// Creates the MAS/fileIPAM source. +// Creates the MAS/fileIpam source. func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) { var filePath string var name string @@ -70,7 +70,7 @@ func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) } return &fileIpamSource{ - name: name, + name: name, filePath: filePath, }, nil } From 17b4eed08efa72bee897b90e7715ce3b56e4be2b Mon Sep 17 00:00:00 2001 From: ashutoshishere04 <54359569+ashutoshishere04@users.noreply.github.com> Date: Thu, 6 Feb 2020 10:41:23 +0530 Subject: [PATCH 6/6] smarter code structure --- cnm/plugin/main.go | 2 +- cns/service/main.go | 2 +- common/config.go | 2 +- ipam/fileIpam.go | 12 ++---------- ipam/fileIpam_test.go | 19 ++++++++++--------- ipam/manager.go | 2 +- 6 files changed, 16 insertions(+), 23 deletions(-) diff --git a/cnm/plugin/main.go b/cnm/plugin/main.go index 9b3923b7a2..9919b8a555 100644 --- a/cnm/plugin/main.go +++ b/cnm/plugin/main.go @@ -36,7 +36,7 @@ var args = common.ArgumentList{ ValueMap: map[string]interface{}{ common.OptEnvironmentAzure: 0, common.OptEnvironmentMAS: 0, - common.OptEnvironmentFileIPAM: 0, + common.OptEnvironmentFileIpam: 0, }, }, { diff --git a/cns/service/main.go b/cns/service/main.go index 786e48f11c..2be2bde65c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -47,7 +47,7 @@ var args = acn.ArgumentList{ ValueMap: map[string]interface{}{ acn.OptEnvironmentAzure: 0, acn.OptEnvironmentMAS: 0, - acn.OptEnvironmentFileIPAM: 0, + acn.OptEnvironmentFileIpam: 0, }, }, diff --git a/common/config.go b/common/config.go index 4a87d10ca7..35b9f36ea2 100644 --- a/common/config.go +++ b/common/config.go @@ -10,7 +10,7 @@ const ( OptEnvironmentAlias = "e" OptEnvironmentAzure = "azure" OptEnvironmentMAS = "mas" - OptEnvironmentFileIPAM = "fileIpam" + OptEnvironmentFileIpam = "fileIpam" // API server URL. OptAPIServerURL = "api-url" diff --git a/ipam/fileIpam.go b/ipam/fileIpam.go index 163b320832..358e9c8133 100644 --- a/ipam/fileIpam.go +++ b/ipam/fileIpam.go @@ -19,8 +19,6 @@ const ( defaultLinuxFilePath = "/etc/kubernetes/interfaces.json" defaultWindowsFilePath = `c:\k\interfaces.json` windows = "windows" - masName = "MAS" - fileIpamName = "fileIpam" ) // Microsoft Azure Stack IPAM configuration source. @@ -56,20 +54,14 @@ type IPAddress struct { func newFileIpamSource(options map[string]interface{}) (*fileIpamSource, error) { var filePath string var name string - + if runtime.GOOS == windows { filePath = defaultWindowsFilePath } else { filePath = defaultLinuxFilePath } - environment, _ := options[common.OptEnvironment].(string) - if environment == common.OptEnvironmentMAS { - name = masName - } else { - name = fileIpamName - } - + name = options[common.OptEnvironment].(string) return &fileIpamSource{ name: name, filePath: filePath, diff --git a/ipam/fileIpam_test.go b/ipam/fileIpam_test.go index 34f2e788e7..16d9e72a88 100644 --- a/ipam/fileIpam_test.go +++ b/ipam/fileIpam_test.go @@ -23,7 +23,8 @@ func TestNewMasSource(t *testing.T) { t.Fatalf("default file path set incorrectly") } } - if mas.name != "MAS" { + + if mas.name != "mas" { t.Fatalf("mas source Name incorrect") } } @@ -42,7 +43,7 @@ func TestNewFileIpamSource(t *testing.T) { t.Fatalf("default file path set incorrectly") } } - + if fileIpam.name != "fileIpam" { t.Fatalf("fileIpam source Name incorrect") } @@ -189,11 +190,11 @@ func TestPopulateAddressSpaceMultipleSDNInterfaces(t *testing.T) { IsPrimary: true, IPSubnets: []IPSubnet{ { - Prefix: "0.0.0.0/24", + Prefix: "0.0.0.0/24", IPAddresses: []IPAddress{}, }, { - Prefix: "0.1.0.0/24", + Prefix: "0.1.0.0/24", IPAddresses: []IPAddress{}, }, { @@ -206,22 +207,22 @@ func TestPopulateAddressSpaceMultipleSDNInterfaces(t *testing.T) { }, { MacAddress: "111111111111", - IsPrimary: false, + IsPrimary: false, IPSubnets: []IPSubnet{ { - Prefix: "1.0.0.0/24", + Prefix: "1.0.0.0/24", IPAddresses: []IPAddress{}, }, { - Prefix: "1.1.0.0/24", + Prefix: "1.1.0.0/24", IPAddresses: []IPAddress{}, }, }, }, { MacAddress: "222222222222", - IsPrimary: false, - IPSubnets: []IPSubnet{}, + IsPrimary: false, + IPSubnets: []IPSubnet{}, }, }, } diff --git a/ipam/manager.go b/ipam/manager.go index fa414995e4..e9a912780e 100644 --- a/ipam/manager.go +++ b/ipam/manager.go @@ -192,7 +192,7 @@ func (am *addressManager) StartSource(options map[string]interface{}) error { case common.OptEnvironmentMAS: am.source, err = newFileIpamSource(options) - case common.OptEnvironmentFileIPAM : + case common.OptEnvironmentFileIpam: am.source, err = newFileIpamSource(options) case "null":