Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Clean up pkg/internal/testing #1540

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions hack/check-everything.sh
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/builder_suite_test.go
Expand Up @@ -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"
Expand Down
22 changes: 11 additions & 11 deletions pkg/envtest/server.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions pkg/envtest/webhook.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
Expand Down
Expand Up @@ -7,6 +7,8 @@ import (
"time"
)

// TODO(directxman12): interface / release functionality for external port managers

const (
portReserveTime = 1 * time.Minute
portConflictRetry = 100
Expand Down
@@ -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() {
Expand Down
@@ -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
Expand Down
@@ -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.
Expand Down Expand Up @@ -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
}

Expand All @@ -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
Expand All @@ -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
}
vincepri marked this conversation as resolved.
Show resolved Hide resolved

s.processState.Args, err = process.RenderTemplates(args, s)
return err
}

Expand All @@ -135,7 +148,7 @@ func (s *APIServer) populateAPIServerCerts() error {
return statErr
}

ca, err := internal.NewTinyCA()
ca, err := certs.NewTinyCA()
if err != nil {
return err
}
Expand Down Expand Up @@ -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",
}
@@ -1,4 +1,4 @@
package integration_test
package controlplane_test

import (
"testing"
Expand All @@ -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)})
}
@@ -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.
Expand Down Expand Up @@ -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
}
}
Comment on lines 67 to 71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why we need to protect this?

Could we put everything inside a sync.Once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here -- focusing on not changing functionality in this PR

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
}

Expand All @@ -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 }}",
}