Repository: m3db/m3db-operator
Pull request #192 (2020-02-10 18:12:53)

[k8sops] Generalize m3cluster environment helper

codecov[bot]: # [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=h1) Report
> Merging [#192](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=desc) into [master](https://codecov.io/gh/m3db/m3db-operator/commit/bf5714421007bd8771f164ae41e0725742e7a0ab?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `92.85%`.


[![Impacted file tree graph](https://codecov.io/gh/m3db/m3db-operator/pull/192/graphs/tree.svg?width=650&token=jtHDyWhsto&height=150&src=pr)](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
- Coverage   75.17%   75.14%   -0.04%     
==========================================
  Files          30       30              
  Lines        2087     2084       -3     
==========================================
- Hits         1569     1566       -3     
  Misses        391      391              
  Partials      127      127
```


------

[Continue to review full report at Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=footer). Last update [bf57144...66a5197](https://codecov.io/gh/m3db/m3db-operator/pull/192?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/controller/m3admin_client.go b/pkg/controller/m3admin_client.go
index 1c0da57..8b79670 100644
--- a/pkg/controller/m3admin_client.go
+++ b/pkg/controller/m3admin_client.go
@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/m3db/m3db-operator/pkg/k8sops"
 	"github.com/m3db/m3db-operator/pkg/k8sops/m3db"
 	"github.com/m3db/m3db-operator/pkg/m3admin"
 	"github.com/m3db/m3db-operator/pkg/m3admin/namespace"
@@ -107,7 +108,7 @@ func newMultiAdminClient(adminOpts []m3admin.Option, logger *zap.Logger) *multiA
 }
 
 func (m *multiAdminClient) adminClientForCluster(cluster metav1.ObjectMetaAccessor) m3admin.Client {
-	env := m3db.DefaultM3ClusterEnvironmentName(cluster)
+	env := k8sops.DefaultM3ClusterEnvironmentName(cluster)
 	opts := append(m.adminOpts, m3admin.WithEnvironment(env))
 	return m.adminClientFn(opts...)
 }
diff --git a/pkg/k8sops/k8sops.go b/pkg/k8sops/k8sops.go
new file mode 100644
index 0000000..64979a7
--- /dev/null
+++ b/pkg/k8sops/k8sops.go
@@ -0,0 +1,34 @@
+// Copyright (c) 2020 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package k8sops
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// DefaultM3ClusterEnvironmentName returns the environment under which cluster
+// topology and runtime configuration will be stored. This ensures that multiple
+// m3db clusters won't conflict with each other when sharing a backing etcd
+// store.
+func DefaultM3ClusterEnvironmentName(obj metav1.ObjectMetaAccessor) string {
+	m := obj.GetObjectMeta()
+	return m.GetNamespace() + "/" + m.GetName()
+}
diff --git a/pkg/k8sops/m3db/common.go b/pkg/k8sops/m3db/common.go
index 3211547..903a2dd 100644
--- a/pkg/k8sops/m3db/common.go
+++ b/pkg/k8sops/m3db/common.go
@@ -48,6 +48,6 @@ func CoordinatorServiceName(clusterName string) string {
 
 // TODO(schallert): should figure out a better way to abstract this other than
 // exposing all of CoreV1()
-func (k *k8sops) Events(namespace string) typedcorev1.EventInterface {
+func (k *k8sWrapper) Events(namespace string) typedcorev1.EventInterface {
 	return k.kclient.CoreV1().Events(namespace)
 }
diff --git a/pkg/k8sops/m3db/config_map.go b/pkg/k8sops/m3db/config_map.go
index e1ebb79..82a62d3 100644
--- a/pkg/k8sops/m3db/config_map.go
+++ b/pkg/k8sops/m3db/config_map.go
@@ -26,6 +26,7 @@ import (
 	"text/template"
 
 	myspec "github.com/m3db/m3db-operator/pkg/apis/m3dboperator/v1alpha1"
+	"github.com/m3db/m3db-operator/pkg/k8sops"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -76,7 +77,7 @@ func GenerateDefaultConfigMap(cluster *myspec.M3DBCluster) (*corev1.ConfigMap, e
 	}
 
 	config := &configData{
-		Env:            DefaultM3ClusterEnvironmentName(cluster),
+		Env:            k8sops.DefaultM3ClusterEnvironmentName(cluster),
 		Endpoints:      cluster.Spec.EtcdEndpoints,
 		CarbonIngester: cluster.Spec.EnableCarbonIngester,
 	}
@@ -105,15 +106,6 @@ func defaultConfigMapName(clusterName string) string {
 	return "m3db-config-map-" + clusterName
 }
 
-// DefaultM3ClusterEnvironmentName returns the environment under which cluster
-// topology and runtime configuration will be stored. This ensures that multiple
-// m3db clusters won't conflict with each other when sharing a backing etcd
-// store.
-func DefaultM3ClusterEnvironmentName(obj metav1.ObjectMetaAccessor) string {
-	m := obj.GetObjectMeta()
-	return m.GetNamespace() + "/" + m.GetName()
-}
-
 // Build the volume for the pod and the volumeMount for the container containing
 // necessary config map info. If a user specified a configMap of their own we'll
 // mount it, otherwise we mount the default one (the controller is expected to
diff --git a/pkg/k8sops/m3db/crd.go b/pkg/k8sops/m3db/crd.go
index b306943..9c2be2f 100644
--- a/pkg/k8sops/m3db/crd.go
+++ b/pkg/k8sops/m3db/crd.go
@@ -35,7 +35,7 @@ import (
 	"go.uber.org/zap"
 )
 
-func (k *k8sops) CreateOrUpdateCRD(name string, enableValidation bool) error {
+func (k *k8sWrapper) CreateOrUpdateCRD(name string, enableValidation bool) error {
 	if name != myspec.M3DBClustersName {
 		return fmt.Errorf("unrecognized CRD name '%s'", name)
 	}
@@ -74,7 +74,7 @@ func (k *k8sops) CreateOrUpdateCRD(name string, enableValidation bool) error {
 
 // waitForCRDReady waits until we can list resources of the given type,
 // indicating that the resource is ready.
-func (k *k8sops) waitForCRDReady(name string) error {
+func (k *k8sWrapper) waitForCRDReady(name string) error {
 	if name != myspec.M3DBClustersName {
 		return fmt.Errorf("unrecognized CRD name '%s'", name)
 	}
diff --git a/pkg/k8sops/m3db/k8sops.go b/pkg/k8sops/m3db/k8sops.go
index 70fd46b..6f7246a 100644
--- a/pkg/k8sops/m3db/k8sops.go
+++ b/pkg/k8sops/m3db/k8sops.go
@@ -32,18 +32,18 @@ import (
 // Option provides an interface that can be used for setter options with the
 // constructor
 type Option interface {
-	execute(*k8sops) error
+	execute(*k8sWrapper) error
 }
 
-type optionFn func(k *k8sops) error
+type optionFn func(k *k8sWrapper) error
 
-func (fn optionFn) execute(k *k8sops) error {
+func (fn optionFn) execute(k *k8sWrapper) error {
 	return fn(k)
 }
 
 // WithLogger is a setter to override the default logger
 func WithLogger(logger *zap.Logger) Option {
-	return optionFn(func(k *k8sops) error {
+	return optionFn(func(k *k8sWrapper) error {
 		k.logger = logger
 		return nil
 	})
@@ -51,7 +51,7 @@ func WithLogger(logger *zap.Logger) Option {
 
 // WithCRDClient is a setter to override the default CRD client
 func WithCRDClient(crdClient genclient.Interface) Option {
-	return optionFn(func(k *k8sops) error {
+	return optionFn(func(k *k8sWrapper) error {
 		k.crdClient = crdClient
 		return nil
 	})
@@ -59,7 +59,7 @@ func WithCRDClient(crdClient genclient.Interface) Option {
 
 // WithKClient is a setter to override the default Kubernetes client
 func WithKClient(kclient kubernetes.Interface) Option {
-	return optionFn(func(k *k8sops) error {
+	return optionFn(func(k *k8sWrapper) error {
 		k.kclient = kclient
 		return nil
 	})
@@ -67,14 +67,14 @@ func WithKClient(kclient kubernetes.Interface) Option {
 
 // WithExtClient is a setter to override the default Extensions Client
 func WithExtClient(extClient apiextensionsclient.Interface) Option {
-	return optionFn(func(k *k8sops) error {
+	return optionFn(func(k *k8sWrapper) error {
 		k.kubeExt = extClient
 		return nil
 	})
 }
 
-// k8sops defines the kube object
-type k8sops struct {
+// k8sWrapper encapsulates kubernetes API client dependencies.
+type k8sWrapper struct {
 	crdClient genclient.Interface
 	kclient   kubernetes.Interface
 	kubeExt   apiextensionsclient.Interface
@@ -83,7 +83,7 @@ type k8sops struct {
 
 // New creates a new instance of k8sops
 func New(opts ...Option) (K8sops, error) {
-	k8 := &k8sops{}
+	k8 := &k8sWrapper{}
 	for _, o := range opts {
 		if err := o.execute(k8); err != nil {
 			return nil, err
diff --git a/pkg/k8sops/m3db/services.go b/pkg/k8sops/m3db/services.go
index 3e35160..985680b 100644
--- a/pkg/k8sops/m3db/services.go
+++ b/pkg/k8sops/m3db/services.go
@@ -39,7 +39,7 @@ package m3db
 import (
 	myspec "github.com/m3db/m3db-operator/pkg/apis/m3dboperator/v1alpha1"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -48,7 +48,7 @@ import (
 )
 
 // GetService simply gets a service by name
-func (k *k8sops) GetService(cluster *myspec.M3DBCluster, name string) (*v1.Service, error) {
+func (k *k8sWrapper) GetService(cluster *myspec.M3DBCluster, name string) (*v1.Service, error) {
 	service, err := k.kclient.CoreV1().Services(cluster.GetNamespace()).Get(name, metav1.GetOptions{})
 	if err != nil {
 		return nil, err
@@ -57,13 +57,13 @@ func (k *k8sops) GetService(cluster *myspec.M3DBCluster, name string) (*v1.Servi
 }
 
 // DeleteService simply deletes a service by name
-func (k *k8sops) DeleteService(cluster *myspec.M3DBCluster, name string) error {
+func (k *k8sWrapper) DeleteService(cluster *myspec.M3DBCluster, name string) error {
 	k.logger.Info("deleting service", zap.String("service", name))
 	return k.kclient.CoreV1().Services(cluster.GetNamespace()).Delete(name, &metav1.DeleteOptions{})
 }
 
 // EnsureService will create a service by name if it doesn't exist
-func (k *k8sops) EnsureService(cluster *myspec.M3DBCluster, svc *v1.Service) error {
+func (k *k8sWrapper) EnsureService(cluster *myspec.M3DBCluster, svc *v1.Service) error {
 	_, err := k.GetService(cluster, svc.Name)
 	if errors.IsNotFound(err) {
 		k.logger.Info("service doesn't exist, creating it", zap.String("service", svc.Name))


Test patch:
diff --git a/pkg/k8sops/m3db/crd_test.go b/pkg/k8sops/m3db/crd_test.go
index fdb60d3..a24b1c3 100644
--- a/pkg/k8sops/m3db/crd_test.go
+++ b/pkg/k8sops/m3db/crd_test.go
@@ -39,7 +39,7 @@ import (
 )
 
 func TestCreateOrUpdateCRD(t *testing.T) {
-	k := newFakeK8sops(t).(*k8sops)
+	k := newFakeK8sops(t).(*k8sWrapper)
 
 	err := k.CreateOrUpdateCRD("foo", false)
 	assert.Error(t, err)
@@ -80,7 +80,7 @@ func TestCreateOrUpdateCRD_Err(t *testing.T) {
 		},
 	} {
 		t.Run(test.action, func(t *testing.T) {
-			k := newFakeK8sops(t).(*k8sops)
+			k := newFakeK8sops(t).(*k8sWrapper)
 
 			k.kubeExt.(*kubeExtFake.Clientset).Fake.PrependReactor(test.action, "*", func(action ktesting.Action) (bool, runtime.Object, error) {
 				return true, &extv1beta1.CustomResourceDefinition{}, errors.New("test")
@@ -100,7 +100,7 @@ func TestCreateOrUpdateCRD_Err(t *testing.T) {
 }
 
 func TestWaitForCRDReady(t *testing.T) {
-	k := newFakeK8sops(t).(*k8sops)
+	k := newFakeK8sops(t).(*k8sWrapper)
 
 	err := k.waitForCRDReady("foo")
 	assert.Error(t, err)