diff --git a/test/e2e/framework/provider.go b/test/e2e/framework/provider.go index c3e7851f4f..ea8e02c73c 100644 --- a/test/e2e/framework/provider.go +++ b/test/e2e/framework/provider.go @@ -45,6 +45,17 @@ func RegisterProvider(name string, factory Factory) { providers[name] = factory } +// GetProviders returns the names of all currently registered providers. +func GetProviders() []string { + mutex.Lock() + defer mutex.Unlock() + var providerNames []string + for name := range providers { + providerNames = append(providerNames, name) + } + return providerNames +} + func init() { // "local" or "skeleton" can always be used. RegisterProvider("local", func() (ProviderInterface, error) { @@ -53,11 +64,8 @@ func init() { RegisterProvider("skeleton", func() (ProviderInterface, error) { return NullProvider{}, nil }) - // The empty string also works, but triggers a warning. - RegisterProvider("", func() (ProviderInterface, error) { - Logf("The --provider flag is not set. Treating as a conformance test. Some tests may not be run.") - return NullProvider{}, nil - }) + // The empty string used to be accepted in the past, but is not + // a valid value anymore. } // SetupProviderConfig validates the chosen provider and creates diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index e29b40f8a4..900ed01143 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -21,6 +21,8 @@ import ( "fmt" "io/ioutil" "os" + "sort" + "strings" "time" "github.com/onsi/ginkgo/config" @@ -267,7 +269,7 @@ func RegisterClusterFlags() { flag.StringVar(&TestContext.KubeVolumeDir, "volume-dir", "/var/lib/kubelet", "Path to the directory containing the kubelet volumes.") flag.StringVar(&TestContext.CertDir, "cert-dir", "", "Path to the directory containing the certs. Default is empty, which doesn't use certs.") flag.StringVar(&TestContext.RepoRoot, "repo-root", "../../", "Root directory of kubernetes repository, for finding test files.") - flag.StringVar(&TestContext.Provider, "provider", "", "The name of the Kubernetes provider (gce, gke, local, etc.)") + flag.StringVar(&TestContext.Provider, "provider", "", "The name of the Kubernetes provider (gce, gke, local, skeleton (the fallback if not set), etc.)") flag.StringVar(&TestContext.Tooling, "tooling", "", "The tooling in use (kops, gke, etc.)") flag.StringVar(&TestContext.KubectlPath, "kubectl-path", "kubectl", "The kubectl binary to use. For development, you might use 'cluster/kubectl.sh' here.") flag.StringVar(&TestContext.OutputDir, "e2e-output-dir", "/tmp", "Output directory for interesting/useful test data, like performance data, benchmarks, and other metrics.") @@ -402,22 +404,33 @@ func AfterReadingAllFlags(t *TestContextType) { } // Make sure that all test runs have a valid TestContext.CloudConfig.Provider. + // TODO: whether and how long this code is needed is getting discussed + // in https://github.com/kubernetes/kubernetes/issues/70194. + if TestContext.Provider == "" { + // Some users of the e2e.test binary pass --provider=. + // We need to support that, changing it would break those usages. + Logf("The --provider flag is not set. Continuing as if --provider=skeleton had been used.") + TestContext.Provider = "skeleton" + } + var err error TestContext.CloudConfig.Provider, err = SetupProviderConfig(TestContext.Provider) - if err == nil { - return - } - if !os.IsNotExist(errors.Cause(err)) { - Failf("Failed to setup provider config: %v", err) - } - // We allow unknown provider parameters for historic reasons. At least log a - // warning to catch typos. - // TODO (https://github.com/kubernetes/kubernetes/issues/70200): - // - remove the fallback for unknown providers - // - proper error message instead of Failf (which panics) - klog.Warningf("Unknown provider %q, proceeding as for --provider=skeleton.", TestContext.Provider) - TestContext.CloudConfig.Provider, err = SetupProviderConfig("skeleton") if err != nil { - Failf("Failed to setup fallback skeleton provider config: %v", err) + if os.IsNotExist(errors.Cause(err)) { + // Provide a more helpful error message when the provider is unknown. + var providers []string + for _, name := range GetProviders() { + // The empty string is accepted, but looks odd in the output below unless we quote it. + if name == "" { + name = `""` + } + providers = append(providers, name) + } + sort.Strings(providers) + klog.Errorf("Unknown provider %q. The following providers are known: %v", TestContext.Provider, strings.Join(providers, " ")) + } else { + klog.Errorf("Failed to setup provider config for %q: %v", TestContext.Provider, err) + } + os.Exit(1) } }