Ensure testing ControlPlane is restartable

This makes sure that you can start a ControlPlane after stopping it.
This preserves existing functionality.

Note that the ability to re-use users or keep data around is *not*
preserved -- use a fixed CertDir if you want this.

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

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()
 }
 

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() {