-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-37529][K8S][TESTS][FOLLOWUP] Allow dev-run-integration-tests.sh to take a custom Dockerfile #34818
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
| <td><code>spark-r</code></td> | ||
| </tr> | ||
| <tr> | ||
| <td><code>spark.kubernetes.test.dockerFile</code></td> |
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 part is what I forgot to add in #34790 .
| ) | ||
|
|
||
| $TEST_ROOT_DIR/build/mvn install -f $TEST_ROOT_DIR/pom.xml -pl resource-managers/kubernetes/integration-tests $BUILD_DEPENDENCIES_MVN_FLAG -Pscala-$SCALA_VERSION -P$HADOOP_PROFILE -Pkubernetes -Pkubernetes-integration-tests ${properties[@]} | ||
| (cd $TEST_ROOT_DIR; ./build/mvn install -pl resource-managers/kubernetes/integration-tests $BUILD_DEPENDENCIES_MVN_FLAG -Pscala-$SCALA_VERSION -P$HADOOP_PROFILE -Pkubernetes -Pkubernetes-integration-tests ${properties[@]}) |
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 noticed that all the examples of dev-run-integration-tests.sh in integration-tests/README.md don't work because scala-style-config.xml is not found in integration-tests.
This change is to resolve the issue.
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.
Does this work? I'm testing your PR and hit the following.
$ cd resource-managers/kubernetes/integration-tests
$ ./dev/dev-run-integration-tests.sh --docker-file ../docker/src/main/dockerfiles/spark/Dockerfile.java17
++ git rev-parse --show-toplevel
+ TEST_ROOT_DIR=/Users/dongjoon/APACHE/spark-merge
+ DEPLOY_MODE=minikube
+ IMAGE_REPO=docker.io/kubespark
+ SPARK_TGZ=N/A
+ IMAGE_TAG=N/A
+ JAVA_IMAGE_TAG=
+ BASE_IMAGE_NAME=
+ JVM_IMAGE_NAME=
+ PYTHON_IMAGE_NAME=
+ R_IMAGE_NAME=
+ DOCKER_FILE=
+ SPARK_MASTER=
+ NAMESPACE=
+ SERVICE_ACCOUNT=
+ CONTEXT=
+ INCLUDE_TAGS=k8s
+ EXCLUDE_TAGS=
+ JAVA_VERSION=8
+ BUILD_DEPENDENCIES_MVN_FLAG=-am
+ HADOOP_PROFILE=hadoop-3.2
+ MVN=/Users/dongjoon/APACHE/spark-merge/build/mvn
++ /Users/dongjoon/APACHE/spark-merge/build/mvn help:evaluate -Dexpression=scala.binary.version
++ grep -v INFO
++ grep -v WARNING
++ tail -n 1
+ SCALA_VERSION=2.12
+ export SCALA_VERSION
+ echo 2.12
2.12
+ (( 2 ))
+ case $1 in
+ DOCKER_FILE=../docker/src/main/dockerfiles/spark/Dockerfile.java17
+ shift
+ shift
+ (( 0 ))
+ properties=(-Djava.version=$JAVA_VERSION -Dspark.kubernetes.test.sparkTgz=$SPARK_TGZ -Dspark.kubernetes.test.imageTag=$IMAGE_TAG -Dspark.kubernetes.test.imageRepo=$IMAGE_REPO -Dspark.kubernetes.test.deployMode=$DEPLOY_MODE -Dtest.include.tags=$INCLUDE_TAGS)
+ '[' -n '' ']'
+ '[' -n ../docker/src/main/dockerfiles/spark/Dockerfile.java17 ']'
+ properties=(${properties[@]} -Dspark.kubernetes.test.dockerFile=$(realpath $DOCKER_FILE))
++ realpath ../docker/src/main/dockerfiles/spark/Dockerfile.java17
./dev/dev-run-integration-tests.sh: line 153: realpath: command not found
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, realpath seems not to be a standard command. I tested on Pop!_OS 20.04 and CentOS 8.
I'll look for another way.
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.
Python's os.path.realpath should work on both Linux and macOS.
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.
No~ It's Spark bash function in util.sh.
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.
Oh..
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.
Ah, Linux has realpath command (coreutils) but we have own realpath.
|
Test build #145947 has finished for PR 34818 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
| properties=( | ||
| ${properties[@]} | ||
| -Dspark.kubernetes.test.dockerFile=$( | ||
| python3 -c "import os.path; print(os.path.realpath(\"$DOCKER_FILE\"))") ) |
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.
@sarutak We should use realpath consistently in our Spark bash code.
It seems that the root cause was that
- You can a relative path via realpath in some directory
- And, we moved to another directory.
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 didn't notice realpath in util.sh and I used realpath command which is installed on Linux.
OK, I'll use our own realpath here.
|
Test build #145958 has finished for PR 34818 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #145963 has finished for PR 34818 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146007 has finished for PR 34818 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
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.
+1, LGTM. Thank you, @sarutak . It works as described.
|
@dongjoon-hyun Thank you for your advice and review ! |
What changes were proposed in this pull request?
This PR changes
dev-run-integration-tests.shto allow it to take a custom Dockerfile like #34790 did.With this change, this script accepts
--docker-fileoption, which takes a path to a custom Dockerfile.Why are the changes needed?
As of #34790, we can specify a custom Dockerfile by
spark.kubernetes.test.dockerFileproperty when we run the K8s integration tests using Maven.We can run the integration test via
dev-run-integration-tests.shbut there is no way to specify a custom Dockerfile.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Confirmed that the K8s integration tests run with the following command using
Dockerfile.java17.