Refactor controller to make testing easier.

Avoid relying on Clientset structure to call Kubernetes API functions.
While Clientset is a convinient "catch-all" abstraction for calling
REST API related to different Kubernetes objects, it's impossible to
mock. Replacing it wih the kubernetes.Interface would be quite
straightforward, but would require an exra level of mocked interfaces,
because of the versioning. Instead, a new interface is defined, which
contains only the objects we need of the pre-defined versions.

Add one more test.

cmd/main.go

@@ -8,6 +8,7 @@ import (
 	"sync"
 	"syscall"
 
+	"github.com/zalando-incubator/postgres-operator/pkg/cluster"
 	"github.com/zalando-incubator/postgres-operator/pkg/controller"
 	"github.com/zalando-incubator/postgres-operator/pkg/spec"
 	"github.com/zalando-incubator/postgres-operator/pkg/util/config"
@@ -60,7 +61,7 @@ func ControllerConfig() *controller.Config {
 
 	return &controller.Config{
 		RestConfig: restConfig,
-		KubeClient: client,
+		KubeClient: cluster.NewFromKubernetesInterface(client),
 		RestClient: restClient,
 	}
 }

pkg/cluster/cluster.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 
 	"github.com/Sirupsen/logrus"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/api"
 	"k8s.io/client-go/pkg/api/v1"
 	"k8s.io/client-go/pkg/apis/apps/v1beta1"
@@ -36,7 +35,7 @@ var (
 
 // Config contains operator-wide clients and configuration used from a cluster. TODO: remove struct duplication.
 type Config struct {
-	KubeClient          *kubernetes.Clientset //TODO: move clients to the better place?
+	KubeClient          KubernetesClient
 	RestClient          *rest.RESTClient
 	RestConfig          *rest.Config
 	TeamsAPIClient      *teams.API

pkg/cluster/types.go

@@ -1,8 +1,41 @@
 package cluster
 
+import (
+	"k8s.io/client-go/kubernetes"
+	v1beta1 "k8s.io/client-go/kubernetes/typed/apps/v1beta1"
+	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
+	extensions "k8s.io/client-go/kubernetes/typed/extensions/v1beta1"
+)
+
 type PostgresRole string
 
 const (
 	Master  PostgresRole = "master"
 	Replica PostgresRole = "replica"
 )
+
+type KubernetesClient struct {
+	v1core.SecretsGetter
+	v1core.ServicesGetter
+	v1core.EndpointsGetter
+	v1core.PodsGetter
+	v1core.PersistentVolumesGetter
+	v1core.PersistentVolumeClaimsGetter
+	v1core.ConfigMapsGetter
+	v1beta1.StatefulSetsGetter
+	extensions.ThirdPartyResourcesGetter
+}
+
+func NewFromKubernetesInterface(src kubernetes.Interface) (c KubernetesClient) {
+	c = KubernetesClient{}
+	c.PodsGetter = src.CoreV1()
+	c.ServicesGetter = src.CoreV1()
+	c.EndpointsGetter = src.CoreV1()
+	c.SecretsGetter = src.CoreV1()
+	c.ConfigMapsGetter = src.CoreV1()
+	c.PersistentVolumeClaimsGetter = src.CoreV1()
+	c.PersistentVolumesGetter = src.CoreV1()
+	c.StatefulSetsGetter = src.AppsV1beta1()
+	c.ThirdPartyResourcesGetter = src.ExtensionsV1beta1()
+	return
+}

pkg/controller/controller.go

@@ -5,7 +5,6 @@ import (
 	"sync"
 
 	"github.com/Sirupsen/logrus"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/api/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
@@ -19,14 +18,15 @@ import (
 
 type Config struct {
 	RestConfig          *rest.Config
-	KubeClient          *kubernetes.Clientset
+	KubeClient          cluster.KubernetesClient
 	RestClient          *rest.RESTClient
 	TeamsAPIClient      *teams.API
 	InfrastructureRoles map[string]spec.PgUser
 }
 
 type Controller struct {
 	Config
+
 	opConfig *config.Config
 	logger   *logrus.Entry
 
@@ -82,7 +82,7 @@ func (c *Controller) initController() {
 		c.logger.Fatalf("could not register ThirdPartyResource: %v", err)
 	}
 
-	if infraRoles, err := c.getInfrastructureRoles(); err != nil {
+	if infraRoles, err := c.getInfrastructureRoles(&c.opConfig.InfrastructureRolesSecretName); err != nil {
 		c.logger.Warningf("could not get infrastructure roles: %v", err)
 	} else {
 		c.InfrastructureRoles = infraRoles

pkg/controller/pod.go

@@ -29,7 +29,7 @@ func (c *Controller) podListFunc(options api.ListOptions) (runtime.Object, error
 		TimeoutSeconds:  options.TimeoutSeconds,
 	}
 
-	return c.KubeClient.CoreV1().Pods(c.opConfig.Namespace).List(opts)
+	return c.KubeClient.Pods(c.opConfig.Namespace).List(opts)
 }
 
 func (c *Controller) podWatchFunc(options api.ListOptions) (watch.Interface, error) {
@@ -52,7 +52,7 @@ func (c *Controller) podWatchFunc(options api.ListOptions) (watch.Interface, err
 		TimeoutSeconds:  options.TimeoutSeconds,
 	}
 
-	return c.KubeClient.CoreV1Client.Pods(c.opConfig.Namespace).Watch(opts)
+	return c.KubeClient.Pods(c.opConfig.Namespace).Watch(opts)
 }
 
 func (c *Controller) podAdd(obj interface{}) {

pkg/controller/util.go

@@ -51,7 +51,7 @@ func (c *Controller) createTPR() error {
 	TPRName := fmt.Sprintf("%s.%s", constants.TPRName, constants.TPRVendor)
 	tpr := thirdPartyResource(TPRName)
 
-	_, err := c.KubeClient.ExtensionsV1beta1().ThirdPartyResources().Create(tpr)
+	_, err := c.KubeClient.ThirdPartyResources().Create(tpr)
 	if err != nil {
 		if !k8sutil.ResourceAlreadyExists(err) {
 			return err
@@ -64,17 +64,17 @@ func (c *Controller) createTPR() error {
 	return k8sutil.WaitTPRReady(c.RestClient, c.opConfig.TPR.ReadyWaitInterval, c.opConfig.TPR.ReadyWaitTimeout, c.opConfig.Namespace)
 }
 
-func (c *Controller) getInfrastructureRoles() (result map[string]spec.PgUser, err error) {
-	if c.opConfig.InfrastructureRolesSecretName == (spec.NamespacedName{}) {
+func (c *Controller) getInfrastructureRoles(rolesSecret *spec.NamespacedName) (result map[string]spec.PgUser, err error) {
+	if *rolesSecret == (spec.NamespacedName{}) {
 		// we don't have infrastructure roles defined, bail out
 		return nil, nil
 	}
 
 	infraRolesSecret, err := c.KubeClient.
-		Secrets(c.opConfig.InfrastructureRolesSecretName.Namespace).
-		Get(c.opConfig.InfrastructureRolesSecretName.Name)
+		Secrets(rolesSecret.Namespace).
+		Get(rolesSecret.Name)
 	if err != nil {
-		c.logger.Debugf("Infrastructure roles secret name: %s", c.opConfig.InfrastructureRolesSecretName)
+		c.logger.Debugf("Infrastructure roles secret name: %s", *rolesSecret)
 		return nil, fmt.Errorf("could not get infrastructure roles secret: %v", err)
 	}
 

pkg/controller/util_test.go

@@ -1,25 +1,67 @@
 package controller
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
 
+	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/pkg/api/v1"
+
+	"github.com/zalando-incubator/postgres-operator/pkg/cluster"
 	"github.com/zalando-incubator/postgres-operator/pkg/spec"
 	"github.com/zalando-incubator/postgres-operator/pkg/util/config"
 )
 
+const (
+	testInfrastructureRolesSecretName = "infrastructureroles-test"
+)
+
+type mockSecret struct {
+	v1core.SecretInterface
+}
+
+func (c *mockSecret) Get(name string) (*v1.Secret, error) {
+	if name != testInfrastructureRolesSecretName {
+		return nil, fmt.Errorf("NotFound")
+	}
+	secret := &v1.Secret{}
+	secret.Name = mockController.opConfig.ClusterNameLabel
+	secret.Data = map[string][]byte{
+		"user1":     []byte("testrole"),
+		"password1": []byte("testpassword"),
+		"inrole1":   []byte("testinrole"),
+	}
+	return secret, nil
+
+}
+
+type MockSecretGetter struct {
+}
+
+func (c *MockSecretGetter) Secrets(namespace string) v1core.SecretInterface {
+	return &mockSecret{}
+}
+
+func newMockKubernetesClient() cluster.KubernetesClient {
+	return cluster.KubernetesClient{SecretsGetter: &MockSecretGetter{}}
+}
+
 func newMockController() *Controller {
 	controller := NewController(&Config{}, &config.Config{})
 	controller.opConfig.ClusterNameLabel = "cluster-name"
+	controller.opConfig.InfrastructureRolesSecretName =
+		spec.NamespacedName{v1.NamespaceDefault, testInfrastructureRolesSecretName}
 	controller.opConfig.Workers = 4
+	controller.KubeClient = newMockKubernetesClient()
 	return controller
 }
 
 var mockController = newMockController()
 
 func TestPodClusterName(t *testing.T) {
 	var testTable = []struct {
-		in *v1.Pod
+		in       *v1.Pod
 		expected spec.NamespacedName
 	}{
 		{
@@ -30,15 +72,15 @@ func TestPodClusterName(t *testing.T) {
 			&v1.Pod{
 				ObjectMeta: v1.ObjectMeta{
 					Namespace: v1.NamespaceDefault,
-					Labels:  map[string]string{
-							mockController.opConfig.ClusterNameLabel: "testcluster",
-						},
+					Labels: map[string]string{
+						mockController.opConfig.ClusterNameLabel: "testcluster",
 					},
+				},
 			},
 			spec.NamespacedName{v1.NamespaceDefault, "testcluster"},
 		},
 	}
-	for _, test:= range(testTable) {
+	for _, test := range testTable {
 		resp := mockController.podClusterName(test.in)
 		if resp != test.expected {
 			t.Errorf("expected response %v does not match the actual %v", test.expected, resp)
@@ -48,22 +90,65 @@ func TestPodClusterName(t *testing.T) {
 
 func TestClusterWorkerID(t *testing.T) {
 	var testTable = []struct {
-		in spec.NamespacedName
+		in       spec.NamespacedName
 		expected uint32
 	}{
 		{
-			in: spec.NamespacedName{"foo", "bar"},
+			in:       spec.NamespacedName{"foo", "bar"},
 			expected: 2,
 		},
 		{
-			in: spec.NamespacedName{"default", "testcluster"},
+			in:       spec.NamespacedName{"default", "testcluster"},
 			expected: 3,
 		},
 	}
-	for _, test := range(testTable) {
+	for _, test := range testTable {
 		resp := mockController.clusterWorkerID(test.in)
 		if resp != test.expected {
 			t.Errorf("expected response %v does not match the actual %v", test.expected, resp)
 		}
 	}
 }
+
+func TestGetInfrastructureRoles(t *testing.T) {
+	var testTable = []struct {
+		secretName    spec.NamespacedName
+		expectedRoles map[string]spec.PgUser
+		expectedError error
+	}{
+		{
+			spec.NamespacedName{},
+			nil,
+			nil,
+		},
+		{
+			spec.NamespacedName{v1.NamespaceDefault, "null"},
+			nil,
+			fmt.Errorf(`could not get infrastructure roles secret: NotFound`),
+		},
+		{
+			spec.NamespacedName{v1.NamespaceDefault, testInfrastructureRolesSecretName},
+			map[string]spec.PgUser{
+				"testrole": {
+					"testrole",
+					"testpassword",
+					nil,
+					[]string{"testinrole"},
+				},
+			},
+			nil,
+		},
+	}
+	for _, test := range testTable {
+		roles, err := mockController.getInfrastructureRoles(&test.secretName)
+		if err != test.expectedError {
+			if err != nil && test.expectedError != nil && err.Error() == test.expectedError.Error() {
+				continue
+			}
+			t.Errorf("expected error '%v' does not match the actual error '%v'", test.expectedError, err)
+		}
+		if !reflect.DeepEqual(roles, test.expectedRoles) {
+			t.Errorf("expected roles output %v does not match the actual %v", test.expectedRoles, roles)
+		}
+	}
+}

pkg/spec/types.go

@@ -1,9 +1,9 @@
 package spec
 
 import (
+	"database/sql"
 	"fmt"
 	"strings"
-	"database/sql"
 
 	"k8s.io/client-go/pkg/api/v1"
 	"k8s.io/client-go/pkg/types"