From 3f26c08fd0998f40aeafd2d5be15a66bf29f47e1 Mon Sep 17 00:00:00 2001 From: Gareth Smith Date: Mon, 18 Dec 2017 14:41:49 +0000 Subject: [PATCH 1/2] Replace Etcd.PathFinder with Etcd.Path --- pkg/framework/test/etcd.go | 8 ++++---- pkg/framework/test/etcd_test.go | 9 +-------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go index 6f46522cc..e18727801 100644 --- a/pkg/framework/test/etcd.go +++ b/pkg/framework/test/etcd.go @@ -16,7 +16,7 @@ import ( // in the documentation for the `APIServer`, as both implement a `ControlPaneProcess`. type Etcd struct { AddressManager AddressManager - PathFinder BinPathFinder + Path string ProcessStarter SimpleSessionStarter DataDirManager DataDirManager StopTimeout time.Duration @@ -92,7 +92,7 @@ func (e *Etcd) Start() error { "serving insecure client requests on %s", host)) timedOut := time.After(e.StartTimeout) - command := exec.Command(e.PathFinder("etcd"), args...) + command := exec.Command(e.Path, args...) e.session, err = e.ProcessStarter(command, e.stdOut, e.stdErr) if err != nil { return err @@ -107,8 +107,8 @@ func (e *Etcd) Start() error { } func (e *Etcd) ensureInitialized() { - if e.PathFinder == nil { - e.PathFinder = DefaultBinPathFinder + if e.Path == "" { + e.Path = DefaultBinPathFinder("etcd") } if e.AddressManager == nil { diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go index fad7427a0..a13ff5bf8 100644 --- a/pkg/framework/test/etcd_test.go +++ b/pkg/framework/test/etcd_test.go @@ -19,7 +19,6 @@ var _ = Describe("Etcd", func() { var ( fakeSession *testfakes.FakeSimpleSession fakeDataDirManager *testfakes.FakeDataDirManager - fakePathFinder *testfakes.FakeBinPathFinder fakeAddressManager *testfakes.FakeAddressManager etcd *Etcd etcdStopper chan struct{} @@ -28,7 +27,6 @@ var _ = Describe("Etcd", func() { BeforeEach(func() { fakeSession = &testfakes.FakeSimpleSession{} fakeDataDirManager = &testfakes.FakeDataDirManager{} - fakePathFinder = &testfakes.FakeBinPathFinder{} fakeAddressManager = &testfakes.FakeAddressManager{} etcdStopper = make(chan struct{}, 1) @@ -39,7 +37,7 @@ var _ = Describe("Etcd", func() { etcd = &Etcd{ AddressManager: fakeAddressManager, - PathFinder: fakePathFinder.Spy, + Path: "/path/to/some/etcd", DataDirManager: fakeDataDirManager, StopTimeout: 500 * time.Millisecond, } @@ -55,7 +53,6 @@ var _ = Describe("Etcd", func() { fakeSession.ExitCodeReturnsOnCall(0, -1) fakeSession.ExitCodeReturnsOnCall(1, 143) - fakePathFinder.ReturnsOnCall(0, "/path/to/some/etcd") fakeAddressManager.InitializeReturns(1234, "this.is.etcd.listening.for.clients", nil) etcd.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { @@ -70,10 +67,6 @@ var _ = Describe("Etcd", func() { err := etcd.Start() Expect(err).NotTo(HaveOccurred()) - By("...in turn calling the PathFinder") - Expect(fakePathFinder.CallCount()).To(Equal(1)) - Expect(fakePathFinder.ArgsForCall(0)).To(Equal("etcd")) - By("...in turn calling using the AddressManager") Expect(fakeAddressManager.InitializeCallCount()).To(Equal(1)) From e7b219a3334e15e3651c1ca941912be897f17164 Mon Sep 17 00:00:00 2001 From: Gareth Smith Date: Mon, 18 Dec 2017 15:06:11 +0000 Subject: [PATCH 2/2] Replace ApiServer.PathFinder with ApiServer.Path --- pkg/framework/test/apiserver.go | 19 ++-- pkg/framework/test/apiserver_test.go | 9 +- ...mples_api_server_custom_pathfinder_test.go | 19 ---- .../test/testfakes/fake_bin_path_finder.go | 98 ------------------- 4 files changed, 9 insertions(+), 136 deletions(-) delete mode 100644 pkg/framework/test/examples_api_server_custom_pathfinder_test.go delete mode 100644 pkg/framework/test/testfakes/fake_bin_path_finder.go diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go index bb64a9066..585674d97 100644 --- a/pkg/framework/test/apiserver.go +++ b/pkg/framework/test/apiserver.go @@ -21,14 +21,11 @@ type APIServer struct { // AddressManager as shown in the `FirewalledAddressManager` example. AddressManager AddressManager - // PathFinder is used to resolve a symbolic name, "kube-apiserver" in this case, to a name / path to a binary - // to run. - // If not specified, the DefaultBinPathFinder is used, which tries to resolve the binary name from the environment - // or from a fixed assets directory. - // - // You can customise this if, e.g. you need to specify a fixed path. You can pass in a different BinPathFinder as - // shown in the `SpecialPathFinder` example. - PathFinder BinPathFinder + // 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. + Path string // ProcessStarter is a way to hook into how a the APIServer process is started. By default `gexec.Start(...)` is // used to run the process. @@ -130,7 +127,7 @@ func (s *APIServer) Start() error { detectedStart := s.stdErr.Detect(fmt.Sprintf("Serving insecurely on %s:%d", addr, port)) timedOut := time.After(s.StartTimeout) - command := exec.Command(s.PathFinder("kube-apiserver"), args...) + command := exec.Command(s.Path, args...) s.session, err = s.ProcessStarter(command, s.stdOut, s.stdErr) if err != nil { return err @@ -145,8 +142,8 @@ func (s *APIServer) Start() error { } func (s *APIServer) ensureInitialized() { - if s.PathFinder == nil { - s.PathFinder = DefaultBinPathFinder + if s.Path == "" { + s.Path = DefaultBinPathFinder("kube-apiserver") } if s.AddressManager == nil { s.AddressManager = &DefaultAddressManager{} diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go index 9ca704dc5..edaad8b6d 100644 --- a/pkg/framework/test/apiserver_test.go +++ b/pkg/framework/test/apiserver_test.go @@ -23,7 +23,6 @@ var _ = Describe("Apiserver", func() { fakeCertDirManager *testfakes.FakeCertDirManager apiServer *APIServer fakeEtcdProcess *testfakes.FakeControlPlaneProcess - fakePathFinder *testfakes.FakeBinPathFinder fakeAddressManager *testfakes.FakeAddressManager apiServerStopper chan struct{} ) @@ -32,7 +31,6 @@ var _ = Describe("Apiserver", func() { fakeSession = &testfakes.FakeSimpleSession{} fakeCertDirManager = &testfakes.FakeCertDirManager{} fakeEtcdProcess = &testfakes.FakeControlPlaneProcess{} - fakePathFinder = &testfakes.FakeBinPathFinder{} fakeAddressManager = &testfakes.FakeAddressManager{} apiServerStopper = make(chan struct{}, 1) @@ -43,7 +41,7 @@ var _ = Describe("Apiserver", func() { apiServer = &APIServer{ AddressManager: fakeAddressManager, - PathFinder: fakePathFinder.Spy, + Path: "/some/path/to/apiserver", CertDirManager: fakeCertDirManager, Etcd: fakeEtcdProcess, StopTimeout: 500 * time.Millisecond, @@ -61,7 +59,6 @@ var _ = Describe("Apiserver", func() { fakeSession.ExitCodeReturnsOnCall(0, -1) fakeSession.ExitCodeReturnsOnCall(1, 143) - fakePathFinder.Returns("/some/path/to/apiserver") fakeAddressManager.InitializeReturns(1234, "this.is.the.API.server", nil) fakeEtcdProcess.URLReturns("the etcd url", nil) @@ -82,10 +79,6 @@ var _ = Describe("Apiserver", func() { Expect(fakeEtcdProcess.StartCallCount()).To(Equal(1), "the Etcd process should be started exactly once") - By("...in turn calling the PathFinder") - Expect(fakePathFinder.CallCount()).To(Equal(1)) - Expect(fakePathFinder.ArgsForCall(0)).To(Equal("kube-apiserver")) - By("...in turn calling the AddressManager") Expect(fakeAddressManager.InitializeCallCount()).To(Equal(1)) diff --git a/pkg/framework/test/examples_api_server_custom_pathfinder_test.go b/pkg/framework/test/examples_api_server_custom_pathfinder_test.go deleted file mode 100644 index 36e3be3c4..000000000 --- a/pkg/framework/test/examples_api_server_custom_pathfinder_test.go +++ /dev/null @@ -1,19 +0,0 @@ -package test_test - -import ( - "fmt" - - . "k8s.io/kubectl/pkg/framework/test" -) - -func ExampleAPIServer_specialPathFinder() { - apiServer := &APIServer{ - PathFinder: myCustomPathFinder, - } - - fmt.Println(apiServer) -} - -func myCustomPathFinder(_ string) string { - return "/usr/local/bin/kube/1.21-alpha1/special-kube-api" -} diff --git a/pkg/framework/test/testfakes/fake_bin_path_finder.go b/pkg/framework/test/testfakes/fake_bin_path_finder.go deleted file mode 100644 index 4503dcb91..000000000 --- a/pkg/framework/test/testfakes/fake_bin_path_finder.go +++ /dev/null @@ -1,98 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package testfakes - -import ( - "sync" - - "k8s.io/kubectl/pkg/framework/test" -) - -type FakeBinPathFinder struct { - Stub func(symbolicName string) (binPath string) - mutex sync.RWMutex - argsForCall []struct { - symbolicName string - } - returns struct { - result1 string - } - returnsOnCall map[int]struct { - result1 string - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeBinPathFinder) Spy(symbolicName string) (binPath string) { - fake.mutex.Lock() - ret, specificReturn := fake.returnsOnCall[len(fake.argsForCall)] - fake.argsForCall = append(fake.argsForCall, struct { - symbolicName string - }{symbolicName}) - fake.recordInvocation("BinPathFinder", []interface{}{symbolicName}) - fake.mutex.Unlock() - if fake.Stub != nil { - return fake.Stub(symbolicName) - } - if specificReturn { - return ret.result1 - } - return fake.returns.result1 -} - -func (fake *FakeBinPathFinder) CallCount() int { - fake.mutex.RLock() - defer fake.mutex.RUnlock() - return len(fake.argsForCall) -} - -func (fake *FakeBinPathFinder) ArgsForCall(i int) string { - fake.mutex.RLock() - defer fake.mutex.RUnlock() - return fake.argsForCall[i].symbolicName -} - -func (fake *FakeBinPathFinder) Returns(result1 string) { - fake.Stub = nil - fake.returns = struct { - result1 string - }{result1} -} - -func (fake *FakeBinPathFinder) ReturnsOnCall(i int, result1 string) { - fake.Stub = nil - if fake.returnsOnCall == nil { - fake.returnsOnCall = make(map[int]struct { - result1 string - }) - } - fake.returnsOnCall[i] = struct { - result1 string - }{result1} -} - -func (fake *FakeBinPathFinder) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.mutex.RLock() - defer fake.mutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeBinPathFinder) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ test.BinPathFinder = new(FakeBinPathFinder).Spy