Refactored the IPAM code and moved to ipmanager package

pkg/controller/constants.go

@@ -146,12 +146,6 @@ const (
 
 	nginxMonitorPort int32 = 8081
 
-	NotEnabled = iota
-	InvalidInput
-	NotRequested
-	Requested
-	Allocated
-
 	timeoutSmall  = 5 * time.Second
 	timeoutMedium = 30 * time.Second
 	timeoutLarge  = 180 * time.Second

pkg/controller/controller.go

@@ -18,24 +18,19 @@ package controller
 
 import (
 	"fmt"
+	"github.com/F5Networks/f5-ipam-controller/pkg/ipammachinery"
 	cisapiv1 "github.com/F5Networks/k8s-bigip-ctlr/v3/config/apis/cis/v1"
+	"github.com/F5Networks/k8s-bigip-ctlr/v3/config/client/clientset/versioned"
+	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/clustermanager"
+	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/ipmanager"
 	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/prometheus"
 	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/tokenmanager"
+	log "github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/vlogger"
 	"os"
-	"strings"
 	"sync"
 	"time"
-	"unicode"
-
-	ficV1 "github.com/F5Networks/f5-ipam-controller/pkg/ipamapis/apis/fic/v1"
-	"github.com/F5Networks/f5-ipam-controller/pkg/ipammachinery"
-	"github.com/F5Networks/k8s-bigip-ctlr/v3/config/client/clientset/versioned"
-	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/clustermanager"
-	log "github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/vlogger"
 
 	routeclient "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1"
-	extClient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
-	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -136,6 +131,7 @@ func (ctlr *Controller) NewRequestHandler(userAgent string, httpClientMetrics bo
 		httpClientMetrics: httpClientMetrics,
 	}
 }
+
 func (ctlr *Controller) setupIPAM(params Params) {
 	if params.IPAM {
 		ipamParams := ipammachinery.Params{
@@ -145,76 +141,13 @@ func (ctlr *Controller) setupIPAM(params Params) {
 		}
 
 		ipamClient := ipammachinery.NewIPAMClient(ipamParams)
-		ctlr.ipamCli = ipamClient
 
-		ctlr.registerIPAMCRD()
-		time.Sleep(3 * time.Second)
-		_ = ctlr.createIPAMResource()
-	}
-}
+		ctlr.ipamHandler = ipmanager.NewIpamHandler(ctlr.ControllerIdentifier, params.Config, ipamClient)
 
-// Register IPAM CRD
-func (ctlr *Controller) registerIPAMCRD() {
-	err := ipammachinery.RegisterCRD(ctlr.clientsets.kubeAPIClient)
-	if err != nil {
-		log.Errorf("[IPAM] error while registering CRD %v", err)
-	}
-}
-
-// Create IPAM CRD
-func (ctlr *Controller) createIPAMResource() error {
-
-	frameIPAMResourceName := func() string {
-		prtn := ""
-		for _, ch := range DEFAULT_PARTITION {
-			elem := string(ch)
-			if unicode.IsUpper(ch) {
-				elem = strings.ToLower(elem) + "-"
-			}
-			prtn += elem
-		}
-		if string(prtn[len(prtn)-1]) == "-" {
-			prtn = prtn + ipamCRName
-		} else {
-			prtn = prtn + "." + ipamCRName
-		}
-
-		prtn = strings.Replace(prtn, "_", "-", -1)
-		prtn = strings.Replace(prtn, "--", "-", -1)
-
-		hostsplit := strings.Split(os.Getenv("HOSTNAME"), "-")
-		var host string
-		if len(hostsplit) > 2 {
-			host = strings.Join(hostsplit[0:len(hostsplit)-2], "-")
-		} else {
-			host = strings.Join(hostsplit, "-")
-		}
-		return strings.Join([]string{host, prtn}, ".")
-	}
-
-	crName := frameIPAMResourceName()
-	f5ipam := &ficV1.IPAM{
-		ObjectMeta: metaV1.ObjectMeta{
-			Name:      crName,
-			Namespace: IPAMNamespace,
-		},
-		Spec: ficV1.IPAMSpec{
-			HostSpecs: make([]*ficV1.HostSpec, 0),
-		},
-		Status: ficV1.IPAMStatus{
-			IPStatus: make([]*ficV1.IPSpec, 0),
-		},
-	}
-	ctlr.ipamCR = IPAMNamespace + "/" + crName
-
-	ipamCR, err := ctlr.ipamCli.Create(f5ipam)
-	if err == nil {
-		log.Debugf("[IPAM] Created IPAM Custom Resource: \n%v\n", ipamCR)
-		return nil
+		ctlr.ipamHandler.RegisterIPAMCRD()
+		time.Sleep(3 * time.Second)
+		_ = ctlr.ipamHandler.CreateIPAMResource()
 	}
-
-	log.Debugf("[IPAM] error while creating IPAM custom resource %v", err.Error())
-	return err
 }
 
 // createLabelSelector returns label used to identify F5 specific
@@ -248,15 +181,6 @@ func (ctlr *Controller) setupClients(config *rest.Config) error {
 		return fmt.Errorf("Failed to create kubeClient: %v", err)
 	}
 
-	var ipamCRConfig *rest.Config
-	if ipamCRConfig, err = rest.InClusterConfig(); err != nil {
-		log.Errorf("error creating client configuration: %v", err)
-	}
-	kubeIPAMClient, err := extClient.NewForConfig(ipamCRConfig)
-	if err != nil {
-		log.Errorf("Failed to create client: %v", err)
-	}
-
 	var rclient *routeclient.RouteV1Client
 	if ctlr.managedResources.ManageRoutes {
 		rclient, err = routeclient.NewForConfig(config)
@@ -269,7 +193,6 @@ func (ctlr *Controller) setupClients(config *rest.Config) error {
 	ctlr.clientsets = &ClientSets{
 		kubeClient:    kubeClient,
 		kubeCRClient:  kubeCRClient,
-		kubeAPIClient: kubeIPAMClient,
 		routeClientV1: rclient,
 	}
 	return nil
@@ -284,8 +207,8 @@ func (ctlr *Controller) Start() {
 	// Start Informers
 	ctlr.startInformers()
 
-	if ctlr.ipamCli != nil {
-		go ctlr.ipamCli.Start()
+	if ctlr.ipamHandler != nil {
+		go ctlr.ipamHandler.IpamCli.Start()
 	}
 
 	stopChan := make(chan struct{})
@@ -300,8 +223,8 @@ func (ctlr *Controller) Start() {
 func (ctlr *Controller) Stop() {
 	// stop the informers
 	ctlr.stopInformers()
-	if ctlr.ipamCli != nil {
-		ctlr.ipamCli.Stop()
+	if ctlr.ipamHandler != nil {
+		ctlr.ipamHandler.IpamCli.Stop()
 	}
 
 }

pkg/controller/controller_suit_test.go

@@ -4,13 +4,15 @@ import (
 	"bytes"
 	"fmt"
 	cisapiv1 "github.com/F5Networks/k8s-bigip-ctlr/v3/config/apis/cis/v1"
+	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/ipmanager"
 	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/tokenmanager"
 	mockhc "github.com/f5devcentral/mockhttpclient"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	routeapi "github.com/openshift/api/route/v1"
 	"io/ioutil"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/rest"
 	"net/http"
 	"sync"
 	"testing"
@@ -56,6 +58,7 @@ func newMockController() *mockController {
 			PostParams:       PostParams{},
 			clientsets:       &ClientSets{},
 			managedResources: ManagedResources{ManageVirtualServer: true, ManageIL: true, ManageEDNS: true, ManageTransportServer: true, ManageTLSProfile: true, ManageSecrets: true},
+			ipamHandler:      ipmanager.NewIpamHandler("test", &rest.Config{}, nil),
 		},
 	}
 }

pkg/controller/informers.go

@@ -724,7 +724,7 @@ func (ctlr *Controller) getEventHandlerForIPAM() *cache.ResourceEventHandlerFunc
 func (ctlr *Controller) enqueueIPAM(obj interface{}) {
 	ipamObj := obj.(*ficV1.IPAM)
 
-	if ipamObj.Namespace+"/"+ipamObj.Name != ctlr.ipamCR {
+	if ipamObj.Namespace+"/"+ipamObj.Name != ctlr.ipamHandler.IPAMCR {
 		return
 	}
 
@@ -744,7 +744,7 @@ func (ctlr *Controller) enqueueUpdatedIPAM(oldObj, newObj interface{}) {
 	oldIpam := oldObj.(*ficV1.IPAM)
 	curIpam := newObj.(*ficV1.IPAM)
 
-	if curIpam.Namespace+"/"+curIpam.Name != ctlr.ipamCR {
+	if curIpam.Namespace+"/"+curIpam.Name != ctlr.ipamHandler.IPAMCR {
 		return
 	}
 
@@ -767,7 +767,7 @@ func (ctlr *Controller) enqueueUpdatedIPAM(oldObj, newObj interface{}) {
 func (ctlr *Controller) enqueueDeletedIPAM(obj interface{}) {
 	ipamObj := obj.(*ficV1.IPAM)
 
-	if ipamObj.Namespace+"/"+ipamObj.Name != ctlr.ipamCR {
+	if ipamObj.Namespace+"/"+ipamObj.Name != ctlr.ipamHandler.IPAMCR {
 		return
 	}
 

pkg/controller/informers_test.go

@@ -3,8 +3,10 @@ package controller
 import (
 	"context"
 	ficV1 "github.com/F5Networks/f5-ipam-controller/pkg/ipamapis/apis/fic/v1"
+	"github.com/F5Networks/f5-ipam-controller/pkg/ipammachinery"
 	cisapiv1 "github.com/F5Networks/k8s-bigip-ctlr/v3/config/apis/cis/v1"
 	crdfake "github.com/F5Networks/k8s-bigip-ctlr/v3/config/client/clientset/versioned/fake"
+	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/ipmanager"
 	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/teem"
 	"github.com/F5Networks/k8s-bigip-ctlr/v3/pkg/test"
 	. "github.com/onsi/ginkgo"
@@ -13,6 +15,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 	"sync"
@@ -610,7 +613,9 @@ var _ = Describe("Informers Tests", func() {
 		})
 
 		It("IPAM", func() {
-			mockCtlr.ipamCR = "default/SampleIPAM"
+			fakeIpamCli := ipammachinery.NewFakeIPAMClient(nil, nil, nil)
+			mockCtlr.ipamHandler = ipmanager.NewIpamHandler("test", &rest.Config{}, fakeIpamCli)
+			mockCtlr.ipamHandler.IPAMCR = "default/SampleIPAM"
 
 			hostSpec := &ficV1.HostSpec{
 				Host:      "test.com",

pkg/controller/resourceConfig.go

@@ -26,8 +26,6 @@ import (
 	"strconv"
 	"strings"
 
-	ficV1 "github.com/F5Networks/f5-ipam-controller/pkg/ipamapis/apis/fic/v1"
-
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/cache"
 
@@ -60,7 +58,6 @@ func (rs *ResourceStore) Init() {
 	rs.nplStore = make(NPLStore)
 	rs.extdSpecMap = make(extendedSpecMap)
 	rs.invertedNamespaceLabelMap = make(map[string]string)
-	rs.ipamContext = make(map[string]ficV1.IPSpec)
 	rs.processedNativeResources = make(map[resourceRef]struct{})
 	rs.externalClustersConfig = make(map[string]cisapiv1.ExternalClusterConfig)
 }

pkg/controller/responseHandler.go

@@ -1,8 +1,6 @@
 package controller
 
 import (
-	ficV1 "github.com/F5Networks/f5-ipam-controller/pkg/ipamapis/apis/fic/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"strings"
 	"sync"
 	"time"
@@ -66,7 +64,9 @@ func (ctlr *Controller) responseHandler(respChan chan *agentConfig) {
 					continue
 				}
 				for rscKey, kind := range meta {
-					ctlr.removeUnusedIPAMEntries(kind)
+					if ctlr.ipamHandler != nil && (kind == VirtualServer || kind == TransportServer) {
+						ctlr.ipamHandler.RemoveUnusedIPAMEntries()
+					}
 					ns := strings.Split(rscKey, "/")[0]
 					switch kind {
 					//case VirtualServer:
@@ -153,52 +153,3 @@ func (ctlr *Controller) responseHandler(respChan chan *agentConfig) {
 		}
 	}
 }
-
-func (ctlr *Controller) removeUnusedIPAMEntries(kind string) {
-	// Remove Unused IPAM entries in IPAM CR after CIS restarts, applicable to only first PostCall
-	if !ctlr.firstPostResponse && ctlr.ipamCli != nil && (kind == VirtualServer || kind == TransportServer) {
-		ctlr.firstPostResponse = true
-		toRemoveIPAMEntries := &ficV1.IPAM{
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: make(map[string]string),
-			},
-		}
-		ipamCR := ctlr.getIPAMCR()
-		for _, hostSpec := range ipamCR.Spec.HostSpecs {
-			found := false
-			ctlr.cacheIPAMHostSpecs.Lock()
-			for cacheIndex, cachehostSpec := range ctlr.cacheIPAMHostSpecs.IPAM.Spec.HostSpecs {
-				if (hostSpec.IPAMLabel == cachehostSpec.IPAMLabel && hostSpec.Host == cachehostSpec.Host) ||
-					(hostSpec.IPAMLabel == cachehostSpec.IPAMLabel && hostSpec.Key == cachehostSpec.Key) ||
-					(hostSpec.IPAMLabel == cachehostSpec.IPAMLabel && hostSpec.Key == cachehostSpec.Key && hostSpec.Host == cachehostSpec.Host) {
-					if len(ctlr.cacheIPAMHostSpecs.IPAM.Spec.HostSpecs) > cacheIndex {
-						ctlr.cacheIPAMHostSpecs.IPAM.Spec.HostSpecs = append(ctlr.cacheIPAMHostSpecs.IPAM.Spec.HostSpecs[:cacheIndex], ctlr.cacheIPAMHostSpecs.IPAM.Spec.HostSpecs[cacheIndex+1:]...)
-					}
-					found = true
-					break
-				}
-			}
-			ctlr.cacheIPAMHostSpecs.Unlock()
-			if !found {
-				// To remove
-				toRemoveIPAMEntries.Spec.HostSpecs = append(toRemoveIPAMEntries.Spec.HostSpecs, hostSpec)
-			}
-		}
-		for _, removeIPAMentry := range toRemoveIPAMEntries.Spec.HostSpecs {
-			ipamCR = ctlr.getIPAMCR()
-			for index, hostSpec := range ipamCR.Spec.HostSpecs {
-				if (hostSpec.IPAMLabel == removeIPAMentry.IPAMLabel && hostSpec.Host == removeIPAMentry.Host) ||
-					(hostSpec.IPAMLabel == removeIPAMentry.IPAMLabel && hostSpec.Key == removeIPAMentry.Key) ||
-					(hostSpec.IPAMLabel == removeIPAMentry.IPAMLabel && hostSpec.Key == removeIPAMentry.Key && hostSpec.Host == removeIPAMentry.Host) {
-					_, err := ctlr.RemoveIPAMCRHostSpec(ipamCR, removeIPAMentry.Key, index)
-					if err != nil {
-						log.Errorf("[IPAM] ipam hostspec update error: %v", err)
-					}
-					break
-				}
-			}
-		}
-		// Delete cacheIPAMHostSpecs
-		ctlr.cacheIPAMHostSpecs = CacheIPAM{}
-	}
-}