Skip to content

Conversation

@holdenk
Copy link
Contributor

@holdenk holdenk commented Sep 11, 2019

What changes were proposed in this pull request?

Switch from using a Thread sleep for waiting for commands to finish to just waiting for the command to finish with a watcher & improve the error messages in the SecretsTestsSuite.

Why are the changes needed?

Currently some of the Spark Kubernetes tests have race conditions with command execution, and the frequent use of eventually makes debugging test failures difficult.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing tests pass after removal of thread.sleep

@SparkQA
Copy link

SparkQA commented Sep 11, 2019

Test build #110490 has finished for PR 25765 at commit 16ba894.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 11, 2019

@SparkQA
Copy link

SparkQA commented Sep 11, 2019

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

Test build #110492 has finished for PR 25765 at commit 2ab68b5.

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

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

@holdenk
Copy link
Contributor Author

holdenk commented Sep 12, 2019

jenkins retest this please

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

Test build #110527 has finished for PR 25765 at commit 2ab68b5.

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

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

@shaneknapp
Copy link
Contributor

test this please

@shaneknapp
Copy link
Contributor

(i triggered this because i renamed the k8s prb job, and just want to be sure nothing breaks)

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

Test build #110540 has finished for PR 25765 at commit 2ab68b5.

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

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15516/

@SparkQA
Copy link

SparkQA commented Sep 12, 2019

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15516/

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Test build #110549 has finished for PR 25765 at commit de35d67.

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

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15525/

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15525/

@holdenk
Copy link
Contributor Author

holdenk commented Sep 13, 2019

Jenkins retest this please

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Test build #110557 has finished for PR 25765 at commit de35d67.

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

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15532/

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15532/

@shaneknapp
Copy link
Contributor

test this please

@shaneknapp
Copy link
Contributor

(sorry for barging in again but i'm testing a small infra change on the ubuntu workers)

;)

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Test build #110571 has finished for PR 25765 at commit de35d67.

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

@holdenk
Copy link
Contributor Author

holdenk commented Sep 13, 2019

It's 100% ok :)

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15546/

@shaneknapp
Copy link
Contributor

It's 100% ok :)

awesome. i'm getting rid of those crappy, unrelated-to-the-build lsof errors in the logs.

@SparkQA
Copy link

SparkQA commented Sep 13, 2019

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15546/

@shaneknapp
Copy link
Contributor

It's 100% ok :)

awesome. i'm getting rid of those crappy, unrelated-to-the-build lsof errors in the logs.

aaaaaand those error messages are gone!

+ /home/jenkins/bin/kill_zinc_nailgun.py --zinc-port 3714

@holdenk
Copy link
Contributor Author

holdenk commented Sep 13, 2019

Awesome @shaneknapp :D

val SECRET_MOUNT_PATH = "/etc/secret"
val ENV_SECRET_KEY_1 = "username"
val ENV_SECRET_KEY_2 = "password"
val ENV_SECRET_KEY_1_CAP = ENV_SECRET_KEY_1.toUpperCase(Locale.ROOT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part of the change related, necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, although I figured cleaning up hard coded strings while I was there would be good.

@SparkQA
Copy link

SparkQA commented Sep 16, 2019

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15617/

@SparkQA
Copy link

SparkQA commented Sep 16, 2019

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/15617/

.writingOutput(out)
.writingError(System.err)
.withTTY()
.usingListener(listener)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool I was looking for something like that in the past :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rocking :)

@holdenk
Copy link
Contributor Author

holdenk commented Sep 19, 2019

If no one objects I plan to merge this tomorrow.

@asfgit asfgit closed this in 4080c4b Sep 20, 2019
@holdenk
Copy link
Contributor Author

holdenk commented Sep 20, 2019

Merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants