From e961722e9b729e9ca73d68f45220ba7dd3902d22 Mon Sep 17 00:00:00 2001 From: dougbtv Date: Fri, 3 Aug 2018 10:49:31 -0400 Subject: [PATCH] Updates default network readiness to use apimachinery wait.ExponentialBackoff --- README.md | 7 +++--- multus/multus.go | 50 ++++++++++++++++--------------------------- multus/multus_test.go | 50 +------------------------------------------ types/conf.go | 11 +++------- types/conf_test.go | 30 +++++++++++--------------- types/types.go | 3 +-- 6 files changed, 39 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index e00504a02..0603987e9 100644 --- a/README.md +++ b/README.md @@ -499,12 +499,11 @@ For example, if you use Flannel as a default network, the recommended method for In this manner, you may prevent pods from crash looping, and instead wait for that default network to be ready. -Two options are available to configure this functionality: +Only one option is necessary to configure this functionality: -* `defaultnetworkfile`: The path to a file whose existance denotes that the default network is ready. -* `defaultnetworkwaitseconds`: The number of seconds to wait for that file to become available. Defaults to 120 seconds. +* `readinessindicatorfile`: The path to a file whose existance denotes that the default network is ready. -*NOTE*: If `defaultnetworkfile` is unset, or is an empty string, this functionality will be disabled, and is disabled by default. +*NOTE*: If `readinessindicatorfile` is unset, or is an empty string, this functionality will be disabled, and is disabled by default. ## Testing Multus CNI diff --git a/multus/multus.go b/multus/multus.go index f1e141f9d..4b4cc0987 100644 --- a/multus/multus.go +++ b/multus/multus.go @@ -24,7 +24,7 @@ import ( "io/ioutil" "os" "path/filepath" - "k8s.io/client-go/util/retry" + "time" "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/invoke" @@ -35,8 +35,16 @@ import ( k8s "github.com/intel/multus-cni/k8sclient" "github.com/intel/multus-cni/types" "github.com/vishvananda/netlink" + "k8s.io/apimachinery/pkg/util/wait" ) +var defaultReadinessBackoff = wait.Backoff{ + Steps: 4, + Duration: 250 * time.Millisecond, + Factor: 4.0, + Jitter: 0.1, +} + func saveScratchNetConf(containerID, dataDir string, netconf []byte) error { if err := os.MkdirAll(dataDir, 0700); err != nil { return fmt.Errorf("failed to create the multus data directory(%q): %v", dataDir, err) @@ -205,34 +213,6 @@ func delPlugins(exec invoke.Exec, argIfname string, delegates []*types.DelegateN return nil } -// Sits in a wait loop until a file indicating the readiness of the "default network" -// is present (or until a user-defined timeout is reached) -func waitForDefaultNetwork(indicatorFile string, waitSeconds int) error { - // If there's no file to wait for, then, this is essentially disabled. - if len(indicatorFile) > 0 { - attempts := 0 - found := false - // Sleep in a loop until we find that file. - for attempts < waitSeconds { - attempts++ - if _, err := os.Stat(indicatorFile); err == nil { - found = true - attempts = waitSeconds - } else { - time.Sleep(1 * time.Second) - } - } - - if !found { - return fmt.Errorf("Multus: Timeout (%v seconds) finding default network file: %v", waitSeconds, indicatorFile) - } - return nil - - } - - return nil -} - func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cnitypes.Result, error) { n, err := types.LoadNetConf(args.StdinData) if err != nil { @@ -244,9 +224,15 @@ func cmdAdd(args *skel.CmdArgs, exec invoke.Exec, kubeClient k8s.KubeClient) (cn return nil, fmt.Errorf("Multus: Err in getting k8s args: %v", err) } - if err = waitForDefaultNetwork(n.DefaultNetworkFile, n.DefaultNetworkWaitSeconds); err != nil { - return nil, err - } + wait.ExponentialBackoff(defaultReadinessBackoff, func() (bool, error) { + _, err := os.Stat(n.DefaultNetworkFile) + switch { + case err == nil: + return true, nil + default: + return false, nil + } + }) numK8sDelegates, kc, err := k8s.TryLoadK8sDelegates(k8sArgs, n, kubeClient) if err != nil { diff --git a/multus/multus_test.go b/multus/multus_test.go index fb43978b8..b04d05ec8 100644 --- a/multus/multus_test.go +++ b/multus/multus_test.go @@ -173,8 +173,7 @@ var _ = Describe("multus operations", func() { StdinData: []byte(`{ "name": "node-cni-network", "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 3, + "readinessindicatorfile": "/tmp/foo.multus.conf", "delegates": [{ "name": "weave1", "cniVersion": "0.2.0", @@ -320,51 +319,4 @@ var _ = Describe("multus operations", func() { Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) }) - It("times out waiting for default networks", func() { - args := &skel.CmdArgs{ - ContainerID: "123456789", - Netns: testNS.Path(), - IfName: "eth0", - StdinData: []byte(`{ - "name": "defaultnetwork", - "type": "multus", - "defaultnetworkfile": "/tmp/foo.multus.conf", - "defaultnetworkwaitseconds": 1, - "delegates": [{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" - }] -}`), - } - - // Always remove the defaultnetworkfile - configPath := "/tmp/foo.multus.conf" - if _, errStat := os.Stat(configPath); errStat == nil { - errRemove := os.Remove(configPath) - Expect(errRemove).NotTo(HaveOccurred()) - } - - fExec := &fakeExec{} - expectedResult1 := &types020.Result{ - CNIVersion: "0.2.0", - IP4: &types020.IPConfig{ - IP: *testhelpers.EnsureCIDR("1.1.1.2/24"), - }, - } - expectedConf1 := `{ - "name": "weave1", - "cniVersion": "0.2.0", - "type": "weave-net" -}` - fExec.addPlugin(nil, "eth0", expectedConf1, expectedResult1, nil) - - os.Setenv("CNI_COMMAND", "ADD") - os.Setenv("CNI_IFNAME", "eth0") - _, err := cmdAdd(args, fExec, nil) - - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("Multus: Timeout (1 seconds) finding default network file: /tmp/foo.multus.conf")) - - }) }) diff --git a/types/conf.go b/types/conf.go index 996c90d7e..08c92ce6d 100644 --- a/types/conf.go +++ b/types/conf.go @@ -28,11 +28,10 @@ import ( ) const ( - defaultCNIDir = "/var/lib/cni/multus" - defaultConfDir = "/etc/cni/multus/net.d" - defaultBinDir = "/opt/cni/bin" + defaultCNIDir = "/var/lib/cni/multus" + defaultConfDir = "/etc/cni/multus/net.d" + defaultBinDir = "/opt/cni/bin" defaultDefaultNetworkFile = "" - defaultDefaultNetworkWaitSeconds = 120 ) func LoadDelegateNetConfList(bytes []byte, delegateConf *DelegateNetConf) error { @@ -185,10 +184,6 @@ func LoadNetConf(bytes []byte) (*NetConf, error) { netconf.DefaultNetworkFile = defaultDefaultNetworkFile } - if netconf.DefaultNetworkWaitSeconds < 1 { - netconf.DefaultNetworkWaitSeconds = defaultDefaultNetworkWaitSeconds - } - for idx, rawConf := range netconf.RawDelegates { bytes, err := json.Marshal(rawConf) if err != nil { diff --git a/types/conf_test.go b/types/conf_test.go index 1260b4862..c88fff598 100644 --- a/types/conf_test.go +++ b/types/conf_test.go @@ -82,8 +82,8 @@ var _ = Describe("config operations", func() { Expect(err).To(HaveOccurred()) }) - It("has defaults set for network readiness", func() { - conf := `{ + It("has defaults set for network readiness", func() { + conf := `{ "name": "defaultnetwork", "type": "multus", "kubeconfig": "/etc/kubernetes/kubelet.conf", @@ -94,18 +94,16 @@ var _ = Describe("config operations", func() { "isDefaultGateway": true }] }` - netConf, err := LoadNetConf([]byte(conf)) - Expect(err).NotTo(HaveOccurred()) - Expect(netConf.DefaultNetworkFile).To(Equal("")) - Expect(netConf.DefaultNetworkWaitSeconds).To(Equal(120)) - }) + netConf, err := LoadNetConf([]byte(conf)) + Expect(err).NotTo(HaveOccurred()) + Expect(netConf.DefaultNetworkFile).To(Equal("")) + }) - It("honors overrides for network readiness", func() { - conf := `{ + It("honors overrides for network readiness", func() { + conf := `{ "name": "defaultnetwork", "type": "multus", - "defaultnetworkfile": "/etc/cni/net.d/foo", - "defaultnetworkwaitseconds": 30, + "readinessindicatorfile": "/etc/cni/net.d/foo", "kubeconfig": "/etc/kubernetes/kubelet.conf", "delegates": [{ "cniVersion": "0.3.0", @@ -114,11 +112,9 @@ var _ = Describe("config operations", func() { "isDefaultGateway": true }] }` - netConf, err := LoadNetConf([]byte(conf)) - Expect(err).NotTo(HaveOccurred()) - Expect(netConf.DefaultNetworkFile).To(Equal("/etc/cni/net.d/foo")) - Expect(netConf.DefaultNetworkWaitSeconds).To(Equal(30)) - }) - + netConf, err := LoadNetConf([]byte(conf)) + Expect(err).NotTo(HaveOccurred()) + Expect(netConf.DefaultNetworkFile).To(Equal("/etc/cni/net.d/foo")) + }) }) diff --git a/types/types.go b/types/types.go index 54ae40b36..b12d248bd 100644 --- a/types/types.go +++ b/types/types.go @@ -43,8 +43,7 @@ type NetConf struct { LogFile string `json:"logFile"` LogLevel string `json:"logLevel"` // Default network readiness options - DefaultNetworkFile string `json:defaultnetworkfile` - DefaultNetworkWaitSeconds int `json:defaultnetworkwaitseconds` + DefaultNetworkFile string `json:readinessindicatorfile` } type NetworkStatus struct {