diff --git a/pkg/internal/testing/controlplane/apiserver.go b/pkg/internal/testing/controlplane/apiserver.go index ca392a831e..2d4f61cbe1 100644 --- a/pkg/internal/testing/controlplane/apiserver.go +++ b/pkg/internal/testing/controlplane/apiserver.go @@ -167,10 +167,8 @@ func (s *APIServer) Start() error { } func (s *APIServer) prepare() error { - if s.processState == nil { - if err := s.setProcessState(); err != nil { - return err - } + if err := s.setProcessState(); err != nil { + return err } if err := s.Authn.Start(); err != nil { return err @@ -235,6 +233,10 @@ func (s *APIServer) setProcessState() error { var err error + // unconditionally re-set this so we can successfully restart + // TODO(directxman12): we supported this in the past, but do we actually + // want to support re-using an API server object to restart? The loss + // of provisioned users is surprising to say the least. s.processState = &process.State{ Dir: s.CertDir, Path: s.Path, @@ -411,6 +413,9 @@ func (s *APIServer) populateAPIServerCerts() error { // Stop stops this process gracefully, waits for its termination, and cleans up // the CertDir if necessary. func (s *APIServer) Stop() error { + if s.processState.DirNeedsCleaning { + s.CertDir = "" // reset the directory if it was randomly allocated, so that we can safely restart + } if s.processState != nil { if err := s.processState.Stop(); err != nil { return err diff --git a/pkg/internal/testing/controlplane/etcd.go b/pkg/internal/testing/controlplane/etcd.go index b0a5bb5e47..2471a3ae7e 100644 --- a/pkg/internal/testing/controlplane/etcd.go +++ b/pkg/internal/testing/controlplane/etcd.go @@ -89,10 +89,8 @@ type Etcd struct { // Start starts the etcd, waits for it to come up, and returns an error, if one // occoured. func (e *Etcd) Start() error { - if e.processState == nil { - if err := e.setProcessState(); err != nil { - return err - } + if err := e.setProcessState(); err != nil { + return err } return e.processState.Start(e.Out, e.Err) } @@ -105,6 +103,10 @@ func (e *Etcd) setProcessState() error { StopTimeout: e.StopTimeout, } + // unconditionally re-set this so we can successfully restart + // TODO(directxman12): we supported this in the past, but do we actually + // want to support re-using an API server object to restart? The loss + // of provisioned users is surprising to say the least. if err := e.processState.Init("etcd"); err != nil { return err } @@ -140,6 +142,9 @@ func (e *Etcd) setProcessState() error { // Stop stops this process gracefully, waits for its termination, and cleans up // the DataDir if necessary. func (e *Etcd) Stop() error { + if e.processState.DirNeedsCleaning { + e.DataDir = "" // reset the directory if it was randomly allocated, so that we can safely restart + } return e.processState.Stop() } diff --git a/pkg/internal/testing/controlplane/plane_test.go b/pkg/internal/testing/controlplane/plane_test.go index d6465997db..714e76e8a4 100644 --- a/pkg/internal/testing/controlplane/plane_test.go +++ b/pkg/internal/testing/controlplane/plane_test.go @@ -47,6 +47,18 @@ var _ = Describe("Control Plane", func() { Expect(plane.Etcd).To(BeIdenticalTo(etcd)) }) + It("should be able to restart", func() { + // NB(directxman12): currently restarting invalidates all current users + // when using CertAuthn. We need to support restarting as per our previous + // contract, but it's not clear how much else we actually need to handle, or + // whether or not this is a safe operation. + plane := &ControlPlane{} + Expect(plane.Start()).To(Succeed()) + Expect(plane.Stop()).To(Succeed()) + Expect(plane.Start()).To(Succeed()) + Expect(plane.Stop()).To(Succeed()) + }) + Context("after having started", func() { var plane *ControlPlane BeforeEach(func() {