diff --git a/pkg/framework/test/apiserver.go b/pkg/framework/test/apiserver.go index bc486e1f1..0ac192c16 100644 --- a/pkg/framework/test/apiserver.go +++ b/pkg/framework/test/apiserver.go @@ -3,7 +3,6 @@ package test import ( "fmt" "io" - "net/url" "os/exec" "time" @@ -14,11 +13,10 @@ import ( // APIServer knows how to run a kubernetes apiserver. Set it up with the path to a precompiled binary. type APIServer struct { - // The path to the apiserver binary - Path string + AddressManager AddressManager + PathFinder BinPathFinder ProcessStarter simpleSessionStarter CertDirManager certDirManager - Config *APIServerConfig Etcd FixtureProcess session SimpleSession stdOut *gbytes.Buffer @@ -32,10 +30,8 @@ type certDirManager interface { //go:generate counterfeiter . certDirManager -var apiServerBinPathFinder = DefaultBinPathFinder - // NewAPIServer creates a new APIServer Fixture Process -func NewAPIServer(config *APIServerConfig) (*APIServer, error) { +func NewAPIServer() (*APIServer, error) { starter := func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { return gexec.Start(command, out, err) } @@ -46,8 +42,6 @@ func NewAPIServer(config *APIServerConfig) (*APIServer, error) { } return &APIServer{ - Path: apiServerBinPathFinder("kube-apiserver"), - Config: config, ProcessStarter: starter, CertDirManager: NewTempDirManager(), Etcd: etcd, @@ -55,34 +49,43 @@ func NewAPIServer(config *APIServerConfig) (*APIServer, error) { } // URL returns the URL APIServer is listening on. Clients can use this to connect to APIServer. -func (s *APIServer) URL() string { - return s.Config.APIServerURL +func (s *APIServer) URL() (string, error) { + port, err := s.AddressManager.Port() + if err != nil { + return "", err + } + host, err := s.AddressManager.Host() + if err != nil { + return "", err + } + return fmt.Sprintf("http://%s:%d", host, port), nil } // Start starts the apiserver, waits for it to come up, and returns an error, if occoured. func (s *APIServer) Start() error { - if err := s.Config.Validate(); err != nil { - return err - } + s.ensureInitialized() - err := s.Etcd.Start() + port, addr, err := s.AddressManager.Initialize("localhost") if err != nil { return err } - s.stdOut = gbytes.NewBuffer() - s.stdErr = gbytes.NewBuffer() - certDir, err := s.CertDirManager.Create() if err != nil { return err } - clientURL, err := url.Parse(s.Config.APIServerURL) + err = s.Etcd.Start() if err != nil { return err } + etcdURLString, err := s.Etcd.URL() + if err != nil { + s.Etcd.Stop() + return err + } + args := []string{ "--authorization-mode=Node,RBAC", "--runtime-config=admissionregistration.k8s.io/v1alpha1", @@ -91,16 +94,16 @@ func (s *APIServer) Start() error { "--admission-control-config-file=", "--bind-address=0.0.0.0", "--storage-backend=etcd3", - fmt.Sprintf("--etcd-servers=%s", s.Etcd.URL()), + fmt.Sprintf("--etcd-servers=%s", etcdURLString), fmt.Sprintf("--cert-dir=%s", certDir), - fmt.Sprintf("--insecure-port=%s", clientURL.Port()), - fmt.Sprintf("--insecure-bind-address=%s", clientURL.Hostname()), + fmt.Sprintf("--insecure-port=%d", port), + fmt.Sprintf("--insecure-bind-address=%s", addr), } - detectedStart := s.stdErr.Detect(fmt.Sprintf("Serving insecurely on %s", clientURL.Host)) + detectedStart := s.stdErr.Detect(fmt.Sprintf("Serving insecurely on %s:%d", addr, port)) timedOut := time.After(20 * time.Second) - command := exec.Command(s.Path, args...) + command := exec.Command(s.PathFinder("kube-apiserver"), args...) s.session, err = s.ProcessStarter(command, s.stdOut, s.stdErr) if err != nil { return err @@ -114,6 +117,18 @@ func (s *APIServer) Start() error { } } +func (s *APIServer) ensureInitialized() { + if s.PathFinder == nil { + s.PathFinder = DefaultBinPathFinder + } + if s.AddressManager == nil { + s.AddressManager = &DefaultAddressManager{} + } + + s.stdOut = gbytes.NewBuffer() + s.stdErr = gbytes.NewBuffer() +} + // Stop stops this process gracefully, waits for its termination, and cleans up the cert directory. func (s *APIServer) Stop() { if s.session != nil { diff --git a/pkg/framework/test/apiserver_config.go b/pkg/framework/test/apiserver_config.go deleted file mode 100644 index 19ae5e694..000000000 --- a/pkg/framework/test/apiserver_config.go +++ /dev/null @@ -1,14 +0,0 @@ -package test - -import "github.com/asaskevich/govalidator" - -// APIServerConfig is a struct holding data to configure the API Server process -type APIServerConfig struct { - APIServerURL string `valid:"required,url"` -} - -// Validate checks that the config contains only valid URLs -func (c *APIServerConfig) Validate() error { - _, err := govalidator.ValidateStruct(c) - return err -} diff --git a/pkg/framework/test/apiserver_config_test.go b/pkg/framework/test/apiserver_config_test.go deleted file mode 100644 index ca4a16708..000000000 --- a/pkg/framework/test/apiserver_config_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package test_test - -import ( - . "k8s.io/kubectl/pkg/framework/test" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("APIServerConfig", func() { - It("does not error on valid config", func() { - conf := &APIServerConfig{ - APIServerURL: "http://this.is.some.url:1234", - } - err := conf.Validate() - Expect(err).NotTo(HaveOccurred()) - }) - - It("errors on empty config", func() { - conf := &APIServerConfig{} - err := conf.Validate() - Expect(err).To(MatchError(ContainSubstring("APIServerURL: non zero value required"))) - }) - - It("errors on malformed URLs", func() { - conf := &APIServerConfig{ - APIServerURL: "something not URLish", - } - err := conf.Validate() - Expect(err).To(MatchError(ContainSubstring("APIServerURL: something not URLish does not validate as url"))) - }) -}) diff --git a/pkg/framework/test/apiserver_constructor_test.go b/pkg/framework/test/apiserver_constructor_test.go deleted file mode 100644 index 4b73cac8f..000000000 --- a/pkg/framework/test/apiserver_constructor_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("NewAPIServer", func() { - var oldAPIServerBinPathFinder BinPathFinder - BeforeEach(func() { - oldAPIServerBinPathFinder = apiServerBinPathFinder - }) - AfterEach(func() { - apiServerBinPathFinder = oldAPIServerBinPathFinder - }) - - It("can construct a properly configured APIServer", func() { - config := &APIServerConfig{ - APIServerURL: "some APIServer URL", - } - apiServerBinPathFinder = func(name string) string { - Expect(name).To(Equal("kube-apiserver")) - return "some api server path" - } - - apiServer, err := NewAPIServer(config) - - Expect(err).NotTo(HaveOccurred()) - Expect(apiServer).NotTo(BeNil()) - Expect(apiServer.ProcessStarter).NotTo(BeNil()) - Expect(apiServer.CertDirManager).NotTo(BeNil()) - Expect(apiServer.Path).To(Equal("some api server path")) - Expect(apiServer.Etcd).NotTo(BeNil()) - Expect(apiServer.Config).To(Equal(config)) - }) -}) diff --git a/pkg/framework/test/apiserver_test.go b/pkg/framework/test/apiserver_test.go index 569e87309..7f9bd813e 100644 --- a/pkg/framework/test/apiserver_test.go +++ b/pkg/framework/test/apiserver_test.go @@ -20,117 +20,198 @@ var _ = Describe("Apiserver", func() { fakeSession *testfakes.FakeSimpleSession fakeCertDirManager *testfakes.FakeCertDirManager apiServer *APIServer - apiServerConfig *APIServerConfig fakeEtcdProcess *testfakes.FakeFixtureProcess + fakePathFinder *testfakes.FakeBinPathFinder + fakeAddressManager *testfakes.FakeAddressManager ) BeforeEach(func() { fakeSession = &testfakes.FakeSimpleSession{} fakeCertDirManager = &testfakes.FakeCertDirManager{} fakeEtcdProcess = &testfakes.FakeFixtureProcess{} + fakePathFinder = &testfakes.FakeBinPathFinder{} + fakeAddressManager = &testfakes.FakeAddressManager{} - apiServerConfig = &APIServerConfig{ - APIServerURL: "http://this.is.the.API.server:8080", - } apiServer = &APIServer{ - Path: "", + AddressManager: fakeAddressManager, + PathFinder: fakePathFinder.Spy, CertDirManager: fakeCertDirManager, - Config: apiServerConfig, Etcd: fakeEtcdProcess, } }) - It("can be queried for the URL it listens on", func() { - Expect(apiServer.URL()).To(Equal("http://this.is.the.API.server:8080")) + 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() { + //TODO Break this long test up + sessionBuffer := gbytes.NewBuffer() + fmt.Fprint(sessionBuffer, "Everything is fine") + fakeSession.BufferReturns(sessionBuffer) + + 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) + + apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + Expect(command.Args).To(ContainElement("--insecure-port=1234")) + Expect(command.Args).To(ContainElement("--insecure-bind-address=this.is.the.API.server")) + Expect(command.Args).To(ContainElement("--etcd-servers=the etcd url")) + Expect(command.Path).To(Equal("/some/path/to/apiserver")) + fmt.Fprint(err, "Serving insecurely on this.is.the.API.server:1234") + return fakeSession, nil + } + + By("Starting the API Server") + err := apiServer.Start() + Expect(err).NotTo(HaveOccurred()) + + By("...in turn starting Etcd") + 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 PortFinder") + Expect(fakeAddressManager.InitializeCallCount()).To(Equal(1)) + Expect(fakeAddressManager.InitializeArgsForCall(0)).To(Equal("localhost")) + + By("...getting the URL of Etcd") + 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)) + Expect(fakeCertDirManager.CreateCallCount()).To(Equal(1)) + + By("Stopping the API Server") + apiServer.Stop() + + Expect(fakeEtcdProcess.StopCallCount()).To(Equal(1)) + Expect(apiServer).To(gexec.Exit(143)) + Expect(fakeSession.TerminateCallCount()).To(Equal(1)) + Expect(fakeSession.WaitCallCount()).To(Equal(1)) + Expect(fakeSession.ExitCodeCallCount()).To(Equal(2)) + Expect(fakeCertDirManager.DestroyCallCount()).To(Equal(1)) + }) + }) + + Context("when starting etcd fails", func() { + It("propagates the error, and does not start the process", func() { + fakeEtcdProcess.StartReturnsOnCall(0, fmt.Errorf("starting etcd failed")) + apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + Expect(true).To(BeFalse(), + "the api server process starter shouldn't be called if starting etcd fails") + return nil, nil + } + + err := apiServer.Start() + Expect(err).To(MatchError(ContainSubstring("starting etcd failed"))) + }) + }) + + Context("when getting the URL of Etcd fails", func() { + It("propagates the error, stop Etcd and keep APIServer down", func() { + fakeEtcdProcess.URLReturns("", fmt.Errorf("no etcd url")) + + apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + Expect(true).To(BeFalse(), + "the api server process starter shouldn't be called if getting etcd's URL fails") + return nil, nil + } + + err := apiServer.Start() + Expect(err).To(MatchError(ContainSubstring("no etcd url"))) + Expect(fakeEtcdProcess.StopCallCount()).To(Equal(1)) + }) + }) + + Context("when the certificate directory cannot be created", func() { + It("propagates the error, and does not start any process", func() { + fakeCertDirManager.CreateReturnsOnCall(0, "", fmt.Errorf("Error on cert directory creation.")) + + apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + Expect(true).To(BeFalse(), + "the api server process starter shouldn't be called if creating the cert dir fails") + return nil, nil + } + + err := apiServer.Start() + Expect(err).To(MatchError(ContainSubstring("Error on cert directory creation."))) + Expect(fakeEtcdProcess.StartCallCount()).To(Equal(0)) + }) + }) + + Context("when the address manager fails to get a new address", func() { + It("propagates the error and does not start any process", func() { + fakeAddressManager.InitializeReturns(0, "", fmt.Errorf("some error finding a free port")) + + apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + Expect(true).To(BeFalse(), + "the api server process starter shouldn't be called if getting a free port fails") + return nil, nil + } + + Expect(apiServer.Start()).To(MatchError(ContainSubstring("some error finding a free port"))) + Expect(fakeEtcdProcess.StartCallCount()).To(Equal(0)) + }) + }) + + Context("when the starter returns an error", func() { + It("propagates the error", func() { + apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + return nil, fmt.Errorf("Some error in the apiserver starter.") + } + + err := apiServer.Start() + Expect(err).To(MatchError(ContainSubstring("Some error in the apiserver starter."))) + }) + }) + + Context("when we try to stop a server that hasn't been started", func() { + It("is a noop and does not call exit on the session", func() { + apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + return fakeSession, nil + } + apiServer.Stop() + Expect(fakeSession.ExitCodeCallCount()).To(Equal(0)) + }) + }) + }) - 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) - - apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - fmt.Fprint(err, "Serving insecurely on this.is.the.API.server:8080") - return fakeSession, nil - } - - By("Starting the API Server") - err := apiServer.Start() + Describe("querying the server for its URL", func() { + It("can be queried for the URL it listens on", func() { + fakeAddressManager.HostReturns("the.host.for.api.server", nil) + fakeAddressManager.PortReturns(5678, nil) + apiServerURL, err := apiServer.URL() Expect(err).NotTo(HaveOccurred()) - - By("starting Etcd") - Expect(fakeEtcdProcess.StartCallCount()).To(Equal(1), - "the Etcd process should be started exactly once") - - 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)) - Expect(fakeCertDirManager.CreateCallCount()).To(Equal(1)) - - By("Stopping the API Server") - apiServer.Stop() - - Expect(fakeEtcdProcess.StopCallCount()).To(Equal(1)) - Expect(apiServer).To(gexec.Exit(143)) - Expect(fakeSession.TerminateCallCount()).To(Equal(1)) - Expect(fakeSession.WaitCallCount()).To(Equal(1)) - Expect(fakeSession.ExitCodeCallCount()).To(Equal(2)) - Expect(fakeCertDirManager.DestroyCallCount()).To(Equal(1)) + Expect(apiServerURL).To(Equal("http://the.host.for.api.server:5678")) }) - }) - Context("when starting etcd fails", func() { - It("propagates the error, and does not start the process", func() { - fakeEtcdProcess.StartReturnsOnCall(0, fmt.Errorf("starting etcd failed")) - apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - Expect(true).To(BeFalse(), - "the api server process starter shouldn't be called if starting etcd fails") - return nil, nil - } + Context("when we query for the URL before starting the server", func() { - err := apiServer.Start() - Expect(err).To(MatchError(ContainSubstring("starting etcd failed"))) - }) - }) + Context("and so the addressmanager fails to give us a port", func() { + It("propagates the failure", func() { + fakeAddressManager.PortReturns(0, fmt.Errorf("boom")) + _, err := apiServer.URL() + Expect(err).To(MatchError(ContainSubstring("boom"))) + }) + }) - Context("when the certificate directory cannot be created", func() { - It("propagates the error, and does not start the process", func() { - fakeCertDirManager.CreateReturnsOnCall(0, "", fmt.Errorf("Error on cert directory creation.")) + Context("and so the addressmanager fails to give us a host", func() { + It("propagates the failure", func() { + fakeAddressManager.HostReturns("", fmt.Errorf("biff!")) + _, err := apiServer.URL() + Expect(err).To(MatchError(ContainSubstring("biff!"))) + }) + }) - apiServer.ProcessStarter = func(Command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - Expect(true).To(BeFalse(), - "the api server process starter shouldn't be called if creating the cert dir fails") - return nil, nil - } - - err := apiServer.Start() - Expect(err).To(MatchError(ContainSubstring("Error on cert directory creation."))) - }) - }) - - Context("when the starter returns an error", func() { - It("propagates the error", func() { - apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - return nil, fmt.Errorf("Some error in the apiserver starter.") - } - - err := apiServer.Start() - Expect(err).To(MatchError(ContainSubstring("Some error in the apiserver starter."))) - }) - }) - - Context("when we try to stop a server that hasn't been started", func() { - It("is a noop and does not call exit on the session", func() { - apiServer.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - return fakeSession, nil - } - apiServer.Stop() - Expect(fakeSession.ExitCodeCallCount()).To(Equal(0)) }) }) }) diff --git a/pkg/framework/test/bin_path_finder.go b/pkg/framework/test/bin_path_finder.go index f6ad7965b..53a55d37e 100644 --- a/pkg/framework/test/bin_path_finder.go +++ b/pkg/framework/test/bin_path_finder.go @@ -22,6 +22,8 @@ func init() { // 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 diff --git a/pkg/framework/test/democli/integration/integration_test.go b/pkg/framework/test/democli/integration/integration_test.go index bd7a7713d..5be382406 100644 --- a/pkg/framework/test/democli/integration/integration_test.go +++ b/pkg/framework/test/democli/integration/integration_test.go @@ -21,7 +21,8 @@ var _ = Describe("DemoCLI Integration", func() { }) It("can get a list of pods", func() { - apiURL := fixtures.APIServerURL() + apiURL, err := fixtures.APIServerURL() + Expect(err).NotTo(HaveOccurred()) command := exec.Command(pathToDemoCommand, "listPods", "--api-url", apiURL) session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) diff --git a/pkg/framework/test/etcd.go b/pkg/framework/test/etcd.go index 505d8bb5c..aeb76cf62 100644 --- a/pkg/framework/test/etcd.go +++ b/pkg/framework/test/etcd.go @@ -15,7 +15,7 @@ import ( // Etcd knows how to run an etcd server. Set it up with the path to a precompiled binary. type Etcd struct { - Path string + PathFinder BinPathFinder ProcessStarter simpleSessionStarter DataDirManager dataDirManager Config *EtcdConfig @@ -43,8 +43,6 @@ type SimpleSession interface { type simpleSessionStarter func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) -var etcdBinPathFinder = DefaultBinPathFinder - // NewEtcd returns a Etcd process configured with sane defaults func NewEtcd() (*Etcd, error) { starter := func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { @@ -57,36 +55,23 @@ func NewEtcd() (*Etcd, error) { } return &Etcd{ - Path: etcdBinPathFinder("etcd"), ProcessStarter: starter, DataDirManager: NewTempDirManager(), Config: config, }, nil } -// NewEtcdWithBinaryAndConfig returns a Etcd process, using the handed in binary and config. -func NewEtcdWithBinaryAndConfig(pathToEtcd string, config *EtcdConfig) *Etcd { - starter := func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { - return gexec.Start(command, out, err) - } - - etcd := &Etcd{ - Path: pathToEtcd, - ProcessStarter: starter, - DataDirManager: NewTempDirManager(), - Config: config, - } - - return etcd -} - // URL returns the URL Etcd is listening on. Clients can use this to connect to Etcd. -func (e *Etcd) URL() string { - return e.Config.ClientURL +func (e *Etcd) URL() (string, error) { + return e.Config.ClientURL, nil } // Start starts the etcd, waits for it to come up, and returns an error, if occoured. func (e *Etcd) Start() error { + if e.PathFinder == nil { + e.PathFinder = DefaultBinPathFinder + } + if err := e.Config.Validate(); err != nil { return err } @@ -120,7 +105,7 @@ func (e *Etcd) Start() error { "serving insecure client requests on %s", clientURL.Host)) timedOut := time.After(20 * time.Second) - command := exec.Command(e.Path, args...) + command := exec.Command(e.PathFinder("etcd"), args...) e.session, err = e.ProcessStarter(command, e.stdOut, e.stdErr) if err != nil { return err diff --git a/pkg/framework/test/etcd_constructor_test.go b/pkg/framework/test/etcd_constructor_test.go deleted file mode 100644 index 38f4d9bb9..000000000 --- a/pkg/framework/test/etcd_constructor_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("Etcd", func() { - Context("when constructed with the zero-config constructor", func() { - var ( - previousBinPathFinder BinPathFinder - ) - BeforeEach(func() { - previousBinPathFinder = etcdBinPathFinder - etcdBinPathFinder = func(name string) (binPath string) { - return "/the/path/to/etcd" - } - }) - AfterEach(func() { - etcdBinPathFinder = previousBinPathFinder - }) - It("gets a sensible path", func() { - etcd, err := NewEtcd() - Expect(err).NotTo(HaveOccurred()) - Expect(etcd.Path).To(Equal("/the/path/to/etcd")) - }) - }) -}) diff --git a/pkg/framework/test/etcd_test.go b/pkg/framework/test/etcd_test.go index c9cd78ff5..d0e16c1c8 100644 --- a/pkg/framework/test/etcd_test.go +++ b/pkg/framework/test/etcd_test.go @@ -17,6 +17,7 @@ var _ = Describe("Etcd", func() { var ( fakeSession *testfakes.FakeSimpleSession fakeDataDirManager *testfakes.FakeDataDirManager + fakePathFinder *testfakes.FakeBinPathFinder etcd *Etcd etcdConfig *EtcdConfig ) @@ -24,6 +25,7 @@ var _ = Describe("Etcd", func() { BeforeEach(func() { fakeSession = &testfakes.FakeSimpleSession{} fakeDataDirManager = &testfakes.FakeDataDirManager{} + fakePathFinder = &testfakes.FakeBinPathFinder{} etcdConfig = &EtcdConfig{ ClientURL: "http://this.is.etcd.listening.for.clients:1234", @@ -31,14 +33,16 @@ var _ = Describe("Etcd", func() { } etcd = &Etcd{ - Path: "", + PathFinder: fakePathFinder.Spy, DataDirManager: fakeDataDirManager, Config: etcdConfig, } }) It("can be queried for the port it listens on", func() { - Expect(etcd.URL()).To(Equal("http://this.is.etcd.listening.for.clients:1234")) + etcdURL, err := etcd.URL() + Expect(err).NotTo(HaveOccurred()) + Expect(etcdURL).To(Equal("http://this.is.etcd.listening.for.clients:1234")) }) Context("when given a path to a binary that runs for a long time", func() { @@ -49,8 +53,10 @@ var _ = Describe("Etcd", func() { fakeSession.ExitCodeReturnsOnCall(0, -1) fakeSession.ExitCodeReturnsOnCall(1, 143) + fakePathFinder.ReturnsOnCall(0, "/path/to/some/etcd") etcd.ProcessStarter = func(command *exec.Cmd, out, err io.Writer) (SimpleSession, error) { + 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 } @@ -59,6 +65,10 @@ 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")) + Eventually(etcd).Should(gbytes.Say("Everything is dandy")) Expect(fakeSession.ExitCodeCallCount()).To(Equal(0)) Expect(etcd).NotTo(gexec.Exit()) diff --git a/pkg/framework/test/fixtures.go b/pkg/framework/test/fixtures.go index ec92c2378..b93ccd820 100644 --- a/pkg/framework/test/fixtures.go +++ b/pkg/framework/test/fixtures.go @@ -1,10 +1,5 @@ package test -import ( - "fmt" - "net" -) - // Fixtures is a struct that knows how to start all your test fixtures. // // Right now, that means Etcd and your APIServer. This is likely to increase in future. @@ -17,23 +12,16 @@ type Fixtures struct { // and other internals. type FixtureProcess interface { Start() error + //TODO Stop should return an error Stop() - URL() string + URL() (string, error) } //go:generate counterfeiter . FixtureProcess // NewFixtures will give you a Fixtures struct that's properly wired together. func NewFixtures() (*Fixtures, error) { - apiServerConfig := &APIServerConfig{} - - if url, urlErr := getHTTPListenURL(); urlErr == nil { - apiServerConfig.APIServerURL = url - } else { - return nil, urlErr - } - - apiServer, err := NewAPIServer(apiServerConfig) + apiServer, err := NewAPIServer() if err != nil { return nil, err } @@ -75,33 +63,6 @@ func (f *Fixtures) Stop() error { } // APIServerURL returns the URL to the APIServer. Clients can use this URL to connect to the APIServer. -func (f *Fixtures) APIServerURL() string { +func (f *Fixtures) APIServerURL() (string, error) { return f.APIServer.URL() } - -func getHTTPListenURL() (url string, err error) { - host := "127.0.0.1" - port, err := getFreePort(host) - if err != nil { - return "", err - } - return fmt.Sprintf("http://%s:%d", host, port), nil -} - -func getFreePort(host string) (port int, err error) { - var addr *net.TCPAddr - addr, err = net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:0", host)) - if err != nil { - return - } - var l *net.TCPListener - l, err = net.ListenTCP("tcp", addr) - if err != nil { - return - } - defer func() { - err = l.Close() - }() - - return l.Addr().(*net.TCPAddr).Port, nil -} diff --git a/pkg/framework/test/fixtures_test.go b/pkg/framework/test/fixtures_test.go index 05e561c23..4df52276c 100644 --- a/pkg/framework/test/fixtures_test.go +++ b/pkg/framework/test/fixtures_test.go @@ -51,9 +51,10 @@ var _ = Describe("Fixtures", func() { }) It("can be queried for the APIServer URL", func() { - fakeAPIServerProcess.URLReturns("some url to the apiserver") + fakeAPIServerProcess.URLReturns("some url to the apiserver", nil) - url := fixtures.APIServerURL() + url, err := fixtures.APIServerURL() + Expect(err).NotTo(HaveOccurred()) Expect(url).To(Equal("some url to the apiserver")) }) diff --git a/pkg/framework/test/integration/integration_test.go b/pkg/framework/test/integration/integration_test.go index fb64ba21a..d9e340132 100644 --- a/pkg/framework/test/integration/integration_test.go +++ b/pkg/framework/test/integration/integration_test.go @@ -25,9 +25,13 @@ var _ = Describe("The Testing Framework", func() { Expect(err).NotTo(HaveOccurred(), "Expected fixtures to start successfully") var apiServerURL, etcdClientURL *url.URL - etcdClientURL, err = url.Parse(fixtures.APIServer.(*test.APIServer).Etcd.URL()) + etcdUrlString, err := fixtures.APIServer.(*test.APIServer).Etcd.URL() Expect(err).NotTo(HaveOccurred()) - apiServerURL, err = url.Parse(fixtures.APIServerURL()) + etcdClientURL, err = url.Parse(etcdUrlString) + Expect(err).NotTo(HaveOccurred()) + urlString, err := fixtures.APIServerURL() + Expect(err).NotTo(HaveOccurred()) + apiServerURL, err = url.Parse(urlString) Expect(err).NotTo(HaveOccurred()) isEtcdListeningForClients := isSomethingListeningOnPort(etcdClientURL.Host) diff --git a/pkg/framework/test/port_finder.go b/pkg/framework/test/port_finder.go index 233ae00b7..f4fd09654 100644 --- a/pkg/framework/test/port_finder.go +++ b/pkg/framework/test/port_finder.go @@ -5,11 +5,70 @@ import ( "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(host string) (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. Given a hostname it returns an address, +// a tuple consisting of a free port and the hostname resolved to its IP. +func (d *DefaultAddressManager) Initialize(host string) (port int, resolvedHost string, err error) { + if d.port != 0 { + return 0, "", fmt.Errorf("this DefaultAddressManager is already initialized") + } + addr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:0", host)) + 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 +} + // PortFinder is the signature of a function returning a free port and a resolved // address to listen/bind on, and an erro in case there were some problems finding // a free pair of address & port type PortFinder func(host string) (port int, resolvedAddress string, err error) +//go:generate counterfeiter . PortFinder + // DefaultPortFinder is the default implementation of PortFinder: It resolves that // hostname or IP handed in and asks the kernel for a random port. To make that a bit // safer, it also tries to bind to that port to make sure it is not in use. If all goes diff --git a/pkg/framework/test/port_finder_test.go b/pkg/framework/test/port_finder_test.go index faae4208d..f8b98e2ac 100644 --- a/pkg/framework/test/port_finder_test.go +++ b/pkg/framework/test/port_finder_test.go @@ -3,10 +3,90 @@ package test_test import ( . "k8s.io/kubectl/pkg/framework/test" + "fmt" + "net" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) +var _ = Describe("DefaultAddressManager", func() { + var defaultAddressManager *DefaultAddressManager + BeforeEach(func() { + defaultAddressManager = &DefaultAddressManager{} + }) + + Describe("Initialize", func() { + It("returns a free port and an address to bind to", func() { + port, host, err := defaultAddressManager.Initialize("localhost") + + Expect(err).NotTo(HaveOccurred()) + Expect(host).To(Equal("127.0.0.1")) + Expect(port).NotTo(Equal(0)) + + addr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:%d", host, port)) + Expect(err).NotTo(HaveOccurred()) + l, err := net.ListenTCP("tcp", addr) + defer func() { + Expect(l.Close()).To(Succeed()) + }() + Expect(err).NotTo(HaveOccurred()) + }) + + Context("when given an invalid hostname", func() { + It("propagates the error", func() { + _, _, err := defaultAddressManager.Initialize("this is not a hostname") + + Expect(err).To(MatchError(ContainSubstring("no such host"))) + }) + }) + + Context("when given a hostname that we don't have permission to listen on", func() { + It("propagates the error", func() { + _, _, err := defaultAddressManager.Initialize("example.com") + + Expect(err).To(MatchError(ContainSubstring("bind: can't assign requested address"))) + }) + }) + + Context("initialized multiple times", func() { + It("fails", func() { + _, _, err := defaultAddressManager.Initialize("localhost") + Expect(err).NotTo(HaveOccurred()) + _, _, err = defaultAddressManager.Initialize("localhost") + 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() + Expect(err).To(MatchError(ContainSubstring("not initialized yet"))) + }) + It("returns the same port as previously allocated by Initialize", func() { + expectedPort, _, err := defaultAddressManager.Initialize("localhost") + Expect(err).NotTo(HaveOccurred()) + actualPort, err := defaultAddressManager.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() + Expect(err).To(MatchError(ContainSubstring("not initialized yet"))) + }) + It("returns the same port as previously allocated by Initialize", func() { + _, expectedHost, err := defaultAddressManager.Initialize("localhost") + Expect(err).NotTo(HaveOccurred()) + actualHost, err := defaultAddressManager.Host() + Expect(err).NotTo(HaveOccurred()) + Expect(actualHost).To(Equal(expectedHost)) + }) + + }) +}) + var _ = Describe("DefaultPortFinder", func() { It("returns a free port and an address to bind to", func() { port, addr, err := DefaultPortFinder("127.0.0.1") diff --git a/pkg/framework/test/testfakes/fake_address_manager.go b/pkg/framework/test/testfakes/fake_address_manager.go new file mode 100644 index 000000000..7c24dfb1a --- /dev/null +++ b/pkg/framework/test/testfakes/fake_address_manager.go @@ -0,0 +1,220 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package testfakes + +import ( + "sync" + + "k8s.io/kubectl/pkg/framework/test" +) + +type FakeAddressManager struct { + InitializeStub func(host string) (port int, resolvedAddress string, err error) + initializeMutex sync.RWMutex + initializeArgsForCall []struct { + host string + } + initializeReturns struct { + result1 int + result2 string + result3 error + } + initializeReturnsOnCall map[int]struct { + result1 int + result2 string + result3 error + } + HostStub func() (string, error) + hostMutex sync.RWMutex + hostArgsForCall []struct{} + hostReturns struct { + result1 string + result2 error + } + hostReturnsOnCall map[int]struct { + result1 string + result2 error + } + PortStub func() (int, error) + portMutex sync.RWMutex + portArgsForCall []struct{} + portReturns struct { + result1 int + result2 error + } + portReturnsOnCall map[int]struct { + result1 int + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeAddressManager) Initialize(host string) (port int, resolvedAddress string, err error) { + fake.initializeMutex.Lock() + ret, specificReturn := fake.initializeReturnsOnCall[len(fake.initializeArgsForCall)] + fake.initializeArgsForCall = append(fake.initializeArgsForCall, struct { + host string + }{host}) + fake.recordInvocation("Initialize", []interface{}{host}) + fake.initializeMutex.Unlock() + if fake.InitializeStub != nil { + return fake.InitializeStub(host) + } + if specificReturn { + return ret.result1, ret.result2, ret.result3 + } + return fake.initializeReturns.result1, fake.initializeReturns.result2, fake.initializeReturns.result3 +} + +func (fake *FakeAddressManager) InitializeCallCount() int { + fake.initializeMutex.RLock() + defer fake.initializeMutex.RUnlock() + return len(fake.initializeArgsForCall) +} + +func (fake *FakeAddressManager) InitializeArgsForCall(i int) string { + fake.initializeMutex.RLock() + defer fake.initializeMutex.RUnlock() + return fake.initializeArgsForCall[i].host +} + +func (fake *FakeAddressManager) InitializeReturns(result1 int, result2 string, result3 error) { + fake.InitializeStub = nil + fake.initializeReturns = struct { + result1 int + result2 string + result3 error + }{result1, result2, result3} +} + +func (fake *FakeAddressManager) InitializeReturnsOnCall(i int, result1 int, result2 string, result3 error) { + fake.InitializeStub = nil + if fake.initializeReturnsOnCall == nil { + fake.initializeReturnsOnCall = make(map[int]struct { + result1 int + result2 string + result3 error + }) + } + fake.initializeReturnsOnCall[i] = struct { + result1 int + result2 string + result3 error + }{result1, result2, result3} +} + +func (fake *FakeAddressManager) Host() (string, error) { + fake.hostMutex.Lock() + ret, specificReturn := fake.hostReturnsOnCall[len(fake.hostArgsForCall)] + fake.hostArgsForCall = append(fake.hostArgsForCall, struct{}{}) + fake.recordInvocation("Host", []interface{}{}) + fake.hostMutex.Unlock() + if fake.HostStub != nil { + return fake.HostStub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fake.hostReturns.result1, fake.hostReturns.result2 +} + +func (fake *FakeAddressManager) HostCallCount() int { + fake.hostMutex.RLock() + defer fake.hostMutex.RUnlock() + return len(fake.hostArgsForCall) +} + +func (fake *FakeAddressManager) HostReturns(result1 string, result2 error) { + fake.HostStub = nil + fake.hostReturns = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeAddressManager) HostReturnsOnCall(i int, result1 string, result2 error) { + fake.HostStub = nil + if fake.hostReturnsOnCall == nil { + fake.hostReturnsOnCall = make(map[int]struct { + result1 string + result2 error + }) + } + fake.hostReturnsOnCall[i] = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeAddressManager) Port() (int, error) { + fake.portMutex.Lock() + ret, specificReturn := fake.portReturnsOnCall[len(fake.portArgsForCall)] + fake.portArgsForCall = append(fake.portArgsForCall, struct{}{}) + fake.recordInvocation("Port", []interface{}{}) + fake.portMutex.Unlock() + if fake.PortStub != nil { + return fake.PortStub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fake.portReturns.result1, fake.portReturns.result2 +} + +func (fake *FakeAddressManager) PortCallCount() int { + fake.portMutex.RLock() + defer fake.portMutex.RUnlock() + return len(fake.portArgsForCall) +} + +func (fake *FakeAddressManager) PortReturns(result1 int, result2 error) { + fake.PortStub = nil + fake.portReturns = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeAddressManager) PortReturnsOnCall(i int, result1 int, result2 error) { + fake.PortStub = nil + if fake.portReturnsOnCall == nil { + fake.portReturnsOnCall = make(map[int]struct { + result1 int + result2 error + }) + } + fake.portReturnsOnCall[i] = struct { + result1 int + result2 error + }{result1, result2} +} + +func (fake *FakeAddressManager) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.initializeMutex.RLock() + defer fake.initializeMutex.RUnlock() + fake.hostMutex.RLock() + defer fake.hostMutex.RUnlock() + fake.portMutex.RLock() + defer fake.portMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeAddressManager) 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.AddressManager = new(FakeAddressManager) diff --git a/pkg/framework/test/testfakes/fake_bin_path_finder.go b/pkg/framework/test/testfakes/fake_bin_path_finder.go new file mode 100644 index 000000000..4503dcb91 --- /dev/null +++ b/pkg/framework/test/testfakes/fake_bin_path_finder.go @@ -0,0 +1,98 @@ +// 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 diff --git a/pkg/framework/test/testfakes/fake_fixture_process.go b/pkg/framework/test/testfakes/fake_fixture_process.go index 7a7ef58e1..eca820dc1 100644 --- a/pkg/framework/test/testfakes/fake_fixture_process.go +++ b/pkg/framework/test/testfakes/fake_fixture_process.go @@ -20,14 +20,16 @@ type FakeFixtureProcess struct { StopStub func() stopMutex sync.RWMutex stopArgsForCall []struct{} - URLStub func() string + URLStub func() (string, error) uRLMutex sync.RWMutex uRLArgsForCall []struct{} uRLReturns struct { result1 string + result2 error } uRLReturnsOnCall map[int]struct { result1 string + result2 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex @@ -89,7 +91,7 @@ func (fake *FakeFixtureProcess) StopCallCount() int { return len(fake.stopArgsForCall) } -func (fake *FakeFixtureProcess) URL() string { +func (fake *FakeFixtureProcess) URL() (string, error) { fake.uRLMutex.Lock() ret, specificReturn := fake.uRLReturnsOnCall[len(fake.uRLArgsForCall)] fake.uRLArgsForCall = append(fake.uRLArgsForCall, struct{}{}) @@ -99,9 +101,9 @@ func (fake *FakeFixtureProcess) URL() string { return fake.URLStub() } if specificReturn { - return ret.result1 + return ret.result1, ret.result2 } - return fake.uRLReturns.result1 + return fake.uRLReturns.result1, fake.uRLReturns.result2 } func (fake *FakeFixtureProcess) URLCallCount() int { @@ -110,23 +112,26 @@ func (fake *FakeFixtureProcess) URLCallCount() int { return len(fake.uRLArgsForCall) } -func (fake *FakeFixtureProcess) URLReturns(result1 string) { +func (fake *FakeFixtureProcess) URLReturns(result1 string, result2 error) { fake.URLStub = nil fake.uRLReturns = struct { result1 string - }{result1} + result2 error + }{result1, result2} } -func (fake *FakeFixtureProcess) URLReturnsOnCall(i int, result1 string) { +func (fake *FakeFixtureProcess) URLReturnsOnCall(i int, result1 string, result2 error) { fake.URLStub = nil if fake.uRLReturnsOnCall == nil { fake.uRLReturnsOnCall = make(map[int]struct { result1 string + result2 error }) } fake.uRLReturnsOnCall[i] = struct { result1 string - }{result1} + result2 error + }{result1, result2} } func (fake *FakeFixtureProcess) Invocations() map[string][][]interface{} {