From 4eb6ae1553a59771eeefc7a64e1213e2b8bd789b Mon Sep 17 00:00:00 2001 From: Tomofumi Hayashi Date: Thu, 11 Jun 2020 13:04:20 +0900 Subject: [PATCH] Fix error handling on cmdDel Fix #519 --- go.mod | 2 +- go.sum | 3 +++ k8sclient/k8sclient.go | 6 +++--- kubeletclient/kubeletclient.go | 2 +- multus/multus.go | 28 +++++++++++++++++++++++++--- multus/multus_test.go | 16 +++++++++++++--- netutils/netutils.go | 2 +- types/conf_test.go | 2 +- 8 files changed, 48 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index f2ac77ce1..ce8663a9f 100644 --- a/go.mod +++ b/go.mod @@ -17,8 +17,8 @@ require ( golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect golang.org/x/net v0.0.0-20200226121028-0de0cce0169b google.golang.org/grpc v1.23.0 - gopkg.in/yaml.v2 v2.2.8 // indirect gopkg.in/natefinch/lumberjack.v2 v2.0.0 + gopkg.in/yaml.v2 v2.2.8 // indirect k8s.io/api v0.0.0-20181115043458-b799cb063522 k8s.io/apimachinery v0.0.0-20181110190943-2a7c93004028 k8s.io/client-go v0.0.0-20181115111358-9bea17718df8 diff --git a/go.sum b/go.sum index 0878e6534..e49c1ad99 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= +github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/Microsoft/go-winio v0.4.11/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA= @@ -244,6 +245,8 @@ gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKW gopkg.in/inf.v0 v0.9.0 h1:3zYtXIO92bvsdS3ggAdA8Gb4Azj0YU+TVY1uGYNFA8o= gopkg.in/inf.v0 v0.9.0/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= +gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= +gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/k8sclient/k8sclient.go b/k8sclient/k8sclient.go index 8a310f1ac..27398928e 100644 --- a/k8sclient/k8sclient.go +++ b/k8sclient/k8sclient.go @@ -37,12 +37,12 @@ import ( "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/skel" cnitypes "github.com/containernetworking/cni/pkg/types" - "gopkg.in/intel/multus-cni.v3/kubeletclient" - "gopkg.in/intel/multus-cni.v3/logging" - "gopkg.in/intel/multus-cni.v3/types" nettypes "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" netclient "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/typed/k8s.cni.cncf.io/v1" netutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils" + "gopkg.in/intel/multus-cni.v3/kubeletclient" + "gopkg.in/intel/multus-cni.v3/logging" + "gopkg.in/intel/multus-cni.v3/types" ) const ( diff --git a/kubeletclient/kubeletclient.go b/kubeletclient/kubeletclient.go index eed27978a..6f6061f9e 100644 --- a/kubeletclient/kubeletclient.go +++ b/kubeletclient/kubeletclient.go @@ -5,10 +5,10 @@ import ( "path/filepath" "time" + "golang.org/x/net/context" "gopkg.in/intel/multus-cni.v3/checkpoint" "gopkg.in/intel/multus-cni.v3/logging" "gopkg.in/intel/multus-cni.v3/types" - "golang.org/x/net/context" v1 "k8s.io/api/core/v1" "k8s.io/kubernetes/pkg/kubelet/apis/podresources" podresourcesapi "k8s.io/kubernetes/pkg/kubelet/apis/podresources/v1alpha1" diff --git a/multus/multus.go b/multus/multus.go index e67004ce8..9640dfda8 100644 --- a/multus/multus.go +++ b/multus/multus.go @@ -701,11 +701,33 @@ func cmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er } } + kubeClient, err = k8s.GetK8sClient(in.Kubeconfig, kubeClient) + if err != nil { + return cmdErr(nil, "error getting k8s client: %v", err) + } + pod := (*v1.Pod)(nil) if kubeClient != nil { pod, err = kubeClient.GetPod(string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_NAME)) if err != nil { - if !errors.IsNotFound(err) { + var waitErr error + // in case of ServiceUnavailable, retry 10 times with 0.5 sec interval + if errors.IsServiceUnavailable(err) { + pollDuration := 500 * time.Millisecond + pollTimeout := 5 * time.Second + waitErr = wait.PollImmediate(pollDuration, pollTimeout, func() (bool, error) { + pod, err = kubeClient.GetPod(string(k8sArgs.K8S_POD_NAMESPACE), string(k8sArgs.K8S_POD_NAME)) + return pod != nil, err + }) + // retry failed, then return error with retry out + if waitErr != nil { + return cmdErr(k8sArgs, "error getting pod by service unavailable: %v", err) + } + } else if errors.IsNotFound(err) { + // If not found, proceed to remove interface with cache + pod = nil + } else { + // Other case, return error return cmdErr(k8sArgs, "error getting pod: %v", err) } } @@ -714,8 +736,8 @@ func cmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er // Read the cache to get delegates json for the pod netconfBytes, path, err := consumeScratchNetConf(args.ContainerID, in.CNIDir) if err != nil { - // Fetch delegates again if cache is not exist - if os.IsNotExist(err) { + // Fetch delegates again if cache is not exist and pod info can be read + if os.IsNotExist(err) && pod != nil { if in.ClusterNetwork != "" { _, err = k8s.GetDefaultNetworks(pod, in, kubeClient, nil) if err != nil { diff --git a/multus/multus_test.go b/multus/multus_test.go index 2413b7f77..5b1ae66d9 100644 --- a/multus/multus_test.go +++ b/multus/multus_test.go @@ -39,6 +39,9 @@ import ( "gopkg.in/intel/multus-cni.v3/logging" testhelpers "gopkg.in/intel/multus-cni.v3/testing" "gopkg.in/intel/multus-cni.v3/types" + + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" @@ -1564,8 +1567,14 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { os.Setenv("CNI_COMMAND", "DEL") os.Setenv("CNI_IFNAME", "eth0") - // set fKubeClient to nil to emulate no pod info + + // delete pod to emulate no pod info clientInfo.DeletePod(fakePod.ObjectMeta.Namespace, fakePod.ObjectMeta.Name) + nilPod, err := clientInfo.Client.Core().Pods(fakePod.ObjectMeta.Namespace).Get( + fakePod.ObjectMeta.Name, metav1.GetOptions{}) + Expect(nilPod).To(BeNil()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + err = cmdDel(args, fExec, clientInfo) Expect(err).NotTo(HaveOccurred()) Expect(fExec.delIndex).To(Equal(len(fExec.plugins))) @@ -2070,8 +2079,9 @@ var _ = Describe("multus operations cniVersion 0.2.0 config", func() { fExec.addPlugin020(nil, "eth0", expectedConf1, nil, nil) os.Setenv("CNI_COMMAND", "ADD") os.Setenv("CNI_IFNAME", "eth0") - - err := cmdDel(args, fExec, nil) + _, err := cmdAdd(args, fExec, nil) + Expect(err).NotTo(HaveOccurred()) + err = cmdDel(args, fExec, nil) Expect(err).NotTo(HaveOccurred()) }) }) diff --git a/netutils/netutils.go b/netutils/netutils.go index ab5f3cba6..fe4675ab5 100644 --- a/netutils/netutils.go +++ b/netutils/netutils.go @@ -20,8 +20,8 @@ import ( cnitypes "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" - "gopkg.in/intel/multus-cni.v3/logging" "github.com/vishvananda/netlink" + "gopkg.in/intel/multus-cni.v3/logging" "net" "strings" ) diff --git a/types/conf_test.go b/types/conf_test.go index 9f9848aaa..6264c964d 100644 --- a/types/conf_test.go +++ b/types/conf_test.go @@ -26,8 +26,8 @@ import ( types020 "github.com/containernetworking/cni/pkg/types/020" "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" - testhelpers "gopkg.in/intel/multus-cni.v3/testing" netutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils" + testhelpers "gopkg.in/intel/multus-cni.v3/testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega"