Fix error handling on cmdDel

Fix #519
k8snetworkplumbingwg · Jul 9, 2020 · 12e50d1 · 12e50d1
1 parent ecb7933
commit 12e50d1
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 12 deletions.
diff --git 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
@@ -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
@@ -701,11 +701,33 @@ func cmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo) er
 		}
 	}
 
+	kubeClient, err = k8s.GetK8sClient(n.Kubeconfig, kubeClient)
+	if err != nil {
+		return nil, 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
@@ -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
@@ -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
@@ -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"