Improved unit test coverage (#3314)

cmd/k8s-bigip-ctlr/main.go

@@ -940,6 +940,7 @@ func initController(
 			StaticRouteNodeCIDR:         *staticRouteNodeCIDR,
 			MultiClusterMode:            *multiClusterMode,
 		},
+		true,
 	)
 
 	return ctlr

cmd/k8s-bigip-ctlr/main_test.go

@@ -17,15 +17,18 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"github.com/F5Networks/k8s-bigip-ctlr/v2/pkg/agent/as3"
 	"github.com/F5Networks/k8s-bigip-ctlr/v2/pkg/agent/cccl"
-	"io/ioutil"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 	"os"
 	"os/exec"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/F5Networks/k8s-bigip-ctlr/v2/pkg/appmanager"
@@ -457,19 +460,43 @@ var _ = Describe("Main Tests", func() {
 				"--bigip-partition=velcro1",
 				"--bigip-url=bigip.example.com",
 				"--pool-member-type=nodeport",
+				"--trusted-certs-cfgmap=default/foomap",
 			}
 			flags.Parse(os.Args)
 			os.Mkdir("/tmp/k8s-test-creds", 0755)
-			err := ioutil.WriteFile("/tmp/k8s-test-creds/username", []byte("user"), 0755)
+			err := os.WriteFile("/tmp/k8s-test-creds/username", []byte("user"), 0755)
 			Expect(err).ToNot(HaveOccurred())
-			err = ioutil.WriteFile("/tmp/k8s-test-creds/password", []byte("pass"), 0755)
+			err = os.WriteFile("/tmp/k8s-test-creds/password", []byte("pass"), 0755)
 			Expect(err).ToNot(HaveOccurred())
 
 			err = getCredentials()
 			Expect(err).ToNot(HaveOccurred())
 			Expect(*bigIPURL).To(Equal("https://bigip.example.com"))
 			Expect(*bigIPUsername).To(Equal("user"))
 			Expect(*bigIPPassword).To(Equal("pass"))
+			kubeClient = fake.NewSimpleClientset()
+			cfgFoo := &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foomap", Namespace: "default"}, Data: map[string]string{"data": "foo"}}
+			_, err = kubeClient.CoreV1().ConfigMaps("default").Create(context.TODO(), cfgFoo, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			cfgFoo, err = getConfigMapUsingNamespaceAndName("default", "foomap")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(cfgFoo.Data["data"]).To(Equal("foo"))
+			//check for invlaid configmap
+			_, err = getConfigMapUsingNamespaceAndName("default", "invalid")
+			Expect(err).ToNot(BeNil())
+			//check for valid bigip trusted certs
+			os.Args[6] = "--trusted-certs-cfgmap=default/foomap"
+			out := getBIGIPTrustedCerts()
+			Expect(strings.TrimSpace(out)).To(Equal("foo"))
+			//check for invalid bigip trusted certs
+			os.Args[6] = "--trusted-certs-cfgmap= "
+			flags.Parse(os.Args)
+			out = getBIGIPTrustedCerts()
+			Expect(out).To(Equal(""))
+			os.Args[6] = "--trusted-certs-cfgmap=default"
+			flags.Parse(os.Args)
+			out = getBIGIPTrustedCerts()
+			Expect(out).To(Equal(""))
 
 			// Test url variations
 			os.Args[4] = "--bigip-url=fail://bigip.example.com"
@@ -503,14 +530,14 @@ var _ = Describe("Main Tests", func() {
 			}
 			flags.Parse(os.Args)
 			os.Mkdir("/tmp/k8s-test-creds", 0755)
-			err := ioutil.WriteFile("/tmp/k8s-test-creds/username", []byte("user"), 0755)
+			err := os.WriteFile("/tmp/k8s-test-creds/username", []byte("user"), 0755)
 			Expect(err).ToNot(HaveOccurred())
-			err = ioutil.WriteFile("/tmp/k8s-test-creds/password", []byte("pass"), 0755)
+			err = os.WriteFile("/tmp/k8s-test-creds/password", []byte("pass"), 0755)
 			Expect(err).ToNot(HaveOccurred())
 			os.Mkdir("/tmp/k8s-test-gtm-creds", 0755)
-			err = ioutil.WriteFile("/tmp/k8s-test-gtm-creds/username", []byte("user-gtm"), 0755)
+			err = os.WriteFile("/tmp/k8s-test-gtm-creds/username", []byte("user-gtm"), 0755)
 			Expect(err).ToNot(HaveOccurred())
-			err = ioutil.WriteFile("/tmp/k8s-test-gtm-creds/password", []byte("pass-gtm"), 0755)
+			err = os.WriteFile("/tmp/k8s-test-gtm-creds/password", []byte("pass-gtm"), 0755)
 			Expect(err).ToNot(HaveOccurred())
 
 			err = getCredentials()

pkg/controller/controller.go

@@ -135,7 +135,7 @@ const (
 )
 
 // NewController creates a new Controller Instance.
-func NewController(params Params) *Controller {
+func NewController(params Params, startController bool) *Controller {
 
 	ctlr := &Controller{
 		namespaces:            make(map[string]bool),
@@ -192,7 +192,7 @@ func NewController(params Params) *Controller {
 		ctlr.shareNodes = true
 	}
 
-	if err := ctlr.setupClients(params.Config); err != nil {
+	if err := ctlr.setupClients(params.Config, params.IPAM); err != nil {
 		log.Errorf("Failed to Setup Clients: %v", err)
 	}
 
@@ -260,14 +260,15 @@ func NewController(params Params) *Controller {
 		}
 		ctlr.vxlanMgr = vxlanMgr
 	}
+	if startController {
+		go ctlr.responseHandler(ctlr.Agent.respChan)
 
-	go ctlr.responseHandler(ctlr.Agent.respChan)
+		go ctlr.Start()
 
-	go ctlr.Start()
-
-	go ctlr.setOtherSDNType()
-	// Start the CIS health check
-	go ctlr.CISHealthCheck()
+		go ctlr.setOtherSDNType()
+		// Start the CIS health check
+		go ctlr.CISHealthCheck()
+	}
 
 	return ctlr
 }
@@ -377,7 +378,7 @@ func createLabelSelector(label string) (labels.Selector, error) {
 }
 
 // setupClients sets Kubernetes Clients.
-func (ctlr *Controller) setupClients(config *rest.Config) error {
+func (ctlr *Controller) setupClients(config *rest.Config, ipamClient bool) error {
 	var kubeCRClient *versioned.Clientset
 	var err error
 	kubeCRClient, err = versioned.NewForConfig(config)
@@ -390,13 +391,12 @@ 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 kubeIPAMClient *extClient.Clientset
+	if ipamClient {
+		kubeIPAMClient, err = extClient.NewForConfig(config)
+		if err != nil {
+			log.Errorf("Failed to create client: %v", err)
+		}
 	}
 
 	var rclient *routeclient.RouteV1Client

pkg/controller/controller_test.go

@@ -7,6 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/rest"
 )
 
 var _ = Describe("OtherSDNType", func() {
@@ -40,4 +41,26 @@ var _ = Describe("OtherSDNType", func() {
 		mockCtlr.setOtherSDNType()
 		Expect(mockCtlr.TeemData.SDNType).To(Equal("other"), "SDNType should be other")
 	})
+	It("Create a new controller object", func() {
+		ctlrOpenShift := NewController(Params{
+			Mode:           OpenShiftMode,
+			PoolMemberType: Cluster,
+			Config:         &rest.Config{},
+			NamespaceLabel: "ctlr=cis",
+			VXLANMode:      "multi-point",
+			VXLANName:      "vxlan0",
+			Agent:          newMockAgent(&test.MockWriter{FailStyle: test.Success}),
+		}, false)
+		Expect(ctlrOpenShift.processedHostPath).NotTo(BeNil(), "processedHostPath object should not be nil")
+		Expect(ctlrOpenShift.shareNodes).To(BeFalse(), "shareNodes should not be enable")
+		Expect(ctlrOpenShift.vxlanMgr).To(BeNil(), "vxlanMgr should be created")
+		ctlrK8s := NewController(Params{
+			Mode:           CustomResourceMode,
+			PoolMemberType: NodePort,
+			Config:         &rest.Config{},
+			IPAM:           true,
+		}, false)
+		Expect(ctlrK8s.processedHostPath).To(BeNil(), "processedHostPath object should be nil")
+		Expect(ctlrK8s.shareNodes).To(BeTrue(), "shareNodes should be enable")
+	})
 })

pkg/controller/multiClusterInformers.go

@@ -228,12 +228,14 @@ func (ctlr *Controller) stopDeletedGlobalCMMultiClusterInformers() error {
 	return nil
 }
 
-func (ctlr *Controller) stopMultiClusterInformers(clusterName string) error {
+func (ctlr *Controller) stopMultiClusterInformers(clusterName string, stopInformer bool) error {
 
 	// remove the informers for clusters whose config has been removed
 	if clsSet, ok := ctlr.multiClusterPoolInformers[clusterName]; ok {
 		for _, nsPoolInf := range clsSet {
-			nsPoolInf.stop()
+			if stopInformer {
+				nsPoolInf.stop()
+			}
 			delete(ctlr.multiClusterPoolInformers, clusterName)
 		}
 	}
@@ -244,14 +246,14 @@ func (ctlr *Controller) stopMultiClusterInformers(clusterName string) error {
 }
 
 // setup multi cluster informer
-func (ctlr *Controller) setupAndStartMultiClusterInformers(svcKey MultiClusterServiceKey) error {
+func (ctlr *Controller) setupAndStartMultiClusterInformers(svcKey MultiClusterServiceKey, startInformer bool) error {
 	if config, ok := ctlr.multiClusterConfigs.ClusterConfigs[svcKey.clusterName]; ok {
 		restClient := config.KubeClient.CoreV1().RESTClient()
-		if err := ctlr.addMultiClusterNamespacedInformers(svcKey.clusterName, svcKey.namespace, restClient, true); err != nil {
+		if err := ctlr.addMultiClusterNamespacedInformers(svcKey.clusterName, svcKey.namespace, restClient, startInformer); err != nil {
 			log.Errorf("[MultiCluster] unable to setup informer for cluster: %v, namespace: %v, Error: %v", svcKey.clusterName, svcKey.namespace, err)
 			return err
 		}
-		err := ctlr.setupMultiClusterNodeInformers(svcKey.clusterName)
+		err := ctlr.setupMultiClusterNodeInformers(svcKey.clusterName, startInformer)
 		if err != nil {
 			return err
 		}
@@ -271,33 +273,35 @@ func (ctlr *Controller) setupAndStartHAClusterInformers(clusterName string) erro
 			return err
 		}
 	}
-	err := ctlr.setupMultiClusterNodeInformers(clusterName)
+	err := ctlr.setupMultiClusterNodeInformers(clusterName, true)
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
 // setupMultiClusterNodeInformers sets up and starts node informers for cluster if it hasn't been started
-func (ctlr *Controller) setupMultiClusterNodeInformers(clusterName string) error {
+func (ctlr *Controller) setupMultiClusterNodeInformers(clusterName string, startInformer bool) error {
 	if _, ok := ctlr.multiClusterNodeInformers[clusterName]; !ok {
 		nodeInf := ctlr.getNodeInformer(clusterName)
 		ctlr.addNodeEventUpdateHandler(&nodeInf)
-		nodeInf.start()
-		time.Sleep(100 * time.Millisecond)
 		ctlr.multiClusterNodeInformers[clusterName] = &nodeInf
-		nodesIntfc := nodeInf.nodeInformer.GetIndexer().List()
-		var nodesList []corev1.Node
-		for _, obj := range nodesIntfc {
-			node := obj.(*corev1.Node)
-			nodesList = append(nodesList, *node)
-		}
-		sort.Sort(NodeList(nodesList))
-		nodes, err := ctlr.getNodes(nodesList)
-		if err != nil {
-			return err
+		if startInformer {
+			nodeInf.start()
+			time.Sleep(100 * time.Millisecond)
+			nodesIntfc := nodeInf.nodeInformer.GetIndexer().List()
+			var nodesList []corev1.Node
+			for _, obj := range nodesIntfc {
+				node := obj.(*corev1.Node)
+				nodesList = append(nodesList, *node)
+			}
+			sort.Sort(NodeList(nodesList))
+			nodes, err := ctlr.getNodes(nodesList)
+			if err != nil {
+				return err
+			}
+			nodeInf.oldNodes = nodes
 		}
-		nodeInf.oldNodes = nodes
 	}
 	return nil
 }

pkg/controller/multiClusterInformers_test.go

@@ -0,0 +1,47 @@
+package controller
+
+import (
+	"github.com/F5Networks/k8s-bigip-ctlr/v2/pkg/clustermanager"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	k8sfake "k8s.io/client-go/kubernetes/fake"
+)
+
+var _ = Describe("MultiClusterInformers", func() {
+	var mockCtlr *mockController
+	BeforeEach(func() {
+		mockCtlr = newMockController()
+		mockCtlr.multiClusterConfigs = clustermanager.NewMultiClusterConfig()
+		clusterName := "cluster-1"
+		mockCtlr.multiClusterConfigs.ClusterConfigs[clusterName] = clustermanager.ClusterConfig{KubeClient: k8sfake.NewSimpleClientset()}
+		mockCtlr.multiClusterPoolInformers = make(map[string]map[string]*MultiClusterPoolInformer)
+		mockCtlr.multiClusterNodeInformers = make(map[string]*NodeInformer)
+		mockCtlr.multiClusterResources = newMultiClusterResourceStore()
+	})
+	It("Setup and start multi-cluster informers NodePortLocal", func() {
+		mockCtlr.PoolMemberType = NodePortLocal
+		svcKey := MultiClusterServiceKey{
+			serviceName: "svc-1",
+			namespace:   "ns",
+			clusterName: "cluster-1",
+		}
+		err := mockCtlr.setupAndStartMultiClusterInformers(svcKey, false)
+		Expect(err).To(BeNil())
+		poolInf, found := mockCtlr.getNamespaceMultiClusterPoolInformer(svcKey.namespace, svcKey.clusterName)
+		Expect(found).To(BeTrue())
+		Expect(poolInf).ToNot(BeNil())
+		mockCtlr.stopMultiClusterInformers(svcKey.clusterName, false)
+		Expect(len(mockCtlr.multiClusterPoolInformers)).To(Equal(0))
+		Expect(len(mockCtlr.multiClusterNodeInformers)).To(Equal(0))
+	})
+	It("Setup and start multi-cluster informers Cluster", func() {
+		mockCtlr.PoolMemberType = Cluster
+		svcKey := MultiClusterServiceKey{
+			serviceName: "svc-1",
+			namespace:   "ns",
+			clusterName: "cluster-1",
+		}
+		err := mockCtlr.setupAndStartMultiClusterInformers(svcKey, false)
+		Expect(err).To(BeNil())
+	})
+})

pkg/controller/multiClusterWorker.go

@@ -50,9 +50,9 @@ func (ctlr *Controller) processResourceExternalClusterServices(rscKey resourceRe
 			// if informer not found for cluster, setup and start informer
 			_, clusterKeyFound := ctlr.multiClusterPoolInformers[svc.ClusterName]
 			if !clusterKeyFound {
-				ctlr.setupAndStartMultiClusterInformers(svcKey)
+				ctlr.setupAndStartMultiClusterInformers(svcKey, true)
 			} else if _, found := ctlr.multiClusterPoolInformers[svc.ClusterName][svc.Namespace]; !found {
-				ctlr.setupAndStartMultiClusterInformers(svcKey)
+				ctlr.setupAndStartMultiClusterInformers(svcKey, true)
 			}
 		} else {
 			log.Warningf("[MultiCluster] invalid cluster reference found cluster: %v resource:%v", svc.ClusterName, rscKey)
@@ -127,7 +127,7 @@ func (ctlr *Controller) deleteUnrefereedMultiClusterInformers() {
 		if len(svcs) == 0 && ((ctlr.haModeType == StandAloneCIS || ctlr.haModeType == StandBy) ||
 			ctlr.multiClusterConfigs.HAPairClusterName != clusterName) {
 			delete(ctlr.multiClusterResources.clusterSvcMap, clusterName)
-			ctlr.stopMultiClusterInformers(clusterName)
+			ctlr.stopMultiClusterInformers(clusterName, true)
 		}
 	}
 }