From f3d79e152eaac1cd1c5ac79e0a0bbd0997abdb17 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Mon, 28 Jan 2019 14:35:23 +0100
Subject: [PATCH] e2e: reject unknown providers

This finishes the work started for 1.13: instead of merely warning
about an unknown value given to --profile, the test/e2e/e2e.test
binary will now print an error and refuse to run.

Fixes: #70200
---
 test/e2e/framework/provider.go     | 11 +++++++++
 test/e2e/framework/test_context.go | 36 +++++++++++++++++-------------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/test/e2e/framework/provider.go b/test/e2e/framework/provider.go
index c3e7851f4fe..9c8a902b99e 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) {
diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go
index 58df02d4371..0fc595cdc76 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"
@@ -264,7 +266,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, 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.")
@@ -399,22 +401,26 @@ 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.
 	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)
 	}
 }