Cleaner: reconciling the handling of "no release service" and "no IP" edge cases

pkg/danmep/calico.go

@@ -1,33 +1,33 @@
 package danmep
 
 import (
-    "fmt"
-    "log"
-    "net"
-    "os/exec"
+	"fmt"
+	"log"
+	"net"
+	"os/exec"
 
-    danmipam "github.com/nokia/danm/pkg/ipam"
+	danmipam "github.com/nokia/danm/pkg/ipam"
 )
 
 type calicoReleaseIPServiceImpl releaseIPServiceImplBase
 
-func (h *calicoReleaseIPServiceImpl) IsIPAllocatedByMe(ip string) bool {
-    return ip != danmipam.NoneAllocType && ip != "" &&
-        ! danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Cidr) &&
-        ! danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Pool6.Cidr) &&
-        h.ep.Spec.NetworkType == "calico"
+func (h *calicoReleaseIPServiceImpl) IsIPAllocatedByMe(ip string, neType string) bool {
+	return ip != danmipam.NoneAllocType && ip != "" &&
+		!danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Cidr) &&
+		!danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Pool6.Cidr) &&
+		neType == "calico"
 }
 
 func (h *calicoReleaseIPServiceImpl) ReleaseIP(ip string) error {
-    parsedIp := net.ParseIP(ip)
-    if parsedIp == nil {
-        parsedIp, _, _ = net.ParseCIDR(ip)
-    }
-    cmd := exec.Command("calicoctl", "ipam", "release", fmt.Sprintf("--ip=%s", parsedIp))
-    log.Printf("release calico managed IP: %s", cmd)
+	parsedIp := net.ParseIP(ip)
+	if parsedIp == nil {
+		parsedIp, _, _ = net.ParseCIDR(ip)
+	}
+	cmd := exec.Command("calicoctl", "ipam", "release", fmt.Sprintf("--ip=%s", parsedIp))
+	log.Printf("release calico managed IP: %s", cmd)
 
-    if output, err := cmd.CombinedOutput(); err != nil {
-        return fmt.Errorf("could not release calico managed IP %s, because: %s | output: %s", ip, err, output)
-    }
-    return nil
+	if output, err := cmd.CombinedOutput(); err != nil {
+		return fmt.Errorf("could not release calico managed IP %s, because: %s | output: %s", ip, err, output)
+	}
+	return nil
 }

pkg/danmep/danm.go

@@ -1,16 +1,16 @@
 package danmep
 
 import (
-    danmipam "github.com/nokia/danm/pkg/ipam"
+	danmipam "github.com/nokia/danm/pkg/ipam"
 )
 
 type danmReleaseIPServiceImpl releaseIPServiceImplBase
 
-func (h *danmReleaseIPServiceImpl) IsIPAllocatedByMe(ip string) bool {
-    return danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Cidr) ||
-        danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Pool6.Cidr)
+func (h *danmReleaseIPServiceImpl) IsIPAllocatedByMe(ip string, neType string) bool {
+	return danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Cidr) ||
+		danmipam.WasIpAllocatedByDanm(ip, h.dnet.Spec.Options.Pool6.Cidr)
 }
 
 func (h *danmReleaseIPServiceImpl) ReleaseIP(ip string) error {
-    return danmipam.Free(h.danmClient, *h.dnet, ip)
+	return danmipam.Free(h.danmClient, *h.dnet, ip)
 }

pkg/danmep/interface.go

@@ -1,65 +1,72 @@
 package danmep
 
 import (
-    "context"
-    "fmt"
+	"context"
+	"fmt"
+	"log"
 
-    danmtypes "github.com/nokia/danm/crd/apis/danm/v1"
-    danmclientset "github.com/nokia/danm/crd/client/clientset/versioned"
-    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	danmtypes "github.com/nokia/danm/crd/apis/danm/v1"
+	danmclientset "github.com/nokia/danm/crd/client/clientset/versioned"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // GetSupportedReleaseIPHandlers gets all supported static CNI implementation that cleaner can use
 // when deleting dangling DanmEps
-func GetSupportedReleaseIPHandlers(danmClient danmclientset.Interface, dnet *danmtypes.DanmNet, ep *danmtypes.DanmEp) []interface{} {
-    return []interface{}{
-        &danmReleaseIPServiceImpl{danmClient, dnet, ep},
-        &calicoReleaseIPServiceImpl{danmClient, dnet, ep},
-    }
+func GetSupportedReleaseIPHandlers(danmClient danmclientset.Interface, dnet *danmtypes.DanmNet) []interface{} {
+	return []interface{}{
+		&danmReleaseIPServiceImpl{danmClient, dnet},
+		&calicoReleaseIPServiceImpl{danmClient, dnet},
+	}
 }
 
 // ReleaseIPInterface need to be implemented by every static CNI IPAM release plugin
 // that want to invoked when Cleaner is cleaning up dangling DanmEps
 type ReleaseIPInterface interface {
-    ReleaseIP(ip string) error
-    IsIPAllocatedByMe(ip string) bool
+	IsIPAllocatedByMe(ip string, neType string) bool
+	ReleaseIP(ip string) error
 }
 
 type releaseIPServiceImplBase struct {
-    danmClient danmclientset.Interface
-    dnet *danmtypes.DanmNet
-    ep *danmtypes.DanmEp
+	danmClient danmclientset.Interface
+	dnet       *danmtypes.DanmNet
 }
 
-// SelectReleaseIpServiceImplementation looks up and returns the
-// first suitable ReleaseIP service implementation for given IP address
-// returns nil when no suitable implementation found
-func SelectReleaseIpServiceImplementation(danmClient danmclientset.Interface, dnet *danmtypes.DanmNet, ep *danmtypes.DanmEp, ip string) ReleaseIPInterface {
-    for _, impl := range GetSupportedReleaseIPHandlers(danmClient, dnet, ep) {
-        if service, ok := impl.(ReleaseIPInterface); ok && service.IsIPAllocatedByMe(ip) {
-            return service
-        }
-    }
-    return nil
+// DeleteDanmEp selects a ReleaseIPService implementation for allocated IPv4 and IPv6 IP addresses in a given DanmEp and invokes them to free up the allocated IPs in a remote control plane
+// After successfully freeing up all IPs, it deletes the DanmEp as well
+// Returns error if a registered service for the network was unable to free any of the IPs, or if the DanmEp could not be deleted
+func DeleteDanmEp(danmClient danmclientset.Interface, ep *danmtypes.DanmEp, dnet *danmtypes.DanmNet) error {
+	v4Error := releaseIP(danmClient, dnet, ep.Spec.NetworkType, ep.Spec.Iface.Address)
+	v6Error := releaseIP(danmClient, dnet, ep.Spec.NetworkType, ep.Spec.Iface.AddressIPv6)
+	if v4Error != nil || v6Error != nil {
+		return fmt.Errorf("one of the registered IP releasing service failed with an error: V4: %s, V6: %s", v4Error, v6Error)
+	}
+	return danmClient.DanmV1().DanmEps(ep.ObjectMeta.Namespace).Delete(context.TODO(), ep.ObjectMeta.Name, metav1.DeleteOptions{})
 }
 
-// DeleteDanmEp selects an ReleaseIPService implementation for allocated IPv4 and IPv6 IP addresses in a
-// given DanmEp and invokes them separately trying ot free up allocated IPs, after successfully freeing up
-// all IPs, it deletes the DanmEp, returns error if unable to free any of the IPs or unable to delete DanmEp
-func DeleteDanmEp(danmClient danmclientset.Interface, ep *danmtypes.DanmEp, dnet *danmtypes.DanmNet) error {
-    if service := SelectReleaseIpServiceImplementation(danmClient, dnet, ep, ep.Spec.Iface.Address); service != nil {
-        if err := service.ReleaseIP(ep.Spec.Iface.Address); err != nil {
-            return fmt.Errorf("unable to release ipv4 IP because: %s", err)
-        }
-    } else {
-      return fmt.Errorf("unable to release ipv4 IP because: no releaseIP Service selected")
-    }
-    if service := SelectReleaseIpServiceImplementation(danmClient, dnet, ep, ep.Spec.Iface.AddressIPv6); service != nil {
-        if err := service.ReleaseIP(ep.Spec.Iface.AddressIPv6); err != nil {
-            return fmt.Errorf("unable to release ipv6 IP because: %s", err)
-        }
-    } else {
-        return fmt.Errorf("unable to release ipv6 IP because: no releaseIP Service selected")
-    }
-    return danmClient.DanmV1().DanmEps(ep.ObjectMeta.Namespace).Delete(context.TODO(), ep.ObjectMeta.Name, metav1.DeleteOptions{})
+func releaseIP(danmClient danmclientset.Interface, dnet *danmtypes.DanmNet, neType string, ip string) error {
+	if ip == "" {
+		return nil
+	}
+	service := selectReleaseIpServiceImplementation(danmClient, dnet, neType, ip)
+	if service == nil {
+		log.Println("WARNING: no releaseIP Service is found for IP:" + ip + " in network:" + dnet.ObjectMeta.Name + ", deleting Endpoint without additional clean-up!")
+		return nil
+	}
+	err := service.ReleaseIP(ip)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// selectReleaseIpServiceImplementation looks up and returns the
+// first suitable ReleaseIP service implementation for given IP address
+// returns nil when no suitable implementation found
+func selectReleaseIpServiceImplementation(danmClient danmclientset.Interface, dnet *danmtypes.DanmNet, neType string, ip string) ReleaseIPInterface {
+	for _, impl := range GetSupportedReleaseIPHandlers(danmClient, dnet) {
+		if service, ok := impl.(ReleaseIPInterface); ok && service.IsIPAllocatedByMe(ip, neType) {
+			return service
+		}
+	}
+	return nil
 }