Single way of mocking Kubernetes client/dynamic client

Signed-off-by: David Gageot <david@gageot.net>
dgageot · Sep 3, 2019 · 95bcad7 · 95bcad7
1 parent e2ecf51
commit 95bcad7
Show file tree

Hide file tree

Showing 15 changed files with 62 additions and 57 deletions.
diff --git a/integration/util.go b/integration/util.go
@@ -24,7 +24,7 @@ import (
 	"testing"
 	"time"
 
-	kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
+	pkgkubernetes "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
@@ -46,7 +46,7 @@ func Run(t *testing.T, dir, command string, args ...string) {
 
 // SetupNamespace creates a Kubernetes namespace to run a test.
 func SetupNamespace(t *testing.T) (*v1.Namespace, *NSKubernetesClient, func()) {
-	client, err := kubernetesutil.GetClientset()
+	client, err := pkgkubernetes.Client()
 	if err != nil {
 		t.Fatalf("Test setup error: getting kubernetes client: %s", err)
 	}

diff --git a/pkg/skaffold/build/cluster/kaniko.go b/pkg/skaffold/build/cluster/kaniko.go
@@ -51,12 +51,12 @@ func (b *Builder) runKanikoBuild(ctx context.Context, out io.Writer, artifact *l
 	}
 
 	// Create pod
-	client, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
-		return "", errors.Wrap(err, "")
+		return "", errors.Wrap(err, "getting kubernetes client")
 	}
-	pods := client.CoreV1().Pods(b.Namespace)
 
+	pods := client.CoreV1().Pods(b.Namespace)
 	podSpec := s.Pod(args)
 	pod, err := pods.Create(podSpec)
 	if err != nil {

diff --git a/pkg/skaffold/build/cluster/secret.go b/pkg/skaffold/build/cluster/secret.go
@@ -32,7 +32,7 @@ import (
 func (b *Builder) setupPullSecret(out io.Writer) (func(), error) {
 	color.Default.Fprintf(out, "Creating kaniko secret [%s/%s]...\n", b.Namespace, b.PullSecretName)
 
-	client, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
 		return nil, errors.Wrap(err, "getting kubernetes client")
 	}
@@ -82,7 +82,7 @@ func (b *Builder) setupDockerConfigSecret(out io.Writer) (func(), error) {
 
 	color.Default.Fprintf(out, "Creating docker config secret [%s]...\n", b.DockerConfig.SecretName)
 
-	client, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
 		return nil, errors.Wrap(err, "getting kubernetes client")
 	}

diff --git a/pkg/skaffold/build/cluster/sources/localdir.go b/pkg/skaffold/build/cluster/sources/localdir.go
@@ -98,10 +98,11 @@ func (g *LocalDir) Pod(args []string) *v1.Pod {
 // Via kubectl exec, we extract the tarball to the empty dir
 // Then, via kubectl exec, create the /tmp/complete file via kubectl exec to complete the init container
 func (g *LocalDir) ModifyPod(ctx context.Context, p *v1.Pod) error {
-	client, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
-		return errors.Wrap(err, "getting clientset")
+		return errors.Wrap(err, "getting kubernetes client")
 	}
+
 	if err := kubernetes.WaitForPodInitialized(ctx, client.CoreV1().Pods(p.Namespace), p.Name); err != nil {
 		return errors.Wrap(err, "waiting for pod to initialize")
 	}

diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go
@@ -69,13 +69,13 @@ func labelDeployResults(labels map[string]string, results []Artifact) {
 	// use the kubectl client to update all k8s objects with a skaffold watermark
 	dynClient, err := kubernetes.DynamicClient()
 	if err != nil {
-		logrus.Warnf("error retrieving kubernetes dynamic client: %s", err.Error())
+		logrus.Warnf("error getting kubernetes dynamic client: %s", err.Error())
 		return
 	}
 
-	client, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
-		logrus.Warnf("error retrieving kubernetes client: %s", err.Error())
+		logrus.Warnf("error getting kubernetes client: %s", err.Error())
 		return
 	}
 

diff --git a/pkg/skaffold/deploy/status_check.go b/pkg/skaffold/deploy/status_check.go
@@ -29,7 +29,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
-	kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
+	pkgkubernetes "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 )
 
@@ -44,10 +44,11 @@ var (
 )
 
 func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *runcontext.RunContext) error {
-	client, err := kubernetesutil.GetClientset()
+	client, err := pkgkubernetes.Client()
 	if err != nil {
-		return err
+		return errors.Wrap(err, "getting kubernetes client")
 	}
+
 	deadline := getDeadline(runCtx.Cfg.Deploy.StatusCheckDeadlineSeconds)
 
 	dMap, err := getDeployments(client, runCtx.Opts.Namespace, defaultLabeller, deadline)

diff --git a/pkg/skaffold/kubernetes/client.go b/pkg/skaffold/kubernetes/client.go
@@ -27,15 +27,21 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
-func GetClientset() (kubernetes.Interface, error) {
+// for tests
+var (
+	Client        = getClientset
+	DynamicClient = getDynamicClient
+)
+
+func getClientset() (kubernetes.Interface, error) {
 	config, err := context.GetRestClientConfig()
 	if err != nil {
 		return nil, errors.Wrap(err, "getting client config for kubernetes client")
 	}
 	return kubernetes.NewForConfig(config)
 }
 
-func GetDynamicClient() (dynamic.Interface, error) {
+func getDynamicClient() (dynamic.Interface, error) {
 	config, err := context.GetRestClientConfig()
 	if err != nil {
 		return nil, errors.Wrap(err, "getting client config for dynamic client")

diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go
@@ -34,10 +34,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 )
 
-// Client is for tests
-var Client = GetClientset
-var DynamicClient = GetDynamicClient
-
 // LogAggregator aggregates the logs for all the deployed pods.
 type LogAggregator struct {
 	output      io.Writer

diff --git a/pkg/skaffold/kubernetes/owner.go b/pkg/skaffold/kubernetes/owner.go
@@ -23,11 +23,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-var (
-	// For testing
-	getClientSet = GetClientset
-)
-
 // TopLevelOwnerKey returns a key associated with the top level
 // owner of a Kubernetes resource in the form Kind-Name
 func TopLevelOwnerKey(obj metav1.Object, kind string) string {
@@ -47,7 +42,7 @@ func TopLevelOwnerKey(obj metav1.Object, kind string) string {
 }
 
 func ownerMetaObject(ns string, owner metav1.OwnerReference) (metav1.Object, error) {
-	client, err := getClientSet()
+	client, err := Client()
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/skaffold/kubernetes/owner_test.go b/pkg/skaffold/kubernetes/owner_test.go
@@ -21,14 +21,12 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 
+	"github.com/GoogleContainerTools/skaffold/testutil"
+	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	batchv1beta1 "k8s.io/api/batch/v1beta1"
-
-	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-
-	"github.com/GoogleContainerTools/skaffold/testutil"
 	"k8s.io/client-go/kubernetes"
 	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
 )
@@ -103,8 +101,10 @@ func TestTopLevelOwnerKey(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			client := fakekubeclientset.NewSimpleClientset(test.objects...)
-			t.Override(&getClientSet, mockClient(client))
+			t.Override(&Client, mockClient(client))
+
 			actual := TopLevelOwnerKey(test.initialObject, test.kind)
+
 			t.CheckDeepEqual(test.expected, actual)
 		})
 	}
@@ -277,8 +277,10 @@ func TestOwnerMetaObject(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			client := fakekubeclientset.NewSimpleClientset(test.objects...)
-			t.Override(&getClientSet, mockClient(client))
+			t.Override(&Client, mockClient(client))
+
 			actual, err := ownerMetaObject("ns", test.or)
+
 			t.CheckNoError(err)
 			t.CheckDeepEqual(test.expected, actual)
 		})

diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go
@@ -40,7 +40,6 @@ var (
 	// For testing
 	retrieveAvailablePort = util.GetAvailablePort
 	retrieveServices      = retrieveServiceResources
-	getClientSet          = kubernetes.GetClientset
 )
 
 // NewResourceForwarder returns a struct that tracks and port-forwards pods as they are created and modified
@@ -101,14 +100,14 @@ func (p *ResourceForwarder) getCurrentEntry(resource latest.PortForwardResource)
 // retrieveServiceResources retrieves all services in the cluster matching the given label
 // as a list of PortForwardResources
 func retrieveServiceResources(label string, namespaces []string) ([]*latest.PortForwardResource, error) {
-	clientset, err := getClientSet()
+	client, err := kubernetes.Client()
 	if err != nil {
-		return nil, errors.Wrap(err, "getting clientset")
+		return nil, errors.Wrap(err, "getting kubernetes client")
 	}
 
 	var resources []*latest.PortForwardResource
 	for _, ns := range namespaces {
-		services, err := clientset.CoreV1().Services(ns).List(metav1.ListOptions{
+		services, err := client.CoreV1().Services(ns).List(metav1.ListOptions{
 			LabelSelector: label,
 		})
 		if err != nil {

diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go
@@ -24,19 +24,19 @@ import (
 	"testing"
 	"time"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
+	kubernetesutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/GoogleContainerTools/skaffold/testutil"
 	"github.com/google/go-cmp/cmp"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
-
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
-	"github.com/GoogleContainerTools/skaffold/testutil"
 	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
 )
 
@@ -344,8 +344,10 @@ func TestRetrieveServices(t *testing.T) {
 				objs[i] = s
 			}
 			client := fakekubeclientset.NewSimpleClientset(objs...)
-			t.Override(&getClientSet, mockClient(client))
+			t.Override(&kubernetesutil.Client, mockClient(client))
+
 			actual, err := retrieveServiceResources(fmt.Sprintf("%s=9876-6789", deploy.RunIDLabel), test.namespaces)
+
 			t.CheckNoError(err)
 			t.CheckDeepEqual(test.expected, actual)
 		})

diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go
@@ -223,7 +223,7 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex
 
 	client, err := kubernetes.Client()
 	if err != nil {
-		return errors.Wrap(err, "getting k8s client")
+		return errors.Wrap(err, "getting kubernetes client")
 	}
 
 	numSynced := 0

diff --git a/pkg/webhook/kubernetes/deployment.go b/pkg/webhook/kubernetes/deployment.go
@@ -48,9 +48,9 @@ const (
 // 		2. A container to run hugo server
 // and one emptyDir volume to hold the git repository
 func CreateDeployment(pr *github.PullRequestEvent, svc *v1.Service, externalIP string) (*appsv1.Deployment, error) {
-	clientset, err := pkgkubernetes.GetClientset()
+	client, err := pkgkubernetes.Client()
 	if err != nil {
-		return nil, errors.Wrap(err, "getting clientset")
+		return nil, errors.Wrap(err, "getting kubernetes client")
 	}
 
 	deploymentLabels := svc.Spec.Selector
@@ -119,18 +119,20 @@ func CreateDeployment(pr *github.PullRequestEvent, svc *v1.Service, externalIP s
 			},
 		},
 	}
-	return clientset.AppsV1().Deployments(constants.Namespace).Create(d)
+	return client.AppsV1().Deployments(constants.Namespace).Create(d)
 }
 
 // WaitForDeploymentToStabilize waits till the Deployment has stabilized
 func WaitForDeploymentToStabilize(d *appsv1.Deployment, ip string) error {
-	client, err := pkgkubernetes.GetClientset()
+	client, err := pkgkubernetes.Client()
 	if err != nil {
-		return errors.Wrap(err, "getting clientset")
+		return errors.Wrap(err, "getting kubernetes client")
 	}
+
 	if err := pkgkubernetes.WaitForDeploymentToStabilize(context.Background(), client, d.Namespace, d.Name, 5*time.Minute); err != nil {
 		return errors.Wrap(err, "waiting for deployment to stabilize")
 	}
+
 	// wait up to five minutes for the URL to return a valid endpoint
 	url := BaseURL(ip)
 	log.Printf("Waiting up to 2 minutes for %s to return an OK response...", url)

diff --git a/pkg/webhook/kubernetes/service.go b/pkg/webhook/kubernetes/service.go
@@ -33,12 +33,12 @@ import (
 // CreateService creates a service for the deployment to bind to
 // and returns the external IP of the service
 func CreateService(pr *github.PullRequestEvent) (*v1.Service, error) {
-	clientset, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
-		return nil, errors.Wrap(err, "getting clientset")
+		return nil, errors.Wrap(err, "getting kubernetes client")
 	}
-	l := labels.GenerateLabelsFromPR(pr.GetNumber())
 
+	l := labels.GenerateLabelsFromPR(pr.GetNumber())
 	key, val := labels.RetrieveLabel(pr.GetNumber())
 	selector := map[string]string{key: val}
 
@@ -57,7 +57,7 @@ func CreateService(pr *github.PullRequestEvent) (*v1.Service, error) {
 			Selector: selector,
 		},
 	}
-	return clientset.CoreV1().Services(constants.Namespace).Create(svc)
+	return client.CoreV1().Services(constants.Namespace).Create(svc)
 }
 
 // GetExternalIP polls the service until an external IP is available and returns it
@@ -82,9 +82,10 @@ func serviceName(prNumber int) string {
 }
 
 func getService(svc *v1.Service) (*v1.Service, error) {
-	clientset, err := kubernetes.GetClientset()
+	client, err := kubernetes.Client()
 	if err != nil {
-		return nil, errors.Wrap(err, "getting clientset")
+		return nil, errors.Wrap(err, "getting kubernetes client")
 	}
-	return clientset.CoreV1().Services(svc.Namespace).Get(svc.Name, metav1.GetOptions{})
+
+	return client.CoreV1().Services(svc.Namespace).Get(svc.Name, metav1.GetOptions{})
 }