[SPARK-24551][K8S] Add integration tests for secrets #21652

@foxish @erikerlandson I got this failure: "namespaces "59da2249348347a39391b4389ab418ab" not found."
I don't think it relates to the PR, I haven't seen anything locally. Secret is created in the namespace defined in the tests, I don't touch it.

skonto · 2018-06-29T07:08:17Z

@felixcheung @liyinan926 @ssuchter review pls.

foxish · 2018-07-02T20:14:13Z

jenkins, retest this please

foxish

Looks like a great start. I like that we're now getting the output from pod exec and validating it.

foxish · 2018-07-02T20:17:52Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

@@ -74,10 +76,12 @@ private[spark] class KubernetesSuite extends SparkFunSuite
    testBackend = IntegrationTestBackendFactory.getTestBackend
    testBackend.initialize()
    kubernetesTestComponents = new KubernetesTestComponents(testBackend.getKubernetesClient)
+    createTestSecret()


I'm wondering if we should add these tests to a separate class? This file is growing in size and we can separate them out a bit for better code understanding.

You are right probably we should. Ok let me try do this.

foxish · 2018-07-02T20:18:29Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

+    runSparkPiAndVerifyCompletion(
+      driverPodChecker = (driverPod: Pod) => {
+        doBasicDriverPodCheck(driverPod)
+          checkSecrets(driverPod)


nit: indentation

ssuchter · 2018-07-02T20:21:05Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

+      .withTTY()
+      .exec(cmd.toArray: _*)
+    // wait to get some result back
+    Thread.sleep(1000)


This is a pretty quick timeout. Is there some way to have a longer timeout but to return instantly if the watch command exits? E.g. some kind of "join" method call on the watch instance that itself has a timeout?

Ok will have a look. It does work because I guess its minikube and it is fast, I didn't see any issues. But might be a problem don't know.

@ssuchter I tried the approach here: https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/kubernetes/examples/ExecLoopExample.java
and also mentioned here:
https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/kubernetes/examples/ExecPipesExample.java#L60
Latches were not useful to me. The examples there use the timeout approach (actually I recall that I looked at the examples before writing the code here).
The ideal approach would be to check continuously on the watch output on a future (eg. look what this class does: https://github.com/fabric8io/kubernetes-client/blob/9b2128ca5f2362991c99640d8a0325d60741644c/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/InputStreamPumper.java) and wait until it completes with some time out. But that didnt work well so far for me.
So I guess that sleep is the most easy solution.

ssuchter

This overall looks pretty nice, thanks! I had just a couple questions - please take a look.

ssuchter · 2018-07-02T20:23:10Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

@@ -74,10 +76,12 @@ private[spark] class KubernetesSuite extends SparkFunSuite
    testBackend = IntegrationTestBackendFactory.getTestBackend
    testBackend.initialize()
    kubernetesTestComponents = new KubernetesTestComponents(testBackend.getKubernetesClient)
+    createTestSecret()


Should this be done before every test, or just the one that is using the secrets?

I guess we can add it before the specific test. My rationale was that this is something I require before the tests are run.

ssuchter · 2018-07-02T20:23:24Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

  }

  override def afterAll(): Unit = {
    testBackend.cleanUp()
+    deleteTestSecret()


Same question about every test versus just one of them.

SparkQA · 2018-07-02T20:27:31Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/626/

SparkQA · 2018-07-02T20:29:36Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/626/

SparkQA · 2018-07-02T20:34:00Z

Test build #92538 has finished for PR 21652 at commit 7602dbc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

skonto · 2018-07-04T14:01:12Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala

+        },
+        appArgs = Array("1000") // give it enough time for all execs to be visible
+      )
+    } finally {


@ssuchter moved the secret creation in here. I had the idea of using before and after and checking for a flag to identify the current test (scalatest with FunSuite does not give you that AFAIK) but I think this is cleaner.

SparkQA · 2018-07-04T14:07:20Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/673/

SparkQA · 2018-07-04T14:09:11Z

Test build #92610 has finished for PR 21652 at commit 2bda40c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-04T14:12:27Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/673/

skonto · 2018-07-04T15:27:14Z

@foxish @ssuchter I refactored the tests in different classes. There are some clear benefits now.
KubernetesSuite should only provide the env setup now. If we want real separation in the future we can just create real Suites, now the tests are included as traits.
Pls check my comment above about the timeout issue.

SparkQA · 2018-07-04T15:33:03Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/675/

SparkQA · 2018-07-17T20:16:16Z

Test build #93199 has finished for PR 21652 at commit f843638.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

shaneknapp · 2018-07-17T21:10:05Z

test this please

SparkQA · 2018-07-17T21:24:26Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1075/

SparkQA · 2018-07-17T21:25:18Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1075/

SparkQA · 2018-07-17T21:30:30Z

Test build #93204 has finished for PR 21652 at commit f843638.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

shaneknapp · 2018-07-17T21:48:28Z

test this please (i tied this build to amp-jenkins-staging-worker-02 to unblock things)

SparkQA · 2018-07-17T22:04:13Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1076/

SparkQA · 2018-07-17T22:05:04Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1076/

shaneknapp · 2018-07-17T22:08:39Z

argh fixing 02 again and will retrigger then once it's done.

i'm really sorry that things have been so flaky recently.

SparkQA · 2018-07-17T22:11:16Z

Test build #93205 has finished for PR 21652 at commit f843638.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

shaneknapp · 2018-07-17T22:28:50Z

test this please

SparkQA · 2018-07-17T22:43:33Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1077/

SparkQA · 2018-07-17T22:49:24Z

Test build #93206 has finished for PR 21652 at commit f843638.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-17T22:50:47Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1077/

skonto · 2018-07-18T09:35:37Z

@shaneknapp thanks, @srowen @foxish gentle ping.

skonto · 2018-07-18T09:37:36Z

...gration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/TestConstants.scala

@@ -16,7 +16,7 @@
 */
 package org.apache.spark.deploy.k8s.integrationtest

-package object constants {
+object TestConstants {


I removed the package objects and replaced them with normal ones. The package statement was not ok and their naming convention was not following scala rules.

SparkQA · 2018-07-18T09:48:22Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1091/

SparkQA · 2018-07-18T09:54:39Z

Test build #93228 has finished for PR 21652 at commit 5265129.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-18T09:56:03Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1091/

skonto · 2018-07-19T20:31:50Z

@foxish @srowen I rebased again could you review and merge pls. It is ready now.

SparkQA · 2018-07-19T20:43:31Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1140/

SparkQA · 2018-07-19T20:52:21Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/1140/

skonto · 2018-07-19T21:05:58Z

bin/docker-image-tool.sh

@@ -135,7 +135,7 @@ BASEDOCKERFILE=
 PYDOCKERFILE=
 NOCACHEARG=
 BUILD_PARAMS=
-while getopts f:mr:t:n:b: option
+while getopts f:p:mr:t:n:b: option


@debasishg spotted this.

SparkQA · 2018-07-20T00:55:37Z

Test build #93298 has finished for PR 21652 at commit 67df340.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

skonto · 2018-07-20T09:21:26Z

@felixcheung @srowen may I have a merge pls?

srowen · 2018-07-20T12:56:16Z

Merged to master

skonto force-pushed the add-secret-its branch 2 times, most recently from f0d59cc to 7602dbc Compare June 28, 2018 03:59

skonto changed the title ~~[SPARK-24551][K8s] Add integration tests for secrets~~ [SPARK-24551][K8S] Add integration tests for secrets Jun 28, 2018

skonto mentioned this pull request Jun 30, 2018

[SPARK-24428][K8S] Fix unused code #21462

Closed

foxish reviewed Jul 2, 2018

View reviewed changes

ssuchter reviewed Jul 2, 2018

View reviewed changes

ssuchter suggested changes Jul 2, 2018

View reviewed changes

skonto force-pushed the add-secret-its branch from 7602dbc to 2bda40c Compare July 4, 2018 13:52

skonto commented Jul 4, 2018

View reviewed changes

skonto force-pushed the add-secret-its branch 2 times, most recently from 9bd17b3 to 9de4a7a Compare July 4, 2018 15:26

skonto force-pushed the add-secret-its branch from 9de4a7a to 135aa65 Compare July 4, 2018 15:29

skonto force-pushed the add-secret-its branch from f843638 to 5265129 Compare July 18, 2018 09:35

skonto commented Jul 18, 2018

View reviewed changes

add secret tests

67df340

skonto force-pushed the add-secret-its branch from 5265129 to 67df340 Compare July 19, 2018 20:30

skonto commented Jul 19, 2018

View reviewed changes

asfgit closed this in 20ce1a8 Jul 20, 2018

[SPARK-24551][K8S] Add integration tests for secrets #21652

[SPARK-24551][K8S] Add integration tests for secrets #21652

Uh oh!

Conversation

skonto commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

skonto commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

skonto commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skonto commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxish commented Jul 2, 2018

Uh oh!

foxish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssuchter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 2, 2018

Uh oh!

SparkQA commented Jul 2, 2018

Uh oh!

SparkQA commented Jul 2, 2018

Uh oh!

skonto Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 4, 2018

Uh oh!

SparkQA commented Jul 4, 2018

Uh oh!

SparkQA commented Jul 4, 2018

Uh oh!

skonto commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

skonto commented Jun 28, 2018 •

edited

Loading

skonto commented Jun 28, 2018 •

edited

Loading

skonto commented Jun 28, 2018 •

edited

Loading

skonto commented Jun 29, 2018 •

edited

Loading

skonto Jul 4, 2018 •

edited

Loading

skonto Jul 4, 2018 •

edited

Loading

skonto Jul 4, 2018 •

edited

Loading

skonto commented Jul 4, 2018 •

edited

Loading

skonto Jul 18, 2018 •

edited

Loading

skonto commented Jul 19, 2018 •

edited

Loading

skonto commented Jul 20, 2018 •

edited

Loading