From 84e6727884fbd4271d528551842702e48861d12f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Fri, 8 Dec 2017 14:13:56 +0000
Subject: [PATCH] Add a default constructor for EtcdConfig

---
 pkg/framework/test/etcd_config.go      | 28 ++++++++-
 pkg/framework/test/etcd_config_test.go | 80 ++++++++++++++++++--------
 2 files changed, 83 insertions(+), 25 deletions(-)

diff --git a/pkg/framework/test/etcd_config.go b/pkg/framework/test/etcd_config.go
index 3b112eec4..20d57c38f 100644
--- a/pkg/framework/test/etcd_config.go
+++ b/pkg/framework/test/etcd_config.go
@@ -1,6 +1,10 @@
 package test
 
-import "github.com/asaskevich/govalidator"
+import (
+	"fmt"
+
+	"github.com/asaskevich/govalidator"
+)
 
 // EtcdConfig is a struct holding data to configure the Etcd process
 type EtcdConfig struct {
@@ -8,6 +12,28 @@ type EtcdConfig struct {
 	PeerURL   string `valid:"required,url"`
 }
 
+var etcdPortFinder = DefaultPortFinder
+
+// NewEtcdConfig returns a simple config for Etcd with sane default values
+func NewEtcdConfig() (etcdConfig *EtcdConfig, err error) {
+	conf := &EtcdConfig{}
+	host := "localhost"
+
+	if port, addr, err := etcdPortFinder(host); err == nil {
+		conf.ClientURL = fmt.Sprintf("http://%s:%d", addr, port)
+	} else {
+		return nil, err
+	}
+
+	if port, addr, err := etcdPortFinder(host); err == nil {
+		conf.PeerURL = fmt.Sprintf("http://%s:%d", addr, port)
+	} else {
+		return nil, err
+	}
+
+	return conf, nil
+}
+
 // Validate checks that the config contains only valid URLs
 func (c *EtcdConfig) Validate() error {
 	_, err := govalidator.ValidateStruct(c)
diff --git a/pkg/framework/test/etcd_config_test.go b/pkg/framework/test/etcd_config_test.go
index 7e01d7701..88a53a7bf 100644
--- a/pkg/framework/test/etcd_config_test.go
+++ b/pkg/framework/test/etcd_config_test.go
@@ -1,36 +1,68 @@
-package test_test
+package test
 
 import (
-	. "k8s.io/kubectl/pkg/framework/test"
+	"fmt"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("EtcdConfig", func() {
-	It("does not error on valid config", func() {
-		conf := &EtcdConfig{
-			PeerURL:   "http://this.is.some.url:1234",
-			ClientURL: "http://this.is.another.url/",
-		}
-		err := conf.Validate()
-		Expect(err).NotTo(HaveOccurred())
-	})
+	Context("Constructor", func() {
+		var (
+			previousPortFinder PortFinder
+		)
+		BeforeEach(func() {
+			previousPortFinder = etcdPortFinder
+		})
+		AfterEach(func() {
+			etcdPortFinder = previousPortFinder
+		})
 
-	It("errors on empty config", func() {
-		conf := &EtcdConfig{}
-		err := conf.Validate()
-		Expect(err).To(MatchError(ContainSubstring("PeerURL: non zero value required")))
-		Expect(err).To(MatchError(ContainSubstring("ClientURL: non zero value required")))
-	})
+		It("sets some sane default URLs", func() {
+			etcdPortFinder = func(host string) (port int, addr string, err error) {
+				return 42, "1.2.3.4", nil
+			}
 
-	It("errors on malformed URLs", func() {
-		conf := &EtcdConfig{
-			PeerURL:   "something not URLish",
-			ClientURL: "something not URLesc",
-		}
-		err := conf.Validate()
-		Expect(err).To(MatchError(ContainSubstring("PeerURL: something not URLish does not validate as url")))
-		Expect(err).To(MatchError(ContainSubstring("ClientURL: something not URLesc does not validate as url")))
+			conf, err := NewEtcdConfig()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(conf.ClientURL).To(Equal("http://1.2.3.4:42"))
+		})
+
+		It("prop[agates and error while trying to find a port", func() {
+			etcdPortFinder = func(host string) (port int, addr string, err error) {
+				return 43, "5.6.7.8", fmt.Errorf("oh nooos, no port")
+			}
+
+			_, err := NewEtcdConfig()
+			Expect(err).To(MatchError(ContainSubstring("oh nooos, no port")))
+		})
+	})
+	Context("Validate()", func() {
+		It("does not error on valid config", func() {
+			conf := &EtcdConfig{
+				PeerURL:   "http://this.is.some.url:1234",
+				ClientURL: "http://this.is.another.url/",
+			}
+			err := conf.Validate()
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("errors on empty config", func() {
+			conf := &EtcdConfig{}
+			err := conf.Validate()
+			Expect(err).To(MatchError(ContainSubstring("PeerURL: non zero value required")))
+			Expect(err).To(MatchError(ContainSubstring("ClientURL: non zero value required")))
+		})
+
+		It("errors on malformed URLs", func() {
+			conf := &EtcdConfig{
+				PeerURL:   "something not URLish",
+				ClientURL: "something not URLesc",
+			}
+			err := conf.Validate()
+			Expect(err).To(MatchError(ContainSubstring("PeerURL: something not URLish does not validate as url")))
+			Expect(err).To(MatchError(ContainSubstring("ClientURL: something not URLesc does not validate as url")))
+		})
 	})
 })