Merge pull request #1540 from DirectXMan12/refactor/shuffle-internal-…

…testing

✨ Clean up pkg/internal/testing

hack/check-everything.sh

@@ -37,11 +37,6 @@ tmp_bin=/tmp/cr-tests-bin
 )
 source <(${tmp_bin}/setup-envtest use --use-env -p env ${ENVTEST_K8S_VERSION})
 
-# link the assets into integration
-for tool in kube-apiserver etcd kubectl; do
-    ln -f -s "${KUBEBUILDER_ASSETS:?unable find envtest assets}/${tool}" "${hack_dir}/../pkg/internal/testing/integration/assets/bin/${tool}"
-done
-
 ${hack_dir}/verify.sh
 ${hack_dir}/test-all.sh
 

pkg/builder/builder_suite_test.go

@@ -29,7 +29,7 @@ import (
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"

pkg/envtest/server.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane"
 
 	logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
 )
@@ -82,20 +82,20 @@ func (te *Environment) getBinAssetPath(binary string) string {
 	return filepath.Join(defaultKubebuilderPath, binary)
 }
 
-// ControlPlane is the re-exported ControlPlane type from the internal integration package
-type ControlPlane = integration.ControlPlane
+// ControlPlane is the re-exported ControlPlane type from the internal testing package
+type ControlPlane = controlplane.ControlPlane
 
-// APIServer is the re-exported APIServer type from the internal integration package
-type APIServer = integration.APIServer
+// APIServer is the re-exported APIServer type from the internal testing package
+type APIServer = controlplane.APIServer
 
-// Etcd is the re-exported Etcd type from the internal integration package
-type Etcd = integration.Etcd
+// Etcd is the re-exported Etcd type from the internal testing package
+type Etcd = controlplane.Etcd
 
 // Environment creates a Kubernetes test environment that will start / stop the Kubernetes control plane and
 // install extension APIs
 type Environment struct {
 	// ControlPlane is the ControlPlane including the apiserver and etcd
-	ControlPlane integration.ControlPlane
+	ControlPlane controlplane.ControlPlane
 
 	// Scheme is used to determine if conversion webhooks should be enabled
 	// for a particular CRD / object.
@@ -219,10 +219,10 @@ func (te *Environment) Start() (*rest.Config, error) {
 		}
 	} else {
 		if te.ControlPlane.APIServer == nil {
-			te.ControlPlane.APIServer = &integration.APIServer{Args: te.getAPIServerFlags()}
+			te.ControlPlane.APIServer = &controlplane.APIServer{Args: te.getAPIServerFlags()}
 		}
 		if te.ControlPlane.Etcd == nil {
-			te.ControlPlane.Etcd = &integration.Etcd{}
+			te.ControlPlane.Etcd = &controlplane.Etcd{}
 		}
 
 		if os.Getenv(envAttachOutput) == "true" {
@@ -357,4 +357,4 @@ func (te *Environment) useExistingCluster() bool {
 
 // DefaultKubeAPIServerFlags exposes the default args for the APIServer so that
 // you can use those to append your own additional arguments.
-var DefaultKubeAPIServerFlags = integration.APIServerDefaultArgs
+var DefaultKubeAPIServerFlags = controlplane.APIServerDefaultArgs

pkg/envtest/webhook.go

@@ -33,8 +33,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration"
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/certs"
 	"sigs.k8s.io/yaml"
 )
 
@@ -274,7 +274,7 @@ func (p *webhookPoller) poll() (done bool, err error) {
 
 // setupCA creates CA for testing and writes them to disk
 func (o *WebhookInstallOptions) setupCA() error {
-	hookCA, err := integration.NewTinyCA()
+	hookCA, err := certs.NewTinyCA()
 	if err != nil {
 		return fmt.Errorf("unable to set up webhook CA: %v", err)
 	}

pkg/internal/testing/integration/addr/manager.go → pkg/internal/testing/addr/manager.go

@@ -7,6 +7,8 @@ import (
 	"time"
 )
 
+// TODO(directxman12): interface / release functionality for external port managers
+
 const (
 	portReserveTime   = 1 * time.Minute
 	portConflictRetry = 100

pkg/internal/testing/integration/addr/manager_test.go → pkg/internal/testing/addr/manager_test.go

@@ -1,13 +1,13 @@
 package addr_test
 
 import (
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr"
-
 	"net"
 	"strconv"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
 )
 
 var _ = Describe("SuggestAddress", func() {

pkg/internal/testing/integration/internal/tinyca.go → pkg/internal/testing/certs/tinyca.go

@@ -1,4 +1,4 @@
-package internal
+package certs
 
 // NB(directxman12): nothing has verified that this has good settings.  In fact,
 // the setting generated here are probably terrible, but they're fine for integration

pkg/internal/testing/integration/apiserver.go → pkg/internal/testing/controlplane/apiserver.go

@@ -1,16 +1,19 @@
-package integration
+package controlplane
 
 import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/url"
 	"os"
 	"path/filepath"
+	"strconv"
 	"time"
 
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr"
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/certs"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/process"
 )
 
 // APIServer knows how to run a kubernetes apiserver.
@@ -68,38 +71,34 @@ type APIServer struct {
 	Out io.Writer
 	Err io.Writer
 
-	processState *internal.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 = &internal.ProcessState{}
-
-	s.processState.DefaultedProcessInput, err = internal.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
 	}
 
@@ -111,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
@@ -123,9 +133,12 @@ func (s *APIServer) setProcessState() error {
 		return err
 	}
 
-	s.processState.Args, err = internal.RenderTemplates(
-		internal.DoAPIServerArgDefaulting(s.Args), s,
-	)
+	args := s.Args
+	if len(args) == 0 {
+		args = APIServerDefaultArgs
+	}
+
+	s.processState.Args, err = process.RenderTemplates(args, s)
 	return err
 }
 
@@ -135,7 +148,7 @@ func (s *APIServer) populateAPIServerCerts() error {
 		return statErr
 	}
 
-	ca, err := internal.NewTinyCA()
+	ca, err := certs.NewTinyCA()
 	if err != nil {
 		return err
 	}
@@ -171,7 +184,16 @@ func (s *APIServer) Stop() error {
 
 // APIServerDefaultArgs exposes the default args for the APIServer so that you
 // can use those to append your own additional arguments.
-//
-// The internal default arguments are explicitly copied here, we don't want to
-// allow users to change the internal ones.
-var APIServerDefaultArgs = append([]string{}, internal.APIServerDefaultArgs...)
+var APIServerDefaultArgs = []string{
+	"--advertise-address=127.0.0.1",
+	"--etcd-servers={{ if .EtcdURL }}{{ .EtcdURL.String }}{{ end }}",
+	"--cert-dir={{ .CertDir }}",
+	"--insecure-port={{ if .URL }}{{ .URL.Port }}{{ end }}",
+	"--insecure-bind-address={{ if .URL }}{{ .URL.Hostname }}{{ end }}",
+	"--secure-port={{ if .SecurePort }}{{ .SecurePort }}{{ end }}",
+	// we're keeping this disabled because if enabled, default SA is missing which would force all tests to create one
+	// in normal apiserver operation this SA is created by controller, but that is not run in integration environment
+	"--disable-admission-plugins=ServiceAccount",
+	"--service-cluster-ip-range=10.0.0.0/24",
+	"--allow-privileged=true",
+}

pkg/internal/testing/integration/integration_suite_test.go → pkg/internal/testing/controlplane/controlplane_suite_test.go

@@ -1,4 +1,4 @@
-package integration_test
+package controlplane_test
 
 import (
 	"testing"
@@ -12,6 +12,6 @@ import (
 func TestIntegration(t *testing.T) {
 	t.Parallel()
 	RegisterFailHandler(Fail)
-	suiteName := "Integration Framework Unit Tests"
+	suiteName := "Control Plane Standup Unit Tests"
 	RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
 }

pkg/internal/testing/integration/etcd.go → pkg/internal/testing/controlplane/etcd.go

@@ -1,12 +1,14 @@
-package integration
+package controlplane
 
 import (
 	"io"
-	"time"
-
+	"net"
 	"net/url"
+	"strconv"
+	"time"
 
-	"sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/process"
 )
 
 // Etcd knows how to run an etcd server.
@@ -55,48 +57,61 @@ type Etcd struct {
 	Out io.Writer
 	Err io.Writer
 
-	processState *internal.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 = &internal.ProcessState{}
-
-	e.processState.DefaultedProcessInput, err = internal.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 = internal.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
 	e.StopTimeout = e.processState.StopTimeout
 
-	e.processState.Args, err = internal.RenderTemplates(
-		internal.DoEtcdArgDefaulting(e.Args), e,
-	)
+	args := e.Args
+	if len(args) == 0 {
+		args = EtcdDefaultArgs
+	}
+
+	e.processState.Args, err = process.RenderTemplates(args, e)
 	return err
 }
 
@@ -108,7 +123,9 @@ func (e *Etcd) Stop() error {
 
 // EtcdDefaultArgs exposes the default args for Etcd so that you
 // can use those to append your own additional arguments.
-//
-// The internal default arguments are explicitly copied here, we don't want to
-// allow users to change the internal ones.
-var EtcdDefaultArgs = append([]string{}, internal.EtcdDefaultArgs...)
+var EtcdDefaultArgs = []string{
+	"--listen-peer-urls=http://localhost:0",
+	"--advertise-client-urls={{ if .URL }}{{ .URL.String }}{{ end }}",
+	"--listen-client-urls={{ if .URL }}{{ .URL.String }}{{ end }}",
+	"--data-dir={{ .DataDir }}",
+}