Repository: knative/pkg
Pull request #1820 (2020-10-19 21:00:24)

Remove re-definition of kubeconfig from pkg/test

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1820#" title="Author self-approved">n3wscott</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [n3wscott]
- ~~[injection/OWNERS](https://github.com/knative/pkg/blob/master/injection/OWNERS)~~ [n3wscott]
- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/master/metrics/OWNERS)~~ [n3wscott]
- ~~[test/OWNERS](https://github.com/knative/pkg/blob/master/test/OWNERS)~~ [n3wscott]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=h1) Report
> Merging [#1820](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=desc) into [master](https://codecov.io/gh/knative/pkg/commit/ee0539ddff70cd28eabc3fc086a669f2c1f65d38?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1820/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1820      +/-   ##
==========================================
- Coverage   67.80%   67.80%   -0.01%     
==========================================
  Files         222      222              
  Lines        9438     9441       +3     
==========================================
+ Hits         6399     6401       +2     
- Misses       2716     2718       +2     
+ Partials      323      322       -1     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [injection/config.go](https://codecov.io/gh/knative/pkg/pull/1820/diff?src=pr&el=tree#diff-aW5qZWN0aW9uL2NvbmZpZy5nbw==) | `0.00% <0.00%> (ø)` | |
| [metrics/metricstest/metricstest.go](https://codecov.io/gh/knative/pkg/pull/1820/diff?src=pr&el=tree#diff-bWV0cmljcy9tZXRyaWNzdGVzdC9tZXRyaWNzdGVzdC5nbw==) | `0.00% <ø> (ø)` | |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1820/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `91.39% <0.00%> (+1.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=footer). Last update [ee0539d...4e9f1af](https://codecov.io/gh/knative/pkg/pull/1820?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-prow-robot: @n3wscott: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 4a57e2284808982f7a5f0245449292bc6e1811d2 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1820/pull-knative-pkg-unit-tests/1318317943712387072) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1820). [Your PR dashboard](https://gubernator.knative.dev/pr/n3wscott).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
coryrc: `... Define your own interface.`

why?
knative-prow-robot: @n3wscott: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 4a57e2284808982f7a5f0245449292bc6e1811d2 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1820/pull-knative-pkg-unit-tests/1318317943712387072) | `/test pull-knative-pkg-unit-tests`
pull-knative-pkg-build-tests | 4a57e2284808982f7a5f0245449292bc6e1811d2 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1820/pull-knative-pkg-build-tests/1318317943712387074) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1820). [Your PR dashboard](https://gubernator.knative.dev/pr/n3wscott).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: New changes are detected. LGTM label has been removed.
n3wscott: Drat, missed one in the null class
n3wscott: > `... Define your own interface.`
> 
> why?

There is no need to define a central interface for the testing class, and doing so causes a circular dependency.

Patch:
diff --git a/injection/config.go b/injection/config.go
index bae2b2e25..d8228fe5c 100644
--- a/injection/config.go
+++ b/injection/config.go
@@ -24,41 +24,68 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
+	"sync"
 
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/klog"
 )
 
+// Environment holds the config for flag based config.
+type Environment struct {
+	// ServerURL - The address of the Kubernetes API server. Overrides any value in kubeconfig.
+	ServerURL string
+	// Kubeconfig - Path to a kubeconfig.
+	Kubeconfig string // Note: named Kubeconfig because of legacy reasons vs KubeConfig.
+	// Burst - Maximum burst for throttle.
+	Burst int
+	// QPS - Maximum QPS to the server from the client.
+	QPS float64
+}
+
+var (
+	env  *Environment
+	once sync.Once
+)
+
+func Flags() *Environment {
+	once.Do(func() {
+		env = new(Environment)
+
+		flag.StringVar(&env.ServerURL, "server", "",
+			"The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.")
+
+		flag.StringVar(&env.Kubeconfig, "kubeconfig", "",
+			"Path to a kubeconfig. Only required if out-of-cluster.")
+
+		flag.IntVar(&env.Burst, "kube-api-burst", 0, "Maximum burst for throttle.")
+
+		flag.Float64Var(&env.QPS, "kube-api-qps", 0, "Maximum QPS to the server from the client.")
+	})
+	return env
+}
+
 // ParseAndGetRESTConfigOrDie parses the rest config flags and creates a client or
 // dies by calling log.Fatalf.
 func ParseAndGetRESTConfigOrDie() *rest.Config {
-	var (
-		serverURL = flag.String("server", "",
-			"The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.")
-		kubeconfig = flag.String("kubeconfig", "",
-			"Path to a kubeconfig. Only required if out-of-cluster.")
+	env := Flags()
 
-		burst = flag.Int("kube-api-burst", 0, "Maximum burst for throttle.")
-		qps   = flag.Float64("kube-api-qps", 0, "Maximum QPS to the server from the client.")
-	)
 	klog.InitFlags(flag.CommandLine)
 	flag.Parse()
+	if env.Burst < 0 {
+		log.Fatal("Invalid burst value", env.Burst)
+	}
+	if env.QPS < 0 || env.QPS > math.MaxFloat32 {
+		log.Fatal("Invalid QPS value", env.QPS)
+	}
 
-	cfg, err := GetRESTConfig(*serverURL, *kubeconfig)
+	cfg, err := GetRESTConfig(env.ServerURL, env.Kubeconfig)
 	if err != nil {
 		log.Fatal("Error building kubeconfig: ", err)
 	}
 
-	if *burst < 0 {
-		log.Fatal("Invalid burst value", *burst)
-	}
-	if *qps < 0 || *qps > math.MaxFloat32 {
-		log.Fatal("Invalid QPS value", *qps)
-	}
-
-	cfg.Burst = *burst
-	cfg.QPS = float32(*qps)
+	cfg.Burst = env.Burst
+	cfg.QPS = float32(env.QPS)
 
 	return cfg
 }


Test patch:
diff --git a/apis/testing/conditions.go b/apis/testing/conditions.go
index 68b0018ab..8b0ac4547 100644
--- a/apis/testing/conditions.go
+++ b/apis/testing/conditions.go
@@ -20,9 +20,13 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"knative.dev/pkg/apis"
-	"knative.dev/pkg/test"
 )
 
+type ti interface {
+	Helper()
+	Error(args ...interface{})
+}
+
 // CheckCondition checks if condition `c` on `cc` has value `cs`.
 func CheckCondition(a apis.ConditionAccessor, c apis.ConditionType, cs corev1.ConditionStatus) error {
 	cond := a.GetCondition(c)
@@ -36,7 +40,7 @@ func CheckCondition(a apis.ConditionAccessor, c apis.ConditionType, cs corev1.Co
 }
 
 // CheckConditionOngoing checks if the condition is in state `Unknown`.
-func CheckConditionOngoing(a apis.ConditionAccessor, c apis.ConditionType, t test.T) {
+func CheckConditionOngoing(a apis.ConditionAccessor, c apis.ConditionType, t ti) {
 	t.Helper()
 	if err := CheckCondition(a, c, corev1.ConditionUnknown); err != nil {
 		t.Error(err)
@@ -44,7 +48,7 @@ func CheckConditionOngoing(a apis.ConditionAccessor, c apis.ConditionType, t tes
 }
 
 // CheckConditionFailed checks if the condition is in state `False`.
-func CheckConditionFailed(a apis.ConditionAccessor, c apis.ConditionType, t test.T) {
+func CheckConditionFailed(a apis.ConditionAccessor, c apis.ConditionType, t ti) {
 	t.Helper()
 	if err := CheckCondition(a, c, corev1.ConditionFalse); err != nil {
 		t.Error(err)
@@ -52,7 +56,7 @@ func CheckConditionFailed(a apis.ConditionAccessor, c apis.ConditionType, t test
 }
 
 // CheckConditionSucceeded checks if the condition is in state `True`.
-func CheckConditionSucceeded(a apis.ConditionAccessor, c apis.ConditionType, t test.T) {
+func CheckConditionSucceeded(a apis.ConditionAccessor, c apis.ConditionType, t ti) {
 	t.Helper()
 	if err := CheckCondition(a, c, corev1.ConditionTrue); err != nil {
 		t.Error(err)
diff --git a/metrics/metricstest/metricstest.go b/metrics/metricstest/metricstest.go
index f63e34a52..6a9d9d76a 100644
--- a/metrics/metricstest/metricstest.go
+++ b/metrics/metricstest/metricstest.go
@@ -22,12 +22,16 @@ import (
 
 	"go.opencensus.io/metric/metricproducer"
 	"go.opencensus.io/stats/view"
-	"knative.dev/pkg/test"
 )
 
+type ti interface {
+	Helper()
+	Error(args ...interface{})
+}
+
 // CheckStatsReported checks that there is a view registered with the given name for each string in names,
 // and that each view has at least one record.
-func CheckStatsReported(t test.T, names ...string) {
+func CheckStatsReported(t ti, names ...string) {
 	t.Helper()
 	for _, name := range names {
 		d, err := readRowsFromAllMeters(name)
@@ -42,7 +46,7 @@ func CheckStatsReported(t test.T, names ...string) {
 
 // CheckStatsNotReported checks that there are no records for any views that a name matching a string in names.
 // Names that do not match registered views are considered not reported.
-func CheckStatsNotReported(t test.T, names ...string) {
+func CheckStatsNotReported(t ti, names ...string) {
 	t.Helper()
 	for _, name := range names {
 		d, err := readRowsFromAllMeters(name)
@@ -56,7 +60,7 @@ func CheckStatsNotReported(t test.T, names ...string) {
 
 // CheckCountData checks the view with a name matching string name to verify that the CountData stats
 // reported are tagged with the tags in wantTags and that wantValue matches reported count.
-func CheckCountData(t test.T, name string, wantTags map[string]string, wantValue int64) {
+func CheckCountData(t ti, name string, wantTags map[string]string, wantValue int64) {
 	t.Helper()
 	row, err := checkExactlyOneRow(t, name)
 	if err != nil {
@@ -75,7 +79,7 @@ func CheckCountData(t test.T, name string, wantTags map[string]string, wantValue
 // CheckDistributionData checks the view with a name matching string name to verify that the DistributionData stats reported
 // are tagged with the tags in wantTags and that expectedCount number of records were reported.
 // It also checks that expectedMin and expectedMax match the minimum and maximum reported values, respectively.
-func CheckDistributionData(t test.T, name string, wantTags map[string]string, expectedCount int64, expectedMin float64, expectedMax float64) {
+func CheckDistributionData(t ti, name string, wantTags map[string]string, expectedCount int64, expectedMin float64, expectedMax float64) {
 	t.Helper()
 	row, err := checkExactlyOneRow(t, name)
 	if err != nil {
@@ -101,7 +105,7 @@ func CheckDistributionData(t test.T, name string, wantTags map[string]string, ex
 
 // CheckDistributionRange checks the view with a name matching string name to verify that the DistributionData stats reported
 // are tagged with the tags in wantTags and that expectedCount number of records were reported.
-func CheckDistributionCount(t test.T, name string, wantTags map[string]string, expectedCount int64) {
+func CheckDistributionCount(t ti, name string, wantTags map[string]string, expectedCount int64) {
 	t.Helper()
 	row, err := checkExactlyOneRow(t, name)
 	if err != nil {
@@ -119,13 +123,13 @@ func CheckDistributionCount(t test.T, name string, wantTags map[string]string, e
 }
 
 // GetLastValueData returns the last value for the given metric, verifying tags.
-func GetLastValueData(t test.T, name string, tags map[string]string) float64 {
+func GetLastValueData(t ti, name string, tags map[string]string) float64 {
 	t.Helper()
 	return GetLastValueDataWithMeter(t, name, tags, nil)
 }
 
 // GetLastValueDataWithMeter returns the last value of the given metric using meter, verifying tags.
-func GetLastValueDataWithMeter(t test.T, name string, tags map[string]string, meter view.Meter) float64 {
+func GetLastValueDataWithMeter(t ti, name string, tags map[string]string, meter view.Meter) float64 {
 	t.Helper()
 	if row := lastRow(t, name, meter); row != nil {
 		checkRowTags(t, row, name, tags)
@@ -141,7 +145,7 @@ func GetLastValueDataWithMeter(t test.T, name string, tags map[string]string, me
 
 // CheckLastValueData checks the view with a name matching string name to verify that the LastValueData stats
 // reported are tagged with the tags in wantTags and that wantValue matches reported last value.
-func CheckLastValueData(t test.T, name string, wantTags map[string]string, wantValue float64) {
+func CheckLastValueData(t ti, name string, wantTags map[string]string, wantValue float64) {
 	t.Helper()
 	CheckLastValueDataWithMeter(t, name, wantTags, wantValue, nil)
 }
@@ -149,7 +153,7 @@ func CheckLastValueData(t test.T, name string, wantTags map[string]string, wantV
 // CheckLastValueDataWithMeter checks the  view with a name matching the string name in the
 // specified Meter (resource-specific view) to verify that the LastValueData stats are tagged with
 // the tags in wantTags and that wantValue matches the last reported value.
-func CheckLastValueDataWithMeter(t test.T, name string, wantTags map[string]string, wantValue float64, meter view.Meter) {
+func CheckLastValueDataWithMeter(t ti, name string, wantTags map[string]string, wantValue float64, meter view.Meter) {
 	t.Helper()
 	if v := GetLastValueDataWithMeter(t, name, wantTags, meter); v != wantValue {
 		t.Error("Reporter.Report() wrong value", "metric", name, "got", v, "want", wantValue)
@@ -158,7 +162,7 @@ func CheckLastValueDataWithMeter(t test.T, name string, wantTags map[string]stri
 
 // CheckSumData checks the view with a name matching string name to verify that the SumData stats
 // reported are tagged with the tags in wantTags and that wantValue matches the reported sum.
-func CheckSumData(t test.T, name string, wantTags map[string]string, wantValue float64) {
+func CheckSumData(t ti, name string, wantTags map[string]string, wantValue float64) {
 	t.Helper()
 	row, err := checkExactlyOneRow(t, name)
 	if err != nil {
@@ -192,7 +196,7 @@ func Unregister(names ...string) {
 	}
 }
 
-func lastRow(t test.T, name string, meter view.Meter) *view.Row {
+func lastRow(t ti, name string, meter view.Meter) *view.Row {
 	t.Helper()
 	var d []*view.Row
 	var err error
@@ -213,7 +217,7 @@ func lastRow(t test.T, name string, meter view.Meter) *view.Row {
 	return d[len(d)-1]
 }
 
-func checkExactlyOneRow(t test.T, name string) (*view.Row, error) {
+func checkExactlyOneRow(t ti, name string) (*view.Row, error) {
 	rows, err := readRowsFromAllMeters(name)
 	if err != nil || len(rows) == 0 {
 		return nil, fmt.Errorf("could not find row for %q", name)
@@ -242,7 +246,7 @@ func readRowsFromAllMeters(name string) ([]*view.Row, error) {
 	return rows, nil
 }
 
-func checkRowTags(t test.T, row *view.Row, name string, wantTags map[string]string) {
+func checkRowTags(t ti, row *view.Row, name string, wantTags map[string]string) {
 	t.Helper()
 	if wantlen, gotlen := len(wantTags), len(row.Tags); gotlen != wantlen {
 		t.Error("Reporter got wrong number of tags", "metric", name, "got", gotlen, "want", wantlen)
diff --git a/test/clients.go b/test/clients.go
index dd5c6e301..c4e84cff5 100644
--- a/test/clients.go
+++ b/test/clients.go
@@ -23,6 +23,8 @@ import (
 	"fmt"
 	"strings"
 
+	"knative.dev/pkg/test/flags"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -42,8 +44,8 @@ type KubeClient struct {
 // NewSpoofingClient returns a spoofing client to make requests
 func NewSpoofingClient(ctx context.Context, client *KubeClient, logf logging.FormatLogger,
 	domain string, resolvable bool, opts ...spoof.TransportOption) (*spoof.SpoofingClient, error) {
-	return spoof.New(ctx, client, logf, domain, resolvable, Flags.IngressEndpoint,
-		Flags.SpoofRequestInterval, Flags.SpoofRequestTimeout, opts...)
+	return spoof.New(ctx, client, logf, domain, resolvable, flags.Flags().IngressEndpoint,
+		flags.Flags().SpoofRequestInterval, flags.Flags().SpoofRequestTimeout, opts...)
 }
 
 // NewKubeClient instantiates and returns several clientsets required for making request to the
diff --git a/test/e2e_flags.go b/test/e2e_flags.go
index bc6f90bff..0fe58f747 100644
--- a/test/e2e_flags.go
+++ b/test/e2e_flags.go
@@ -22,74 +22,36 @@ package test
 import (
 	"bytes"
 	"flag"
-	"os"
-	"os/user"
-	"path"
 	"text/template"
-	"time"
 
+	"knative.dev/pkg/injection"
+	testflags "knative.dev/pkg/test/flags"
 	"knative.dev/pkg/test/logging"
 )
 
 var (
 	// Flags holds the command line flags or defaults for settings in the user's environment.
 	// See EnvironmentFlags for a list of supported fields.
+	// Deprecated: use test/flags.Flags()
 	Flags = initializeFlags()
 )
 
 // EnvironmentFlags define the flags that are needed to run the e2e tests.
+// Deprecated: use test/flags.Flags() or injection.Flags()
 type EnvironmentFlags struct {
-	Cluster              string        // K8s cluster (defaults to cluster in kubeconfig)
-	Kubeconfig           string        // Path to kubeconfig (defaults to ./kube/config)
-	Namespace            string        // K8s namespace (blank by default, to be overwritten by test suite)
-	IngressEndpoint      string        // Host to use for ingress endpoint
-	ImageTemplate        string        // Template to build the image reference (defaults to {{.Repository}}/{{.Name}}:{{.Tag}})
-	DockerRepo           string        // Docker repo (defaults to $KO_DOCKER_REPO)
-	Tag                  string        // Tag for test images
-	SpoofRequestInterval time.Duration // SpoofRequestInterval is the interval between requests in SpoofingClient
-	SpoofRequestTimeout  time.Duration // SpoofRequestTimeout is the timeout for polling requests in SpoofingClient
+	*injection.Environment
+	*testflags.TestEnvironment
 }
 
 func initializeFlags() *EnvironmentFlags {
-	var f EnvironmentFlags
-	flag.StringVar(&f.Cluster, "cluster", "",
-		"Provide the cluster to test against. Defaults to the current cluster in kubeconfig.")
-
-	// Use KUBECONFIG if available
-	defaultKubeconfig := os.Getenv("KUBECONFIG")
-
-	// If KUBECONFIG env var isn't set then look for $HOME/.kube/config
-	if defaultKubeconfig == "" {
-		if usr, err := user.Current(); err == nil {
-			defaultKubeconfig = path.Join(usr.HomeDir, ".kube/config")
-		}
-	}
-
-	// Allow for --kubeconfig on the cmd line to override the above logic
-	flag.StringVar(&f.Kubeconfig, "kubeconfig", defaultKubeconfig,
-		"Provide the path to the `kubeconfig` file you'd like to use for these tests. The `current-context` will be used.")
-
-	flag.StringVar(&f.Namespace, "namespace", "",
-		"Provide the namespace you would like to use for these tests.")
-
-	flag.StringVar(&f.IngressEndpoint, "ingressendpoint", "", "Provide a static endpoint url to the ingress server used during tests.")
-
-	flag.StringVar(&f.ImageTemplate, "imagetemplate", "{{.Repository}}/{{.Name}}:{{.Tag}}",
-		"Provide a template to generate the reference to an image from the test. Defaults to `{{.Repository}}/{{.Name}}:{{.Tag}}`.")
-
-	flag.DurationVar(&f.SpoofRequestInterval, "spoofinterval", 1*time.Second,
-		"Provide an interval between requests for the SpoofingClient")
-
-	flag.DurationVar(&f.SpoofRequestTimeout, "spooftimeout", 5*time.Minute,
-		"Provide a request timeout for the SpoofingClient")
+	f := new(EnvironmentFlags)
 
-	defaultRepo := os.Getenv("KO_DOCKER_REPO")
-	flag.StringVar(&f.DockerRepo, "dockerrepo", defaultRepo,
-		"Provide the uri of the docker repo you have uploaded the test image to using `uploadtestimage.sh`. Defaults to $KO_DOCKER_REPO")
+	testflags.InitFlags(flag.CommandLine)
 
-	flag.StringVar(&f.Tag, "tag", "latest", "Provide the version tag for the test images.")
+	f.TestEnvironment = testflags.Flags()
+	f.Environment = injection.Flags()
 
-	return &f
+	return f
 }
 
 // TODO(coryrc): Remove once other repos are moved to call logging.InitializeLogger() directly
@@ -99,7 +61,7 @@ func SetupLoggingFlags() {
 
 // ImagePath is a helper function to transform an image name into an image reference that can be pulled.
 func ImagePath(name string) string {
-	tpl, err := template.New("image").Parse(Flags.ImageTemplate)
+	tpl, err := template.New("image").Parse(testflags.Flags().ImageTemplate)
 	if err != nil {
 		panic("could not parse image template: " + err.Error())
 	}
@@ -110,9 +72,9 @@ func ImagePath(name string) string {
 		Name       string
 		Tag        string
 	}{
-		Repository: Flags.DockerRepo,
+		Repository: testflags.Flags().DockerRepo,
 		Name:       name,
-		Tag:        Flags.Tag,
+		Tag:        testflags.Flags().Tag,
 	}); err != nil {
 		panic("could not apply the image template: " + err.Error())
 	}
diff --git a/test/flags/e2e_flags.go b/test/flags/e2e_flags.go
new file mode 100644
index 000000000..fc9032a78
--- /dev/null
+++ b/test/flags/e2e_flags.go
@@ -0,0 +1,79 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// This file contains logic to encapsulate flags which are needed to specify
+// what cluster, etc. to use for e2e tests.
+
+package flags
+
+import (
+	"flag"
+	"os"
+	"time"
+)
+
+// TestEnvironment define the config that are needed to run the e2e tests.
+type TestEnvironment struct {
+	Cluster              string        // K8s cluster (defaults to cluster in kubeconfig)
+	Namespace            string        // K8s namespace (blank by default, to be overwritten by test suite)
+	IngressEndpoint      string        // Host to use for ingress endpoint
+	ImageTemplate        string        // Template to build the image reference (defaults to {{.Repository}}/{{.Name}}:{{.Tag}})
+	DockerRepo           string        // Docker repo (defaults to $KO_DOCKER_REPO)
+	Tag                  string        // Tag for test images
+	SpoofRequestInterval time.Duration // SpoofRequestInterval is the interval between requests in SpoofingClient
+	SpoofRequestTimeout  time.Duration // SpoofRequestTimeout is the timeout for polling requests in SpoofingClient
+}
+
+var f *TestEnvironment
+
+// InitFlags is for explicitly initializing the flags.
+func InitFlags(flagset *flag.FlagSet) {
+	if flagset == nil {
+		flagset = flag.CommandLine
+	}
+
+	f = new(TestEnvironment)
+
+	flagset.StringVar(&f.Cluster, "cluster", "",
+		"Provide the cluster to test against. Defaults to the current cluster in kubeconfig.")
+
+	flagset.StringVar(&f.Namespace, "namespace", "",
+		"Provide the namespace you would like to use for these tests.")
+
+	flagset.StringVar(&f.IngressEndpoint, "ingressendpoint", "", "Provide a static endpoint url to the ingress server used during tests.")
+
+	flagset.StringVar(&f.ImageTemplate, "imagetemplate", "{{.Repository}}/{{.Name}}:{{.Tag}}",
+		"Provide a template to generate the reference to an image from the test. Defaults to `{{.Repository}}/{{.Name}}:{{.Tag}}`.")
+
+	flagset.DurationVar(&f.SpoofRequestInterval, "spoofinterval", 1*time.Second,
+		"Provide an interval between requests for the SpoofingClient")
+
+	flagset.DurationVar(&f.SpoofRequestTimeout, "spooftimeout", 5*time.Minute,
+		"Provide a request timeout for the SpoofingClient")
+
+	defaultRepo := os.Getenv("KO_DOCKER_REPO")
+	flagset.StringVar(&f.DockerRepo, "dockerrepo", defaultRepo,
+		"Provide the uri of the docker repo you have uploaded the test image to using `uploadtestimage.sh`. Defaults to $KO_DOCKER_REPO")
+
+	flagset.StringVar(&f.Tag, "tag", "latest", "Provide the version tag for the test images.")
+}
+
+// Flags returns the command line flags or defaults for settings in the user's
+// environment. See TestEnvironment for a list of supported fields.
+// Caller must call InitFlags() and flags.Parse() before calling Flags().
+func Flags() *TestEnvironment {
+	return f
+}
diff --git a/test/logstream/interface.go b/test/logstream/interface.go
index 88418fb7a..6c2109d8f 100644
--- a/test/logstream/interface.go
+++ b/test/logstream/interface.go
@@ -31,10 +31,17 @@ import (
 // deferred so that the logstream can be stopped when the test is complete.
 type Canceler = logstreamv2.Canceler
 
+type ti interface {
+	Name() string
+	Error(args ...interface{})
+	Log(args ...interface{})
+	Logf(fmt string, args ...interface{})
+}
+
 // Start begins streaming the logs from system components with a `key:` matching
 // `test.ObjectNameForTest(t)` to `t.Log`.  It returns a Canceler, which must
 // be called before the test completes.
-func Start(t test.TLegacy) Canceler {
+func Start(t ti) Canceler {
 	// Do this lazily to make import ordering less important.
 	once.Do(func() {
 		if ns := os.Getenv(system.NamespaceEnvKey); ns != "" {
@@ -56,7 +63,7 @@ func Start(t test.TLegacy) Canceler {
 }
 
 type streamer interface {
-	Start(t test.TLegacy) Canceler
+	Start(t ti) Canceler
 }
 
 var (
@@ -68,7 +75,7 @@ type shim struct {
 	logstreamv2.Source
 }
 
-func (s *shim) Start(t test.TLegacy) Canceler {
+func (s *shim) Start(t ti) Canceler {
 	name := helpers.ObjectPrefixForTest(t)
 	canceler, err := s.StartStream(name, t.Logf)
 
diff --git a/test/logstream/null.go b/test/logstream/null.go
index 22d21c628..13697aae4 100644
--- a/test/logstream/null.go
+++ b/test/logstream/null.go
@@ -16,14 +16,12 @@ limitations under the License.
 
 package logstream
 
-import "knative.dev/pkg/test"
-
 type null struct{}
 
 var _ streamer = (*null)(nil)
 
 // Start implements streamer
-func (*null) Start(t test.TLegacy) Canceler {
+func (*null) Start(t ti) Canceler {
 	t.Log("logstream was requested, but SYSTEM_NAMESPACE was unset.")
 	return func() {}
 }
diff --git a/test/request.go b/test/request.go
index 334eec7e3..96c055a79 100644
--- a/test/request.go
+++ b/test/request.go
@@ -27,6 +27,8 @@ import (
 	"sync"
 	"time"
 
+	"knative.dev/pkg/test/flags"
+
 	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/test/logging"
 	"knative.dev/pkg/test/spoof"
@@ -171,7 +173,7 @@ func WaitForEndpointState(
 	resolvable bool,
 	opts ...interface{}) (*spoof.Response, error) {
 	return WaitForEndpointStateWithTimeout(ctx, kubeClient, logf, url, inState,
-		desc, resolvable, Flags.SpoofRequestTimeout, opts...)
+		desc, resolvable, flags.Flags().SpoofRequestTimeout, opts...)
 }
 
 // WaitForEndpointStateWithTimeout will poll an endpoint until inState indicates the state is achieved
diff --git a/test/tinterface.go b/test/tinterface.go
index 58565193a..4cda2c90a 100644
--- a/test/tinterface.go
+++ b/test/tinterface.go
@@ -20,6 +20,7 @@ limitations under the License.
 
 package test
 
+// Deprecated: Do not use this. Define your own interface.
 type T interface {
 	Name() string
 	Helper()
@@ -29,6 +30,7 @@ type T interface {
 	Error(args ...interface{})
 }
 
+// Deprecated: Do not use this. Define your own interface.
 type TLegacy interface {
 	T
 	Logf(fmt string, args ...interface{}) // It gets passed to things in logstream