From 97dc673d161ddf0a2355291ea378506208c823be Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Fri, 1 Nov 2019 15:48:44 -0700 Subject: [PATCH 1/4] modified wireserver call to non-blocking. Fixed logging issue in ipam --- cni/ipam/ipam.go | 7 +------ cni/ipam/plugin/main.go | 16 +++++++++++++++- ipam/azure.go | 12 +++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/cni/ipam/ipam.go b/cni/ipam/ipam.go index 9aa458080a..2c17ef92b1 100644 --- a/cni/ipam/ipam.go +++ b/cni/ipam/ipam.go @@ -19,11 +19,6 @@ import ( cniTypesCurr "github.com/containernetworking/cni/pkg/types/current" ) -const ( - // Plugin name. - name = "azure-vnet-ipam" -) - var ( ipv4DefaultRouteDstPrefix = net.IPNet{net.IPv4zero, net.IPv4Mask(0, 0, 0, 0)} ) @@ -35,7 +30,7 @@ type ipamPlugin struct { } // NewPlugin creates a new ipamPlugin object. -func NewPlugin(config *common.PluginConfig) (*ipamPlugin, error) { +func NewPlugin(name string, config *common.PluginConfig) (*ipamPlugin, error) { // Setup base plugin. plugin, err := cni.NewPlugin(name, config.Version) if err != nil { diff --git a/cni/ipam/plugin/main.go b/cni/ipam/plugin/main.go index 6ab1e49f3c..7278cb956e 100644 --- a/cni/ipam/plugin/main.go +++ b/cni/ipam/plugin/main.go @@ -10,6 +10,11 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/ipam" "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/log" +) + +const ( + name = "azure-vnet-ipam" ) // Version is populated by make during build. @@ -20,7 +25,16 @@ func main() { var config common.PluginConfig config.Version = version - ipamPlugin, err := ipam.NewPlugin(&config) + log.SetName(name) + log.SetLevel(log.LevelInfo) + if err := log.SetTarget(log.TargetLogfile); err != nil { + fmt.Printf("Failed to setup cni logging: %v\n", err) + return + } + + defer log.Close() + + ipamPlugin, err := ipam.NewPlugin(name, &config) if err != nil { fmt.Printf("Failed to create IPAM plugin, err:%v.\n", err) os.Exit(1) diff --git a/ipam/azure.go b/ipam/azure.go index 286c883f55..059318c4b8 100644 --- a/ipam/azure.go +++ b/ipam/azure.go @@ -5,6 +5,7 @@ package ipam import ( "encoding/xml" + "fmt" "net" "net/http" "strings" @@ -84,14 +85,23 @@ func (s *azureSource) refresh() error { return err } + httpClient := &http.Client{Timeout: time.Second * 5} + + log.Printf("[ipam] Wireserver call %v to retrieve IP List", s.queryUrl) // Fetch configuration. - resp, err := http.Get(s.queryUrl) + resp, err := httpClient.Get(s.queryUrl) if err != nil { + log.Printf("[ipam] wireserver call failed with: %v", err) return err } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + log.Printf("[ipam] http return error code for wireserver call %d", resp.StatusCode) + return fmt.Errorf("http error %d", resp.StatusCode) + } + // Decode XML document. var doc common.XmlDocument decoder := xml.NewDecoder(resp.Body) From a95bb2808c51c9d5f20f5d42c3537c1441b61e4b Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Fri, 1 Nov 2019 16:27:36 -0700 Subject: [PATCH 2/4] fixed ut --- cni/ipam/ipam_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/ipam/ipam_test.go b/cni/ipam/ipam_test.go index ad67e9fef5..0a6738b21f 100644 --- a/cni/ipam/ipam_test.go +++ b/cni/ipam/ipam_test.go @@ -51,7 +51,7 @@ func TestMain(m *testing.M) { } // Create the plugin. - plugin, err = NewPlugin(&config) + plugin, err = NewPlugin("ipamtest", &config) if err != nil { fmt.Printf("Failed to create IPAM plugin, err:%v.\n", err) return From 98e9f414d25ac4a774588fa21976e2b0858e9a8f Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Thu, 7 Nov 2019 11:58:10 -0800 Subject: [PATCH 3/4] addressing review comments --- ipam/azure.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipam/azure.go b/ipam/azure.go index 059318c4b8..368c3096c9 100644 --- a/ipam/azure.go +++ b/ipam/azure.go @@ -98,8 +98,8 @@ func (s *azureSource) refresh() error { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - log.Printf("[ipam] http return error code for wireserver call %d", resp.StatusCode) - return fmt.Errorf("http error %d", resp.StatusCode) + log.Errorf("[ipam] http return error code for wireserver call %+v", resp) + return fmt.Errorf("wireserver http error %+v", resp) } // Decode XML document. From a206b61cc7ef3142ffa66ffb0f295ed0f672dce9 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 19 Nov 2019 16:25:51 -0800 Subject: [PATCH 4/4] used inithttpclient function --- ipam/azure.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ipam/azure.go b/ipam/azure.go index 368c3096c9..808eec7e15 100644 --- a/ipam/azure.go +++ b/ipam/azure.go @@ -18,9 +18,12 @@ import ( const ( // Host URL to query. azureQueryUrl = "http://168.63.129.16/machine/plugins?comp=nmagent&type=getinterfaceinfov1" - // Minimum time interval between consecutive queries. azureQueryInterval = 10 * time.Second + // http connection timeout + httpConnectionTimeout = 10 + // http response header timeout + responseHeaderTimeout = 10 ) // Microsoft Azure IPAM configuration source. @@ -85,7 +88,11 @@ func (s *azureSource) refresh() error { return err } - httpClient := &http.Client{Timeout: time.Second * 5} + httpClient := common.InitHttpClient(httpConnectionTimeout, responseHeaderTimeout) + if httpClient == nil { + log.Errorf("[ipam] Failed intializing http client") + return fmt.Errorf("Error intializing http client") + } log.Printf("[ipam] Wireserver call %v to retrieve IP List", s.queryUrl) // Fetch configuration.