diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go index c1c7c1861..030174d0f 100644 --- a/pkg/framework/test/apiserver.go +++ b/pkg/framework/test/apiserver.go @@ -18,6 +18,7 @@ type APIServer struct { Path string ProcessStarter simpleSessionStarter CertDirManager certDirManager + Config *APIServerConfig session SimpleSession stdOut *gbytes.Buffer stdErr *gbytes.Buffer @@ -28,10 +29,16 @@ type certDirManager interface { Destroy() error } +// APIServerConfig is a struct holding data to configure the API Server process +type APIServerConfig struct { + EtcdURL string + APIServerURL string +} + //go:generate counterfeiter . certDirManager // NewAPIServer creates a new APIServer Fixture Process -func NewAPIServer(pathToAPIServer string) *APIServer { +func NewAPIServer(pathToAPIServer string, config *APIServerConfig) *APIServer { starter := func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { return gexec.Start(command, out, err) } @@ -40,13 +47,14 @@ func NewAPIServer(pathToAPIServer string) *APIServer { Path: pathToAPIServer, ProcessStarter: starter, CertDirManager: NewTempDirManager(), + Config: config, } return apiserver } // Start starts the apiserver, waits for it to come up, and returns an error, if occoured. -func (s *APIServer) Start(config map[string]string) error { +func (s *APIServer) Start() error { s.stdOut = gbytes.NewBuffer() s.stdErr = gbytes.NewBuffer() @@ -55,16 +63,7 @@ func (s *APIServer) Start(config map[string]string) error { return err } - etcdURL, ok := config["etcdURL"] - if !ok { - return fmt.Errorf("config setting 'etcdURL' not found") - } - apiServerURL, ok := config["apiServerURL"] - if !ok { - return fmt.Errorf("config setting 'apiServerURL' not found") - } - - url, err := url.Parse(apiServerURL) + clientURL, err := url.Parse(s.Config.APIServerURL) if err != nil { return err } @@ -77,13 +76,13 @@ func (s *APIServer) Start(config map[string]string) error { "--admission-control-config-file=", "--bind-address=0.0.0.0", "--storage-backend=etcd3", - fmt.Sprintf("--etcd-servers=%s", etcdURL), + fmt.Sprintf("--etcd-servers=%s", s.Config.EtcdURL), fmt.Sprintf("--cert-dir=%s", certDir), - fmt.Sprintf("--insecure-port=%s", url.Port()), - fmt.Sprintf("--insecure-bind-address=%s", url.Hostname()), + fmt.Sprintf("--insecure-port=%s", clientURL.Port()), + fmt.Sprintf("--insecure-bind-address=%s", clientURL.Hostname()), } - detectedStart := s.stdErr.Detect(fmt.Sprintf("Serving insecurely on %s", url.Host)) + detectedStart := s.stdErr.Detect(fmt.Sprintf("Serving insecurely on %s", clientURL.Host)) timedOut := time.After(20 * time.Second) command := exec.Command(s.Path, args...) diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go index 69cdea16e..3473cd569 100644 --- a/pkg/framework/test/apiserver_test.go +++ b/pkg/framework/test/apiserver_test.go @@ -20,21 +20,21 @@ var _ = Describe("Apiserver", func() { fakeSession *testfakes.FakeSimpleSession fakeCertDirManager *testfakes.FakeCertDirManager apiServer *APIServer - apiServerConfig map[string]string + apiServerConfig *APIServerConfig ) BeforeEach(func() { fakeSession = &testfakes.FakeSimpleSession{} fakeCertDirManager = &testfakes.FakeCertDirManager{} + apiServerConfig = &APIServerConfig{ + EtcdURL: "http://this.is.etcd:2345/", + APIServerURL: "http://this.is.the.API.server:8080", + } apiServer = &APIServer{ Path: "", CertDirManager: fakeCertDirManager, - } - - apiServerConfig = map[string]string{ - "apiServerURL": "http://this.is.the.API.server:8080", - "etcdURL": "http://this.is.etcd:2345/", + Config: apiServerConfig, } }) @@ -53,7 +53,7 @@ var _ = Describe("Apiserver", func() { } By("Starting the API Server") - err := apiServer.Start(apiServerConfig) + err := apiServer.Start() Expect(err).NotTo(HaveOccurred()) Eventually(apiServer).Should(gbytes.Say("Everything is fine")) @@ -82,7 +82,7 @@ var _ = Describe("Apiserver", func() { return fakeSession, nil } - err := apiServer.Start(apiServerConfig) + err := apiServer.Start() Expect(err).To(MatchError(ContainSubstring("Error on cert directory creation."))) Expect(processStarterCounter).To(Equal(0)) }) @@ -94,7 +94,7 @@ var _ = Describe("Apiserver", func() { return nil, fmt.Errorf("Some error in the apiserver starter.") } - err := apiServer.Start(apiServerConfig) + err := apiServer.Start() Expect(err).To(MatchError(ContainSubstring("Some error in the apiserver starter."))) }) }) diff --git a/pkg/framework/test/democli/integration/integration_suite_test.go b/pkg/framework/test/democli/integration/integration_suite_test.go index 83117c276..070e30ffc 100644 --- a/pkg/framework/test/democli/integration/integration_suite_test.go +++ b/pkg/framework/test/democli/integration/integration_suite_test.go @@ -44,7 +44,9 @@ var _ = BeforeSuite(func() { Expect(pathToEtcd).NotTo(BeEmpty(), "Path to etcd cannot be empty, set $TEST_ETCD_BIN") Expect(pathToAPIServer).NotTo(BeEmpty(), "Path to apiserver cannot be empty, set $TEST_APISERVER_BIN") - fixtures = test.NewFixtures(pathToEtcd, pathToAPIServer) + fixtures, err = test.NewFixtures(pathToEtcd, pathToAPIServer) + Expect(err).NotTo(HaveOccurred()) + err = fixtures.Start() Expect(err).NotTo(HaveOccurred()) }) diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go index 49a3430da..1df3c8623 100644 --- a/pkg/framework/test/etcd.go +++ b/pkg/framework/test/etcd.go @@ -18,6 +18,7 @@ type Etcd struct { Path string ProcessStarter simpleSessionStarter DataDirManager dataDirManager + Config *EtcdConfig session SimpleSession stdOut *gbytes.Buffer stdErr *gbytes.Buffer @@ -28,6 +29,12 @@ type dataDirManager interface { Destroy() error } +// EtcdConfig is a struct holding data to configure the Etcd process +type EtcdConfig struct { + ClientURL string + PeerURL string +} + //go:generate counterfeiter . dataDirManager // SimpleSession describes a CLI session. You can get output, and you can kill it. It is implemented by *gexec.Session. @@ -43,7 +50,7 @@ type SimpleSession interface { type simpleSessionStarter func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) // NewEtcd constructs an Etcd Fixture Process -func NewEtcd(pathToEtcd string) *Etcd { +func NewEtcd(pathToEtcd string, config *EtcdConfig) *Etcd { starter := func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { return gexec.Start(command, out, err) } @@ -52,13 +59,14 @@ func NewEtcd(pathToEtcd string) *Etcd { Path: pathToEtcd, ProcessStarter: starter, DataDirManager: NewTempDirManager(), + Config: config, } return etcd } // Start starts the etcd, waits for it to come up, and returns an error, if occoured. -func (e *Etcd) Start(config map[string]string) error { +func (e *Etcd) Start() error { e.stdOut = gbytes.NewBuffer() e.stdErr = gbytes.NewBuffer() @@ -67,34 +75,25 @@ func (e *Etcd) Start(config map[string]string) error { return err } - clientURL, ok := config["clientURL"] - if !ok { - return fmt.Errorf("config setting 'clientURL' not found") - } - peerURL, ok := config["peerURL"] - if !ok { - return fmt.Errorf("config setting 'peerURL' not found") - } - args := []string{ "--debug", "--advertise-client-urls", - clientURL, + e.Config.ClientURL, "--listen-client-urls", - clientURL, + e.Config.ClientURL, "--listen-peer-urls", - peerURL, + e.Config.PeerURL, "--data-dir", dataDir, } - url, err := url.Parse(clientURL) + clientURL, err := url.Parse(e.Config.ClientURL) if err != nil { return err } detectedStart := e.stdErr.Detect(fmt.Sprintf( - "serving insecure client requests on %s", url.Host)) + "serving insecure client requests on %s", clientURL.Host)) timedOut := time.After(20 * time.Second) command := exec.Command(e.Path, args...) diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go index 297d123e2..94278f592 100644 --- a/pkg/framework/test/etcd_test.go +++ b/pkg/framework/test/etcd_test.go @@ -20,21 +20,22 @@ var _ = Describe("Etcd", func() { fakeSession *testfakes.FakeSimpleSession fakeDataDirManager *testfakes.FakeDataDirManager etcd *Etcd - etcdConfig map[string]string + etcdConfig *EtcdConfig ) BeforeEach(func() { fakeSession = &testfakes.FakeSimpleSession{} fakeDataDirManager = &testfakes.FakeDataDirManager{} + etcdConfig = &EtcdConfig{ + ClientURL: "http://this.is.etcd.listening.for.clients:1234", + PeerURL: "http://this.is.etcd.listening.for.peers:1235", + } + etcd = &Etcd{ Path: "", DataDirManager: fakeDataDirManager, - } - - etcdConfig = map[string]string{ - "clientURL": "http://this.is.etcd.listening.for.clients:1234", - "peerURL": "http://this.is.etcd.listening.for.peers:1235", + Config: etcdConfig, } }) @@ -53,7 +54,7 @@ var _ = Describe("Etcd", func() { } By("Starting the Etcd Server") - err := etcd.Start(etcdConfig) + err := etcd.Start() Expect(err).NotTo(HaveOccurred()) Eventually(etcd).Should(gbytes.Say("Everything is dandy")) @@ -82,7 +83,7 @@ var _ = Describe("Etcd", func() { return fakeSession, nil } - err := etcd.Start(etcdConfig) + err := etcd.Start() Expect(err).To(MatchError(ContainSubstring("Error on directory creation."))) Expect(processStarterCounter).To(Equal(0)) }) @@ -94,7 +95,7 @@ var _ = Describe("Etcd", func() { return nil, fmt.Errorf("Some error in the starter.") } - err := etcd.Start(etcdConfig) + err := etcd.Start() Expect(err).To(MatchError(ContainSubstring("Some error in the starter."))) }) }) diff --git a/pkg/framework/test/fixtures.go b/pkg/framework/test/fixtures.go index 904b760dc..ecce9ee92 100644 --- a/pkg/framework/test/fixtures.go +++ b/pkg/framework/test/fixtures.go @@ -12,7 +12,6 @@ type Fixtures struct { Etcd FixtureProcess APIServer FixtureProcess Config FixturesConfig - URLGetter listenURLGetter } // FixturesConfig is a datastructure that exposes configuration that should be used by clients to talk @@ -25,60 +24,61 @@ type FixturesConfig struct { // This interface is potentially going to be expanded to e.g. allow access to the processes StdOut/StdErr // and other internals. type FixtureProcess interface { - Start(config map[string]string) error + Start() error Stop() } //go:generate counterfeiter . FixtureProcess // NewFixtures will give you a Fixtures struct that's properly wired together. -func NewFixtures(pathToEtcd, pathToAPIServer string) *Fixtures { - fixtures := &Fixtures{ - Etcd: NewEtcd(pathToEtcd), - APIServer: NewAPIServer(pathToAPIServer), - URLGetter: getHTTPListenURL, +func NewFixtures(pathToEtcd, pathToAPIServer string) (*Fixtures, error) { + etcdConfig := &EtcdConfig{} + apiServerConfig := &APIServerConfig{} + + if url, err := getHTTPListenURL(); err == nil { + etcdConfig.ClientURL = url + apiServerConfig.EtcdURL = url + } else { + return nil, err } - return fixtures + if url, err := getHTTPListenURL(); err == nil { + etcdConfig.PeerURL = url + } else { + return nil, err + } + + if url, err := getHTTPListenURL(); err == nil { + apiServerConfig.APIServerURL = url + } else { + return nil, err + } + + fixtures := &Fixtures{ + Etcd: NewEtcd(pathToEtcd, etcdConfig), + APIServer: NewAPIServer(pathToAPIServer, apiServerConfig), + } + + fixtures.Config = FixturesConfig{ + APIServerURL: apiServerConfig.APIServerURL, + } + + return fixtures, nil } // Start will start all your fixtures. To stop them, call Stop(). func (f *Fixtures) Start() error { - type configs map[string]string - - etcdClientURL, err := f.URLGetter() - if err != nil { - return err - } - etcdPeerURL, err := f.URLGetter() - if err != nil { - return err - } - apiServerURL, err := f.URLGetter() - if err != nil { - return err - } - - etcdConf := configs{ - "peerURL": etcdPeerURL, - "clientURL": etcdClientURL, - } - apiServerConf := configs{ - "etcdURL": etcdClientURL, - "apiServerURL": apiServerURL, - } - started := make(chan error) - starter := func(process FixtureProcess, conf configs) { - started <- process.Start(conf) + starter := func(process FixtureProcess) { + started <- process.Start() } - processes := map[FixtureProcess]configs{ - f.Etcd: etcdConf, - f.APIServer: apiServerConf, + processes := []FixtureProcess{ + f.Etcd, + f.APIServer, } - for process, config := range processes { - go starter(process, config) + for _, process := range processes { + go starter(process) } for range processes { @@ -87,10 +87,6 @@ func (f *Fixtures) Start() error { } } - f.Config = FixturesConfig{ - APIServerURL: apiServerURL, - } - return nil } @@ -101,10 +97,6 @@ func (f *Fixtures) Stop() error { return nil } -type listenURLGetter func() (url string, err error) - -//go:generate counterfeiter . listenURLGetter - func getHTTPListenURL() (url string, err error) { host := "127.0.0.1" port, err := getFreePort(host) diff --git a/pkg/framework/test/fixtures_test.go b/pkg/framework/test/fixtures_test.go index 818858588..5275432d8 100644 --- a/pkg/framework/test/fixtures_test.go +++ b/pkg/framework/test/fixtures_test.go @@ -12,7 +12,8 @@ import ( var _ = Describe("Fixtures", func() { It("can construct a properly wired Fixtures struct", func() { - f := NewFixtures("path to etcd", "path to apiserver") + f, err := NewFixtures("path to etcd", "path to apiserver") + Expect(err).NotTo(HaveOccurred()) Expect(f.Etcd.(*Etcd).Path).To(Equal("path to etcd")) Expect(f.APIServer.(*APIServer).Path).To(Equal("path to apiserver")) }) @@ -21,24 +22,20 @@ var _ = Describe("Fixtures", func() { var ( fakeEtcdProcess *testfakes.FakeFixtureProcess fakeAPIServerProcess *testfakes.FakeFixtureProcess - fakeListenURLGetter *testfakes.FakeListenURLGetter fixtures Fixtures ) BeforeEach(func() { fakeEtcdProcess = &testfakes.FakeFixtureProcess{} fakeAPIServerProcess = &testfakes.FakeFixtureProcess{} - fakeListenURLGetter = &testfakes.FakeListenURLGetter{} fixtures = Fixtures{ Etcd: fakeEtcdProcess, APIServer: fakeAPIServerProcess, - URLGetter: fakeListenURLGetter.Spy, } }) It("can start them", func() { err := fixtures.Start() Expect(err).NotTo(HaveOccurred()) - Expect(fakeListenURLGetter.CallCount()).To(Equal(3)) By("starting Etcd") Expect(fakeEtcdProcess.StartCallCount()).To(Equal(1), @@ -53,7 +50,6 @@ var _ = Describe("Fixtures", func() { It("wraps the error", func() { fakeEtcdProcess.StartReturns(fmt.Errorf("some error")) err := fixtures.Start() - Expect(fakeListenURLGetter.CallCount()).To(Equal(3)) Expect(err).To(MatchError(ContainSubstring("some error"))) }) }) @@ -62,7 +58,6 @@ var _ = Describe("Fixtures", func() { It("wraps the error", func() { fakeAPIServerProcess.StartReturns(fmt.Errorf("another error")) err := fixtures.Start() - Expect(fakeListenURLGetter.CallCount()).To(Equal(3)) Expect(err).To(MatchError(ContainSubstring("another error"))) }) }) diff --git a/pkg/framework/test/integration/integration_test.go b/pkg/framework/test/integration/integration_test.go index 76b031b03..e4d1816f0 100644 --- a/pkg/framework/test/integration/integration_test.go +++ b/pkg/framework/test/integration/integration_test.go @@ -14,17 +14,37 @@ import ( var _ = Describe("The Testing Framework", func() { It("Successfully manages the fixtures lifecycle", func() { - fixtures := test.NewFixtures(defaultPathToEtcd, defaultPathToApiserver) + var err error + var fixtures *test.Fixtures - By("Starting all the fixture processes") - err := fixtures.Start() - Expect(err).NotTo(HaveOccurred(), "Expected fixtures to start successfully") - - apiServerURL, err := url.Parse(fixtures.Config.APIServerURL) + fixtures, err = test.NewFixtures(defaultPathToEtcd, defaultPathToApiserver) Expect(err).NotTo(HaveOccurred()) + By("Starting all the fixture processes") + err = fixtures.Start() + Expect(err).NotTo(HaveOccurred(), "Expected fixtures to start successfully") + + apiServerConf := fixtures.APIServer.(*test.APIServer).Config + etcdConf := fixtures.Etcd.(*test.Etcd).Config + + var apiServerURL, etcdClientURL, etcdPeerURL *url.URL + etcdClientURL, err = url.Parse(etcdConf.ClientURL) + Expect(err).NotTo(HaveOccurred()) + etcdPeerURL, err = url.Parse(etcdConf.PeerURL) + Expect(err).NotTo(HaveOccurred()) + apiServerURL, err = url.Parse(apiServerConf.APIServerURL) + Expect(err).NotTo(HaveOccurred()) + + isEtcdListeningForClients := isSomethingListeningOnPort(etcdClientURL.Host) + isEtcdListeningForPeers := isSomethingListeningOnPort(etcdPeerURL.Host) isAPIServerListening := isSomethingListeningOnPort(apiServerURL.Host) + By("Ensuring Etcd is listening") + Expect(isEtcdListeningForClients()).To(BeTrue(), + fmt.Sprintf("Expected Etcd to listen for clients on %s,", etcdClientURL.Host)) + Expect(isEtcdListeningForPeers()).To(BeTrue(), + fmt.Sprintf("Expected Etcd to listen for peers on %s,", etcdPeerURL.Host)) + By("Ensuring APIServer is listening") Expect(isAPIServerListening()).To(BeTrue(), fmt.Sprintf("Expected APIServer to listen on %s", apiServerURL.Host)) @@ -33,13 +53,19 @@ var _ = Describe("The Testing Framework", func() { err = fixtures.Stop() Expect(err).NotTo(HaveOccurred(), "Expected fixtures to stop successfully") + By("Ensuring Etcd is not listening anymore") + Expect(isEtcdListeningForClients()).To(BeFalse(), "Expected Etcd not to listen for clients anymore") + Expect(isEtcdListeningForPeers()).To(BeFalse(), "Expected Etcd not to listen for peers anymore") + By("Ensuring APIServer is not listening anymore") Expect(isAPIServerListening()).To(BeFalse(), "Expected APIServer not to listen anymore") }) Measure("It should be fast to bring up and tear down the fixtures", func(b Benchmarker) { b.Time("lifecycle", func() { - fixtures := test.NewFixtures(defaultPathToEtcd, defaultPathToApiserver) + fixtures, err := test.NewFixtures(defaultPathToEtcd, defaultPathToApiserver) + Expect(err).NotTo(HaveOccurred()) + fixtures.Start() fixtures.Stop() }) diff --git a/pkg/framework/test/testfakes/fake_fixture_process.go b/pkg/framework/test/testfakes/fake_fixture_process.go index e59ac3e10..9b0c0cb21 100644 --- a/pkg/framework/test/testfakes/fake_fixture_process.go +++ b/pkg/framework/test/testfakes/fake_fixture_process.go @@ -8,12 +8,10 @@ import ( ) type FakeFixtureProcess struct { - StartStub func(config map[string]string) error + StartStub func() error startMutex sync.RWMutex - startArgsForCall []struct { - config map[string]string - } - startReturns struct { + startArgsForCall []struct{} + startReturns struct { result1 error } startReturnsOnCall map[int]struct { @@ -26,16 +24,14 @@ type FakeFixtureProcess struct { invocationsMutex sync.RWMutex } -func (fake *FakeFixtureProcess) Start(config map[string]string) error { +func (fake *FakeFixtureProcess) Start() error { fake.startMutex.Lock() ret, specificReturn := fake.startReturnsOnCall[len(fake.startArgsForCall)] - fake.startArgsForCall = append(fake.startArgsForCall, struct { - config map[string]string - }{config}) - fake.recordInvocation("Start", []interface{}{config}) + fake.startArgsForCall = append(fake.startArgsForCall, struct{}{}) + fake.recordInvocation("Start", []interface{}{}) fake.startMutex.Unlock() if fake.StartStub != nil { - return fake.StartStub(config) + return fake.StartStub() } if specificReturn { return ret.result1 @@ -49,12 +45,6 @@ func (fake *FakeFixtureProcess) StartCallCount() int { return len(fake.startArgsForCall) } -func (fake *FakeFixtureProcess) StartArgsForCall(i int) map[string]string { - fake.startMutex.RLock() - defer fake.startMutex.RUnlock() - return fake.startArgsForCall[i].config -} - func (fake *FakeFixtureProcess) StartReturns(result1 error) { fake.StartStub = nil fake.startReturns = struct { diff --git a/pkg/framework/test/testfakes/fake_listen_urlgetter.go b/pkg/framework/test/testfakes/fake_listen_urlgetter.go deleted file mode 100644 index 2aa226b05..000000000 --- a/pkg/framework/test/testfakes/fake_listen_urlgetter.go +++ /dev/null @@ -1,89 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package testfakes - -import ( - "sync" -) - -type FakeListenURLGetter struct { - Stub func() (url string, err error) - mutex sync.RWMutex - argsForCall []struct{} - returns struct { - result1 string - result2 error - } - returnsOnCall map[int]struct { - result1 string - result2 error - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeListenURLGetter) Spy() (url string, err error) { - fake.mutex.Lock() - ret, specificReturn := fake.returnsOnCall[len(fake.argsForCall)] - fake.argsForCall = append(fake.argsForCall, struct{}{}) - fake.recordInvocation("listenURLGetter", []interface{}{}) - fake.mutex.Unlock() - if fake.Stub != nil { - return fake.Stub() - } - if specificReturn { - return ret.result1, ret.result2 - } - return fake.returns.result1, fake.returns.result2 -} - -func (fake *FakeListenURLGetter) CallCount() int { - fake.mutex.RLock() - defer fake.mutex.RUnlock() - return len(fake.argsForCall) -} - -func (fake *FakeListenURLGetter) Returns(result1 string, result2 error) { - fake.Stub = nil - fake.returns = struct { - result1 string - result2 error - }{result1, result2} -} - -func (fake *FakeListenURLGetter) ReturnsOnCall(i int, result1 string, result2 error) { - fake.Stub = nil - if fake.returnsOnCall == nil { - fake.returnsOnCall = make(map[int]struct { - result1 string - result2 error - }) - } - fake.returnsOnCall[i] = struct { - result1 string - result2 error - }{result1, result2} -} - -func (fake *FakeListenURLGetter) 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 *FakeListenURLGetter) 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) -}