From 36db85d7296e34d1787e4059b6e386a69b4ce4a7 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 10 Aug 2021 10:22:19 -0700 Subject: [PATCH 1/5] remove lock for version command. --- cni/cni.go | 11 ++-- cni/network/plugin/main.go | 126 ++++++++++++++++++++----------------- 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/cni/cni.go b/cni/cni.go index bfcd1b1854..afb1d7fb4d 100644 --- a/cni/cni.go +++ b/cni/cni.go @@ -9,11 +9,12 @@ import ( const ( // CNI commands. - Cmd = "CNI_COMMAND" - CmdAdd = "ADD" - CmdGet = "GET" - CmdDel = "DEL" - CmdUpdate = "UPDATE" + Cmd = "CNI_COMMAND" + CmdAdd = "ADD" + CmdGet = "GET" + CmdDel = "DEL" + CmdUpdate = "UPDATE" + CmdVersion = "VERSION" // nonstandard CNI spec command, used to dump CNI state to stdout CmdGetEndpointsState = "GET_ENDPOINT_STATE" diff --git a/cni/network/plugin/main.go b/cni/network/plugin/main.go index 98b8ec6a9f..959952185a 100644 --- a/cni/network/plugin/main.go +++ b/cni/network/plugin/main.go @@ -11,6 +11,8 @@ import ( "reflect" "time" + "github.com/Azure/azure-container-networking/platform" + "github.com/Azure/azure-container-networking/nns" "github.com/Azure/azure-container-networking/cni" @@ -18,7 +20,6 @@ import ( "github.com/Azure/azure-container-networking/common" acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" - "github.com/Azure/azure-container-networking/platform" "github.com/Azure/azure-container-networking/telemetry" "github.com/containernetworking/cni/pkg/skel" ) @@ -127,7 +128,6 @@ func handleIfCniUpdate(update func(*skel.CmdArgs) error) (bool, error) { // Main is the entry point for CNI network plugin. func main() { - startTime := time.Now() // Initialize and parse command line arguments. @@ -143,6 +143,7 @@ func main() { config common.PluginConfig err error logDirectory string // This sets empty string i.e. current location + tb *telemetry.TelemetryBuffer ) log.SetName(name) @@ -168,84 +169,87 @@ func main() { cniReport := reportManager.Report.(*telemetry.CNIReport) - upTime, err := platform.GetLastRebootTime() - if err == nil { - cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05") - } - - cniReport.GetReport(pluginName, version, ipamQueryURL) - netPlugin, err := network.NewPlugin(name, &config, &nns.GrpcClient{}) if err != nil { log.Printf("Failed to create network plugin, err:%v.\n", err) return } - // CNI Acquires lock - if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil { - log.Errorf("Failed to initialize key-value store of network plugin, err:%v.\n", err) - tb := telemetry.NewTelemetryBuffer() - if tberr := tb.Connect(); tberr == nil { - reportPluginError(reportManager, tb, err) - tb.Close() + // Check CNI_COMMAND value + cniCmd := os.Getenv(cni.Cmd) + + if cniCmd != cni.CmdVersion { + log.Printf("CNI_COMMAND environment variable set to %s", cniCmd) + + cniReport.GetReport(pluginName, version, ipamQueryURL) + + upTime, err := platform.GetLastRebootTime() + if err == nil { + cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05") } - if isSafe, _ := netPlugin.Plugin.IsSafeToRemoveLock(name); isSafe { - log.Printf("[CNI] Removing lock file as process holding lock exited") - if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil { - log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit) + // CNI Acquires lock + if err = netPlugin.Plugin.InitializeKeyValueStore(&config); err != nil { + log.Errorf("Failed to initialize key-value store of network plugin, err:%v.\n", err) + tb := telemetry.NewTelemetryBuffer() + if tberr := tb.Connect(); tberr == nil { + reportPluginError(reportManager, tb, err) + tb.Close() } - } - return - } + if isSafe, _ := netPlugin.Plugin.IsSafeToRemoveLock(name); isSafe { + log.Printf("[CNI] Removing lock file as process holding lock exited") + if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(true); errUninit != nil { + log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit) + } + } - defer func() { - if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil { - log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit) + return } - if recover() != nil { - os.Exit(1) - } - }() + defer func() { + if errUninit := netPlugin.Plugin.UninitializeKeyValueStore(false); errUninit != nil { + log.Errorf("Failed to uninitialize key-value store of network plugin, err:%v.\n", errUninit) + } - // Start telemetry process if not already started. This should be done inside lock, otherwise multiple process - // end up creating/killing telemetry process results in undesired state. - tb := telemetry.NewTelemetryBuffer() - tb.ConnectToTelemetryService(telemetryNumRetries, telemetryWaitTimeInMilliseconds) - defer tb.Close() + if recover() != nil { + os.Exit(1) + } + }() - netPlugin.SetCNIReport(cniReport, tb) + // Start telemetry process if not already started. This should be done inside lock, otherwise multiple process + // end up creating/killing telemetry process results in undesired state. + tb = telemetry.NewTelemetryBuffer() + tb.ConnectToTelemetryService(telemetryNumRetries, telemetryWaitTimeInMilliseconds) + defer tb.Close() - t := time.Now() - cniReport.Timestamp = t.Format("2006-01-02 15:04:05") + netPlugin.SetCNIReport(cniReport, tb) - if err = netPlugin.Start(&config); err != nil { - log.Errorf("Failed to start network plugin, err:%v.\n", err) - reportPluginError(reportManager, tb, err) - panic("network plugin start fatal error") - } + t := time.Now() + cniReport.Timestamp = t.Format("2006-01-02 15:04:05") - // Check CNI_COMMAND value for logging purposes. - cniCmd := os.Getenv(cni.Cmd) - log.Printf("CNI_COMMAND environment variable set to %s", cniCmd) - - // used to dump state - if cniCmd == cni.CmdGetEndpointsState { - log.Printf("Retrieving state") - simpleState, err := netPlugin.GetAllEndpointState("azure") - if err != nil { - log.Errorf("Failed to get Azure CNI state, err:%v.\n", err) - return + if err = netPlugin.Start(&config); err != nil { + log.Errorf("Failed to start network plugin, err:%v.\n", err) + reportPluginError(reportManager, tb, err) + panic("network plugin start fatal error") } - err = simpleState.PrintResult() - if err != nil { - log.Errorf("Failed to print state result to stdout with err %v\n", err) - } + // used to dump state + if cniCmd == cni.CmdGetEndpointsState { + log.Printf("Retrieving state") + simpleState, err := netPlugin.GetAllEndpointState("azure") + if err != nil { + log.Errorf("Failed to get Azure CNI state, err:%v.\n", err) + return + } - return + err = simpleState.PrintResult() + if err != nil { + log.Errorf("Failed to print state result to stdout with err %v\n", err) + } + + return + } } handled, err := handleIfCniUpdate(netPlugin.Update) @@ -255,6 +259,10 @@ func main() { log.Errorf("Failed to execute network plugin, err:%v.\n", err) } + if cniCmd == cni.CmdVersion { + return + } + netPlugin.Stop() // release cni lock From 641fa4d0489a8a78a5f0da9d11f32b6c850dbab5 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 10 Aug 2021 12:12:03 -0700 Subject: [PATCH 2/5] linter error fix --- cni/network/plugin/main.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cni/network/plugin/main.go b/cni/network/plugin/main.go index 959952185a..a689fcdad4 100644 --- a/cni/network/plugin/main.go +++ b/cni/network/plugin/main.go @@ -11,15 +11,13 @@ import ( "reflect" "time" - "github.com/Azure/azure-container-networking/platform" - - "github.com/Azure/azure-container-networking/nns" - "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/network" - "github.com/Azure/azure-container-networking/common" acn "github.com/Azure/azure-container-networking/common" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/nns" + "github.com/Azure/azure-container-networking/platform" "github.com/Azure/azure-container-networking/telemetry" "github.com/containernetworking/cni/pkg/skel" ) From f9521b3674f4df4cad9752c35268f04e6bc38443 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 10 Aug 2021 12:34:57 -0700 Subject: [PATCH 3/5] sort imports --- cni/network/plugin/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cni/network/plugin/main.go b/cni/network/plugin/main.go index a689fcdad4..23eefc5436 100644 --- a/cni/network/plugin/main.go +++ b/cni/network/plugin/main.go @@ -13,8 +13,8 @@ import ( "github.com/Azure/azure-container-networking/cni" "github.com/Azure/azure-container-networking/cni/network" - acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/common" + acn "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/nns" "github.com/Azure/azure-container-networking/platform" From 54e842e52e62d0aea3b3be5f0a3f3ce36052df90 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 10 Aug 2021 12:49:19 -0700 Subject: [PATCH 4/5] addressed issues --- cni/cni.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cni/cni.go b/cni/cni.go index afb1d7fb4d..817e41ab5e 100644 --- a/cni/cni.go +++ b/cni/cni.go @@ -10,10 +10,15 @@ import ( const ( // CNI commands. Cmd = "CNI_COMMAND" + // CNI ADD command. CmdAdd = "ADD" + // CNI GET command. CmdGet = "GET" + // CNI DEL command. CmdDel = "DEL" + // CNI UPDATE command. CmdUpdate = "UPDATE" + // CNI VERSION command. CmdVersion = "VERSION" // nonstandard CNI spec command, used to dump CNI state to stdout From abf657b043982f816b97c8b3849407e47678b18f Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Tue, 10 Aug 2021 12:54:58 -0700 Subject: [PATCH 5/5] linter fix --- cni/cni.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cni/cni.go b/cni/cni.go index 817e41ab5e..ec694352ec 100644 --- a/cni/cni.go +++ b/cni/cni.go @@ -9,16 +9,16 @@ import ( const ( // CNI commands. - Cmd = "CNI_COMMAND" - // CNI ADD command. - CmdAdd = "ADD" - // CNI GET command. - CmdGet = "GET" - // CNI DEL command. - CmdDel = "DEL" - // CNI UPDATE command. - CmdUpdate = "UPDATE" - // CNI VERSION command. + Cmd = "CNI_COMMAND" + // CmdAdd - CNI ADD command. + CmdAdd = "ADD" + // CmdGet - CNI GET command. + CmdGet = "GET" + // CmdDel - CNI DEL command. + CmdDel = "DEL" + // CmdUpdate - CNI UPDATE command. + CmdUpdate = "UPDATE" + // CmdVersion - CNI VERSION command. CmdVersion = "VERSION" // nonstandard CNI spec command, used to dump CNI state to stdout