From 2c2f826b21c6f7c6d770beaabc69cc3e6221707b Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Fri, 7 Aug 2020 12:00:35 -0700 Subject: [PATCH 1/3] cni to not consider reboot time and rehydrate --- cni/network/network.go | 2 +- cni/plugin.go | 14 +------- cnm/network/network.go | 2 +- cnms/service/networkmonitor.go | 2 +- ipam/manager.go | 1 + network/manager.go | 62 ++++++++++++++++------------------ 6 files changed, 35 insertions(+), 48 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index e576b004bd..8cb9b9c342 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -116,7 +116,7 @@ func (plugin *netPlugin) Start(config *common.PluginConfig) error { common.LogNetworkInterfaces() // Initialize network manager. - err = plugin.nm.Initialize(config) + err = plugin.nm.Initialize(config, false) if err != nil { log.Printf("[cni-net] Failed to initialize network manager, err:%v.", err) return err diff --git a/cni/plugin.go b/cni/plugin.go index a1ed3fe338..855ddb2012 100644 --- a/cni/plugin.go +++ b/cni/plugin.go @@ -160,19 +160,7 @@ func (plugin *Plugin) InitializeKeyValueStore(config *common.PluginConfig) error } // Force unlock the json store if the lock file is left on the node after reboot - if lockFileModTime, err := plugin.Store.GetLockFileModificationTime(); err == nil { - rebootTime, err := platform.GetLastRebootTime() - log.Printf("[cni] reboot time %v storeLockFile mod time %v", rebootTime, lockFileModTime) - if err == nil && rebootTime.After(lockFileModTime) { - log.Printf("[cni] Detected Reboot") - - if err := plugin.Store.Unlock(true); err != nil { - log.Printf("[cni] Failed to force unlock store due to error %v", err) - } else { - log.Printf("[cni] Force unlocked the store successfully") - } - } - } + removeLockFileAfterReboot(plugin) } // Acquire store lock. diff --git a/cnm/network/network.go b/cnm/network/network.go index 599db6e4e7..077be98ca6 100644 --- a/cnm/network/network.go +++ b/cnm/network/network.go @@ -71,7 +71,7 @@ func (plugin *netPlugin) Start(config *common.PluginConfig) error { } // Initialize network manager. - err = plugin.nm.Initialize(config) + err = plugin.nm.Initialize(config, true) if err != nil { log.Printf("[net] Failed to initialize network manager, err:%v.", err) return err diff --git a/cnms/service/networkmonitor.go b/cnms/service/networkmonitor.go index f1564f0dba..57069075c6 100644 --- a/cnms/service/networkmonitor.go +++ b/cnms/service/networkmonitor.go @@ -152,7 +152,7 @@ func main() { return } - if err := nm.Initialize(&config); err != nil { + if err := nm.Initialize(&config, false); err != nil { log.Printf("[monitor] Failed while initializing network manager %+v", err) } diff --git a/ipam/manager.go b/ipam/manager.go index 06c697699d..247d00f0d8 100644 --- a/ipam/manager.go +++ b/ipam/manager.go @@ -363,6 +363,7 @@ func (am *addressManager) RequestAddress(asId, poolId, address string, options m err = am.save() if err != nil { + ap.releaseAddress(addr, options) return "", err } diff --git a/network/manager.go b/network/manager.go index d5468f50da..9cf1585514 100644 --- a/network/manager.go +++ b/network/manager.go @@ -51,7 +51,7 @@ type networkManager struct { // NetworkManager API. type NetworkManager interface { - Initialize(config *common.PluginConfig) error + Initialize(config *common.PluginConfig, isRehydrationRequired bool) error Uninitialize() AddExternalInterface(ifName string, subnet string) error @@ -81,12 +81,12 @@ func NewNetworkManager() (NetworkManager, error) { } // Initialize configures network manager. -func (nm *networkManager) Initialize(config *common.PluginConfig) error { +func (nm *networkManager) Initialize(config *common.PluginConfig, isRehydrationRequired bool) error { nm.Version = config.Version nm.store = config.Store // Restore persisted state. - err := nm.restore() + err := nm.restore(isRehydrationRequired) return err } @@ -95,7 +95,7 @@ func (nm *networkManager) Uninitialize() { } // Restore reads network manager state from persistent store. -func (nm *networkManager) restore() error { +func (nm *networkManager) restore(isRehydrationRequired bool) error { // Skip if a store is not provided. if nm.store == nil { log.Printf("[net] network store is nil") @@ -107,9 +107,6 @@ func (nm *networkManager) restore() error { // Ignore the persisted state if it is older than the last reboot time. // Read any persisted state. - nm.Lock() - defer nm.Unlock() - err := nm.store.Read(storeKey, nm) if err != nil { if err == store.ErrKeyNotFound { @@ -122,38 +119,39 @@ func (nm *networkManager) restore() error { } } - modTime, err := nm.store.GetModificationTime() - if err == nil { - rebootTime, err := platform.GetLastRebootTime() - log.Printf("[net] reboot time %v store mod time %v", rebootTime, modTime) - if err == nil && rebootTime.After(modTime) { - log.Printf("[net] Detected Reboot") - rebooted = true - if clearNwConfig, err := platform.ClearNetworkConfiguration(); clearNwConfig { - if err != nil { - log.Printf("[net] Failed to clear network configuration, err:%v\n", err) - return err - } + if isRehydrationRequired { + modTime, err := nm.store.GetModificationTime() + if err == nil { + rebootTime, err := platform.GetLastRebootTime() + log.Printf("[net] reboot time %v store mod time %v", rebootTime, modTime) + if err == nil && rebootTime.After(modTime) { + log.Printf("[net] Detected Reboot") + rebooted = true + if clearNwConfig, err := platform.ClearNetworkConfiguration(); clearNwConfig { + if err != nil { + log.Printf("[net] Failed to clear network configuration, err:%v\n", err) + return err + } - // Delete the networks left behind after reboot - for _, extIf := range nm.ExternalInterfaces { - for _, nw := range extIf.Networks { - log.Printf("[net] Deleting the network %s on reboot\n", nw.Id) - nm.deleteNetwork(nw.Id) + // Delete the networks left behind after reboot + for _, extIf := range nm.ExternalInterfaces { + for _, nw := range extIf.Networks { + log.Printf("[net] Deleting the network %s on reboot\n", nw.Id) + nm.deleteNetwork(nw.Id) + } } - } - // Clear networkManager contents - nm.TimeStamp = time.Time{} - for extIfName := range nm.ExternalInterfaces { - delete(nm.ExternalInterfaces, extIfName) - } + // Clear networkManager contents + nm.TimeStamp = time.Time{} + for extIfName := range nm.ExternalInterfaces { + delete(nm.ExternalInterfaces, extIfName) + } - return nil + return nil + } } } } - // Populate pointers. for _, extIf := range nm.ExternalInterfaces { for _, nw := range extIf.Networks { From 0a09b1f107c47be9fcc9e0c9dc61c67cbb36e95f Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Fri, 7 Aug 2020 12:10:05 -0700 Subject: [PATCH 2/3] added missing files --- cni/plugin_linux.go | 11 +++++++++++ cni/plugin_windows.go | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 cni/plugin_linux.go create mode 100644 cni/plugin_windows.go diff --git a/cni/plugin_linux.go b/cni/plugin_linux.go new file mode 100644 index 0000000000..1a950712c2 --- /dev/null +++ b/cni/plugin_linux.go @@ -0,0 +1,11 @@ +package cni + +import ( + "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/platform" +) + +func removeLockFileAfterReboot(plugin *Plugin) { + rebootTime, _ := platform.GetLastRebootTime() + log.Printf("[cni] reboot time %v", rebootTime) +} diff --git a/cni/plugin_windows.go b/cni/plugin_windows.go new file mode 100644 index 0000000000..34a96403a4 --- /dev/null +++ b/cni/plugin_windows.go @@ -0,0 +1,22 @@ +package cni + +import ( + "github.com/Azure/azure-container-networking/log" + "github.com/Azure/azure-container-networking/platform" +) + +func removeLockFileAfterReboot(plugin *Plugin) { + if lockFileModTime, err := plugin.Store.GetLockFileModificationTime(); err == nil { + rebootTime, err := platform.GetLastRebootTime() + log.Printf("[cni] reboot time %v storeLockFile mod time %v", rebootTime, lockFileModTime) + if err == nil && rebootTime.After(lockFileModTime) { + log.Printf("[cni] Detected Reboot") + + if err := plugin.Store.Unlock(true); err != nil { + log.Printf("[cni] Failed to force unlock store due to error %v", err) + } else { + log.Printf("[cni] Force unlocked the store successfully") + } + } + } +} From f9dc9a540add327de15202e4a509cc8e7e91b46e Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Fri, 7 Aug 2020 14:41:23 -0700 Subject: [PATCH 3/3] fixed ut --- network/manager_test.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/network/manager_test.go b/network/manager_test.go index 2038998be6..0040ebd855 100644 --- a/network/manager_test.go +++ b/network/manager_test.go @@ -51,7 +51,7 @@ var ( Context("When restore is nil", func() { It("Should return nil", func() { nm := &networkManager{} - err := nm.restore() + err := nm.restore(false) Expect(err).NotTo(HaveOccurred()) }) }) @@ -63,7 +63,7 @@ var ( ReadError: store.ErrKeyNotFound, }, } - err := nm.restore() + err := nm.restore(false) Expect(err).NotTo(HaveOccurred()) }) }) @@ -75,7 +75,7 @@ var ( ReadError: errors.New("error for test"), }, } - err := nm.restore() + err := nm.restore(false) Expect(err).To(HaveOccurred()) }) }) @@ -91,11 +91,11 @@ var ( ExternalInterfaces: map[string]*externalInterface{}, } nm.ExternalInterfaces[extIfName] = &externalInterface{ - Name: extIfName, + Name: extIfName, Networks: map[string]*network{}, } nm.ExternalInterfaces[extIfName].Networks[nwId] = &network{} - err := nm.restore() + err := nm.restore(false) Expect(err).NotTo(HaveOccurred()) Expect(nm.ExternalInterfaces[extIfName].Networks[nwId].extIf.Name).To(Equal(extIfName)) }) @@ -104,7 +104,7 @@ var ( Describe("Test save", func() { Context("When store is nil", func() { - It("Should return nil", func(){ + It("Should return nil", func() { nm := &networkManager{} err := nm.save() Expect(err).NotTo(HaveOccurred()) @@ -112,7 +112,7 @@ var ( }) }) Context("When store.Write return error", func() { - It("Should raise error", func(){ + It("Should raise error", func() { nm := &networkManager{ store: &testutils.KeyValueStoreMock{ WriteError: errors.New("error for test"), @@ -120,7 +120,8 @@ var ( } err := nm.save() Expect(err).To(HaveOccurred()) - Expect(nm.TimeStamp).NotTo(Equal(time.Time{}))}) + Expect(nm.TimeStamp).NotTo(Equal(time.Time{})) + }) }) }) @@ -198,9 +199,9 @@ var ( } nm.ExternalInterfaces[ifName].Networks[nwId] = &network{ Endpoints: map[string]*endpoint{ - "ep1":&endpoint{}, - "ep2":&endpoint{}, - "ep3":&endpoint{}, + "ep1": &endpoint{}, + "ep2": &endpoint{}, + "ep3": &endpoint{}, }, } num := nm.GetNumberOfEndpoints(ifName, nwId) @@ -220,9 +221,9 @@ var ( } nm.ExternalInterfaces[ifName].Networks[nwId] = &network{ Endpoints: map[string]*endpoint{ - "ep1":&endpoint{}, - "ep2":&endpoint{}, - "ep3":&endpoint{}, + "ep1": &endpoint{}, + "ep2": &endpoint{}, + "ep3": &endpoint{}, }, } num := nm.GetNumberOfEndpoints("", nwId)