chore(deisctl): refactor config/etcd code

In order to reuse the code for accessing a
back-end config store (e.g. etcd) in the
new upgrade feature, and in order to test
that same code, the previous commit made
some compromises that might have affected
the long-term maintainability of deisctl.
This is a follow-up commit to refactor
some of that.  It establishes cleaner
boundaries between config back-end
model, interface, and that interface's
two implementations-- the existing one
based on etcd and a new, in-memory "mock"
used for testing.

Author: Kent Rancourt

deisctl/client/client.go

@@ -9,6 +9,8 @@ import (
 	"github.com/deis/deis/deisctl/backend"
 	"github.com/deis/deis/deisctl/backend/fleet"
 	"github.com/deis/deis/deisctl/cmd"
+	"github.com/deis/deis/deisctl/config"
+	"github.com/deis/deis/deisctl/config/etcd"
 	"github.com/deis/deis/deisctl/units"
 
 	docopt "github.com/docopt/docopt-go"
@@ -35,7 +37,8 @@ type DeisCtlClient interface {
 
 // Client uses a backend to implement the DeisCtlClient interface.
 type Client struct {
-	Backend backend.Backend
+	Backend       backend.Backend
+	configBackend config.Backend
 }
 
 // NewClient returns a Client using the requested backend.
@@ -57,7 +60,13 @@ func NewClient(requestedBackend string) (*Client, error) {
 	default:
 		return nil, errors.New("invalid backend")
 	}
-	return &Client{Backend: backend}, nil
+
+	cb, err := etcd.NewConfigBackend()
+	if err != nil {
+		return nil, err
+	}
+
+	return &Client{Backend: backend, configBackend: cb}, nil
 }
 
 // UpgradePrep prepares a running cluster to be upgraded
@@ -85,7 +94,7 @@ Usage:
 		return err
 	}
 
-	return cmd.UpgradeTakeover(c.Backend)
+	return cmd.UpgradeTakeover(c.Backend, c.configBackend)
 }
 
 // RollingRestart attempts a rolling restart of an instance unit
@@ -151,7 +160,7 @@ Examples:
 		key = args["<key>"].([]string)
 	}
 
-	return cmd.Config(args["<target>"].(string), action, key)
+	return cmd.Config(args["<target>"].(string), action, key, c.configBackend)
 }
 
 // Install loads the definitions of components from local unit files.
@@ -186,7 +195,7 @@ Options:
 	}
 	cmd.RouterMeshSize = uint8(parsedValue)
 
-	return cmd.Install(args["<target>"].([]string), c.Backend, cmd.CheckRequiredKeys)
+	return cmd.Install(args["<target>"].([]string), c.Backend, c.configBackend, cmd.CheckRequiredKeys)
 }
 
 // Journal prints log output for the specified components.

deisctl/cmd/cmd.go

@@ -107,14 +107,14 @@ func RollingRestart(target string, b backend.Backend) error {
 	return nil
 }
 
-// CheckRequiredKeys exist in etcd
-func CheckRequiredKeys() error {
-	if err := config.CheckConfig("/deis/platform/", "domain"); err != nil {
+// CheckRequiredKeys exist in config backend
+func CheckRequiredKeys(cb config.Backend) error {
+	if err := config.CheckConfig("/deis/platform/", "domain", cb); err != nil {
 		return fmt.Errorf(`Missing platform domain, use:
 deisctl config platform set domain=<your-domain>`)
 	}
 
-	if err := config.CheckConfig("/deis/platform/", "sshPrivateKey"); err != nil {
+	if err := config.CheckConfig("/deis/platform/", "sshPrivateKey", cb); err != nil {
 		fmt.Printf(`Warning: Missing sshPrivateKey, "deis run" will be unavailable. Use:
 deisctl config platform set sshPrivateKey=<path-to-key>
 `)
@@ -286,15 +286,15 @@ func Journal(targets []string, b backend.Backend) error {
 
 // Install loads the definitions of components from local unit files.
 // After Install, the components will be available to Start.
-func Install(targets []string, b backend.Backend, checkKeys func() error) error {
+func Install(targets []string, b backend.Backend, cb config.Backend, checkKeys func(config.Backend) error) error {
 
 	// if target is platform, install all services
 	if len(targets) == 1 {
 		switch targets[0] {
 		case PlatformCommand:
-			return InstallPlatform(b, checkKeys, false)
+			return InstallPlatform(b, cb, checkKeys, false)
 		case StatelessPlatformCommand:
-			return InstallPlatform(b, checkKeys, true)
+			return InstallPlatform(b, cb, checkKeys, true)
 		case mesos:
 			return InstallMesos(b)
 		case swarm:
@@ -439,11 +439,11 @@ func splitScaleTarget(target string) (c string, num int, err error) {
 
 // Config gets or sets a configuration value from the cluster.
 //
-// A configuration value is stored and retrieved from a key/value store (in this case, etcd)
+// A configuration value is stored and retrieved from a key/value store
 // at /deis/<component>/<config>. Configuration values are typically used for component-level
 // configuration, such as enabling TLS for the routers.
-func Config(target string, action string, key []string) error {
-	if err := config.Config(target, action, key); err != nil {
+func Config(target string, action string, key []string, cb config.Backend) error {
+	if err := config.Config(target, action, key, cb); err != nil {
 		return err
 	}
 	return nil

deisctl/cmd/cmd_test.go

@@ -13,7 +13,8 @@ import (
 	"testing"
 
 	"github.com/deis/deis/deisctl/backend"
-	"github.com/deis/deis/deisctl/etcdclient"
+	"github.com/deis/deis/deisctl/config"
+	"github.com/deis/deis/deisctl/config/model"
 	"github.com/deis/deis/deisctl/test/mock"
 	"github.com/deis/deis/deisctl/units"
 )
@@ -86,7 +87,7 @@ func (backend *backendStub) SSHExec(target, command string) error {
 
 var _ backend.Backend = &backendStub{}
 
-func fakeCheckKeys() error {
+func fakeCheckKeys(cb config.Backend) error {
 	return nil
 }
 
@@ -306,7 +307,7 @@ func TestUpgradePrep(t *testing.T) {
 
 func TestUpgradeTakeover(t *testing.T) {
 	t.Parallel()
-	testMock := mock.Client{Expected: []*etcdclient.ServiceKey{{Key: "/deis/services/app1", Value: "foo", TTL: 10},
+	testMock := mock.ConfigBackend{Expected: []*model.ConfigNode{{Key: "/deis/services/app1", Value: "foo", TTL: 10},
 		{Key: "/deis/services/app2", Value: "8000", TTL: 10}}}
 
 	b := backendStub{}
@@ -477,9 +478,11 @@ func TestInstall(t *testing.T) {
 	t.Parallel()
 
 	b := backendStub{}
+	cb := mock.ConfigBackend{}
+
 	expected := []string{"router@1", "router@2"}
 
-	Install(expected, &b, fakeCheckKeys)
+	Install(expected, &b, &cb, fakeCheckKeys)
 
 	if !reflect.DeepEqual(b.installedUnits, expected) {
 		t.Error(fmt.Errorf("Expected %v, Got %v", expected, b.installedUnits))
@@ -490,11 +493,13 @@ func TestInstallPlatform(t *testing.T) {
 	t.Parallel()
 
 	b := backendStub{}
+	cb := mock.ConfigBackend{}
+
 	expected := []string{"store-daemon", "store-monitor", "store-metadata", "store-volume",
 		"store-gateway@1", "logger", "logspout", "database", "registry@1",
 		"controller", "builder", "publisher", "router@1", "router@2", "router@3"}
 
-	Install([]string{"platform"}, &b, fakeCheckKeys)
+	Install([]string{"platform"}, &b, &cb, fakeCheckKeys)
 
 	if !reflect.DeepEqual(b.installedUnits, expected) {
 		t.Error(fmt.Errorf("Expected %v, Got %v", expected, b.installedUnits))
@@ -505,12 +510,14 @@ func TestInstallPlatformWithCustomRouterMeshSize(t *testing.T) {
 	t.Parallel()
 
 	b := backendStub{}
+	cb := mock.ConfigBackend{}
+
 	expected := []string{"store-daemon", "store-monitor", "store-metadata", "store-volume",
 		"store-gateway@1", "logger", "logspout", "database", "registry@1",
 		"controller", "builder", "publisher", "router@1", "router@2", "router@3", "router@4", "router@5"}
 	RouterMeshSize = 5
 
-	Install([]string{"platform"}, &b, fakeCheckKeys)
+	Install([]string{"platform"}, &b, &cb, fakeCheckKeys)
 	RouterMeshSize = DefaultRouterMeshSize
 
 	if !reflect.DeepEqual(b.installedUnits, expected) {
@@ -522,10 +529,12 @@ func TestInstallStatelessPlatform(t *testing.T) {
 	t.Parallel()
 
 	b := backendStub{}
+	cb := mock.ConfigBackend{}
+
 	expected := []string{"logspout", "registry@1",
 		"controller", "builder", "publisher", "router@1", "router@2", "router@3"}
 
-	Install([]string{"stateless-platform"}, &b, fakeCheckKeys)
+	Install([]string{"stateless-platform"}, &b, &cb, fakeCheckKeys)
 
 	if !reflect.DeepEqual(b.installedUnits, expected) {
 		t.Error(fmt.Errorf("Expected %v, Got %v", expected, b.installedUnits))
@@ -536,9 +545,11 @@ func TestInstallSwarm(t *testing.T) {
 	t.Parallel()
 
 	b := backendStub{}
+	cb := mock.ConfigBackend{}
+
 	expected := []string{"swarm-node", "swarm-manager"}
 
-	Install([]string{"swarm"}, &b, fakeCheckKeys)
+	Install([]string{"swarm"}, &b, &cb, fakeCheckKeys)
 
 	if !reflect.DeepEqual(b.installedUnits, expected) {
 		t.Error(fmt.Errorf("Expected %v, Got %v", expected, b.installedUnits))

deisctl/cmd/platform.go

@@ -6,14 +6,15 @@ import (
 	"sync"
 
 	"github.com/deis/deis/deisctl/backend"
+	"github.com/deis/deis/deisctl/config"
 	"github.com/deis/deis/pkg/prettyprint"
 )
 
 // InstallPlatform loads all components' definitions from local unit files.
 // After InstallPlatform, all components will be available for StartPlatform.
-func InstallPlatform(b backend.Backend, checkKeys func() error, stateless bool) error {
+func InstallPlatform(b backend.Backend, cb config.Backend, checkKeys func(config.Backend) error, stateless bool) error {
 
-	if err := checkKeys(); err != nil {
+	if err := checkKeys(cb); err != nil {
 		return err
 	}
 

deisctl/cmd/upgrade.go

@@ -5,7 +5,8 @@ import (
 	"sync"
 
 	"github.com/deis/deis/deisctl/backend"
-	"github.com/deis/deis/deisctl/etcdclient"
+	"github.com/deis/deis/deisctl/config"
+	"github.com/deis/deis/deisctl/config/model"
 )
 
 // UpgradePrep stops and uninstalls all components except router and publisher
@@ -43,18 +44,18 @@ func UpgradePrep(b backend.Backend) error {
 	return nil
 }
 
-func listPublishedServices(etcdClient etcdclient.Client) ([]*etcdclient.ServiceKey, error) {
-	nodes, err := etcdClient.GetRecursive("deis/services")
+func listPublishedServices(cb config.Backend) ([]*model.ConfigNode, error) {
+	nodes, err := cb.GetRecursive("deis/services")
 	if err != nil {
 		return nil, err
 	}
 
 	return nodes, nil
 }
 
-func republishServices(ttl uint64, nodes []*etcdclient.ServiceKey, etcdClient etcdclient.Client) error {
+func republishServices(ttl uint64, nodes []*model.ConfigNode, cb config.Backend) error {
 	for _, node := range nodes {
-		_, err := etcdClient.Update(node.Key, node.Value, ttl)
+		_, err := cb.SetWithTTL(node.Key, node.Value, ttl)
 		if err != nil {
 			return err
 		}
@@ -64,23 +65,19 @@ func republishServices(ttl uint64, nodes []*etcdclient.ServiceKey, etcdClient et
 }
 
 // UpgradeTakeover gracefully starts a platform stopped with UpgradePrep
-func UpgradeTakeover(b backend.Backend) error {
-	etcdClient, err := etcdclient.GetEtcdClient()
-	if err != nil {
-		return err
-	}
+func UpgradeTakeover(b backend.Backend, cb config.Backend) error {
 
-	if err := doUpgradeTakeOver(b, etcdClient); err != nil {
+	if err := doUpgradeTakeOver(b, cb); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func doUpgradeTakeOver(b backend.Backend, etcdClient etcdclient.Client) error {
+func doUpgradeTakeOver(b backend.Backend, cb config.Backend) error {
 	var wg sync.WaitGroup
 
-	nodes, err := listPublishedServices(etcdClient)
+	nodes, err := listPublishedServices(cb)
 	if err != nil {
 		return err
 	}
@@ -90,7 +87,7 @@ func doUpgradeTakeOver(b backend.Backend, etcdClient etcdclient.Client) error {
 	b.Destroy([]string{"publisher"}, &wg, Stdout, Stderr)
 	wg.Wait()
 
-	if err := republishServices(1800, nodes, etcdClient); err != nil {
+	if err := republishServices(1800, nodes, cb); err != nil {
 		return err
 	}
 

deisctl/config/backend.go

@@ -0,0 +1,12 @@
+package config
+
+import "github.com/deis/deis/deisctl/config/model"
+
+// Backend is an interface for any sort of underlying key/value config store
+type Backend interface {
+	Get(string) (string, error)
+	Set(string, string) (string, error)
+	SetWithTTL(string, string, uint64) (string, error)
+	Delete(string) error
+	GetRecursive(string) ([]*model.ConfigNode, error)
+}