[SPARK-25809][K8S] Further simplify code

- Combine cloud backends into one
- Remove unecessary manipulation of System properties
- Documentation tweaks
- Ensure we clean up Spark Master URL when overriding master URL in
  KubeConfigBackend
- Reuse utils method from Spark Core rather than duplicating in
  integration tests module
- Fix Bash indentation issues
- Insert extra new line prior to error output

Author: rvesse

resource-managers/kubernetes/integration-tests/README.md

@@ -26,38 +26,40 @@ The main useful options are outlined below.
 
 ## Using a different backend
 
-The integration test backend i.e. the K8S cluster used for testing is controlled by the `--deploy-mode` option.  By default this
-is set to `minikube`, the available backends are their perquisites are as follows.
+The integration test backend i.e. the K8S cluster used for testing is controlled by the `--deploy-mode` option.  By 
+default this is set to `minikube`, the available backends are their perequisites are as follows.  
 
 ### `minikube`
 
-Uses the local `minikube` cluster, this requires that `minikube` 0.23.0 or greater be installed and that it be allocated at least 
-4 CPUs and 6GB memory (some users have reported success with as few as 3 CPUs and 4GB memory).  The tests will check if `minikube` 
-is started and abort early if it isn't currently running.
+Uses the local `minikube` cluster, this requires that `minikube` 0.23.0 or greater be installed and that it be allocated 
+at least 4 CPUs and 6GB memory (some users have reported success with as few as 3 CPUs and 4GB memory).  The tests will 
+check if `minikube` is started and abort early if it isn't currently running.
 
 ### `docker-for-desktop`
 
 Since July 2018 Docker for Desktop provide an optional Kubernetes cluster that can be enabled as described in this 
-[blog post](https://blog.docker.com/2018/07/kubernetes-is-now-available-in-docker-desktop-stable-channel/).  Assuming this is enabled
-using this backend will auto-configure itself from the `docker-for-desktop` context that Docker creates in your `~/.kube/config` file. 
-If your config file is in a different location you should set the `KUBECONFIG` environment variable appropriately.
+[blog post](https://blog.docker.com/2018/07/kubernetes-is-now-available-in-docker-desktop-stable-channel/).  Assuming 
+this is enabled using this backend will auto-configure itself from the `docker-for-desktop` context that Docker creates 
+in your `~/.kube/config` file. If your config file is in a different location you should set the `KUBECONFIG` 
+environment variable appropriately.
 
-### `cloud` and `cloud-url`
+### `cloud` 
 
-These closely related backends configure the tests to use an arbitrary Kubernetes cluster running in the cloud or otherwise.
+These cloud backend configures the tests to use an arbitrary Kubernetes cluster running in the cloud or otherwise.
 
-The `cloud` backend auto-configures the cluster to use from your K8S config file, this is assumed to be `~/.kube/config` unless the
-`KUBECONFIG` environment variable is set to override this location.  By default this will use whatever your current context is in the
-config file, to use an alternative context from your config file you can specify the `--context <context>` flag with the desired context.
+The `cloud` backend auto-configures the cluster to use from your K8S config file, this is assumed to be `~/.kube/config`
+unless the `KUBECONFIG` environment variable is set to override this location.  By default this will use whatever your 
+current context is in the config file, to use an alternative context from your config file you can specify the 
+`--context <context>` flag with the desired context.
 
-The `cloud-url` backend configures the cluster to simply use a K8S master URL, this should be supplied via the 
-`--spark-master <master-url>` flag.
+You can optionally use a different K8S master URL than the one your K8S config file specified, this should be supplied 
+via the `--spark-master <master-url>` flag.
 
 ## Re-using Docker Images
 
 By default, the test framework will build new Docker images on every test execution. A unique image tag is generated,
-and it is written to file at `target/imageTag.txt`. To reuse the images built in a previous run, or to use a Docker image tag
-that you have built by other means already, pass the tag to the test script:
+and it is written to file at `target/imageTag.txt`. To reuse the images built in a previous run, or to use a Docker 
+image tag that you have built by other means already, pass the tag to the test script:
 
     dev/dev-run-integration-tests.sh --image-tag <tag>
 
@@ -67,34 +69,40 @@ where if you still want to use images that were built before by the test framewo
 
 ## Spark Distribution Under Test
 
-The Spark code to test is handed to the integration test system via a tarball. Here is the option that is used to specify the tarball:
+The Spark code to test is handed to the integration test system via a tarball. Here is the option that is used to 
+specify the tarball:
 
 * `--spark-tgz <path-to-tgz>` - set `<path-to-tgz>` to point to a tarball containing the Spark distribution to test.
 
-This Tarball should be created by first running `dev/make-distribution.sh` passing the `--tgz` flag and `-Pkubernetes` as one of the
-options to ensure that Kubernetes support is included in the distribution.  For more details on building a runnable distribution please
-see the [Building Spark](https://spark.apache.org/docs/latest/building-spark.html#building-a-runnable-distribution) documentation.
+This Tarball should be created by first running `dev/make-distribution.sh` passing the `--tgz` flag and `-Pkubernetes` 
+as one of the options to ensure that Kubernetes support is included in the distribution.  For more details on building a
+runnable distribution please see the 
+[Building Spark](https://spark.apache.org/docs/latest/building-spark.html#building-a-runnable-distribution) 
+documentation.
 
-**TODO:** Don't require the packaging of the built Spark artifacts into this tarball, just read them out of the current tree.
+**TODO:** Don't require the packaging of the built Spark artifacts into this tarball, just read them out of the current 
+tree.
 
 ## Customizing the Namespace and Service Account
 
-If no namespace is specified then a temporary namespace will be created and deleted during the test run.  Similarly if no service
-account is specified then the `default` service account for the namespace will be used.
+If no namespace is specified then a temporary namespace will be created and deleted during the test run.  Similarly if 
+no service account is specified then the `default` service account for the namespace will be used.
 
-Using the `--namespace <namespace>` flag sets `<namespace>` to the namespace in which the tests should be run.  If this is supplied 
-then the tests assume this namespace exists in the K8S cluster and will not attempt to create it.  Additionally this namespace must 
-have an appropriately authorized service account which can be customised via the `--service-account` flag.
+Using the `--namespace <namespace>` flag sets `<namespace>` to the namespace in which the tests should be run.  If this 
+is supplied then the tests assume this namespace exists in the K8S cluster and will not attempt to create it.  
+Additionally this namespace must have an appropriately authorized service account which can be customised via the 
+`--service-account` flag.
 
-The `--service-account <service account name>` flag sets `<service account name>` to the name of the Kubernetes service account to
-use in the namespace specified by the `--namespace` flag. The service account is expected to have permissions to get, list, watch,
-and create pods. For clusters with RBAC turned on, it's important that the right permissions are granted to the service account
-in the namespace through an appropriate role and role binding. A reference RBAC configuration is provided in `dev/spark-rbac.yaml`.
+The `--service-account <service account name>` flag sets `<service account name>` to the name of the Kubernetes service 
+account to use in the namespace specified by the `--namespace` flag. The service account is expected to have permissions
+to get, list, watch, and create pods. For clusters with RBAC turned on, it's important that the right permissions are 
+granted to the service account in the namespace through an appropriate role and role binding. A reference RBAC 
+configuration is provided in `dev/spark-rbac.yaml`.
 
 # Running the Test Directly
 
-If you prefer to run just the integration tests directly then you can customise the behaviour via properties passed to Maven using the
-`-Dproperty=value` option e.g.
+If you prefer to run just the integration tests directly, then you can customise the behaviour via passing system 
+properties to Maven.  For example:
 
     mvn integration-test -am -pl :spark-kubernetes-integration-tests_2.11 \
                             -Pkubernetes -Phadoop-2.7 -Dhadoop.version=2.7.3 \
@@ -108,8 +116,8 @@ If you prefer to run just the integration tests directly then you can customise
                             
 ## Available Maven Properties
 
-The following are the available Maven properties that can be passed.  For the most part these correspond to flags passed to the 
-wrapper scripts and using the wrapper scripts will simply set these appropriately behind the scenes.
+The following are the available Maven properties that can be passed.  For the most part these correspond to flags passed 
+to the wrapper scripts and using the wrapper scripts will simply set these appropriately behind the scenes.
 
 <table>
   <tr>
@@ -134,63 +142,65 @@ wrapper scripts and using the wrapper scripts will simply set these appropriatel
   <tr>
     <td><code>spark.kubernetes.test.deployMode</code></td>
     <td>
-      The integration test backend to use.  Acceptable values are <code>minikube</code>, <code>docker-for-desktop</code>, 
-      <code>cloud</code> and <code>cloud-url</code>.
+      The integration test backend to use.  Acceptable values are <code>minikube</code>, 
+      <code>docker-for-desktop</code> and <code>cloud</code>.
     <td><code>minikube</code></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.kubeConfigContext</code></td>
     <td>
-      When using the <code>cloud</code> backend specifies the context from the users K8S config file that should be used as the 
-      target cluster for integration testing.  If not set and using the <code>cloud</code> backend then your current context 
-      will be used.
+      When using the <code>cloud</code> backend specifies the context from the users K8S config file that should be used
+      as the target cluster for integration testing.  If not set and using the <code>cloud</code> backend then your 
+      current context will be used.
     </td>
     <td></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.master</code></td>
     <td>
-      When using the <code>cloud-url</code> backend must be specified to indicate the K8S master URL to communicate with.
+      When using the <code>cloud-url</code> backend must be specified to indicate the K8S master URL to communicate 
+      with.
     </td>
     <td></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.imageTag</code></td>
     <td>
-      A specific image tag to use, when set assumes images with those tags are already built and available in the specified image 
-      repository.  When set to <code>N/A</code> (the default) fresh images will be built.
+      A specific image tag to use, when set assumes images with those tags are already built and available in the 
+      specified image repository.  When set to <code>N/A</code> (the default) fresh images will be built.
     </td>
     <td><code>N/A</code>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.imageTagFile</code></td>
     <td>
-      A file containing the image tag to use, if no specific image tag is set then fresh images will be built with a generated 
-      tag and that tag written to this file.
+      A file containing the image tag to use, if no specific image tag is set then fresh images will be built with a 
+      generated tag and that tag written to this file.
     </td>
     <td><code>${project.build.directory}/imageTag.txt</code></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.imageRepo</code></td>
     <td>
-      The Docker image repository that contains the images to be used if a specific image tag is set or to which the images will 
-      be pushed to if fresh images are being built.
+      The Docker image repository that contains the images to be used if a specific image tag is set or to which the 
+      images will be pushed to if fresh images are being built.
     </td>
     <td><code>docker.io/kubespark</code></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.namespace</code></td>
     <td>
-      A specific Kubernetes namespace to run the tests in.  If specified then the tests assume that this namespace already exists.
-      When not specified a temporary namespace for the tests will be created and deleted as part of the test run.
+      A specific Kubernetes namespace to run the tests in.  If specified then the tests assume that this namespace 
+      already exists. When not specified a temporary namespace for the tests will be created and deleted as part of the
+      test run.
     </td>
     <td></td>
   </tr>
   <tr>
     <td><code>spark.kubernetes.test.serviceAccountName</code></td>
     <td>
-      A specific Kubernetes service account to use for running the tests.  If not specified then the namespaces default service
-      account will be used and that must have sufficient permissions or the tests will fail.
+      A specific Kubernetes service account to use for running the tests.  If not specified then the namespaces default
+      service account will be used and that must have sufficient permissions or the tests will fail.
     </td>
     <td></td>
   </tr>

resource-managers/kubernetes/integration-tests/scripts/setup-integration-test-env.sh

@@ -73,7 +73,7 @@ then
   cd $UNPACKED_SPARK_TGZ
 
   case $DEPLOY_MODE in
-    cloud|cloud-url)
+    cloud)
       # Build images
       $UNPACKED_SPARK_TGZ/bin/docker-image-tool.sh -r $IMAGE_REPO -t $IMAGE_TAG build
 
@@ -86,18 +86,18 @@ then
       fi
       ;;
 
-     docker-for-desktop)
+    docker-for-desktop)
        # Only need to build as this will place it in our local Docker repo which is all
        # we need for Docker for Desktop to work so no need to also push
        $UNPACKED_SPARK_TGZ/bin/docker-image-tool.sh -r $IMAGE_REPO -t $IMAGE_TAG build
        ;;
 
-     minikube)
+    minikube)
        # Only need to build and if we do this with the -m option for minikube we will
        # build the images directly using the minikube Docker daemon so no need to push
        $UNPACKED_SPARK_TGZ/bin/docker-image-tool.sh -m -r $IMAGE_REPO -t $IMAGE_TAG build
        ;;
-     *)
+    *)
        echo "Unrecognized deploy mode $DEPLOY_MODE" && exit 1
        ;;
   esac

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/ProcessUtils.scala

@@ -41,7 +41,7 @@ object ProcessUtils extends Logging {
     assert(proc.waitFor(timeout, TimeUnit.SECONDS),
       s"Timed out while executing ${fullCommand.mkString(" ")}")
     assert(proc.exitValue == 0,
-      s"Failed to execute ${fullCommand.mkString(" ")}${if (dumpErrors) outputLines.mkString("\n")}")
+      s"Failed to execute ${fullCommand.mkString(" ")}${if (dumpErrors) "\n" + outputLines.mkString("\n")}")
     outputLines
   }
 }

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/TestConstants.scala

@@ -20,7 +20,6 @@ object TestConstants {
   val BACKEND_MINIKUBE = "minikube"
   val BACKEND_DOCKER_FOR_DESKTOP = "docker-for-desktop"
   val BACKEND_CLOUD = "cloud"
-  val BACKEND_CLOUD_URL = "cloud-url"
 
   val CONFIG_KEY_DEPLOY_MODE = "spark.kubernetes.test.deployMode"
   val CONFIG_KEY_KUBE_CONFIG_CONTEXT = "spark.kubernetes.test.kubeConfigContext"

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/Utils.scala

@@ -27,36 +27,4 @@ object Utils extends Logging {
     val resource = createResource
     try f.apply(resource) finally resource.close()
   }
-
-  def checkAndGetK8sMasterUrl(rawMasterURL: String): String = {
-    require(rawMasterURL.startsWith("k8s://"),
-      "Kubernetes master URL must start with k8s://.")
-    val masterWithoutK8sPrefix = rawMasterURL.substring("k8s://".length)
-
-    // To handle master URLs, e.g., k8s://host:port.
-    if (!masterWithoutK8sPrefix.contains("://")) {
-      val resolvedURL = s"https://$masterWithoutK8sPrefix"
-      logInfo("No scheme specified for kubernetes master URL, so defaulting to https. Resolved " +
-        s"URL is $resolvedURL.")
-      return s"k8s://$resolvedURL"
-    }
-
-    val masterScheme = new URI(masterWithoutK8sPrefix).getScheme
-    val resolvedURL = masterScheme.toLowerCase match {
-      case "https" =>
-        masterWithoutK8sPrefix
-      case "http" =>
-        logWarning("Kubernetes master URL uses HTTP instead of HTTPS.")
-        masterWithoutK8sPrefix
-      case null =>
-        val resolvedURL = s"https://$masterWithoutK8sPrefix"
-        logInfo("No scheme specified for kubernetes master URL, so defaulting to https. Resolved " +
-          s"URL is $resolvedURL.")
-        resolvedURL
-      case _ =>
-        throw new IllegalArgumentException("Invalid Kubernetes master scheme: " + masterScheme)
-    }
-
-    s"k8s://$resolvedURL"
-  }
 }

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/backend/IntegrationTestBackend.scala

@@ -19,7 +19,7 @@ package org.apache.spark.deploy.k8s.integrationtest.backend
 
 import io.fabric8.kubernetes.client.DefaultKubernetesClient
 import org.apache.spark.deploy.k8s.integrationtest.TestConstants._
-import org.apache.spark.deploy.k8s.integrationtest.backend.cloud.{KubeConfigBackend, CloudTestBackend}
+import org.apache.spark.deploy.k8s.integrationtest.backend.cloud.KubeConfigBackend
 import org.apache.spark.deploy.k8s.integrationtest.backend.docker.DockerForDesktopBackend
 import org.apache.spark.deploy.k8s.integrationtest.backend.minikube.MinikubeTestBackend
 
@@ -35,8 +35,7 @@ private[spark] object IntegrationTestBackendFactory {
       .getOrElse(BACKEND_MINIKUBE)
     deployMode match {
       case BACKEND_MINIKUBE => MinikubeTestBackend
-      case BACKEND_CLOUD => new KubeConfigBackend(null)
-      case BACKEND_CLOUD_URL => CloudTestBackend
+      case BACKEND_CLOUD => new KubeConfigBackend(System.getProperty(CONFIG_KEY_KUBE_CONFIG_CONTEXT))
       case BACKEND_DOCKER_FOR_DESKTOP => DockerForDesktopBackend
       case _ => throw new IllegalArgumentException("Invalid " +
         CONFIG_KEY_DEPLOY_MODE + ": " + deployMode)