From 46fe38e2c5ae06da40ef678faa53b29384917e91 Mon Sep 17 00:00:00 2001
From: Tomofumi Hayashi <tohayash@redhat.com>
Date: Mon, 2 Oct 2023 16:25:03 +0900
Subject: [PATCH] Suppress status unset in cmdDel

This change stops to update status in CNI's DEL command.
There are two reasons:

1. cmd DEL is invoked at only pod deletion, hence k8s does not
guarantee the pod and it may be already deleted. Hence this
API may failed.

2. In stateful set's pod recreation case, it may have race
condition to update the status at cmd DEL case.
In stateful set case, same pod name, i.e. stateful-0, is deleted
and then created again. In this case, if old Pod's CNI DEL command
is not finished before new Pod's creation, then SetStatus function
is failed due to pod UID mismatch.
---
 cmd/cert-approver/main.go | 12 ++++++++++--
 pkg/multus/multus.go      | 31 -------------------------------
 2 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/cmd/cert-approver/main.go b/cmd/cert-approver/main.go
index 9e891f262..52068a7c0 100644
--- a/cmd/cert-approver/main.go
+++ b/cmd/cert-approver/main.go
@@ -70,19 +70,26 @@ const (
 )
 
 var (
+	// ControllerName provides controller name
 	ControllerName = "csr-approver"
+	// NamePrefix specifies which name in certification request should be target to approve
 	NamePrefix     = "system:multus"
+	// Organization specifies which org in certification request should be target to approve
 	Organization   = []string{"system:multus"}
+	// Groups specifies which group in certification request should be target to approve
 	Groups         = sets.New[string]("system:nodes", "system:multus", "system:authenticated")
+	// UserPrefixes specifies which name prefix in certification request should be target to approve
 	UserPrefixes   = sets.New[string]("system:node", NamePrefix)
+	// Usages specifies which usage in certification request should be target to approve
 	Usages         = sets.New[certificatesv1.KeyUsage](
 		certificatesv1.UsageDigitalSignature,
 		certificatesv1.UsageClientAuth)
 )
 
+// NewCertController creates certcontroller
 func NewCertController() (*CertController, error) {
 	var clientset kubernetes.Interface
-	/* setup Kubernetes API client */
+	// setup Kubernetes API client
 	config, err := rest.InClusterConfig()
 	if err != nil {
 		return nil, err
@@ -131,6 +138,7 @@ func NewCertController() (*CertController, error) {
 	return c, nil
 }
 
+// Run starts controller
 func (c *CertController) Run(stopCh <-chan struct{}) {
 	defer utilruntime.HandleCrash()
 	defer c.queue.ShutDown()
@@ -347,7 +355,7 @@ func isApprovedOrDenied(status *certificatesv1.CertificateSigningRequestStatus)
 func main() {
 	klog.Infof("starting cert-approver")
 
-	//Start watching for pod creations
+	// Start watching for pod creations
 	certController, err := NewCertController()
 	if err != nil {
 		klog.Fatal(err)
diff --git a/pkg/multus/multus.go b/pkg/multus/multus.go
index 9fe2c84b5..77541fb56 100644
--- a/pkg/multus/multus.go
+++ b/pkg/multus/multus.go
@@ -814,21 +814,7 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo, po
 		return err
 	}
 
-	skipStatusUpdate := false
 	netns, err := ns.GetNS(args.Netns)
-	if err != nil {
-		// if NetNs is passed down by the Cloud Orchestration Engine, or if it called multiple times
-		// so don't return an error if the device is already removed.
-		// https://github.com/kubernetes/kubernetes/issues/43014#issuecomment-287164444
-		_, ok := err.(ns.NSPathNotExistErr)
-		skipStatusUpdate = true
-		if ok {
-			logging.Debugf("CmdDel: WARNING netns may not exist, netns: %s, err: %s", args.Netns, err)
-		} else {
-			logging.Debugf("CmdDel: WARNING failed to open netns %q: %v", netns, err)
-		}
-	}
-
 	if netns != nil {
 		defer netns.Close()
 	}
@@ -853,8 +839,6 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo, po
 	if err != nil {
 		// GetPod may be failed but just do print error in its log and continue to delete
 		logging.Errorf("Multus: GetPod failed: %v, but continue to delete", err)
-		// skip status update because k8s api seems to be stucked
-		skipStatusUpdate = true
 	}
 
 	// Read the cache to get delegates json for the pod
@@ -919,21 +903,6 @@ func CmdDel(args *skel.CmdArgs, exec invoke.Exec, kubeClient *k8s.ClientInfo, po
 		}
 	}
 
-	// unset the network status annotation in apiserver, only in case Multus as kubeconfig
-	if kubeClient != nil {
-		if !skipStatusUpdate {
-			if !types.CheckSystemNamespaces(string(k8sArgs.K8S_POD_NAMESPACE), in.SystemNamespaces) {
-				err := k8s.SetNetworkStatus(kubeClient, k8sArgs, nil, in)
-				if err != nil {
-					// error happen but continue to delete
-					logging.Errorf("Multus: error unsetting the networks status: %v", err)
-				}
-			}
-		} else {
-			logging.Debugf("WARNING: Unset SetNetworkStatus skipped")
-		}
-	}
-
 	e := delPlugins(exec, pod, args, k8sArgs, in.Delegates, len(in.Delegates)-1, in.RuntimeConfig, in)
 
 	// Enable Option only delegate plugin delete success to delete cache file