-
Notifications
You must be signed in to change notification settings - Fork 9
Cloud testing scripts #16
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.
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
Conversation
|
CI is now working against GKE - https://k8s-testgrid.appspot.com/sig-big-data#spark-k8s-periodic, and using the changes in this branch. Looks like there are a couple of failures in the runs of the integration tests to be addressed as well. |
4f7022f to
1f9ebc3
Compare
|
@kimoonkim, integration test failure looks like a minikube issue on the jenkins node. |
|
Yes, there was a minikube issue. I don't know the root cause, but it seems to have disappeared. |
|
rerun integration tests please |
kimoonkim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. I like the change. A few minor suggestions below. PTAL.
| } | ||
|
|
||
| ### Basic Validation ### | ||
| if [ ! -d "integration-test" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good check. I like it.
e2e/e2e-cloud.sh
Outdated
| tag=$(git rev-parse HEAD | cut -c -6) | ||
| echo "Spark distribution built at SHA $tag" | ||
|
|
||
| cd dist && ./sbin/build-push-docker-images.sh -r $IMAGE_REPO -t $tag build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this separation of docker image building and running integration tests.
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a one-liner comment for what this script does?
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment for what this script does and what are the requirements for running the script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| # Install basic dependencies | ||
| echo "deb http://http.debian.net/debian jessie-backports main" >> /etc/apt/sources.list | ||
| apt-get update && apt-get install -y curl wget git tar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. So this works only for debian? Maybe mention this as a requirement at the file level comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned above - this is specific to the k8s testing environment used by the Kubernetes project. It's not intended to be used elsewhere.
e2e/e2e-prow.sh
Outdated
| root=$(pwd) | ||
| master=$(kubectl cluster-info | head -n 1 | grep -oE "https?://[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}(:[0-9]+)?") | ||
| repo="https://github.com/apache/spark" | ||
| image_repo="gcr.io/spark-testing-191023" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this magic number 191023? Is it ok to hard code?
|
|
||
| cd "$(dirname "$0")"/../ | ||
| ./e2e/e2e-cloud.sh -m $master -r $repo -i $image_repo | ||
| ls -1 ./integration-test/target/surefire-reports/*.xml | cat -n | while read n f; do cp "$f" "/workspace/_artifacts/junit_0$n.xml"; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment why do we create the junit_ files?
|
@kimoonkim, added comments and a new script to run for minikube. Should just work out of the box. PTAL. |
e2e/runner.sh
Outdated
| ### Set sensible defaults ### | ||
| REPO="https://github.com/apache/spark" | ||
| IMAGE_REPO="docker.io/kubespark" | ||
| DEPLOY_MODE="cloud" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume minikube is better as default as it is less assuming than GCE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm, will flip that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| fi | ||
| else | ||
| # -m option for minikube. | ||
| cd dist && ./sbin/build-push-docker-images.sh -m -r $IMAGE_REPO -t $tag build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Where are this build dir and the build script? What if they don't exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build and push are arguments that the script takes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They need not be valid directories. The build script is part of the upstream spark distribution, and exists on our fork as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. This is spark/dist dir inside a spark repo.
|
LGTM and I approved just now. Thanks for writing this change! |
|
Thanks for reviewing. Merging now |
|
(not deleting the branch for now because the cloud jobs are still using it) |
runner.sh is useful in general for anyone looking to test against an arbitrary k8s cluster.
e2e-prow.sh has some specific setup for the prow testing environment.
e2e-minikube.sh has setup for local minikube environments.
Tested with:
cc @liyinan926 @kimoonkim