Testing: Remove legacy cruft from process

This removes some legacy cruft from the process abstraction, namely: - the legacy health check support (startup message), since our supported etcd versions support the /health endpoint - URL populatation, since we end up needing multiple ports for the api server - gexec usage, since we don't need to check wait messages any more and the equivalent "send SIGTERM" and "wait for process to exit" parts are pretty straightforward for our usecases.
kubernetes-sigs · May 25, 2021 · 78a2f9f · 78a2f9f
1 parent 481f7b9
commit 78a2f9f
Show file tree

Hide file tree

Showing 6 changed files with 378 additions and 473 deletions.
diff --git a/pkg/internal/testing/controlplane/apiserver.go b/pkg/internal/testing/controlplane/apiserver.go
@@ -4,9 +4,11 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/url"
 	"os"
 	"path/filepath"
+	"strconv"
 	"time"
 
 	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
@@ -69,38 +71,34 @@ type APIServer struct {
 	Out io.Writer
 	Err io.Writer
 
-	processState *process.ProcessState
+	processState *process.State
 }
 
 // Start starts the apiserver, waits for it to come up, and returns an error,
 // if occurred.
 func (s *APIServer) Start() error {
 	if s.processState == nil {
-		if err := s.setProcessState(); err != nil {
+		if err := s.setState(); err != nil {
 			return err
 		}
 	}
 	return s.processState.Start(s.Out, s.Err)
 }
 
-func (s *APIServer) setProcessState() error {
+func (s *APIServer) setState() error {
 	if s.EtcdURL == nil {
 		return fmt.Errorf("expected EtcdURL to be configured")
 	}
 
 	var err error
 
-	s.processState = &process.ProcessState{}
-
-	s.processState.DefaultedProcessInput, err = process.DoDefaulting(
-		"kube-apiserver",
-		s.URL,
-		s.CertDir,
-		s.Path,
-		s.StartTimeout,
-		s.StopTimeout,
-	)
-	if err != nil {
+	s.processState = &process.State{
+		Dir:          s.CertDir,
+		Path:         s.Path,
+		StartTimeout: s.StartTimeout,
+		StopTimeout:  s.StopTimeout,
+	}
+	if err := s.processState.Init("kube-apiserver"); err != nil {
 		return err
 	}
 
@@ -112,9 +110,20 @@ func (s *APIServer) setProcessState() error {
 		}
 	}
 
-	s.processState.HealthCheckEndpoint = "/healthz"
+	if s.URL == nil {
+		port, host, err := addr.Suggest("")
+		if err != nil {
+			return err
+		}
+		s.URL = &url.URL{
+			Scheme: "http",
+			Host:   net.JoinHostPort(host, strconv.Itoa(port)),
+		}
+	}
+
+	s.processState.HealthCheck.URL = *s.URL
+	s.processState.HealthCheck.Path = "/healthz"
 
-	s.URL = &s.processState.URL
 	s.CertDir = s.processState.Dir
 	s.Path = s.processState.Path
 	s.StartTimeout = s.processState.StartTimeout

diff --git a/pkg/internal/testing/controlplane/etcd.go b/pkg/internal/testing/controlplane/etcd.go
@@ -2,10 +2,12 @@ package controlplane
 
 import (
 	"io"
-	"time"
-
+	"net"
 	"net/url"
+	"strconv"
+	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
 	"sigs.k8s.io/controller-runtime/pkg/internal/testing/process"
 )
 
@@ -55,40 +57,50 @@ type Etcd struct {
 	Out io.Writer
 	Err io.Writer
 
-	processState *process.ProcessState
+	// processState contains the actual details about this running process
+	processState *process.State
 }
 
 // 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 {
+		if err := e.setState(); err != nil {
 			return err
 		}
 	}
 	return e.processState.Start(e.Out, e.Err)
 }
 
-func (e *Etcd) setProcessState() error {
+func (e *Etcd) setState() error {
 	var err error
 
-	e.processState = &process.ProcessState{}
-
-	e.processState.DefaultedProcessInput, err = process.DoDefaulting(
-		"etcd",
-		e.URL,
-		e.DataDir,
-		e.Path,
-		e.StartTimeout,
-		e.StopTimeout,
-	)
-	if err != nil {
+	e.processState = &process.State{
+		Dir:          e.DataDir,
+		Path:         e.Path,
+		StartTimeout: e.StartTimeout,
+		StopTimeout:  e.StopTimeout,
+	}
+
+	if err := e.processState.Init("etcd"); err != nil {
 		return err
 	}
 
-	e.processState.StartMessage = getEtcdStartMessage(e.processState.URL)
+	if e.URL == nil {
+		port, host, err := addr.Suggest("")
+		if err != nil {
+			return err
+		}
+		e.URL = &url.URL{
+			Scheme: "http",
+			Host:   net.JoinHostPort(host, strconv.Itoa(port)),
+		}
+	}
+
+	// can use /health as of etcd 3.3.0
+	e.processState.HealthCheck.URL = *e.URL
+	e.processState.HealthCheck.Path = "/health"
 
-	e.URL = &e.processState.URL
 	e.DataDir = e.processState.Dir
 	e.Path = e.processState.Path
 	e.StartTimeout = e.processState.StartTimeout
@@ -117,24 +129,3 @@ var EtcdDefaultArgs = []string{
 	"--listen-client-urls={{ if .URL }}{{ .URL.String }}{{ end }}",
 	"--data-dir={{ .DataDir }}",
 }
-
-// isSecureScheme returns false when the schema is insecure.
-func isSecureScheme(scheme string) bool {
-	// https://github.com/coreos/etcd/blob/d9deeff49a080a88c982d328ad9d33f26d1ad7b6/pkg/transport/listener.go#L53
-	if scheme == "https" || scheme == "unixs" {
-		return true
-	}
-	return false
-}
-
-// getEtcdStartMessage returns an start message to inform if the client is or not insecure.
-// It will return true when the URL informed has the scheme == "https" || scheme == "unixs"
-func getEtcdStartMessage(listenURL url.URL) string {
-	if isSecureScheme(listenURL.Scheme) {
-		// https://github.com/coreos/etcd/blob/a7f1fbe00ec216fcb3a1919397a103b41dca8413/embed/serve.go#L167
-		return "serving client requests on "
-	}
-
-	// https://github.com/coreos/etcd/blob/a7f1fbe00ec216fcb3a1919397a103b41dca8413/embed/serve.go#L124
-	return "serving insecure client requests on "
-}
diff --git a/pkg/internal/testing/process/arguments_test.go b/pkg/internal/testing/process/arguments_test.go