Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix broken spark tests to use yaml files, 1 worker, declare DNS depe… #21626

Merged
merged 1 commit into from
Feb 25, 2016
Merged

Fix broken spark tests to use yaml files, 1 worker, declare DNS depe… #21626

merged 1 commit into from
Feb 25, 2016

Conversation

jayunit100
Copy link
Member

Ok, Another set of fixes for the broken examples #20999 , this time spark yamls.
Also updates docs to declare the explicit dependency on DNS.

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 20, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-bot
Copy link

k8s-bot commented Feb 20, 2016

GCE e2e build/test failed for commit 56eb861b62a3039571202ff7bb191320ce3d5d44.

@jayunit100
Copy link
Member Author

failure <*errors.errorString | 0xc208406a60>: {s: "old replica sets are not fully scaled down",} - assuming thats a flake

@jayunit100
Copy link
Member Author

  • fixes the component label
  • adds DNS as stated requirement
  • implements generic pod list that doesnt require name to be fixed for polling
  • tested and working on GCE
  • separates functions for worker and master so that coordinating components is declarative and not dependend on definition of test functions
    tested and working on GCE:
• [SLOW TEST:45.462 seconds]
[Feature:Example]
/home/jayunit100/Development/gopath/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/examples.go:447
  Spark
  /home/jayunit100/Development/gopath/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/examples.go:205
    should start spark master, driver and workers
    /home/jayunit100/Development/gopath/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/examples.go:204
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Ran 1 of 235 Specs in 48.872 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 234 Skipped PASS

@jayunit100
Copy link
Member Author

cc @mattf @kubernetes/sig-testing

@jayunit100 jayunit100 changed the title Fix broken spark tests to use yaml files, 2 workers, declare DNS depe… Fix broken spark tests to use yaml files, 1 worker, declare DNS depe… Feb 20, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 20, 2016

GCE e2e test build/test passed for commit 3c3dbe0737d8270bf2f802d653bf0e6d65bdaf57.

@k8s-bot
Copy link

k8s-bot commented Feb 20, 2016

GCE e2e build/test failed for commit e7be97b79e148e44314cda9f9da8e01f065c9892.

@jayunit100 jayunit100 mentioned this pull request Feb 20, 2016
Merged
@k8s-bot
Copy link

k8s-bot commented Feb 20, 2016

GCE e2e build/test failed for commit 7b2657a59a8e2c00ddd9f009ecd0295466462b8a.

@k8s-github-robot
Copy link

@jayunit100
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@k8s-bot
Copy link

k8s-bot commented Feb 21, 2016

GCE e2e test build/test passed for commit 7b2657a59a8e2c00ddd9f009ecd0295466462b8a.

@k8s-bot
Copy link

k8s-bot commented Feb 21, 2016

GCE e2e test build/test passed for commit bae110d2d9a2260695d5685890f470dedbfadec0.

zmerlynn
zmerlynn reviewed Feb 22, 2016
View reviewed changes
examples/spark/spark-worker-controller.yaml
@@ -3,7 +3,7 @@ apiVersion: v1
metadata:
name: spark-worker-controller
spec:
replicas: 3
replicas: 1
Copy link
Member

Choose a reason for hiding this comment

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

We don't want the example to bring up 1 worker by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

we can easily modify it to n+1, but what value does that provide?

Copy link
Member

Choose a reason for hiding this comment

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

I mean, 3 is certainly kind of arbitrary, but it at least shows more than one worker running when you spin up the example.

Copy link
Member Author

Choose a reason for hiding this comment

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

the goal is to minimize flakes and test a distilled cluster.
2 is a fully distributed system in every sense of the word, because spark is a strict master->slave architecture.
If we agree with that : Then and there is no fundamental difference between 1, 2 or 3 slaves, is there?

@zmerlynn if you insist ill update to 2 or 3 or .... as the priority is to get the fix in, but i think 1 slave is ideal. just let me know :)

Copy link
Member

Choose a reason for hiding this comment

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

Changing to 2 is fine.

@jayunit100
Copy link
Member Author

@zmerlynn thanks for the review, is this all set?

@k8s-bot
Copy link

k8s-bot commented Feb 22, 2016

GCE e2e test build/test passed for commit 8721c1cd4e3d71b0e578b0b824e59bd0ff70af3f.

zmerlynn
zmerlynn reviewed Feb 22, 2016
View reviewed changes
test/e2e/examples.go
workerControllerJson := mkpath("spark-worker-controller.json")

// TODO: Add Zepplin and Web UI to this example.
serviceJson := mkpath("spark-master-service.yaml")
Copy link
Member

Choose a reason for hiding this comment

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

Total nit: s/Json/Yaml.

@zmerlynn
Copy link
Member

@jayunit100: You need to gofmt it looks like, but it LGTM after that and the nit. (I wouldn't've bothered with the nit otherwise).

@zmerlynn zmerlynn assigned zmerlynn and unassigned ikehz Feb 22, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 22, 2016

GCE e2e build/test failed for commit 2dd36dd9971ef6e5cd4c4480302b5b9a2493f0d7.

@k8s-bot
Copy link

k8s-bot commented Feb 22, 2016

GCE e2e test build/test passed for commit e003947611ac3ed28879576147c508a6e7eb9a8d.

@k8s-github-robot
Copy link

@zmerlynn
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@zmerlynn
Copy link
Member

@k8s-bot unit test this, issue #IGNORE (no test results)

@zmerlynn zmerlynn added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 22, 2016
@zmerlynn
Copy link
Member

LGTM

@jayunit100
Copy link
Member Author

@zmerlynn i had to gofmt, force pushed... i think might a new LGTM ?

@zmerlynn zmerlynn added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Feb 22, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 22, 2016

GCE e2e test build/test passed for commit 782b268.

@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Feb 24, 2016

GCE e2e test build/test passed for commit 782b268.

a-robinson added a commit that referenced this pull request Feb 25, 2016
@a-robinson
Merge pull request #21626 from jayunit100/examples-spark-fix
c031697 
Fix broken spark tests to use yaml files, 1 worker, declare DNS depe…
@a-robinson a-robinson merged commit c031697 into kubernetes:master Feb 25, 2016
openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Dec 18, 2018
@k8s-publishing-bot
Merge pull request kubernetes#21626 from sttts/sttts-aggregator-metrics
e249841 
UPSTREAM: 71380: aggregator: add APIService unavailability metrics

Origin-commit: a2218fc44906711ef8e109118df5a6ca477393fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zmerlynn zmerlynn

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jayunit100 @k8s-github-robot @k8s-bot @zmerlynn @ikehz @googlebot @a-robinson