From 862283579ce6b58b6819ebafa91b9ff8d1f85cc9 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Mon, 15 Mar 2021 14:44:27 -0700 Subject: [PATCH 1/5] Split up testing/integration/internal This moves testing/integration/internal into several more specific packages to prepare to refactor the rest and dedup with the main envtest code. --- .../{integration => }/addr/addr_suite_test.go | 0 .../testing/{integration => }/addr/manager.go | 2 ++ .../{integration => }/addr/manager_test.go | 4 ++-- .../{integration/internal => certs}/tinyca.go | 2 +- pkg/internal/testing/integration/apiserver.go | 14 ++++++++------ .../testing/integration/control_plane.go | 4 ++-- pkg/internal/testing/integration/etcd.go | 9 +++++---- pkg/internal/testing/integration/kubectl.go | 4 ++-- .../internal => process}/arguments.go | 2 +- .../internal => process}/arguments_test.go | 4 ++-- .../internal => process}/bin_path_finder.go | 4 +++- .../bin_path_finder_test.go | 2 +- .../testing/process/internal_suite_test.go | 17 +++++++++++++++++ .../internal => process}/process.go | 4 ++-- .../internal => process}/process_test.go | 6 +++--- 15 files changed, 51 insertions(+), 27 deletions(-) rename pkg/internal/testing/{integration => }/addr/addr_suite_test.go (100%) rename pkg/internal/testing/{integration => }/addr/manager.go (94%) rename pkg/internal/testing/{integration => }/addr/manager_test.go (95%) rename pkg/internal/testing/{integration/internal => certs}/tinyca.go (99%) rename pkg/internal/testing/{integration/internal => process}/arguments.go (96%) rename pkg/internal/testing/{integration/internal => process}/arguments_test.go (96%) rename pkg/internal/testing/{integration/internal => process}/bin_path_finder.go (92%) rename pkg/internal/testing/{integration/internal => process}/bin_path_finder_test.go (99%) create mode 100644 pkg/internal/testing/process/internal_suite_test.go rename pkg/internal/testing/{integration/internal => process}/process.go (98%) rename pkg/internal/testing/{integration/internal => process}/process_test.go (98%) diff --git a/pkg/internal/testing/integration/addr/addr_suite_test.go b/pkg/internal/testing/addr/addr_suite_test.go similarity index 100% rename from pkg/internal/testing/integration/addr/addr_suite_test.go rename to pkg/internal/testing/addr/addr_suite_test.go diff --git a/pkg/internal/testing/integration/addr/manager.go b/pkg/internal/testing/addr/manager.go similarity index 94% rename from pkg/internal/testing/integration/addr/manager.go rename to pkg/internal/testing/addr/manager.go index be82613a20..5ebf331123 100644 --- a/pkg/internal/testing/integration/addr/manager.go +++ b/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 diff --git a/pkg/internal/testing/integration/addr/manager_test.go b/pkg/internal/testing/addr/manager_test.go similarity index 95% rename from pkg/internal/testing/integration/addr/manager_test.go rename to pkg/internal/testing/addr/manager_test.go index 0948bdbaa3..e0779be742 100644 --- a/pkg/internal/testing/integration/addr/manager_test.go +++ b/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() { diff --git a/pkg/internal/testing/integration/internal/tinyca.go b/pkg/internal/testing/certs/tinyca.go similarity index 99% rename from pkg/internal/testing/integration/internal/tinyca.go rename to pkg/internal/testing/certs/tinyca.go index 42991887f7..7f36c44579 100644 --- a/pkg/internal/testing/integration/internal/tinyca.go +++ b/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 diff --git a/pkg/internal/testing/integration/apiserver.go b/pkg/internal/testing/integration/apiserver.go index 119657875e..032b48a060 100644 --- a/pkg/internal/testing/integration/apiserver.go +++ b/pkg/internal/testing/integration/apiserver.go @@ -9,8 +9,10 @@ import ( "path/filepath" "time" - "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/controller-runtime/pkg/internal/testing/integration/internal" + "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) // APIServer knows how to run a kubernetes apiserver. @@ -68,7 +70,7 @@ type APIServer struct { Out io.Writer Err io.Writer - processState *internal.ProcessState + processState *process.ProcessState } // Start starts the apiserver, waits for it to come up, and returns an error, @@ -89,9 +91,9 @@ func (s *APIServer) setProcessState() error { var err error - s.processState = &internal.ProcessState{} + s.processState = &process.ProcessState{} - s.processState.DefaultedProcessInput, err = internal.DoDefaulting( + s.processState.DefaultedProcessInput, err = process.DoDefaulting( "kube-apiserver", s.URL, s.CertDir, @@ -123,7 +125,7 @@ func (s *APIServer) setProcessState() error { return err } - s.processState.Args, err = internal.RenderTemplates( + s.processState.Args, err = process.RenderTemplates( internal.DoAPIServerArgDefaulting(s.Args), s, ) return err @@ -135,7 +137,7 @@ func (s *APIServer) populateAPIServerCerts() error { return statErr } - ca, err := internal.NewTinyCA() + ca, err := certs.NewTinyCA() if err != nil { return err } diff --git a/pkg/internal/testing/integration/control_plane.go b/pkg/internal/testing/integration/control_plane.go index bab0fb20e0..8edadb1e2d 100644 --- a/pkg/internal/testing/integration/control_plane.go +++ b/pkg/internal/testing/integration/control_plane.go @@ -9,12 +9,12 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" + "sigs.k8s.io/controller-runtime/pkg/internal/testing/certs" ) // NewTinyCA creates a new a tiny CA utility for provisioning serving certs and client certs FOR TESTING ONLY. // Don't use this for anything else! -var NewTinyCA = internal.NewTinyCA +var NewTinyCA = certs.NewTinyCA // ControlPlane is a struct that knows how to start your test control plane. // diff --git a/pkg/internal/testing/integration/etcd.go b/pkg/internal/testing/integration/etcd.go index f7f4e192fa..e93bdb894d 100644 --- a/pkg/internal/testing/integration/etcd.go +++ b/pkg/internal/testing/integration/etcd.go @@ -7,6 +7,7 @@ import ( "net/url" "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" + "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) // Etcd knows how to run an etcd server. @@ -55,7 +56,7 @@ type Etcd struct { Out io.Writer Err io.Writer - processState *internal.ProcessState + processState *process.ProcessState } // Start starts the etcd, waits for it to come up, and returns an error, if one @@ -72,9 +73,9 @@ func (e *Etcd) Start() error { func (e *Etcd) setProcessState() error { var err error - e.processState = &internal.ProcessState{} + e.processState = &process.ProcessState{} - e.processState.DefaultedProcessInput, err = internal.DoDefaulting( + e.processState.DefaultedProcessInput, err = process.DoDefaulting( "etcd", e.URL, e.DataDir, @@ -94,7 +95,7 @@ func (e *Etcd) setProcessState() error { e.StartTimeout = e.processState.StartTimeout e.StopTimeout = e.processState.StopTimeout - e.processState.Args, err = internal.RenderTemplates( + e.processState.Args, err = process.RenderTemplates( internal.DoEtcdArgDefaulting(e.Args), e, ) return err diff --git a/pkg/internal/testing/integration/kubectl.go b/pkg/internal/testing/integration/kubectl.go index 8c29736b96..44e42982e1 100644 --- a/pkg/internal/testing/integration/kubectl.go +++ b/pkg/internal/testing/integration/kubectl.go @@ -5,7 +5,7 @@ import ( "io" "os/exec" - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" + "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) // KubeCtl is a wrapper around the kubectl binary. @@ -30,7 +30,7 @@ type KubeCtl struct { // stderr. func (k *KubeCtl) Run(args ...string) (stdout, stderr io.Reader, err error) { if k.Path == "" { - k.Path = internal.BinPathFinder("kubectl") + k.Path = process.BinPathFinder("kubectl") } stdoutBuffer := &bytes.Buffer{} diff --git a/pkg/internal/testing/integration/internal/arguments.go b/pkg/internal/testing/process/arguments.go similarity index 96% rename from pkg/internal/testing/integration/internal/arguments.go rename to pkg/internal/testing/process/arguments.go index 573295d904..c336f02fa7 100644 --- a/pkg/internal/testing/integration/internal/arguments.go +++ b/pkg/internal/testing/process/arguments.go @@ -1,4 +1,4 @@ -package internal +package process import ( "bytes" diff --git a/pkg/internal/testing/integration/internal/arguments_test.go b/pkg/internal/testing/process/arguments_test.go similarity index 96% rename from pkg/internal/testing/integration/internal/arguments_test.go rename to pkg/internal/testing/process/arguments_test.go index f35a410ae4..073c063a97 100644 --- a/pkg/internal/testing/integration/internal/arguments_test.go +++ b/pkg/internal/testing/process/arguments_test.go @@ -1,4 +1,4 @@ -package internal_test +package process_test import ( "net/url" @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" + . "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) var _ = Describe("Arguments", func() { diff --git a/pkg/internal/testing/integration/internal/bin_path_finder.go b/pkg/internal/testing/process/bin_path_finder.go similarity index 92% rename from pkg/internal/testing/integration/internal/bin_path_finder.go rename to pkg/internal/testing/process/bin_path_finder.go index 5597e4ba00..4f663e6ac0 100644 --- a/pkg/internal/testing/integration/internal/bin_path_finder.go +++ b/pkg/internal/testing/process/bin_path_finder.go @@ -1,4 +1,4 @@ -package internal +package process import ( "os" @@ -18,6 +18,8 @@ func init() { assetsPath = filepath.Join(filepath.Dir(thisFile), "..", "assets", "bin") } +// TODO(directxman12): unify this with the logic from envtest + // BinPathFinder checks the an environment variable, derived from the symbolic name, // and falls back to a default assets location when this variable is not set func BinPathFinder(symbolicName string) (binPath string) { diff --git a/pkg/internal/testing/integration/internal/bin_path_finder_test.go b/pkg/internal/testing/process/bin_path_finder_test.go similarity index 99% rename from pkg/internal/testing/integration/internal/bin_path_finder_test.go rename to pkg/internal/testing/process/bin_path_finder_test.go index 490b9e5b50..0a94d7cadc 100644 --- a/pkg/internal/testing/integration/internal/bin_path_finder_test.go +++ b/pkg/internal/testing/process/bin_path_finder_test.go @@ -1,4 +1,4 @@ -package internal +package process import ( "os" diff --git a/pkg/internal/testing/process/internal_suite_test.go b/pkg/internal/testing/process/internal_suite_test.go new file mode 100644 index 0000000000..46b9a60a6b --- /dev/null +++ b/pkg/internal/testing/process/internal_suite_test.go @@ -0,0 +1,17 @@ +package process_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "sigs.k8s.io/controller-runtime/pkg/envtest/printer" +) + +func TestInternal(t *testing.T) { + t.Parallel() + RegisterFailHandler(Fail) + suiteName := "Envtest Process Launcher Suite" + RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) +} diff --git a/pkg/internal/testing/integration/internal/process.go b/pkg/internal/testing/process/process.go similarity index 98% rename from pkg/internal/testing/integration/internal/process.go rename to pkg/internal/testing/process/process.go index 454354cb38..ac7fa786ae 100644 --- a/pkg/internal/testing/integration/internal/process.go +++ b/pkg/internal/testing/process/process.go @@ -1,4 +1,4 @@ -package internal +package process import ( "fmt" @@ -16,7 +16,7 @@ import ( "github.com/onsi/gomega/gbytes" "github.com/onsi/gomega/gexec" - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr" + "sigs.k8s.io/controller-runtime/pkg/internal/testing/addr" ) // ProcessState define the state of the process. diff --git a/pkg/internal/testing/integration/internal/process_test.go b/pkg/internal/testing/process/process_test.go similarity index 98% rename from pkg/internal/testing/integration/internal/process_test.go rename to pkg/internal/testing/process/process_test.go index c3f32016ca..dd933291db 100644 --- a/pkg/internal/testing/integration/internal/process_test.go +++ b/pkg/internal/testing/process/process_test.go @@ -1,4 +1,4 @@ -package internal_test +package process_test import ( "bytes" @@ -15,8 +15,8 @@ import ( . "github.com/onsi/gomega" "github.com/onsi/gomega/gexec" "github.com/onsi/gomega/ghttp" - "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/process" ) const ( From 9ed51aa48b647cd6b007a54047ad92147577527c Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Wed, 24 Mar 2021 18:12:38 -0700 Subject: [PATCH 2/5] Testing move control plane components This moves control plane components to a new internal/testing/controlplane directory, unifying the first and second levels of internal packages. --- .../apiserver.go | 29 +++++++---- .../controlplane_suite_test.go} | 4 +- .../{integration => controlplane}/etcd.go | 45 +++++++++++++---- .../{integration => controlplane}/kubectl.go | 2 +- .../kubectl_test.go | 4 +- .../plane.go} | 2 +- .../testing/integration/internal/apiserver.go | 27 ---------- .../integration/internal/apiserver_test.go | 23 --------- .../testing/integration/internal/etcd.go | 45 ----------------- .../testing/integration/internal/etcd_test.go | 49 ------------------- .../internal/internal_suite_test.go | 17 ------- 11 files changed, 61 insertions(+), 186 deletions(-) rename pkg/internal/testing/{integration => controlplane}/apiserver.go (83%) rename pkg/internal/testing/{integration/integration_suite_test.go => controlplane/controlplane_suite_test.go} (82%) rename pkg/internal/testing/{integration => controlplane}/etcd.go (68%) rename pkg/internal/testing/{integration => controlplane}/kubectl.go (98%) rename pkg/internal/testing/{integration => controlplane}/kubectl_test.go (90%) rename pkg/internal/testing/{integration/control_plane.go => controlplane/plane.go} (99%) delete mode 100644 pkg/internal/testing/integration/internal/apiserver.go delete mode 100644 pkg/internal/testing/integration/internal/apiserver_test.go delete mode 100644 pkg/internal/testing/integration/internal/etcd.go delete mode 100644 pkg/internal/testing/integration/internal/etcd_test.go delete mode 100644 pkg/internal/testing/integration/internal/internal_suite_test.go diff --git a/pkg/internal/testing/integration/apiserver.go b/pkg/internal/testing/controlplane/apiserver.go similarity index 83% rename from pkg/internal/testing/integration/apiserver.go rename to pkg/internal/testing/controlplane/apiserver.go index 032b48a060..a6ea01cb61 100644 --- a/pkg/internal/testing/integration/apiserver.go +++ b/pkg/internal/testing/controlplane/apiserver.go @@ -1,4 +1,4 @@ -package integration +package controlplane import ( "fmt" @@ -11,7 +11,6 @@ import ( "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/integration/internal" "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) @@ -125,9 +124,12 @@ func (s *APIServer) setProcessState() error { return err } - s.processState.Args, err = process.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 } @@ -173,7 +175,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", +} diff --git a/pkg/internal/testing/integration/integration_suite_test.go b/pkg/internal/testing/controlplane/controlplane_suite_test.go similarity index 82% rename from pkg/internal/testing/integration/integration_suite_test.go rename to pkg/internal/testing/controlplane/controlplane_suite_test.go index 446ea35796..8c4617d0b0 100644 --- a/pkg/internal/testing/integration/integration_suite_test.go +++ b/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)}) } diff --git a/pkg/internal/testing/integration/etcd.go b/pkg/internal/testing/controlplane/etcd.go similarity index 68% rename from pkg/internal/testing/integration/etcd.go rename to pkg/internal/testing/controlplane/etcd.go index e93bdb894d..8c906d4597 100644 --- a/pkg/internal/testing/integration/etcd.go +++ b/pkg/internal/testing/controlplane/etcd.go @@ -1,4 +1,4 @@ -package integration +package controlplane import ( "io" @@ -6,7 +6,6 @@ import ( "net/url" - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" ) @@ -87,7 +86,7 @@ func (e *Etcd) setProcessState() error { return err } - e.processState.StartMessage = internal.GetEtcdStartMessage(e.processState.URL) + e.processState.StartMessage = getEtcdStartMessage(e.processState.URL) e.URL = &e.processState.URL e.DataDir = e.processState.Dir @@ -95,9 +94,12 @@ func (e *Etcd) setProcessState() error { e.StartTimeout = e.processState.StartTimeout e.StopTimeout = e.processState.StopTimeout - e.processState.Args, err = process.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 } @@ -109,7 +111,30 @@ 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 }}", +} + +// 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/integration/kubectl.go b/pkg/internal/testing/controlplane/kubectl.go similarity index 98% rename from pkg/internal/testing/integration/kubectl.go rename to pkg/internal/testing/controlplane/kubectl.go index 44e42982e1..7e2e4feace 100644 --- a/pkg/internal/testing/integration/kubectl.go +++ b/pkg/internal/testing/controlplane/kubectl.go @@ -1,4 +1,4 @@ -package integration +package controlplane import ( "bytes" diff --git a/pkg/internal/testing/integration/kubectl_test.go b/pkg/internal/testing/controlplane/kubectl_test.go similarity index 90% rename from pkg/internal/testing/integration/kubectl_test.go rename to pkg/internal/testing/controlplane/kubectl_test.go index c257780329..3c93c0a415 100644 --- a/pkg/internal/testing/integration/kubectl_test.go +++ b/pkg/internal/testing/controlplane/kubectl_test.go @@ -1,4 +1,4 @@ -package integration_test +package controlplane_test import ( "io/ioutil" @@ -6,7 +6,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" + . "sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane" ) var _ = Describe("Kubectl", func() { diff --git a/pkg/internal/testing/integration/control_plane.go b/pkg/internal/testing/controlplane/plane.go similarity index 99% rename from pkg/internal/testing/integration/control_plane.go rename to pkg/internal/testing/controlplane/plane.go index 8edadb1e2d..97db4e6280 100644 --- a/pkg/internal/testing/integration/control_plane.go +++ b/pkg/internal/testing/controlplane/plane.go @@ -1,4 +1,4 @@ -package integration +package controlplane import ( "fmt" diff --git a/pkg/internal/testing/integration/internal/apiserver.go b/pkg/internal/testing/integration/internal/apiserver.go deleted file mode 100644 index 5c0435fa14..0000000000 --- a/pkg/internal/testing/integration/internal/apiserver.go +++ /dev/null @@ -1,27 +0,0 @@ -package internal - -// APIServerDefaultArgs allow tests to run offline, by preventing API server from attempting to -// use default route to determine its --advertise-address. -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", -} - -// DoAPIServerArgDefaulting will set default values to allow tests to run offline when the args are not informed. Otherwise, -// it will return the same []string arg passed as param. -func DoAPIServerArgDefaulting(args []string) []string { - if len(args) != 0 { - return args - } - - return APIServerDefaultArgs -} diff --git a/pkg/internal/testing/integration/internal/apiserver_test.go b/pkg/internal/testing/integration/internal/apiserver_test.go deleted file mode 100644 index 74f5901a46..0000000000 --- a/pkg/internal/testing/integration/internal/apiserver_test.go +++ /dev/null @@ -1,23 +0,0 @@ -package internal_test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" -) - -var _ = Describe("Apiserver", func() { - It("defaults Args if they are empty", func() { - initialArgs := []string{} - defaultedArgs := DoAPIServerArgDefaulting(initialArgs) - Expect(defaultedArgs).To(BeEquivalentTo(APIServerDefaultArgs)) - }) - - It("keeps Args as is if they are not empty", func() { - initialArgs := []string{"--one", "--two=2"} - defaultedArgs := DoAPIServerArgDefaulting(initialArgs) - Expect(defaultedArgs).To(BeEquivalentTo([]string{ - "--one", "--two=2", - })) - }) -}) diff --git a/pkg/internal/testing/integration/internal/etcd.go b/pkg/internal/testing/integration/internal/etcd.go deleted file mode 100644 index 2d108a3e82..0000000000 --- a/pkg/internal/testing/integration/internal/etcd.go +++ /dev/null @@ -1,45 +0,0 @@ -package internal - -import ( - "net/url" -) - -// EtcdDefaultArgs allow tests to run offline, by preventing API server from attempting to -// use default route to determine its urls. -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 }}", -} - -// DoEtcdArgDefaulting will set default values to allow tests to run offline when the args are not informed. Otherwise, -// it will return the same []string arg passed as param. -func DoEtcdArgDefaulting(args []string) []string { - if len(args) != 0 { - return args - } - - return EtcdDefaultArgs -} - -// 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/integration/internal/etcd_test.go b/pkg/internal/testing/integration/internal/etcd_test.go deleted file mode 100644 index 0a21cd9f39..0000000000 --- a/pkg/internal/testing/integration/internal/etcd_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package internal_test - -import ( - "net/url" - - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/internal" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("Etcd", func() { - It("defaults Args if they are empty", func() { - initialArgs := []string{} - defaultedArgs := DoEtcdArgDefaulting(initialArgs) - Expect(defaultedArgs).To(BeEquivalentTo(EtcdDefaultArgs)) - }) - - It("keeps Args as is if they are not empty", func() { - initialArgs := []string{"--eins", "--zwei=2"} - defaultedArgs := DoEtcdArgDefaulting(initialArgs) - Expect(defaultedArgs).To(BeEquivalentTo([]string{ - "--eins", "--zwei=2", - })) - }) -}) - -var _ = Describe("GetEtcdStartMessage()", func() { - Context("when using a non tls URL", func() { - It("generates valid start message", func() { - url := url.URL{ - Scheme: "http", - Host: "some.insecure.host:1234", - } - message := GetEtcdStartMessage(url) - Expect(message).To(Equal("serving insecure client requests on ")) - }) - }) - Context("when using a tls URL", func() { - It("generates valid start message", func() { - url := url.URL{ - Scheme: "https", - Host: "some.secure.host:8443", - } - message := GetEtcdStartMessage(url) - Expect(message).To(Equal("serving client requests on ")) - }) - }) -}) diff --git a/pkg/internal/testing/integration/internal/internal_suite_test.go b/pkg/internal/testing/integration/internal/internal_suite_test.go deleted file mode 100644 index 358570b9db..0000000000 --- a/pkg/internal/testing/integration/internal/internal_suite_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package internal_test - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" -) - -func TestInternal(t *testing.T) { - t.Parallel() - RegisterFailHandler(Fail) - suiteName := "Internal Suite" - RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) -} From c56312c9b07f9e2546eda016b625bd3507505410 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Wed, 24 Mar 2021 18:24:18 -0700 Subject: [PATCH 3/5] 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. --- .../testing/controlplane/apiserver.go | 41 +- pkg/internal/testing/controlplane/etcd.go | 69 ++- .../testing/process/arguments_test.go | 95 ---- pkg/internal/testing/process/process.go | 237 +++++----- ...al_suite_test.go => process_suite_test.go} | 0 pkg/internal/testing/process/process_test.go | 409 +++++++++--------- 6 files changed, 378 insertions(+), 473 deletions(-) delete mode 100644 pkg/internal/testing/process/arguments_test.go rename pkg/internal/testing/process/{internal_suite_test.go => process_suite_test.go} (100%) diff --git a/pkg/internal/testing/controlplane/apiserver.go b/pkg/internal/testing/controlplane/apiserver.go index a6ea01cb61..eae05eaa6a 100644 --- 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 index 8c906d4597..fde45e20ed 100644 --- 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 deleted file mode 100644 index 073c063a97..0000000000 --- a/pkg/internal/testing/process/arguments_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package process_test - -import ( - "net/url" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" -) - -var _ = Describe("Arguments", func() { - It("templates URLs", func() { - templates := []string{ - "plain URL: {{ .SomeURL }}", - "method on URL: {{ .SomeURL.Hostname }}", - "empty URL: {{ .EmptyURL }}", - "handled empty URL: {{- if .EmptyURL }}{{ .EmptyURL }}{{ end }}", - } - data := struct { - SomeURL *url.URL - EmptyURL *url.URL - }{ - &url.URL{Scheme: "https", Host: "the.host.name:3456"}, - nil, - } - - out, err := RenderTemplates(templates, data) - Expect(err).NotTo(HaveOccurred()) - Expect(out).To(BeEquivalentTo([]string{ - "plain URL: https://the.host.name:3456", - "method on URL: the.host.name", - "empty URL: <nil>", - "handled empty URL:", - })) - }) - - It("templates strings", func() { - templates := []string{ - "a string: {{ .SomeString }}", - "empty string: {{- .EmptyString }}", - } - data := struct { - SomeString string - EmptyString string - }{ - "this is some random string", - "", - } - - out, err := RenderTemplates(templates, data) - Expect(err).NotTo(HaveOccurred()) - Expect(out).To(BeEquivalentTo([]string{ - "a string: this is some random string", - "empty string:", - })) - }) - - It("has no access to unexported fields", func() { - templates := []string{ - "this is just a string", - "this blows up {{ .test }}", - } - data := struct{ test string }{"ooops private"} - - out, err := RenderTemplates(templates, data) - Expect(out).To(BeEmpty()) - Expect(err).To(MatchError( - ContainSubstring("is an unexported field of struct"), - )) - }) - - It("errors when field cannot be found", func() { - templates := []string{"this does {{ .NotExist }}"} - data := struct{ Unused string }{"unused"} - - out, err := RenderTemplates(templates, data) - Expect(out).To(BeEmpty()) - Expect(err).To(MatchError( - ContainSubstring("can't evaluate field"), - )) - }) - - Context("When overriding external default args", func() { - It("does not change the internal default args for APIServer", func() { - integration.APIServerDefaultArgs[0] = "oh no!" - Expect(APIServerDefaultArgs).NotTo(BeEquivalentTo(integration.APIServerDefaultArgs)) - }) - It("does not change the internal default args for Etcd", func() { - integration.EtcdDefaultArgs[0] = "oh no!" - Expect(EtcdDefaultArgs).NotTo(BeEquivalentTo(integration.EtcdDefaultArgs)) - }) - }) -}) diff --git a/pkg/internal/testing/process/process.go b/pkg/internal/testing/process/process.go index ac7fa786ae..fcbbe1f903 100644 --- a/pkg/internal/testing/process/process.go +++ b/pkg/internal/testing/process/process.go @@ -1,6 +1,7 @@ package process import ( + "crypto/tls" "fmt" "io" "io/ioutil" @@ -10,172 +11,190 @@ import ( "os" "os/exec" "path" - "strconv" + "sync" + "syscall" "time" +) - "github.com/onsi/gomega/gbytes" - "github.com/onsi/gomega/gexec" +// ListenAddr represents some listening address and port +type ListenAddr struct { + Address string + Port string +} - "sigs.k8s.io/controller-runtime/pkg/internal/testing/addr" -) +// URL returns a URL for this address with the given scheme and subpath +func (l *ListenAddr) URL(scheme string, path string) *url.URL { + return &url.URL{ + Scheme: scheme, + Host: l.HostPort(), + Path: path, + } +} + +// HostPort returns the joined host-port pair for this address +func (l *ListenAddr) HostPort() string { + return net.JoinHostPort(l.Address, l.Port) +} + +// HealthCheck describes the information needed to health-check a process via +// some health-check URL. +type HealthCheck struct { + url.URL -// ProcessState define the state of the process. -type ProcessState struct { - DefaultedProcessInput - Session *gexec.Session - // Healthcheck Endpoint. If we get http.StatusOK from this endpoint, we - // assume the process is ready to operate. E.g. "/healthz". If this is set, - // we ignore StartMessage. - HealthCheckEndpoint string // HealthCheckPollInterval is the interval which will be used for polling the - // HealthCheckEndpoint. - // If left empty it will default to 100 Milliseconds. - HealthCheckPollInterval time.Duration - // StartMessage is the message to wait for on stderr. If we receive this - // message, we assume the process is ready to operate. Ignored if - // HealthCheckEndpoint is specified. + // endpoint described by Host, Port, and Path. // - // The usage of StartMessage is discouraged, favour HealthCheckEndpoint - // instead! + // If left empty it will default to 100 Milliseconds. + PollInterval time.Duration +} + +// State define the state of the process. +type State struct { + Cmd *exec.Cmd + + // HealthCheck describes how to check if this process is up. If we get an http.StatusOK, + // we assume the process is ready to operate. // - // Deprecated: Use HealthCheckEndpoint in favour of StartMessage - StartMessage string - Args []string + // For example, the /healthz endpoint of the k8s API server, or the /health endpoint of etcd. + HealthCheck HealthCheck - // ready holds wether the process is currently in ready state (hit the ready condition) or not. - // It will be set to true on a successful `Start()` and set to false on a successful `Stop()` - ready bool -} + Args []string + + StopTimeout time.Duration + StartTimeout time.Duration -// DefaultedProcessInput defines the default process input required to perform the test. -type DefaultedProcessInput struct { - URL url.URL Dir string DirNeedsCleaning bool Path string - StopTimeout time.Duration - StartTimeout time.Duration -} -// DoDefaulting sets the default configuration according to the data informed and return an DefaultedProcessInput -// and an error if some requirement was not informed. -func DoDefaulting( - name string, - listenURL *url.URL, - dir string, - path string, - startTimeout time.Duration, - stopTimeout time.Duration, -) (DefaultedProcessInput, error) { - defaults := DefaultedProcessInput{ - Dir: dir, - Path: path, - StartTimeout: startTimeout, - StopTimeout: stopTimeout, - } - - if listenURL == nil { - port, host, err := addr.Suggest("") - if err != nil { - return DefaultedProcessInput{}, err - } - defaults.URL = url.URL{ - Scheme: "http", - Host: net.JoinHostPort(host, strconv.Itoa(port)), - } - } else { - defaults.URL = *listenURL - } + // ready holds wether the process is currently in ready state (hit the ready condition) or not. + // It will be set to true on a successful `Start()` and set to false on a successful `Stop()` + ready bool + + // waitDone is closed when our call to wait finishes up, and indicates that + // our process has terminated. + waitDone chan struct{} + errMu sync.Mutex + exitErr error + exited bool +} - if path == "" { +// Init sets up this process, configuring binary paths if missing, initializing +// temporary directories, etc. +// +// This defaults all defaultable fields. +func (ps *State) Init(name string) error { + if ps.Path == "" { if name == "" { - return DefaultedProcessInput{}, fmt.Errorf("must have at least one of name or path") + return fmt.Errorf("must have at least one of name or path") } - defaults.Path = BinPathFinder(name) + ps.Path = BinPathFinder(name) } - if dir == "" { + if ps.Dir == "" { newDir, err := ioutil.TempDir("", "k8s_test_framework_") if err != nil { - return DefaultedProcessInput{}, err + return err } - defaults.Dir = newDir - defaults.DirNeedsCleaning = true + ps.Dir = newDir + ps.DirNeedsCleaning = true } - if startTimeout == 0 { - defaults.StartTimeout = 20 * time.Second + if ps.StartTimeout == 0 { + ps.StartTimeout = 20 * time.Second } - if stopTimeout == 0 { - defaults.StopTimeout = 20 * time.Second + if ps.StopTimeout == 0 { + ps.StopTimeout = 20 * time.Second } - - return defaults, nil + return nil } type stopChannel chan struct{} // Start starts the apiserver, waits for it to come up, and returns an error, // if occurred. -func (ps *ProcessState) Start(stdout, stderr io.Writer) (err error) { +func (ps *State) Start(stdout, stderr io.Writer) (err error) { if ps.ready { return nil } - command := exec.Command(ps.Path, ps.Args...) + ps.Cmd = exec.Command(ps.Path, ps.Args...) + ps.Cmd.Stdout = stdout + ps.Cmd.Stderr = stderr ready := make(chan bool) timedOut := time.After(ps.StartTimeout) var pollerStopCh stopChannel + pollerStopCh = make(stopChannel) + go pollURLUntilOK(ps.HealthCheck.URL, ps.HealthCheck.PollInterval, ready, pollerStopCh) - if ps.HealthCheckEndpoint != "" { - healthCheckURL := ps.URL - healthCheckURL.Path = ps.HealthCheckEndpoint - pollerStopCh = make(stopChannel) - go pollURLUntilOK(healthCheckURL, ps.HealthCheckPollInterval, ready, pollerStopCh) - } else { - startDetectStream := gbytes.NewBuffer() - ready = startDetectStream.Detect(ps.StartMessage) - stderr = safeMultiWriter(stderr, startDetectStream) - } + ps.waitDone = make(chan struct{}) - ps.Session, err = gexec.Start(command, stdout, stderr) - if err != nil { + if err := ps.Cmd.Start(); err != nil { + ps.errMu.Lock() + defer ps.errMu.Unlock() + ps.exited = true return err } + go func() { + defer close(ps.waitDone) + err := ps.Cmd.Wait() + + ps.errMu.Lock() + defer ps.errMu.Unlock() + ps.exitErr = err + ps.exited = true + }() select { case <-ready: ps.ready = true return nil + case <-ps.waitDone: + if pollerStopCh != nil { + close(pollerStopCh) + } + return fmt.Errorf("timeout waiting for process %s to start successfully "+ + "(it may have failed to start, or stopped unexpectedly before becoming ready)", + path.Base(ps.Path)) case <-timedOut: if pollerStopCh != nil { close(pollerStopCh) } - if ps.Session != nil { - ps.Session.Terminate() + if ps.Cmd != nil { + // intentionally ignore this -- we might've crashed, failed to start, etc + ps.Cmd.Process.Signal(syscall.SIGTERM) //nolint errcheck } return fmt.Errorf("timeout waiting for process %s to start", path.Base(ps.Path)) } } -func safeMultiWriter(writers ...io.Writer) io.Writer { - safeWriters := []io.Writer{} - for _, w := range writers { - if w != nil { - safeWriters = append(safeWriters, w) - } - } - return io.MultiWriter(safeWriters...) +// Exited returns true if the process exited, and may also +// return an error (as per Cmd.Wait) if the process did not +// exit with error code 0. +func (ps *State) Exited() (bool, error) { + ps.errMu.Lock() + defer ps.errMu.Unlock() + return ps.exited, ps.exitErr } func pollURLUntilOK(url url.URL, interval time.Duration, ready chan bool, stopCh stopChannel) { + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + // there's probably certs *somewhere*, + // but it's fine to just skip validating + // them for health checks during testing + InsecureSkipVerify: true, + }, + }, + } if interval <= 0 { interval = 100 * time.Millisecond } for { - res, err := http.Get(url.String()) + res, err := client.Get(url.String()) if err == nil { res.Body.Close() if res.StatusCode == http.StatusOK { @@ -195,23 +214,21 @@ func pollURLUntilOK(url url.URL, interval time.Duration, ready chan bool, stopCh // Stop stops this process gracefully, waits for its termination, and cleans up // the CertDir if necessary. -func (ps *ProcessState) Stop() error { - if ps.Session == nil { +func (ps *State) Stop() error { + if ps.Cmd == nil { return nil } - - // gexec's Session methods (Signal, Kill, ...) do not check if the Process is - // nil, so we are doing this here for now. - // This should probably be fixed in gexec. - if ps.Session.Command.Process == nil { + if done, _ := ps.Exited(); done { return nil } + if err := ps.Cmd.Process.Signal(syscall.SIGTERM); err != nil { + return fmt.Errorf("unable to signal for process %s to stop: %w", ps.Path, err) + } - detectedStop := ps.Session.Terminate().Exited timedOut := time.After(ps.StopTimeout) select { - case <-detectedStop: + case <-ps.waitDone: break case <-timedOut: return fmt.Errorf("timeout waiting for process %s to stop", path.Base(ps.Path)) diff --git a/pkg/internal/testing/process/internal_suite_test.go b/pkg/internal/testing/process/process_suite_test.go similarity index 100% rename from pkg/internal/testing/process/internal_suite_test.go rename to pkg/internal/testing/process/process_suite_test.go diff --git a/pkg/internal/testing/process/process_test.go b/pkg/internal/testing/process/process_test.go index dd933291db..a486cff554 100644 --- a/pkg/internal/testing/process/process_test.go +++ b/pkg/internal/testing/process/process_test.go @@ -7,13 +7,11 @@ import ( "net/http" "net/url" "os" - "os/exec" "strconv" "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/onsi/gomega/gexec" "github.com/onsi/gomega/ghttp" "sigs.k8s.io/controller-runtime/pkg/internal/testing/addr" . "sigs.k8s.io/controller-runtime/pkg/internal/testing/process" @@ -25,159 +23,73 @@ const ( var _ = Describe("Start method", func() { var ( - processState *ProcessState + processState *State + server *ghttp.Server ) BeforeEach(func() { - processState = &ProcessState{} + server = ghttp.NewServer() + + processState = &State{ + Path: "bash", + Args: simpleBashScript, + HealthCheck: HealthCheck{ + URL: getServerURL(server), + }, + } processState.Path = "bash" processState.Args = simpleBashScript - }) - - It("can start a process", func() { - processState.StartTimeout = 10 * time.Second - processState.StartMessage = "loop 5" - - err := processState.Start(nil, nil) - Expect(err).NotTo(HaveOccurred()) - Consistently(processState.Session.ExitCode).Should(BeNumerically("==", -1)) + }) + AfterEach(func() { + server.Close() }) - Context("when a health check endpoint is provided", func() { - var server *ghttp.Server + Context("when process takes too long to start", func() { BeforeEach(func() { - server = ghttp.NewServer() - }) - AfterEach(func() { - server.Close() - }) - - Context("when the healthcheck returns ok", func() { - BeforeEach(func() { - server.RouteToHandler("GET", healthURLPath, ghttp.RespondWith(http.StatusOK, "")) - }) - - It("hits the endpoint, and successfully starts", func() { - processState.HealthCheckEndpoint = healthURLPath - processState.StartTimeout = 100 * time.Millisecond - processState.URL = getServerURL(server) - - err := processState.Start(nil, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(server.ReceivedRequests()).To(HaveLen(1)) - Consistently(processState.Session.ExitCode).Should(BeNumerically("==", -1)) + server.RouteToHandler("GET", healthURLPath, func(resp http.ResponseWriter, _ *http.Request) { + time.Sleep(250 * time.Millisecond) + resp.WriteHeader(http.StatusOK) }) }) + It("returns a timeout error", func() { + processState.StartTimeout = 200 * time.Millisecond - Context("when the healthcheck always returns failure", func() { - BeforeEach(func() { - server.RouteToHandler("GET", healthURLPath, ghttp.RespondWith(http.StatusInternalServerError, "")) - }) - It("returns a timeout error and stops health API checker", func() { - processState.HealthCheckEndpoint = healthURLPath - processState.StartTimeout = 500 * time.Millisecond - processState.URL = getServerURL(server) - - err := processState.Start(nil, nil) - Expect(err).To(MatchError(ContainSubstring("timeout"))) - - nrReceivedRequests := len(server.ReceivedRequests()) - Expect(nrReceivedRequests).To(Equal(5)) - time.Sleep(200 * time.Millisecond) - Expect(nrReceivedRequests).To(Equal(5)) - }) - }) - - Context("when the healthcheck isn't even listening", func() { - BeforeEach(func() { - server.Close() - }) - - It("returns a timeout error", func() { - processState.HealthCheckEndpoint = healthURLPath - processState.StartTimeout = 500 * time.Millisecond - - port, host, err := addr.Suggest("") - Expect(err).NotTo(HaveOccurred()) - - processState.URL = url.URL{ - Scheme: "http", - Host: net.JoinHostPort(host, strconv.Itoa(port)), - } + err := processState.Start(nil, nil) + Expect(err).To(MatchError(ContainSubstring("timeout"))) - err = processState.Start(nil, nil) - Expect(err).To(MatchError(ContainSubstring("timeout"))) - }) + Eventually(func() bool { done, _ := processState.Exited(); return done }).Should(BeTrue()) }) + }) - Context("when the healthcheck fails initially but succeeds eventually", func() { - BeforeEach(func() { - server.AppendHandlers( - ghttp.RespondWith(http.StatusInternalServerError, ""), - ghttp.RespondWith(http.StatusInternalServerError, ""), - ghttp.RespondWith(http.StatusInternalServerError, ""), - ghttp.RespondWith(http.StatusOK, ""), - ) - }) - - It("hits the endpoint repeatedly, and successfully starts", func() { - processState.HealthCheckEndpoint = healthURLPath - processState.StartTimeout = 20 * time.Second - processState.URL = getServerURL(server) - - err := processState.Start(nil, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(server.ReceivedRequests()).To(HaveLen(4)) - Consistently(processState.Session.ExitCode).Should(BeNumerically("==", -1)) - }) - - Context("when the polling interval is not configured", func() { - It("uses the default interval for polling", func() { - processState.HealthCheckEndpoint = "/helathz" - processState.StartTimeout = 300 * time.Millisecond - processState.URL = getServerURL(server) + Context("when the healthcheck returns ok", func() { + BeforeEach(func() { - Expect(processState.Start(nil, nil)).To(MatchError(ContainSubstring("timeout"))) - Expect(server.ReceivedRequests()).To(HaveLen(3)) - }) - }) + server.RouteToHandler("GET", healthURLPath, ghttp.RespondWith(http.StatusOK, "")) + }) - Context("when the polling interval is configured", func() { - BeforeEach(func() { - processState.HealthCheckPollInterval = time.Millisecond * 150 - }) + It("can start a process", func() { + processState.StartTimeout = 10 * time.Second - It("hits the endpoint in the configured interval", func() { - processState.HealthCheckEndpoint = healthURLPath - processState.StartTimeout = 3 * processState.HealthCheckPollInterval - processState.URL = getServerURL(server) + err := processState.Start(nil, nil) + Expect(err).NotTo(HaveOccurred()) - Expect(processState.Start(nil, nil)).To(MatchError(ContainSubstring("timeout"))) - Expect(server.ReceivedRequests()).To(HaveLen(3)) - }) - }) + Consistently(processState.Exited).Should(BeFalse()) }) - }) - - Context("when a health check endpoint is not provided", func() { - - Context("when process takes too long to start", func() { - It("returns a timeout error", func() { - processState.StartTimeout = 200 * time.Millisecond - processState.StartMessage = "loop 5000" - err := processState.Start(nil, nil) - Expect(err).To(MatchError(ContainSubstring("timeout"))) + It("hits the endpoint, and successfully starts", func() { + processState.StartTimeout = 100 * time.Millisecond - Eventually(processState.Session.ExitCode).Should(Equal(143)) - }) + err := processState.Start(nil, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(server.ReceivedRequests()).To(HaveLen(1)) + Consistently(processState.Exited).Should(BeFalse()) }) Context("when the command cannot be started", func() { var err error BeforeEach(func() { - processState = &ProcessState{} + processState = &State{} processState.Path = "/nonexistent" err = processState.Start(nil, nil) @@ -197,43 +109,145 @@ var _ = Describe("Start method", func() { }) }) }) + + Context("when IO is configured", func() { + It("can inspect stdout & stderr", func() { + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + + processState.Args = []string{ + "-c", + ` + echo 'this is stderr' >&2 + echo 'that is stdout' + echo 'i started' >&2 + `, + } + processState.StartTimeout = 1 * time.Second + + Expect(processState.Start(stdout, stderr)).To(Succeed()) + Eventually(processState.Exited).Should(BeTrue()) + + Expect(stdout.String()).To(Equal("that is stdout\n")) + Expect(stderr.String()).To(Equal("this is stderr\ni started\n")) + }) + }) + }) + + Context("when the healthcheck always returns failure", func() { + BeforeEach(func() { + server.RouteToHandler("GET", healthURLPath, ghttp.RespondWith(http.StatusInternalServerError, "")) + }) + It("returns a timeout error and stops health API checker", func() { + processState.HealthCheck.URL = getServerURL(server) + processState.HealthCheck.Path = healthURLPath + processState.StartTimeout = 500 * time.Millisecond + + err := processState.Start(nil, nil) + Expect(err).To(MatchError(ContainSubstring("timeout"))) + + nrReceivedRequests := len(server.ReceivedRequests()) + Expect(nrReceivedRequests).To(Equal(5)) + time.Sleep(200 * time.Millisecond) + Expect(nrReceivedRequests).To(Equal(5)) + }) }) - Context("when IO is configured", func() { - It("can inspect stdout & stderr", func() { - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - - processState.Args = []string{ - "-c", - ` - echo 'this is stderr' >&2 - echo 'that is stdout' - echo 'i started' >&2 - `, + Context("when the healthcheck isn't even listening", func() { + BeforeEach(func() { + server.Close() + }) + + It("returns a timeout error", func() { + processState.HealthCheck.Path = healthURLPath + processState.StartTimeout = 500 * time.Millisecond + + port, host, err := addr.Suggest("") + Expect(err).NotTo(HaveOccurred()) + + processState.HealthCheck.URL = url.URL{ + Scheme: "http", + Host: net.JoinHostPort(host, strconv.Itoa(port)), } - processState.StartMessage = "i started" - processState.StartTimeout = 1 * time.Second - Expect(processState.Start(stdout, stderr)).To(Succeed()) - Eventually(processState.Session).Should(gexec.Exit()) + err = processState.Start(nil, nil) + Expect(err).To(MatchError(ContainSubstring("timeout"))) + }) + }) + + Context("when the healthcheck fails initially but succeeds eventually", func() { + BeforeEach(func() { + server.AppendHandlers( + ghttp.RespondWith(http.StatusInternalServerError, ""), + ghttp.RespondWith(http.StatusInternalServerError, ""), + ghttp.RespondWith(http.StatusInternalServerError, ""), + ghttp.RespondWith(http.StatusOK, ""), + ) + }) + + It("hits the endpoint repeatedly, and successfully starts", func() { + processState.HealthCheck.URL = getServerURL(server) + processState.HealthCheck.Path = healthURLPath + processState.StartTimeout = 20 * time.Second + + err := processState.Start(nil, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(server.ReceivedRequests()).To(HaveLen(4)) + Consistently(processState.Exited).Should(BeFalse()) + }) + + Context("when the polling interval is not configured", func() { + It("uses the default interval for polling", func() { + processState.HealthCheck.URL = getServerURL(server) + processState.HealthCheck.Path = "/helathz" + processState.StartTimeout = 300 * time.Millisecond + + Expect(processState.Start(nil, nil)).To(MatchError(ContainSubstring("timeout"))) + Expect(server.ReceivedRequests()).To(HaveLen(3)) + }) + }) + + Context("when the polling interval is configured", func() { + BeforeEach(func() { + processState.HealthCheck.URL = getServerURL(server) + processState.HealthCheck.Path = healthURLPath + processState.HealthCheck.PollInterval = time.Millisecond * 150 + }) - Expect(stdout.String()).To(Equal("that is stdout\n")) - Expect(stderr.String()).To(Equal("this is stderr\ni started\n")) + It("hits the endpoint in the configured interval", func() { + processState.StartTimeout = 3 * processState.HealthCheck.PollInterval + + Expect(processState.Start(nil, nil)).To(MatchError(ContainSubstring("timeout"))) + Expect(server.ReceivedRequests()).To(HaveLen(3)) + }) }) }) }) var _ = Describe("Stop method", func() { + var ( + server *ghttp.Server + processState *State + ) + BeforeEach(func() { + server = ghttp.NewServer() + server.RouteToHandler("GET", healthURLPath, ghttp.RespondWith(http.StatusOK, "")) + processState = &State{ + Path: "bash", + Args: simpleBashScript, + HealthCheck: HealthCheck{ + URL: getServerURL(server), + }, + } + processState.StartTimeout = 10 * time.Second + }) + + AfterEach(func() { + server.Close() + }) Context("when Stop() is called", func() { - var ( - processState *ProcessState - ) BeforeEach(func() { - var err error - processState = &ProcessState{} - processState.Session, err = gexec.Start(getSimpleCommand(), nil, nil) - Expect(err).NotTo(HaveOccurred()) + Expect(processState.Start(nil, nil)).To(Succeed()) processState.StopTimeout = 10 * time.Second }) @@ -255,13 +269,8 @@ var _ = Describe("Stop method", func() { Context("when the command cannot be stopped", func() { It("returns a timeout error", func() { - var err error - - processState := &ProcessState{} - processState.Session, err = gexec.Start(getSimpleCommand(), nil, nil) - Expect(err).NotTo(HaveOccurred()) - processState.Session.Exited = make(chan struct{}) - processState.StopTimeout = 200 * time.Millisecond + Expect(processState.Start(nil, nil)).To(Succeed()) + processState.StopTimeout = 1 * time.Nanosecond // much shorter than the sleep in the script Expect(processState.Stop()).To(MatchError(ContainSubstring("timeout"))) }) @@ -271,9 +280,7 @@ var _ = Describe("Stop method", func() { It("removes the directory", func() { var err error - processState := &ProcessState{} - processState.Session, err = gexec.Start(getSimpleCommand(), nil, nil) - Expect(err).NotTo(HaveOccurred()) + Expect(processState.Start(nil, nil)).To(Succeed()) processState.Dir, err = ioutil.TempDir("", "k8s_test_framework_") Expect(err).NotTo(HaveOccurred()) processState.DirNeedsCleaning = true @@ -285,67 +292,46 @@ var _ = Describe("Stop method", func() { }) }) -var _ = Describe("DoDefaulting", func() { +var _ = Describe("Init", func() { Context("when all inputs are provided", func() { It("passes them through", func() { - defaults, err := DoDefaulting( - "some name", - &url.URL{Host: "some.host.to.listen.on"}, - "/some/dir", - "/some/path/to/some/bin", - 20*time.Hour, - 65537*time.Millisecond, - ) - Expect(err).NotTo(HaveOccurred()) + ps := &State{ + Dir: "/some/dir", + Path: "/some/path/to/some/bin", + StartTimeout: 20 * time.Hour, + StopTimeout: 65537 * time.Millisecond, + } + + Expect(ps.Init("some name")).To(Succeed()) - Expect(defaults.URL).To(Equal(url.URL{Host: "some.host.to.listen.on"})) - Expect(defaults.Dir).To(Equal("/some/dir")) - Expect(defaults.DirNeedsCleaning).To(BeFalse()) - Expect(defaults.Path).To(Equal("/some/path/to/some/bin")) - Expect(defaults.StartTimeout).To(Equal(20 * time.Hour)) - Expect(defaults.StopTimeout).To(Equal(65537 * time.Millisecond)) + Expect(ps.Dir).To(Equal("/some/dir")) + Expect(ps.DirNeedsCleaning).To(BeFalse()) + Expect(ps.Path).To(Equal("/some/path/to/some/bin")) + Expect(ps.StartTimeout).To(Equal(20 * time.Hour)) + Expect(ps.StopTimeout).To(Equal(65537 * time.Millisecond)) }) }) Context("when inputs are empty", func() { - It("defaults them", func() { - defaults, err := DoDefaulting( - "some name", - nil, - "", - "", - 0, - 0, - ) - Expect(err).NotTo(HaveOccurred()) - - Expect(defaults.Dir).To(BeADirectory()) - Expect(os.RemoveAll(defaults.Dir)).To(Succeed()) - Expect(defaults.DirNeedsCleaning).To(BeTrue()) + It("ps them", func() { + ps := &State{} + Expect(ps.Init("some name")).To(Succeed()) - Expect(defaults.URL).NotTo(BeZero()) - Expect(defaults.URL.Scheme).To(Equal("http")) - Expect(defaults.URL.Hostname()).NotTo(BeEmpty()) - Expect(defaults.URL.Port()).NotTo(BeEmpty()) + Expect(ps.Dir).To(BeADirectory()) + Expect(os.RemoveAll(ps.Dir)).To(Succeed()) + Expect(ps.DirNeedsCleaning).To(BeTrue()) - Expect(defaults.Path).NotTo(BeEmpty()) + Expect(ps.Path).NotTo(BeEmpty()) - Expect(defaults.StartTimeout).NotTo(BeZero()) - Expect(defaults.StopTimeout).NotTo(BeZero()) + Expect(ps.StartTimeout).NotTo(BeZero()) + Expect(ps.StopTimeout).NotTo(BeZero()) }) }) Context("when neither name nor path are provided", func() { It("returns an error", func() { - _, err := DoDefaulting( - "", - nil, - "", - "", - 0, - 0, - ) - Expect(err).To(MatchError("must have at least one of name or path")) + ps := &State{} + Expect(ps.Init("")).To(MatchError("must have at least one of name or path")) }) }) }) @@ -363,12 +349,9 @@ var simpleBashScript = []string{ `, } -func getSimpleCommand() *exec.Cmd { - return exec.Command("bash", simpleBashScript...) -} - func getServerURL(server *ghttp.Server) url.URL { url, err := url.Parse(server.URL()) Expect(err).NotTo(HaveOccurred()) + url.Path = healthURLPath return *url } From 9d53bf1467af23d1c90df47160ec47a8d63cf22a Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 25 May 2021 13:35:23 -0700 Subject: [PATCH 4/5] Remove last of pkg/internal/testing/integration This only contained integration tests, which will be re-introduced in a cleaned up form shortly. --- hack/check-everything.sh | 5 - pkg/internal/testing/integration/.gitignore | 1 - pkg/internal/testing/integration/README.md | 10 - .../testing/integration/assets/bin/.gitkeep | 1 - pkg/internal/testing/integration/doc.go | 112 ------- .../apiserver_integration_test.go | 24 -- .../internal/integration_tests/doc.go | 7 - .../etcd_integration_test.go | 66 ---- .../integration_suite_test.go | 17 -- .../integration_tests/integration_test.go | 288 ------------------ 10 files changed, 531 deletions(-) delete mode 100644 pkg/internal/testing/integration/.gitignore delete mode 100644 pkg/internal/testing/integration/README.md delete mode 100644 pkg/internal/testing/integration/assets/bin/.gitkeep delete mode 100644 pkg/internal/testing/integration/doc.go delete mode 100644 pkg/internal/testing/integration/internal/integration_tests/apiserver_integration_test.go delete mode 100644 pkg/internal/testing/integration/internal/integration_tests/doc.go delete mode 100644 pkg/internal/testing/integration/internal/integration_tests/etcd_integration_test.go delete mode 100644 pkg/internal/testing/integration/internal/integration_tests/integration_suite_test.go delete mode 100644 pkg/internal/testing/integration/internal/integration_tests/integration_test.go diff --git a/hack/check-everything.sh b/hack/check-everything.sh index f0367ac17f..1f02d884f4 100755 --- a/hack/check-everything.sh +++ b/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 diff --git a/pkg/internal/testing/integration/.gitignore b/pkg/internal/testing/integration/.gitignore deleted file mode 100644 index 16308b38c4..0000000000 --- a/pkg/internal/testing/integration/.gitignore +++ /dev/null @@ -1 +0,0 @@ -assets/bin diff --git a/pkg/internal/testing/integration/README.md b/pkg/internal/testing/integration/README.md deleted file mode 100644 index abf9316d44..0000000000 --- a/pkg/internal/testing/integration/README.md +++ /dev/null @@ -1,10 +0,0 @@ -# Integration Testing Framework - -This package has been moved from [https://github.com/kubernetes-sigs/testing_frameworks/tree/master/integration](https://github.com/kubernetes-sigs/testing_frameworks/tree/master/integration). - -A framework for integration testing components of kubernetes. This framework is -intended to work properly both in CI, and on a local dev machine. It therefore -explicitly supports both Linux and Darwin. - -For detailed documentation see the -[![GoDoc](https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/internal/testing/integration?status.svg)](https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/internal/testing/integration). diff --git a/pkg/internal/testing/integration/assets/bin/.gitkeep b/pkg/internal/testing/integration/assets/bin/.gitkeep deleted file mode 100644 index 368201aa28..0000000000 --- a/pkg/internal/testing/integration/assets/bin/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -This directory will be the home of some binaries which are downloaded with `make test` or `.../hack/check-everything.sh`. diff --git a/pkg/internal/testing/integration/doc.go b/pkg/internal/testing/integration/doc.go deleted file mode 100644 index 62a0367311..0000000000 --- a/pkg/internal/testing/integration/doc.go +++ /dev/null @@ -1,112 +0,0 @@ -/* - -Package integration implements an integration testing framework for kubernetes. - -It provides components for standing up a kubernetes API, against which you can test a -kubernetes client, or other kubernetes components. The lifecycle of the components -needed to provide this API is managed by this framework. - -Quickstart - -Add something like the following to -your tests: - - cp := &integration.ControlPlane{} - cp.Start() - kubeCtl := cp.KubeCtl() - stdout, stderr, err := kubeCtl.Run("get", "pods") - // You can check on err, stdout & stderr and build up - // your tests - cp.Stop() - -Components - -Currently the framework provides the following components: - -ControlPlane: The ControlPlane wraps Etcd & APIServer (see below) and wires -them together correctly. A ControlPlane can be stopped & started and can -provide the URL to connect to the API. The ControlPlane can also be asked for a -KubeCtl which is already correctly configured for this ControlPlane. The -ControlPlane is a good entry point for default setups. - -Etcd: Manages an Etcd binary, which can be started, stopped and connected to. -By default Etcd will listen on a random port for http connections and will -create a temporary directory for its data. To configure it differently, see the -Etcd type documentation below. - -APIServer: Manages an Kube-APIServer binary, which can be started, stopped and -connected to. By default APIServer will listen on a random port for http -connections and will create a temporary directory to store the (auto-generated) -certificates. To configure it differently, see the APIServer type -documentation below. - -KubeCtl: Wraps around a `kubectl` binary and can `Run(...)` arbitrary commands -against a kubernetes control plane. - -Binaries - -Etcd, APIServer & KubeCtl use the same mechanism to determine which binaries to -use when they get started. - -1. If the component is configured with a `Path` the framework tries to run that -binary. -For example: - - myEtcd := &Etcd{ - Path: "/some/other/etcd", - } - cp := &integration.ControlPlane{ - Etcd: myEtcd, - } - cp.Start() - -2. If the Path field on APIServer, Etcd or KubeCtl is left unset and an -environment variable named `TEST_ASSET_KUBE_APISERVER`, `TEST_ASSET_ETCD` or -`TEST_ASSET_KUBECTL` is set, its value is used as a path to the binary for the -APIServer, Etcd or KubeCtl. - -3. If neither the `Path` field, nor the environment variable is set, the -framework tries to use the binaries `kube-apiserver`, `etcd` or `kubectl` in -the directory `${FRAMEWORK_DIR}/assets/bin/`. - -Arguments for Etcd and APIServer - -Those components will start without any configuration. However, if you want or -need to, you can override certain configuration -- one of which are the -arguments used when calling the binary. - -When you choose to specify your own set of arguments, those won't be appended -to the default set of arguments, it is your responsibility to provide all the -arguments needed for the binary to start successfully. - -However, the default arguments for APIServer and Etcd are exported as -`APIServerDefaultArgs` and `EtcdDefaultArgs` from this package. Treat those -variables as read-only constants. Internally we have a set of default -arguments for defaulting, the `APIServerDefaultArgs` and `EtcdDefaultArgs` are -just copies of those. So when you override them you loose access to the actual -internal default arguments, but your override won't affect the defaulting. - -All arguments are interpreted as go templates. Those templates have access to -all exported fields of the `APIServer`/`Etcd` struct. It does not matter if -those fields where explicitly set up or if they were defaulted by calling the -`Start()` method, the template evaluation runs just before the binary is -executed and right after the defaulting of all the struct's fields has -happened. - - // When you want to append additional arguments ... - etcd := &Etcd{ - // Additional custom arguments will appended to the set of default - // arguments - Args: append(EtcdDefaultArgs, "--additional=arg"), - DataDir: "/my/special/data/dir", - } - - // When you want to use a custom set of arguments ... - etcd := &Etcd{ - // Only custom arguments will be passed to the binary - Args: []string{"--one=1", "--two=2", "--three=3"}, - DataDir: "/my/special/data/dir", - } - -*/ -package integration diff --git a/pkg/internal/testing/integration/internal/integration_tests/apiserver_integration_test.go b/pkg/internal/testing/integration/internal/integration_tests/apiserver_integration_test.go deleted file mode 100644 index 58770f3578..0000000000 --- a/pkg/internal/testing/integration/internal/integration_tests/apiserver_integration_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package integrationtests - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" -) - -var _ = Describe("APIServer", func() { - Context("when no EtcdURL is provided", func() { - It("does not panic", func() { - apiServer := &APIServer{} - - starter := func() { - Expect(apiServer.Start()).To( - MatchError(ContainSubstring("expected EtcdURL to be configured")), - ) - } - - Expect(starter).NotTo(Panic()) - }) - }) -}) diff --git a/pkg/internal/testing/integration/internal/integration_tests/doc.go b/pkg/internal/testing/integration/internal/integration_tests/doc.go deleted file mode 100644 index 363a126ec5..0000000000 --- a/pkg/internal/testing/integration/internal/integration_tests/doc.go +++ /dev/null @@ -1,7 +0,0 @@ -/* -Package integrationtests holds the integration tests to run against the -framework. - -This file's only purpose is to make godep happy. -*/ -package integrationtests diff --git a/pkg/internal/testing/integration/internal/integration_tests/etcd_integration_test.go b/pkg/internal/testing/integration/internal/integration_tests/etcd_integration_test.go deleted file mode 100644 index 2605e99b87..0000000000 --- a/pkg/internal/testing/integration/internal/integration_tests/etcd_integration_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package integrationtests - -import ( - "bytes" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - . "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" -) - -var _ = Describe("Etcd", func() { - It("sets the properties after defaulting", func() { - etcd := &Etcd{} - - Expect(etcd.URL).To(BeZero()) - Expect(etcd.DataDir).To(BeZero()) - Expect(etcd.Path).To(BeZero()) - Expect(etcd.StartTimeout).To(BeZero()) - Expect(etcd.StopTimeout).To(BeZero()) - - Expect(etcd.Start()).To(Succeed()) - defer func() { - Expect(etcd.Stop()).To(Succeed()) - }() - - Expect(etcd.URL).NotTo(BeZero()) - Expect(etcd.DataDir).NotTo(BeZero()) - Expect(etcd.Path).NotTo(BeZero()) - Expect(etcd.StartTimeout).NotTo(BeZero()) - Expect(etcd.StopTimeout).NotTo(BeZero()) - }) - - It("can inspect IO", func() { - stderr := &bytes.Buffer{} - etcd := &Etcd{ - Err: stderr, - } - - Expect(etcd.Start()).To(Succeed()) - Expect(etcd.Stop()).To(Succeed()) - - Expect(stderr.String()).NotTo(BeEmpty()) - }) - - It("can use user specified Args", func() { - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - etcd := &Etcd{ - Args: []string{"--help"}, - Out: stdout, - Err: stderr, - StartTimeout: 500 * time.Millisecond, - } - - // it will timeout, as we'll never see the "startup message" we are waiting - // for on StdErr - Expect(etcd.Start()).To(MatchError(ContainSubstring("timeout"))) - // Stop is required to cleanup the temporary directory - Expect(etcd.Stop()).To(Succeed()) - - Expect(stdout.String()).To(ContainSubstring("Member:")) - Expect(stderr.String()).To(ContainSubstring("Usage:")) - }) -}) diff --git a/pkg/internal/testing/integration/internal/integration_tests/integration_suite_test.go b/pkg/internal/testing/integration/internal/integration_tests/integration_suite_test.go deleted file mode 100644 index 011b3e5609..0000000000 --- a/pkg/internal/testing/integration/internal/integration_tests/integration_suite_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package integrationtests - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" -) - -func TestIntegration(t *testing.T) { - t.Parallel() - RegisterFailHandler(Fail) - suiteName := "Integration Framework Integration Tests" - RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) -} diff --git a/pkg/internal/testing/integration/internal/integration_tests/integration_test.go b/pkg/internal/testing/integration/internal/integration_tests/integration_test.go deleted file mode 100644 index 4718f9e326..0000000000 --- a/pkg/internal/testing/integration/internal/integration_tests/integration_test.go +++ /dev/null @@ -1,288 +0,0 @@ -package integrationtests - -import ( - "context" - "fmt" - "io/ioutil" - "net" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/internal/testing/integration" -) - -var _ = Describe("The Testing Framework", func() { - var controlPlane *integration.ControlPlane - ctx := context.TODO() - - AfterEach(func() { - Expect(controlPlane.Stop()).To(Succeed()) - }) - - It("Successfully manages the control plane lifecycle", func() { - var err error - - controlPlane = &integration.ControlPlane{} - - By("Starting all the control plane processes") - err = controlPlane.Start() - Expect(err).NotTo(HaveOccurred(), "Expected controlPlane to start successfully") - - apiServerURL := controlPlane.APIURL() - etcdClientURL := controlPlane.APIServer.EtcdURL - - isEtcdListeningForClients := isSomethingListeningOnPort(etcdClientURL.Host) - isAPIServerListening := isSomethingListeningOnPort(apiServerURL.Host) - - By("Ensuring Etcd is listening") - Expect(isEtcdListeningForClients()).To(BeTrue(), - fmt.Sprintf("Expected Etcd to listen for clients on %s,", etcdClientURL.Host)) - - By("Ensuring APIServer is listening") - c, err := controlPlane.RESTClientConfig() - Expect(err).NotTo(HaveOccurred()) - CheckAPIServerIsReady(c) - - By("getting a kubeclient & run it against the control plane") - c.APIPath = "/api" - c.ContentConfig.GroupVersion = &schema.GroupVersion{Version: "v1"} - kubeClient, err := rest.RESTClientFor(c) - Expect(err).NotTo(HaveOccurred()) - result := &corev1.PodList{} - err = kubeClient.Get(). - Namespace("default"). - Resource("pods"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Into(result) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Items).To(BeEmpty()) - - By("getting a kubectl & run it against the control plane") - kubeCtl := controlPlane.KubeCtl() - stdout, stderr, err := kubeCtl.Run("get", "pods") - Expect(err).NotTo(HaveOccurred()) - bytes, err := ioutil.ReadAll(stdout) - Expect(err).NotTo(HaveOccurred()) - Expect(bytes).To(BeEmpty()) - Expect(stderr).To(ContainSubstring("No resources found")) - - By("Stopping all the control plane processes") - err = controlPlane.Stop() - Expect(err).NotTo(HaveOccurred(), "Expected controlPlane to stop successfully") - - By("Ensuring Etcd is not listening anymore") - Expect(isEtcdListeningForClients()).To(BeFalse(), "Expected Etcd not to listen for clients anymore") - - By("Ensuring APIServer is not listening anymore") - Expect(isAPIServerListening()).To(BeFalse(), "Expected APIServer not to listen anymore") - - By("Not erroring when stopping a stopped ControlPlane") - Expect(func() { - Expect(controlPlane.Stop()).To(Succeed()) - }).NotTo(Panic()) - }) - - Context("when Stop() is called on the control plane", func() { - Context("but the control plane is not started yet", func() { - It("does not error", func() { - controlPlane = &integration.ControlPlane{} - - stoppingTheControlPlane := func() { - Expect(controlPlane.Stop()).To(Succeed()) - } - - Expect(stoppingTheControlPlane).NotTo(Panic()) - }) - }) - }) - - Context("when the control plane is configured with its components", func() { - It("it does not default them", func() { - myEtcd, myAPIServer := - &integration.Etcd{StartTimeout: 15 * time.Second}, - &integration.APIServer{StopTimeout: 16 * time.Second} - - controlPlane = &integration.ControlPlane{ - Etcd: myEtcd, - APIServer: myAPIServer, - } - - Expect(controlPlane.Start()).To(Succeed()) - Expect(controlPlane.Etcd).To(BeIdenticalTo(myEtcd)) - Expect(controlPlane.APIServer).To(BeIdenticalTo(myAPIServer)) - Expect(controlPlane.Etcd.StartTimeout).To(Equal(15 * time.Second)) - Expect(controlPlane.APIServer.StopTimeout).To(Equal(16 * time.Second)) - }) - }) - - Context("when etcd already started", func() { - It("starts the control plane successfully", func() { - myEtcd := &integration.Etcd{} - Expect(myEtcd.Start()).To(Succeed()) - - controlPlane = &integration.ControlPlane{ - Etcd: myEtcd, - } - - Expect(controlPlane.Start()).To(Succeed()) - }) - }) - - Context("when control plane is already started", func() { - It("can attempt to start again without errors", func() { - controlPlane = &integration.ControlPlane{} - Expect(controlPlane.Start()).To(Succeed()) - Expect(controlPlane.Start()).To(Succeed()) - }) - }) - - Context("when control plane starts and stops", func() { - It("can attempt to start again without errors", func() { - controlPlane = &integration.ControlPlane{} - Expect(controlPlane.Start()).To(Succeed()) - Expect(controlPlane.Stop()).To(Succeed()) - Expect(controlPlane.Start()).To(Succeed()) - }) - }) - - Measure("It should be fast to bring up and tear down the control plane", func(b Benchmarker) { - b.Time("lifecycle", func() { - controlPlane = &integration.ControlPlane{} - - Expect(controlPlane.Start()).To(Succeed()) - Expect(controlPlane.Stop()).To(Succeed()) - }) - }, 10) -}) - -type portChecker func() bool - -func isSomethingListeningOnPort(hostAndPort string) portChecker { - return func() bool { - conn, err := net.DialTimeout("tcp", hostAndPort, 1*time.Second) - - if err != nil { - return false - } - conn.Close() - return true - } -} - -// CheckAPIServerIsReady checks if the APIServer is really ready and not only -// listening. -// -// While porting some tests in k/k -// (https://github.com/hoegaarden/kubernetes/blob/287fdef1bd98646bc521f4433c1009936d5cf7a2/hack/make-rules/test-cmd-util.sh#L1524-L1535) -// we found, that the APIServer was -// listening but not serving certain APIs yet. -// -// We changed the readiness detection in the PR at -// https://github.com/kubernetes-sigs/testing_frameworks/pull/48. To confirm -// this changed behaviour does what it should do, we used the same test as in -// k/k's test-cmd (see link above) and test if certain well-known known APIs -// are actually available. -func CheckAPIServerIsReady(c *rest.Config) { - ctx := context.TODO() - // check pods, replicationcontrollers and services - c.APIPath = "/api" - c.ContentConfig.GroupVersion = &schema.GroupVersion{Version: "v1"} - kubeClient, err := rest.RESTClientFor(c) - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("pods"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("replicationcontrollers"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("services"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - // check daemonsets, deployments, replicasets and statefulsets, - c.APIPath = "/apis" - c.ContentConfig.GroupVersion = &schema.GroupVersion{Group: "apps", Version: "v1"} - kubeClient, err = rest.RESTClientFor(c) - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("daemonsets"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("deployments"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("replicasets"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("statefulsets"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - // check horizontalpodautoscalers - c.ContentConfig.GroupVersion = &schema.GroupVersion{Group: "autoscaling", Version: "v1"} - kubeClient, err = rest.RESTClientFor(c) - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("horizontalpodautoscalers"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) - - // check jobs - c.ContentConfig.GroupVersion = &schema.GroupVersion{Group: "batch", Version: "v1"} - kubeClient, err = rest.RESTClientFor(c) - Expect(err).NotTo(HaveOccurred()) - - _, err = kubeClient.Get(). - Namespace("default"). - Resource("jobs"). - VersionedParams(&metav1.ListOptions{}, scheme.ParameterCodec). - Do(ctx). - Get() - Expect(err).NotTo(HaveOccurred()) -} From 294af4fa81dde6fec7405d72823e466951fa124f Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Tue, 25 May 2021 13:39:37 -0700 Subject: [PATCH 5/5] Update to use new internal/testing paths This updates envtests, etc to use to the new refactored internal/testing/xyz paths (certs, addr, controlplane). --- pkg/builder/builder_suite_test.go | 2 +- pkg/envtest/server.go | 22 +++++++++++----------- pkg/envtest/webhook.go | 6 +++--- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/builder/builder_suite_test.go b/pkg/builder/builder_suite_test.go index 466e2eee9c..0dc260d4bf 100644 --- a/pkg/builder/builder_suite_test.go +++ b/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" diff --git a/pkg/envtest/server.go b/pkg/envtest/server.go index 63f64ab550..ad59ec3aa4 100644 --- a/pkg/envtest/server.go +++ b/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 diff --git a/pkg/envtest/webhook.go b/pkg/envtest/webhook.go index 880076f898..ad9fe0dbc2 100644 --- a/pkg/envtest/webhook.go +++ b/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) }