Skip to content

Commit

Permalink
[SPARK-49518][K8S][BUILD] Change to using build-helper-maven-plugin
Browse files Browse the repository at this point in the history
… to manage the code for `volcano`

### What changes were proposed in this pull request?
The main changes in this pr are as follows:

1. In `resource-managers/kubernetes/core/pom.xml` and `resource-managers/kubernetes/integration-tests/pom.xml`, the `build-helper-maven-plugin` configuration has been added for the `volcano` profile to ensure that when the profile is activated with `-Pvolcano`, the `volcano/src/main/scala` directory is treated as an additional source path, and `volcano/src/test/scala` directory is treated as an additional test code path.

- `resource-managers/kubernetes/core/pom.xml`

```xml
      <build>
        <plugins>
          <plugin>
            <groupId>org.codehaus.mojo</groupId>
            <artifactId>build-helper-maven-plugin</artifactId>
            <executions>
              <execution>
                <id>add-volcano-source</id>
                <phase>generate-sources</phase>
                <goals>
                  <goal>add-source</goal>
                </goals>
                <configuration>
                  <sources>
                    <source>volcano/src/main/scala</source>
                  </sources>
                </configuration>
              </execution>
              <execution>
                <id>add-volcano-test-sources</id>
                <phase>generate-test-sources</phase>
                <goals>
                  <goal>add-test-source</goal>
                </goals>
                <configuration>
                  <sources>
                    <source>volcano/src/test/scala</source>
                  </sources>
                </configuration>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
```

- `resource-managers/kubernetes/integration-tests/pom.xml`

```xml
      <build>
        <plugins>
          <plugin>
            <groupId>org.codehaus.mojo</groupId>
            <artifactId>build-helper-maven-plugin</artifactId>
            <executions>
              <execution>
                <id>add-volcano-test-sources</id>
                <phase>generate-test-sources</phase>
                <goals>
                  <goal>add-test-source</goal>
                </goals>
                <configuration>
                  <sources>
                    <source>volcano/src/test/scala</source>
                  </sources>
                </configuration>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
```

2. Removed the management configuration for `volcano`-related source/test code in SPARK-36061 | apache#35422.

Since Spark uses the `sbt-pom-reader` plugin in its sbt configuration, the behavior of the `build-helper-maven-plugin` will also propagate to the sbt build process. Therefore, no additional configuration is required in `SparkBuild.scala` after this pr.

### Why are the changes needed?
The previous configuration way was not very friendly to IntelliJ developers: when debugging code in IntelliJ, regardless of whether they were needed or not, the `volcano` profile had to be activated; otherwise compilation errors would occur when running tests that depended on the `kubernetes` module's source code, for example `org.apache.spark.shuffle.ShuffleChecksumUtilsSuite` :

<img width="1465" alt="image" src="https://github.com/user-attachments/assets/e16e3eba-d85e-45ad-bbae-533bd2f8ce0b">

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
1. Pass GitHub Actions

- https://github.com/LuciferYang/spark/actions/runs/10714343021/job/29707907464

<img width="1109" alt="image" src="https://github.com/user-attachments/assets/d893accb-508f-47f5-b19e-e178f6eff128">

- https://github.com/LuciferYang/spark/actions/runs/10714343021/job/29707906573

<img width="1183" alt="image" src="https://github.com/user-attachments/assets/735e0dc7-7d2c-418f-8fcd-200ee10eda0d">

It can be seen that the test cases `VolcanoFeatureStepSuite ` and `VolcanoSuite` have been successfully executed.

2. Manual Testing Using sbt

- Run `build/sbt clean "kubernetes/testOnly *VolcanoFeatureStepSuite" -Pkubernetes`, and without `-Pvolcano`, no tests will be executed:

```
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for kubernetes / Test / testOnly
```

- Run `build/sbt clean "kubernetes/testOnly *VolcanoFeatureStepSuite" -Pkubernetes -Pvolcano`, and with `-Pvolcano`, `VolcanoFeatureStepSuite` will pass the tests:

```
[info] VolcanoFeatureStepSuite:
[info] - SPARK-36061: Driver Pod with Volcano PodGroup (74 milliseconds)
[info] - SPARK-36061: Executor Pod with Volcano PodGroup (8 milliseconds)
[info] - SPARK-38455: Support driver podgroup template (224 milliseconds)
[info] - SPARK-38503: return empty for executor pre resource (1 millisecond)
[info] Run completed in 1 second, 268 milliseconds.
[info] Total number of tests run: 4
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 4, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```

- run `build/sbt clean  "kubernetes/package" -Pkubernetes -Pvolcano`, and with `-Pvolcano`, confirm that `spark-kubernetes_2.13-4.0.0-SNAPSHOT.jar` contains `VolcanoFeatureStep.class`

- run `build/sbt clean  "kubernetes/package" -Pkubernetes`, and without `-Pvolcano`, confirm that `spark-kubernetes_2.13-4.0.0-SNAPSHOT.jar` not contains `VolcanoFeatureStep.class`

3. Manual Testing Using Maven

- Run `build/mvn clean test -pl resource-managers/kubernetes/core -am -Dtest=none -DwildcardSuites=org.apache.spark.deploy.k8s.features.VolcanoFeatureStepSuite -Pkubernetes`, and without `-Pvolcano`, no tests will be executed:

```
Discovery starting.
Discovery completed in 80 milliseconds.
Run starting. Expected test count is: 0
DiscoverySuite:
Run completed in 99 milliseconds.
Total number of tests run: 0
Suites: completed 1, aborted 0
Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
No tests were executed.
```

- Run `build/mvn clean test -pl resource-managers/kubernetes/core -am -Dtest=none -DwildcardSuites=org.apache.spark.deploy.k8s.features.VolcanoFeatureStepSuite -Pkubernetes -Pvolcano`, and with `-Pvolcano`, `VolcanoFeatureStepSuite` will pass the tests:

```
Discovery starting.
Discovery completed in 263 milliseconds.
Run starting. Expected test count is: 4
VolcanoFeatureStepSuite:
- SPARK-36061: Driver Pod with Volcano PodGroup
- SPARK-36061: Executor Pod with Volcano PodGroup
- SPARK-38455: Support driver podgroup template
- SPARK-38503: return empty for executor pre resource
Run completed in 624 milliseconds.
Total number of tests run: 4
Suites: completed 2, aborted 0
Tests: succeeded 4, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
```

- run `build/mvn clean package -pl resource-managers/kubernetes/core -am -DskipTests -Pkubernetes -Pvolcano` and with `-Pvolcano`, confirm that `spark-kubernetes_2.13-4.0.0-SNAPSHOT.jar` contains `VolcanoFeatureStep.class`

- run `build/mvn clean package -pl resource-managers/kubernetes/core -am -DskipTests -Pkubernetes` and without `-Pvolcano`, confirm that `spark-kubernetes_2.13-4.0.0-SNAPSHOT.jar` not contains `VolcanoFeatureStep.class`

4. Testing in IntelliJ (both imported as a Maven project and as an sbt project):
- By default, do not activate `volcano`, and confirm that `volcano`-related code is not recognized as source/test code, and does not affect the compilation and testing of other code.
- Manually activate `volcano`, and confirm that `volcano`-related code is recognized as source/test code, and can be compiled and tested normally.

5. Similar tests were conducted on `kubernetes-integration-tests` module to confirm the validity of the `volcano` profile.

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#47997 from LuciferYang/refactor-volcano.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
  • Loading branch information
LuciferYang authored and dongjoon-hyun committed Sep 5, 2024
1 parent f7292cb commit 2e2715d
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 46 deletions.
12 changes: 0 additions & 12 deletions project/SparkBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,6 @@ object SparkBuild extends PomBuild {

enable(DockerIntegrationTests.settings)(dockerIntegrationTests)

if (!profiles.contains("volcano")) {
enable(Volcano.settings)(kubernetes)
enable(Volcano.settings)(kubernetesIntegrationTests)
}

enable(KubernetesIntegrationTests.settings)(kubernetesIntegrationTests)

enable(YARN.settings)(yarn)
Expand Down Expand Up @@ -1322,13 +1317,6 @@ object SparkR {
)
}

object Volcano {
// Exclude all volcano file for Compile and Test
lazy val settings = Seq(
unmanagedSources / excludeFilter := HiddenFileFilter || "*Volcano*.scala"
)
}

trait SharedUnidocSettings {

import BuildCommons._
Expand Down
51 changes: 34 additions & 17 deletions resource-managers/kubernetes/core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@
<name>Spark Project Kubernetes</name>
<properties>
<sbt.project.name>kubernetes</sbt.project.name>
<volcano.exclude>**/*Volcano*.scala</volcano.exclude>
</properties>

<profiles>
<profile>
<id>volcano</id>
<properties>
<volcano.exclude></volcano.exclude>
</properties>
<dependencies>
<dependency>
<groupId>io.fabric8</groupId>
Expand All @@ -50,6 +46,40 @@
<version>${kubernetes-client.version}</version>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<executions>
<execution>
<id>add-volcano-source</id>
<phase>generate-sources</phase>
<goals>
<goal>add-source</goal>
</goals>
<configuration>
<sources>
<source>volcano/src/main/scala</source>
</sources>
</configuration>
</execution>
<execution>
<id>add-volcano-test-sources</id>
<phase>generate-test-sources</phase>
<goals>
<goal>add-test-source</goal>
</goals>
<configuration>
<sources>
<source>volcano/src/test/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>

Expand Down Expand Up @@ -151,19 +181,6 @@


<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>net.alchim31.maven</groupId>
<artifactId>scala-maven-plugin</artifactId>
<configuration>
<excludes>
<exclude>${volcano.exclude}</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
</pluginManagement>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
<testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
</build>
Expand Down
39 changes: 22 additions & 17 deletions resource-managers/kubernetes/integration-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
<test.exclude.tags></test.exclude.tags>
<test.default.exclude.tags>org.apache.spark.deploy.k8s.integrationtest.YuniKornTag</test.default.exclude.tags>
<test.include.tags></test.include.tags>
<volcano.exclude>**/*Volcano*.scala</volcano.exclude>
</properties>
<packaging>jar</packaging>
<name>Spark Project Kubernetes Integration Tests</name>
Expand Down Expand Up @@ -83,19 +82,6 @@
</dependencies>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>net.alchim31.maven</groupId>
<artifactId>scala-maven-plugin</artifactId>
<configuration>
<excludes>
<exclude>${volcano.exclude}</exclude>
</excludes>
</configuration>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
Expand Down Expand Up @@ -219,16 +205,35 @@
</profile>
<profile>
<id>volcano</id>
<properties>
<volcano.exclude></volcano.exclude>
</properties>
<dependencies>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>volcano-client</artifactId>
<version>${kubernetes-client.version}</version>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<executions>
<execution>
<id>add-volcano-test-sources</id>
<phase>generate-test-sources</phase>
<goals>
<goal>add-test-source</goal>
</goals>
<configuration>
<sources>
<source>volcano/src/test/scala</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>

0 comments on commit 2e2715d

Please sign in to comment.