From 9e3dfdf72d1a208a10ac124fabf38f0c0ccb47dc Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Wed, 11 Aug 2021 11:26:00 +0200
Subject: [PATCH] Do not error on del command

According to the cni spec[1], a plugin should not return an error on
del. If a plugin returns an error cni will not call the following
plugins. Since the plugins are invoked in reverse order on del, the
portmapping and bridge plugins won't get invoked and therefore leave
iptables rules around.

Fixes #67
Fixes containers/podman#10806
Fixes containers/podman#10745

[1] https://github.com/containernetworking/cni/blob/master/SPEC.md#del-remove-container-from-network-or-un-apply-modifications

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 plugins/meta/dnsname/main.go | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/plugins/meta/dnsname/main.go b/plugins/meta/dnsname/main.go
index 1c3e2a5..8208909 100644
--- a/plugins/meta/dnsname/main.go
+++ b/plugins/meta/dnsname/main.go
@@ -105,27 +105,34 @@ func cmdAdd(args *skel.CmdArgs) error {
 	return types.PrintResult(result, netConf.CNIVersion)
 }
 
+// Do not return an error, otherwise cni will stop
+// and not invoke the following plugins del command.
 func cmdDel(args *skel.CmdArgs) error {
 	if err := findDNSMasq(); err != nil {
-		return ErrBinaryNotFound
+		logrus.Error(ErrBinaryNotFound)
+		return nil
 	}
 	netConf, result, podname, err := parseConfig(args.StdinData, args.Args)
 	if err != nil {
-		return errors.Wrap(err, "failed to parse config")
+		logrus.Error(errors.Wrap(err, "failed to parse config"))
+		return nil
 	} else if result == nil {
 		return nil
 	}
 	dnsNameConf, err := newDNSMasqFile(netConf.DomainName, result.Interfaces[0].Name, netConf.Name)
 	if err != nil {
-		return err
+		logrus.Error(err)
+		return nil
 	}
 	domainBaseDir := filepath.Dir(dnsNameConf.PidFile)
 	lock, err := getLock(domainBaseDir)
 	if err != nil {
-		return err
+		logrus.Error(err)
+		return nil
 	}
 	if err := lock.acquire(); err != nil {
-		return err
+		logrus.Error(err)
+		return nil
 	}
 	defer func() {
 		// if the lock isn't given up by another process
@@ -135,20 +142,30 @@ func cmdDel(args *skel.CmdArgs) error {
 	}()
 	shouldHUP, err := removeFromFile(filepath.Join(domainBaseDir, hostsFileName), podname)
 	if err != nil {
-		return err
+		logrus.Error(err)
+		return nil
 	}
 	if !shouldHUP {
 		// if there are no hosts, we should just stop the dnsmasq instance to not take
 		// system resources
 		err = dnsNameConf.stop()
 		if err != nil {
-			return err
+			logrus.Error(err)
+			return nil
 		}
 		// remove the config directory
-		return os.RemoveAll(domainBaseDir)
+		err = os.RemoveAll(domainBaseDir)
+		if err != nil {
+			logrus.Error(err)
+		}
+		return nil
 	}
 	// Now we need to HUP
-	return dnsNameConf.hup()
+	err = dnsNameConf.hup()
+	if err != nil {
+		logrus.Error(err)
+	}
+	return nil
 }
 
 func main() {