[SPARK-35501][SQL][TESTS] Add a feature for removing pulled container image for docker integration tests #32652

sarutak · 2021-05-24T12:53:32Z

What changes were proposed in this pull request?

This PR adds a feature for removing pulled container image after every docker integration test finish.
This feature is enabled by the new propoerty spark.tes.docker.removePulledImage.

Why are the changes needed?

For idempotent.
I'm trying to add docker integration tests to GA in SPARK-35483 (#32631) but I noticed that jdbc.OracleIntegrationSuite consistently fails(https://github.com/sarutak/spark/runs/2646707235?check_suite_focus=true).
I investigated the reason and I found it's short of the storage capacity of the host on GA.

 ORACLE PASSWORD FOR SYS AND SYSTEM: oracle
The location '/opt/oracle' specified for database files has insufficient space.
Database creation needs at least '4.5GB' disk space.
Specify a different database file destination that has enough space in the configuration file '/etc/sysconfig/oracle-xe-18c.conf'.
mv: cannot stat '/opt/oracle/product/18c/dbhomeXE/dbs/spfileXE.ora': No such file or directory
mv: cannot stat '/opt/oracle/product/18c/dbhomeXE/dbs/orapwXE': No such file or directory
ORACLE_HOME = [/home/oracle] ? ORACLE_BASE environment variable is not being set since this
information is not available for the current user ID .
You can set ORACLE_BASE manually if it is required.
Resetting ORACLE_BASE to its previous value or ORACLE_HOME
The Oracle base remains unchanged with value /opt/oracle
#####################################
########### E R R O R ###############
DATABASE SETUP WAS NOT SUCCESSFUL!
Please check output for further info!
########### E R R O R ###############
#####################################
The following output is now a tail of the alert.log:
tail: cannot open '/opt/oracle/diag/rdbms/*/*/trace/alert*.log' for reading: No such file or directory
tail: no files remaining

With this feature, pulled container image is removed and keep the capacity for jdbc.OracleIntegrationSuite in GA.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

I confirmed the following things.

A container image which is absent in the local repository is removed after test finished if spark.test.container.removePulledImage is true.
A container image which is present in the local repository is not removed after the finished even if spark.test.container.removePulledImage is true.
A container image is not removed regardless of presence of the container image in the local repository even if spark.test.container.removePulledImage is true.

SparkQA · 2021-05-24T14:12:03Z

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

SparkQA · 2021-05-24T14:47:10Z

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

SparkQA · 2021-05-24T15:51:58Z

Test build #138872 has finished for PR 32652 at commit 60ea4fd.

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

sarutak · 2021-05-26T01:26:57Z

cc: @dongjoon-hyun @HyukjinKwon

HyukjinKwon · 2021-05-26T01:33:00Z

pom.xml

    <spark.test.home>${session.executionRootDirectory}</spark.test.home>
    <spark.test.webdriver.chrome.driver></spark.test.webdriver.chrome.driver>
    <spark.test.docker.keepContainer>false</spark.test.docker.keepContainer>
+    <spark.test.docker.removePulledImage>false</spark.test.docker.removePulledImage>


I think we should maybe just enable it by default. but I will leave it to you @sarutak and @dongjoon-hyun

I disabled it by default just for behavior compatibility (pulled images are not removed before this change).
But it may be no problem even if we enable it.

…image-rm

dongjoon-hyun · 2021-05-26T02:02:11Z

Thank you for pinging me, @sarutak .

SparkQA · 2021-05-26T03:16:05Z

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

SparkQA · 2021-05-26T03:49:04Z

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

SparkQA · 2021-05-26T04:13:33Z

Test build #138951 has finished for PR 32652 at commit da66bb1.

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

HyukjinKwon · 2021-05-26T08:24:24Z

Merged to master.

dongjoon-hyun · 2021-05-29T03:38:59Z

...-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/DockerJDBCIntegrationSuite.scala

  val keepContainer =
    sys.props.getOrElse("spark.test.docker.keepContainer", "false").toBoolean
+  val removePulledImage =
+    sys.props.getOrElse("spark.test.docker.removePulledImage", "true").toBoolean


If this causes image downloading every time, this is a regression for the non-GA users, isn't it?
I'd keep the default as false and enable this true only at GitHub Action environment if that is trying to resolve GA issue.

I investigated the reason and I found it's short of the storage capacity of the host on GA.

WDYT, @sarutak and @HyukjinKwon ?

To avoid behavior change, it would be better to keep it false but keeping container images in the local repository is side effect and done implicitly before this change. So I thought it is better do keep the images only if users can explicitly set the property to false.
Any thoughts? @dongjoon-hyun @HyukjinKwon

Got it. If you think so, never mind. Since it's already merged, let's leave this AS-IS, @sarutak .

I am okay either way. Please feel free to change if you guys have a preference 👍

dongjoon-hyun · 2021-05-29T03:39:45Z

pom.xml

    <spark.test.home>${session.executionRootDirectory}</spark.test.home>
    <spark.test.webdriver.chrome.driver></spark.test.webdriver.chrome.driver>
    <spark.test.docker.keepContainer>false</spark.test.docker.keepContainer>
+    <spark.test.docker.removePulledImage>true</spark.test.docker.removePulledImage>


sarutak added 2 commits May 24, 2021 19:44

Add pulled image removal feature.

7377ed4

Add the property to pom.xml.

60ea4fd

github-actions bot added BUILD SQL labels May 24, 2021

HyukjinKwon reviewed May 26, 2021

View reviewed changes

HyukjinKwon approved these changes May 26, 2021

View reviewed changes

sarutak added 2 commits May 26, 2021 10:40

Merge branch 'master' of https://github.com/apache/spark into docker-…

1594657

…image-rm

Enabled the property by default.

da66bb1

HyukjinKwon closed this in 116a97e May 26, 2021

dongjoon-hyun reviewed May 29, 2021

View reviewed changes

[SPARK-35501][SQL][TESTS] Add a feature for removing pulled container image for docker integration tests #32652

[SPARK-35501][SQL][TESTS] Add a feature for removing pulled container image for docker integration tests #32652

Conversation

sarutak commented May 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented May 24, 2021

Uh oh!

SparkQA commented May 24, 2021

Uh oh!

SparkQA commented May 24, 2021

Uh oh!

sarutak commented May 26, 2021

Uh oh!

HyukjinKwon May 26, 2021

Choose a reason for hiding this comment

Uh oh!

sarutak May 26, 2021

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented May 26, 2021

Uh oh!

SparkQA commented May 26, 2021

Uh oh!

SparkQA commented May 26, 2021

Uh oh!

SparkQA commented May 26, 2021

Uh oh!

HyukjinKwon commented May 26, 2021

Uh oh!

dongjoon-hyun May 29, 2021 • 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!

sarutak May 29, 2021

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun May 30, 2021

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon May 31, 2021

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun May 29, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sarutak commented May 24, 2021 •

edited

Loading

dongjoon-hyun May 29, 2021 •

edited

Loading