From 40566a76e14cb979ed1a3f2532fc01d6daf9b617 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Mon, 8 Jan 2018 10:58:34 +0000
Subject: [PATCH 1/5] Hide the BinPathFinder in our internal package

---
 pkg/framework/test/apiserver.go                 |  9 +++++----
 pkg/framework/test/etcd.go                      |  3 ++-
 .../test/{ => internal}/bin_path_finder.go      | 17 +++++------------
 .../test/{ => internal}/bin_path_finder_test.go | 16 ++++++++--------
 .../test/internal/internal_suite_test.go        | 13 +++++++++++++
 5 files changed, 33 insertions(+), 25 deletions(-)
 rename pkg/framework/test/{ => internal}/bin_path_finder.go (51%)
 rename pkg/framework/test/{ => internal}/bin_path_finder_test.go (78%)
 create mode 100644 pkg/framework/test/internal/internal_suite_test.go

diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go
index 7165cbfb6..85953e686 100644
--- a/pkg/framework/test/apiserver.go
+++ b/pkg/framework/test/apiserver.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
+	"k8s.io/kubectl/pkg/framework/test/internal"
 )
 
 // APIServer knows how to run a kubernetes apiserver.
@@ -19,9 +20,9 @@ type APIServer struct {
 	Address *url.URL
 
 	// Path is the path to the apiserver binary. If this is left as the empty
-	// string, we will use DefaultBinPathFinder to attempt to locate a binary, by
-	// checking for the TEST_ASSET_KUBE_APISERVER environment variable, and the
-	// default test assets directory.
+	// string, we will attempt to locate a binary, by checking for the
+	// TEST_ASSET_KUBE_APISERVER environment variable, and the default test
+	// assets directory.
 	Path string
 
 	// ProcessStarter is a way to hook into how a the APIServer process is started. By default `gexec.Start(...)` is
@@ -112,7 +113,7 @@ func (s *APIServer) Start() error {
 
 func (s *APIServer) ensureInitialized() error {
 	if s.Path == "" {
-		s.Path = DefaultBinPathFinder("kube-apiserver")
+		s.Path = internal.BinPathFinder("kube-apiserver")
 	}
 	if s.Address == nil {
 		am := &DefaultAddressManager{}
diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go
index aeb48758f..8ac3848b9 100644
--- a/pkg/framework/test/etcd.go
+++ b/pkg/framework/test/etcd.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
+	"k8s.io/kubectl/pkg/framework/test/internal"
 )
 
 // Etcd knows how to run an etcd server.
@@ -86,7 +87,7 @@ func (e *Etcd) Start() error {
 
 func (e *Etcd) ensureInitialized() error {
 	if e.Path == "" {
-		e.Path = DefaultBinPathFinder("etcd")
+		e.Path = internal.BinPathFinder("etcd")
 	}
 	if e.Address == nil {
 		am := &DefaultAddressManager{}
diff --git a/pkg/framework/test/bin_path_finder.go b/pkg/framework/test/internal/bin_path_finder.go
similarity index 51%
rename from pkg/framework/test/bin_path_finder.go
rename to pkg/framework/test/internal/bin_path_finder.go
index 53a55d37e..5597e4ba0 100644
--- a/pkg/framework/test/bin_path_finder.go
+++ b/pkg/framework/test/internal/bin_path_finder.go
@@ -1,4 +1,4 @@
-package test
+package internal
 
 import (
 	"os"
@@ -15,19 +15,12 @@ func init() {
 	if !ok {
 		panic("Could not determine the path of the BinPathFinder")
 	}
-	assetsPath = filepath.Join(filepath.Dir(thisFile), "assets", "bin")
+	assetsPath = filepath.Join(filepath.Dir(thisFile), "..", "assets", "bin")
 }
 
-// BinPathFinder is the signature of a function translating a symbolic name of a binary to
-// a resolvable path to the binary in question
-type BinPathFinder func(symbolicName string) (binPath string)
-
-//go:generate counterfeiter . BinPathFinder
-
-// DefaultBinPathFinder is an implementation of BinPathFinder which checks the an environment
-// variable, derived from the symbolic name, and falls back to a default assets location when
-// this variable is not set
-func DefaultBinPathFinder(symbolicName string) (binPath string) {
+// BinPathFinder checks the an environment variable, derived from the symbolic name,
+// and falls back to a default assets location when this variable is not set
+func BinPathFinder(symbolicName string) (binPath string) {
 	punctuationPattern := regexp.MustCompile("[^A-Z0-9]+")
 	sanitizedName := punctuationPattern.ReplaceAllString(strings.ToUpper(symbolicName), "_")
 	leadingNumberPattern := regexp.MustCompile("^[0-9]+")
diff --git a/pkg/framework/test/bin_path_finder_test.go b/pkg/framework/test/internal/bin_path_finder_test.go
similarity index 78%
rename from pkg/framework/test/bin_path_finder_test.go
rename to pkg/framework/test/internal/bin_path_finder_test.go
index 5f7e5c551..490b9e5b5 100644
--- a/pkg/framework/test/bin_path_finder_test.go
+++ b/pkg/framework/test/internal/bin_path_finder_test.go
@@ -1,4 +1,4 @@
-package test
+package internal
 
 import (
 	"os"
@@ -7,7 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("DefaultBinPathFinder", func() {
+var _ = Describe("BinPathFinder", func() {
 	Context("when relying on the default assets path", func() {
 		var (
 			previousAssetsPath string
@@ -20,7 +20,7 @@ var _ = Describe("DefaultBinPathFinder", func() {
 			assetsPath = previousAssetsPath
 		})
 		It("returns the default path when no env var is configured", func() {
-			binPath := DefaultBinPathFinder("some_bin")
+			binPath := BinPathFinder("some_bin")
 			Expect(binPath).To(Equal("/some/path/assets/bin/some_bin"))
 		})
 	})
@@ -46,20 +46,20 @@ var _ = Describe("DefaultBinPathFinder", func() {
 			}
 		})
 		It("returns the path from the env", func() {
-			binPath := DefaultBinPathFinder("another_symbolic_name")
+			binPath := BinPathFinder("another_symbolic_name")
 			Expect(binPath).To(Equal("/path/to/some_bin.exe"))
 		})
 
 		It("sanitizes the environment variable name", func() {
 			By("cleaning all non-underscore punctuation")
-			binPath := DefaultBinPathFinder("another-symbolic name")
+			binPath := BinPathFinder("another-symbolic name")
 			Expect(binPath).To(Equal("/path/to/some_bin.exe"))
-			binPath = DefaultBinPathFinder("another+symbolic\\name")
+			binPath = BinPathFinder("another+symbolic\\name")
 			Expect(binPath).To(Equal("/path/to/some_bin.exe"))
-			binPath = DefaultBinPathFinder("another=symbolic.name")
+			binPath = BinPathFinder("another=symbolic.name")
 			Expect(binPath).To(Equal("/path/to/some_bin.exe"))
 			By("removing numbers from the beginning of the name")
-			binPath = DefaultBinPathFinder("12another_symbolic_name")
+			binPath = BinPathFinder("12another_symbolic_name")
 			Expect(binPath).To(Equal("/path/to/some_bin.exe"))
 		})
 	})
diff --git a/pkg/framework/test/internal/internal_suite_test.go b/pkg/framework/test/internal/internal_suite_test.go
new file mode 100644
index 000000000..82ed7893b
--- /dev/null
+++ b/pkg/framework/test/internal/internal_suite_test.go
@@ -0,0 +1,13 @@
+package internal_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestInternal(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Internal Suite")
+}

From b786202c38c2506bd8a11ce4b70da1549643ccaf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Mon, 8 Jan 2018 11:42:34 +0000
Subject: [PATCH 2/5] Move AddressManager to an internal package

---
 pkg/framework/test/address_manager.go         | 63 -------------------
 pkg/framework/test/apiserver.go               |  4 +-
 pkg/framework/test/etcd.go                    |  2 +-
 .../test/internal/address_manager.go          | 53 ++++++++++++++++
 .../{ => internal}/address_manager_test.go    | 29 +++++----
 5 files changed, 70 insertions(+), 81 deletions(-)
 delete mode 100644 pkg/framework/test/address_manager.go
 create mode 100644 pkg/framework/test/internal/address_manager.go
 rename pkg/framework/test/{ => internal}/address_manager_test.go (69%)

diff --git a/pkg/framework/test/address_manager.go b/pkg/framework/test/address_manager.go
deleted file mode 100644
index 176aa2bea..000000000
--- a/pkg/framework/test/address_manager.go
+++ /dev/null
@@ -1,63 +0,0 @@
-package test
-
-import (
-	"fmt"
-	"net"
-)
-
-// AddressManager knows how to generate and remember a single address on some
-// local interface for a service to listen on.
-type AddressManager interface {
-	Initialize() (port int, resolvedAddress string, err error)
-	Host() (string, error)
-	Port() (int, error)
-}
-
-//go:generate counterfeiter . AddressManager
-
-// DefaultAddressManager implements an AddressManager. It allocates a new address
-// (interface & port) a process can bind and keeps track of that.
-type DefaultAddressManager struct {
-	port int
-	host string
-}
-
-// Initialize returns a address a process can listen on. It returns
-// a tuple consisting of a free port and the hostname resolved to its IP.
-func (d *DefaultAddressManager) Initialize() (port int, resolvedHost string, err error) {
-	if d.port != 0 {
-		return 0, "", fmt.Errorf("this DefaultAddressManager is already initialized")
-	}
-	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
-	if err != nil {
-		return
-	}
-	l, err := net.ListenTCP("tcp", addr)
-	if err != nil {
-		return
-	}
-	d.port = l.Addr().(*net.TCPAddr).Port
-	defer func() {
-		err = l.Close()
-	}()
-	d.host = addr.IP.String()
-	return d.port, d.host, nil
-}
-
-// Port returns the port that this DefaultAddressManager is managing. Port returns an
-// error if this DefaultAddressManager has not yet been initialized.
-func (d *DefaultAddressManager) Port() (int, error) {
-	if d.port == 0 {
-		return 0, fmt.Errorf("this DefaultAdressManager has is not initialized yet")
-	}
-	return d.port, nil
-}
-
-// Host returns the host that this DefaultAddressManager is managing. Host returns an
-// error if this DefaultAddressManager has not yet been initialized.
-func (d *DefaultAddressManager) Host() (string, error) {
-	if d.host == "" {
-		return "", fmt.Errorf("this DefaultAdressManager has is not initialized yet")
-	}
-	return d.host, nil
-}
diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go
index 85953e686..f905ab13b 100644
--- a/pkg/framework/test/apiserver.go
+++ b/pkg/framework/test/apiserver.go
@@ -16,7 +16,7 @@ import (
 // APIServer knows how to run a kubernetes apiserver.
 type APIServer struct {
 	// Address is the address, a host and a port, the ApiServer should listen on for client connections.
-	// If this is not specified, the DefaultAddressManager is used to determine this address.
+	// If this is not specified, we default to a random free port on localhost.
 	Address *url.URL
 
 	// Path is the path to the apiserver binary. If this is left as the empty
@@ -116,7 +116,7 @@ func (s *APIServer) ensureInitialized() error {
 		s.Path = internal.BinPathFinder("kube-apiserver")
 	}
 	if s.Address == nil {
-		am := &DefaultAddressManager{}
+		am := &internal.AddressManager{}
 		port, host, err := am.Initialize()
 		if err != nil {
 			return err
diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go
index 8ac3848b9..5464cf82c 100644
--- a/pkg/framework/test/etcd.go
+++ b/pkg/framework/test/etcd.go
@@ -90,7 +90,7 @@ func (e *Etcd) ensureInitialized() error {
 		e.Path = internal.BinPathFinder("etcd")
 	}
 	if e.Address == nil {
-		am := &DefaultAddressManager{}
+		am := &internal.AddressManager{}
 		port, host, err := am.Initialize()
 		if err != nil {
 			return err
diff --git a/pkg/framework/test/internal/address_manager.go b/pkg/framework/test/internal/address_manager.go
new file mode 100644
index 000000000..7ad4ab7ea
--- /dev/null
+++ b/pkg/framework/test/internal/address_manager.go
@@ -0,0 +1,53 @@
+package internal
+
+import (
+	"fmt"
+	"net"
+)
+
+// AddressManager allocates a new address (interface & port) a process
+// can bind and keeps track of that.
+type AddressManager struct {
+	port int
+	host string
+}
+
+// Initialize returns a address a process can listen on. It returns
+// a tuple consisting of a free port and the hostname resolved to its IP.
+func (d *AddressManager) Initialize() (port int, resolvedHost string, err error) {
+	if d.port != 0 {
+		return 0, "", fmt.Errorf("this AddressManager is already initialized")
+	}
+	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
+	if err != nil {
+		return
+	}
+	l, err := net.ListenTCP("tcp", addr)
+	if err != nil {
+		return
+	}
+	d.port = l.Addr().(*net.TCPAddr).Port
+	defer func() {
+		err = l.Close()
+	}()
+	d.host = addr.IP.String()
+	return d.port, d.host, nil
+}
+
+// Port returns the port that this AddressManager is managing. Port returns an
+// error if this AddressManager has not yet been initialized.
+func (d *AddressManager) Port() (int, error) {
+	if d.port == 0 {
+		return 0, fmt.Errorf("this AdressManager is not initialized yet")
+	}
+	return d.port, nil
+}
+
+// Host returns the host that this AddressManager is managing. Host returns an
+// error if this AddressManager has not yet been initialized.
+func (d *AddressManager) Host() (string, error) {
+	if d.host == "" {
+		return "", fmt.Errorf("this AdressManager is not initialized yet")
+	}
+	return d.host, nil
+}
diff --git a/pkg/framework/test/address_manager_test.go b/pkg/framework/test/internal/address_manager_test.go
similarity index 69%
rename from pkg/framework/test/address_manager_test.go
rename to pkg/framework/test/internal/address_manager_test.go
index e6b8aef2c..d487b4a7f 100644
--- a/pkg/framework/test/address_manager_test.go
+++ b/pkg/framework/test/internal/address_manager_test.go
@@ -1,7 +1,7 @@
-package test_test
+package internal_test
 
 import (
-	. "k8s.io/kubectl/pkg/framework/test"
+	. "k8s.io/kubectl/pkg/framework/test/internal"
 
 	"fmt"
 	"net"
@@ -10,15 +10,15 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("DefaultAddressManager", func() {
-	var defaultAddressManager *DefaultAddressManager
+var _ = Describe("AddressManager", func() {
+	var addressManager *AddressManager
 	BeforeEach(func() {
-		defaultAddressManager = &DefaultAddressManager{}
+		addressManager = &AddressManager{}
 	})
 
 	Describe("Initialize", func() {
 		It("returns a free port and an address to bind to", func() {
-			port, host, err := defaultAddressManager.Initialize()
+			port, host, err := addressManager.Initialize()
 
 			Expect(err).NotTo(HaveOccurred())
 			Expect(host).To(Equal("127.0.0.1"))
@@ -35,38 +35,37 @@ var _ = Describe("DefaultAddressManager", func() {
 
 		Context("initialized multiple times", func() {
 			It("fails", func() {
-				_, _, err := defaultAddressManager.Initialize()
+				_, _, err := addressManager.Initialize()
 				Expect(err).NotTo(HaveOccurred())
-				_, _, err = defaultAddressManager.Initialize()
+				_, _, err = addressManager.Initialize()
 				Expect(err).To(MatchError(ContainSubstring("already initialized")))
 			})
 		})
 	})
 	Describe("Port", func() {
 		It("returns an error if Initialize has not been called yet", func() {
-			_, err := defaultAddressManager.Port()
+			_, err := addressManager.Port()
 			Expect(err).To(MatchError(ContainSubstring("not initialized yet")))
 		})
 		It("returns the same port as previously allocated by Initialize", func() {
-			expectedPort, _, err := defaultAddressManager.Initialize()
+			expectedPort, _, err := addressManager.Initialize()
 			Expect(err).NotTo(HaveOccurred())
-			actualPort, err := defaultAddressManager.Port()
+			actualPort, err := addressManager.Port()
 			Expect(err).NotTo(HaveOccurred())
 			Expect(actualPort).To(Equal(expectedPort))
 		})
 	})
 	Describe("Host", func() {
 		It("returns an error if Initialize has not been called yet", func() {
-			_, err := defaultAddressManager.Host()
+			_, err := addressManager.Host()
 			Expect(err).To(MatchError(ContainSubstring("not initialized yet")))
 		})
 		It("returns the same port as previously allocated by Initialize", func() {
-			_, expectedHost, err := defaultAddressManager.Initialize()
+			_, expectedHost, err := addressManager.Initialize()
 			Expect(err).NotTo(HaveOccurred())
-			actualHost, err := defaultAddressManager.Host()
+			actualHost, err := addressManager.Host()
 			Expect(err).NotTo(HaveOccurred())
 			Expect(actualHost).To(Equal(expectedHost))
 		})
-
 	})
 })

From 4e6d80ef73713fdc401dfea369a33dc1922539ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Mon, 8 Jan 2018 11:53:36 +0000
Subject: [PATCH 3/5] Rename Directory to CleanableDirectory

... and refactor Etcd tests for the Etcd's data directory
---
 pkg/framework/test/apiserver.go                          | 2 +-
 pkg/framework/test/apiserver_test.go                     | 2 +-
 .../test/{directory.go => cleanable_directory.go}        | 8 ++++----
 .../{directory_test.go => cleanable_directory_test.go}   | 0
 pkg/framework/test/etcd.go                               | 2 +-
 pkg/framework/test/etcd_test.go                          | 9 +++++----
 6 files changed, 12 insertions(+), 11 deletions(-)
 rename pkg/framework/test/{directory.go => cleanable_directory.go} (55%)
 rename pkg/framework/test/{directory_test.go => cleanable_directory_test.go} (100%)

diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go
index f905ab13b..35e554d40 100644
--- a/pkg/framework/test/apiserver.go
+++ b/pkg/framework/test/apiserver.go
@@ -33,7 +33,7 @@ type APIServer struct {
 	ProcessStarter SimpleSessionStarter
 
 	// CertDir is a struct holding a path to a certificate directory and a function to cleanup that directory.
-	CertDir *Directory
+	CertDir *CleanableDirectory
 
 	// Etcd is an implementation of a ControlPlaneProcess and is responsible to run Etcd and provide its coordinates.
 	// If not specified, a brand new instance of Etcd is brought up.
diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go
index 6b13d5acf..d0cacae38 100644
--- a/pkg/framework/test/apiserver_test.go
+++ b/pkg/framework/test/apiserver_test.go
@@ -41,7 +41,7 @@ var _ = Describe("Apiserver", func() {
 		apiServer = &APIServer{
 			Address: &url.URL{Scheme: "http", Host: "the.host.for.api.server:5678"},
 			Path:    "/some/path/to/apiserver",
-			CertDir: &Directory{
+			CertDir: &CleanableDirectory{
 				Path: "/some/path/to/certdir",
 				Cleanup: func() error {
 					cleanupCallCount += 1
diff --git a/pkg/framework/test/directory.go b/pkg/framework/test/cleanable_directory.go
similarity index 55%
rename from pkg/framework/test/directory.go
rename to pkg/framework/test/cleanable_directory.go
index f4a92d89a..4c0fbc43a 100644
--- a/pkg/framework/test/directory.go
+++ b/pkg/framework/test/cleanable_directory.go
@@ -5,19 +5,19 @@ import (
 	"os"
 )
 
-// Directory holds a path to a directory and knows how to tear down / cleanup that directory
-type Directory struct {
+// CleanableDirectory holds a path to a directory and knows how to tear down / cleanup that directory
+type CleanableDirectory struct {
 	Path    string
 	Cleanup func() error
 }
 
-func newDirectory() (*Directory, error) {
+func newDirectory() (*CleanableDirectory, error) {
 	path, err := ioutil.TempDir("", "k8s_test_framework_")
 	if err != nil {
 		return nil, err
 	}
 
-	return &Directory{
+	return &CleanableDirectory{
 		Path: path,
 		Cleanup: func() error {
 			return os.RemoveAll(path)
diff --git a/pkg/framework/test/directory_test.go b/pkg/framework/test/cleanable_directory_test.go
similarity index 100%
rename from pkg/framework/test/directory_test.go
rename to pkg/framework/test/cleanable_directory_test.go
diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go
index 5464cf82c..05f225c45 100644
--- a/pkg/framework/test/etcd.go
+++ b/pkg/framework/test/etcd.go
@@ -21,7 +21,7 @@ type Etcd struct {
 	Address        *url.URL
 	Path           string
 	ProcessStarter SimpleSessionStarter
-	DataDir        *Directory
+	DataDir        *CleanableDirectory
 	StopTimeout    time.Duration
 	StartTimeout   time.Duration
 	session        SimpleSession
diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go
index 6df3d3851..bbcf8a2c9 100644
--- a/pkg/framework/test/etcd_test.go
+++ b/pkg/framework/test/etcd_test.go
@@ -36,8 +36,8 @@ var _ = Describe("Etcd", func() {
 
 		etcd = &Etcd{
 			Path: "/path/to/some/etcd",
-			DataDir: &Directory{
-				Path: "/path/to/some/etcd",
+			DataDir: &CleanableDirectory{
+				Path: "/path/to/some/data",
 				Cleanup: func() error {
 					dataDirCleanupCount += 1
 					return nil
@@ -60,8 +60,9 @@ var _ = Describe("Etcd", func() {
 				etcd.Address = &url.URL{Scheme: "http", Host: "this.is.etcd.listening.for.clients:1234"}
 
 				etcd.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) {
-					Expect(command.Args).To(ContainElement(fmt.Sprintf("--advertise-client-urls=http://%s:%d", "this.is.etcd.listening.for.clients", 1234)))
-					Expect(command.Args).To(ContainElement(fmt.Sprintf("--listen-client-urls=http://%s:%d", "this.is.etcd.listening.for.clients", 1234)))
+					Expect(command.Args).To(ContainElement("--advertise-client-urls=http://this.is.etcd.listening.for.clients:1234"))
+					Expect(command.Args).To(ContainElement("--listen-client-urls=http://this.is.etcd.listening.for.clients:1234"))
+					Expect(command.Args).To(ContainElement("--data-dir=/path/to/some/data"))
 					Expect(command.Path).To(Equal("/path/to/some/etcd"))
 					fmt.Fprint(err, "serving insecure client requests on this.is.etcd.listening.for.clients:1234")
 					return fakeSession, nil

From a6eb5bc2f503e9c061ff4d2f7aa1670d5eca1d50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Mon, 8 Jan 2018 12:11:37 +0000
Subject: [PATCH 4/5] We no-longer need Exit() on APIServer or Etcd

---
 pkg/framework/test/apiserver.go      | 5 -----
 pkg/framework/test/apiserver_test.go | 5 -----
 pkg/framework/test/etcd.go           | 5 -----
 pkg/framework/test/etcd_test.go      | 5 -----
 4 files changed, 20 deletions(-)

diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go
index 35e554d40..adc60061e 100644
--- a/pkg/framework/test/apiserver.go
+++ b/pkg/framework/test/apiserver.go
@@ -181,11 +181,6 @@ func (s *APIServer) Stop() error {
 	return s.CertDir.Cleanup()
 }
 
-// ExitCode returns the exit code of the process, if it has exited. If it hasn't exited yet, ExitCode returns -1.
-func (s *APIServer) ExitCode() int {
-	return s.session.ExitCode()
-}
-
 // Buffer implements the gbytes.BufferProvider interface and returns the stdout of the process
 func (s *APIServer) Buffer() *gbytes.Buffer {
 	return s.session.Buffer()
diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go
index d0cacae38..dc94e663c 100644
--- a/pkg/framework/test/apiserver_test.go
+++ b/pkg/framework/test/apiserver_test.go
@@ -88,18 +88,13 @@ var _ = Describe("Apiserver", func() {
 				Expect(fakeEtcdProcess.URLCallCount()).To(Equal(1))
 
 				Eventually(apiServer).Should(gbytes.Say("Everything is fine"))
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(0))
-				Expect(apiServer).NotTo(gexec.Exit())
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(1))
 
 				By("Stopping the API Server")
 				Expect(apiServer.Stop()).To(Succeed())
 
 				Expect(cleanupCallCount).To(Equal(1))
 				Expect(fakeEtcdProcess.StopCallCount()).To(Equal(1))
-				Expect(apiServer).To(gexec.Exit(143))
 				Expect(fakeSession.TerminateCallCount()).To(Equal(1))
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(2))
 			})
 		})
 
diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go
index 05f225c45..86bdd4db6 100644
--- a/pkg/framework/test/etcd.go
+++ b/pkg/framework/test/etcd.go
@@ -149,11 +149,6 @@ func (e *Etcd) Stop() error {
 	return e.DataDir.Cleanup()
 }
 
-// ExitCode returns the exit code of the process, if it has exited. If it hasn't exited yet, ExitCode returns -1.
-func (e *Etcd) ExitCode() int {
-	return e.session.ExitCode()
-}
-
 // Buffer implements the gbytes.BufferProvider interface and returns the stdout of the process
 func (e *Etcd) Buffer() *gbytes.Buffer {
 	return e.session.Buffer()
diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go
index bbcf8a2c9..ab0512542 100644
--- a/pkg/framework/test/etcd_test.go
+++ b/pkg/framework/test/etcd_test.go
@@ -73,17 +73,12 @@ var _ = Describe("Etcd", func() {
 				Expect(err).NotTo(HaveOccurred())
 
 				Eventually(etcd).Should(gbytes.Say("Everything is dandy"))
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(0))
-				Expect(etcd).NotTo(gexec.Exit())
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(1))
 
 				By("Stopping the Etcd Server")
 				Expect(etcd.Stop()).To(Succeed())
 
 				Expect(dataDirCleanupCount).To(Equal(1))
-				Expect(etcd).To(gexec.Exit(143))
 				Expect(fakeSession.TerminateCallCount()).To(Equal(1))
-				Expect(fakeSession.ExitCodeCallCount()).To(Equal(2))
 			})
 		})
 

From 10ba636acd2e9afdd72feb7916b3fcc511b657c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20H=C3=B6rl?= <hhorl@pivotal.io>
Date: Mon, 8 Jan 2018 12:18:46 +0000
Subject: [PATCH 5/5] We no longer need Buffer() on APIServer or Etcd

---
 pkg/framework/test/apiserver.go      | 5 -----
 pkg/framework/test/apiserver_test.go | 7 -------
 pkg/framework/test/etcd.go           | 5 -----
 pkg/framework/test/etcd_test.go      | 7 -------
 4 files changed, 24 deletions(-)

diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go
index adc60061e..526ec0247 100644
--- a/pkg/framework/test/apiserver.go
+++ b/pkg/framework/test/apiserver.go
@@ -180,8 +180,3 @@ func (s *APIServer) Stop() error {
 	}
 	return s.CertDir.Cleanup()
 }
-
-// Buffer implements the gbytes.BufferProvider interface and returns the stdout of the process
-func (s *APIServer) Buffer() *gbytes.Buffer {
-	return s.session.Buffer()
-}
diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go
index dc94e663c..bdca771fa 100644
--- a/pkg/framework/test/apiserver_test.go
+++ b/pkg/framework/test/apiserver_test.go
@@ -14,7 +14,6 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
 	"k8s.io/kubectl/pkg/framework/test/testfakes"
 )
@@ -56,10 +55,6 @@ var _ = Describe("Apiserver", func() {
 	Describe("starting and stopping the server", func() {
 		Context("when given a path to a binary that runs for a long time", func() {
 			It("can start and stop that binary", func() {
-				sessionBuffer := gbytes.NewBuffer()
-				fmt.Fprint(sessionBuffer, "Everything is fine")
-				fakeSession.BufferReturns(sessionBuffer)
-
 				fakeSession.ExitCodeReturnsOnCall(0, -1)
 				fakeSession.ExitCodeReturnsOnCall(1, 143)
 
@@ -87,8 +82,6 @@ var _ = Describe("Apiserver", func() {
 				By("...getting the URL of Etcd")
 				Expect(fakeEtcdProcess.URLCallCount()).To(Equal(1))
 
-				Eventually(apiServer).Should(gbytes.Say("Everything is fine"))
-
 				By("Stopping the API Server")
 				Expect(apiServer.Stop()).To(Succeed())
 
diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go
index 86bdd4db6..85a76ce44 100644
--- a/pkg/framework/test/etcd.go
+++ b/pkg/framework/test/etcd.go
@@ -148,8 +148,3 @@ func (e *Etcd) Stop() error {
 	}
 	return e.DataDir.Cleanup()
 }
-
-// Buffer implements the gbytes.BufferProvider interface and returns the stdout of the process
-func (e *Etcd) Buffer() *gbytes.Buffer {
-	return e.session.Buffer()
-}
diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go
index ab0512542..d1083514d 100644
--- a/pkg/framework/test/etcd_test.go
+++ b/pkg/framework/test/etcd_test.go
@@ -11,7 +11,6 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
 	. "k8s.io/kubectl/pkg/framework/test"
 	"k8s.io/kubectl/pkg/framework/test/testfakes"
@@ -50,10 +49,6 @@ var _ = Describe("Etcd", func() {
 	Describe("starting and stopping etcd", func() {
 		Context("when given a path to a binary that runs for a long time", func() {
 			It("can start and stop that binary", func() {
-				sessionBuffer := gbytes.NewBuffer()
-				fmt.Fprintf(sessionBuffer, "Everything is dandy")
-				fakeSession.BufferReturns(sessionBuffer)
-
 				fakeSession.ExitCodeReturnsOnCall(0, -1)
 				fakeSession.ExitCodeReturnsOnCall(1, 143)
 
@@ -72,8 +67,6 @@ var _ = Describe("Etcd", func() {
 				err := etcd.Start()
 				Expect(err).NotTo(HaveOccurred())
 
-				Eventually(etcd).Should(gbytes.Say("Everything is dandy"))
-
 				By("Stopping the Etcd Server")
 				Expect(etcd.Stop()).To(Succeed())