Repository navigation
[#13616] feat(k8s-job-executor): Register the k8s job executor, ship it and run built-in jobs on it - #13892
Open
jerryshao wants to merge 3 commits into
Open
[#13616] feat(k8s-job-executor): Register the k8s job executor, ship it and run built-in jobs on it#13892jerryshao wants to merge 3 commits into
jerryshao wants to merge 3 commits into
Conversation
… ship it and run built-in jobs on it - Register `k8s` in `JobExecutorFactory` by class name, so that core doesn't depend on the plugin, and report a missing job executor jar clearly. - Copy the shaded jar of the job executor into `libs` of the distribution. - Add `shadedJarTest`, which loads the shaded jar on the classpath of the Gravitino server, creates the job executor through the factory and submits a job to a stub API server, to catch relocation and dependency conflicts. - Add `spark.builtinJobsJar`, the URI of the built-in jobs jar that the Spark pods can reach. It replaces the executable of a `builtin-` job template when that is a path on the Gravitino server. The stored templates don't change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Distribution assembly and shaded server-classpath compatibility need human confirmation of runtime validation results.
0 open findings
What changed in this PR
Enables Gravitino to load and ship the Kubernetes job executor and run built-in Spark jobs using a cluster-accessible jobs jar.
Changes:
- Registers
k8swith actionable class-loading errors and includes its shaded jar in distributions. - Adds configurable built-in jobs jar substitution and shared resource validation.
- Adds unit coverage and a server-classpath smoke test for the shaded plugin.
| File | Description |
|---|---|
| plugins/k8s-job-executor/src/test/java/org/apache/gravitino/job/k8s/TestK8sJobResourceUtils.java | Tests resource URI classification. |
| plugins/k8s-job-executor/src/test/java/org/apache/gravitino/job/k8s/TestK8sJobExecutorConfigs.java | Tests jobs jar configuration and validation. |
| plugins/k8s-job-executor/src/test/java/org/apache/gravitino/job/k8s/TestK8sJobExecutor.java | Checks factory registration constants. |
| plugins/k8s-job-executor/src/test/java/org/apache/gravitino/job/k8s/spark/TestSparkApplicationUtils.java | Tests built-in executable substitution boundaries. |
| plugins/k8s-job-executor/src/shadedJarTest/java/org/apache/gravitino/job/k8s/TestK8sJobExecutorShadedJar.java | Checks shading, class conflicts, and job submission. |
| plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/spark/SparkApplicationUtils.java | Substitutes cluster-accessible built-in executables. |
| plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sJobResourceUtils.java | Centralizes resource URI classification. |
| plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sJobExecutorConfigs.java | Adds the built-in jobs jar setting. |
| plugins/k8s-job-executor/build.gradle.kts | Configures shaded tests and distribution copying. |
| core/src/test/java/org/apache/gravitino/job/TestJobExecutorFactory.java | Tests missing executor class diagnostics. |
| core/src/main/java/org/apache/gravitino/job/JobExecutorFactory.java | Registers Kubernetes support and improves loading errors. |
| build.gradle.kts | Includes the plugin in distribution assembly. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Coverage Report
Files
|
jerryshao
commented
Oct 16, 2026
jerryshao
left a comment
jerryshao
left a comment
Contributor
Author
There was a problem hiding this comment.
Verdict: ship it
Reviewed 23bc020 (12 files, +516/-15) against origin/main (361b1e6), reading every changed file in full and grepping for callers of each moved or added symbol.
What I verified
- The registration path is complete end to end:
addJarInDir "${GRAVITINO_HOME}/libs"(bin/gravitino.sh.template:186, helper at bin/common.sh.template:98-102) putslibs/*on the server classpath;:plugins:k8s-job-executor:copyLibAndConfigsis in the singlecompileDistributionlist (build.gradle.kts:1189) and drops the shaded jar intodistribution/package/libs, matching the pattern of plugins/idp-basic/build.gradle.kts:70-84. - The
builtin-substitution cannot be triggered by a user template:JobTemplateValidationDispatcher.validateNotBuiltInTemplateName(core/src/main/java/org/apache/gravitino/job/JobTemplateValidationDispatcher.java:146-153) rejects user names with that prefix, and the dispatcher is wired in GravitinoEnv.java:1064. - The substitution covers the whole server-path surface of built-in jobs: every built-in template sets only
withExecutable(resolveExecutable(...));grep -rn "withJars|withFiles|withArchives" maintenance/jobs/src/mainreturns nothing, so no built-in template carries extra server-side resources that would still be rejected. - Moving the scheme check into
K8sJobResourceUtils.isClusterReachableis behaviour-preserving: the anchored^([A-Za-z][A-Za-z0-9+.-]*):withfind()behaves aslookingAt(), and the single-letter andfileexclusions are carried over verbatim from the oldcheckClusterReachable. - The constants the plugin pulls from
core(JobExecutorFactory.K8S_JOB_EXECUTOR_NAME,JobTemplateProvider.BUILTIN_NAME_PREFIX) are compile-timeStringconstants, so they inline and add no runtime dependency oncorefrom the shaded jar. - The new
shadedJarTesttask is picked up by the roottasks.configureEach<Test>(build.gradle.kts:981), so it inheritsuseJUnitPlatform(); itsCopyinto the distribution picks updependsOn(cleanDistributionPackage)from build.gradle.kts:1631-1637. - No legal change is needed: LICENSE.bin already lists Jackson (line 350), SnakeYAML / SnakeYAML Engine (359-360) and Fabric8 Kubernetes Client / Model / zjsonpatch (411-413). No other module declares fabric8 (
grep -rn fabric8 --include=*.kts), so the relocation prefix has no cross-module conflict.
Tests: adequate. The new unit tests cover the default and overridden spark.builtinJobsJar, the three invalid forms, both substituted and kept executables, a my-builtin-job name that must not match the prefix, and a built-in job whose extra jars are still rejected; the shadedJarTest smoke test is the valuable addition, since it exercises relocation, class overlap and a real submission through JobExecutorFactory. I did not find a branch introduced here that is unexercised.
I could not execute the tests in this sandbox: Gradle failed to resolve its plugin dependencies (HTTP 429 from repo.maven.apache.org), so every finding below comes from reading the code, not from a run. The findings are all non-blocking.
Review drafted with Claude, requested by @jerryshao.
jerryshao
commented
Oct 16, 2026
jerryshao
left a comment
jerryshao
left a comment
Contributor
Author
There was a problem hiding this comment.
Verdict: ship it
Reviewed 23bc020 (12 files, +516/-15) against origin/main (361b1e6), reading every changed file in full and grepping for callers of each moved or added symbol.
What I verified
- The registration path is complete end to end:
addJarInDir "${GRAVITINO_HOME}/libs"(bin/gravitino.sh.template:186, helper at bin/common.sh.template:98-102) putslibs/*on the server classpath;:plugins:k8s-job-executor:copyLibAndConfigsis in the singlecompileDistributionlist (build.gradle.kts:1189) and drops the shaded jar intodistribution/package/libs, matching the pattern of plugins/idp-basic/build.gradle.kts:70-84. - The
builtin-substitution cannot be triggered by a user template:JobTemplateValidationDispatcher.validateNotBuiltInTemplateName(core/src/main/java/org/apache/gravitino/job/JobTemplateValidationDispatcher.java:146-153) rejects user names with that prefix, and the dispatcher is wired in GravitinoEnv.java:1064. - The substitution covers the whole server-path surface of built-in jobs: every built-in template sets only
withExecutable(resolveExecutable(...));grep -rn "withJars|withFiles|withArchives" maintenance/jobs/src/mainreturns nothing, so no built-in template carries extra server-side resources that would still be rejected. - Moving the scheme check into
K8sJobResourceUtils.isClusterReachableis behaviour-preserving: the anchored^([A-Za-z][A-Za-z0-9+.-]*):withfind()behaves aslookingAt(), and the single-letter andfileexclusions are carried over verbatim from the oldcheckClusterReachable. - The constants the plugin pulls from
core(JobExecutorFactory.K8S_JOB_EXECUTOR_NAME,JobTemplateProvider.BUILTIN_NAME_PREFIX) are compile-timeStringconstants, so they inline and add no runtime dependency oncorefrom the shaded jar. - The new
shadedJarTesttask is picked up by the roottasks.configureEach<Test>(build.gradle.kts:981), so it inheritsuseJUnitPlatform(); itsCopyinto the distribution picks updependsOn(cleanDistributionPackage)from build.gradle.kts:1631-1637. - No legal change is needed: LICENSE.bin already lists Jackson (line 350), SnakeYAML / SnakeYAML Engine (359-360) and Fabric8 Kubernetes Client / Model / zjsonpatch (411-413). No other module declares fabric8 (
grep -rn fabric8 --include=*.kts), so the relocation prefix has no cross-module conflict.
Tests: adequate. The new unit tests cover the default and overridden spark.builtinJobsJar, the three invalid forms, both substituted and kept executables, a my-builtin-job name that must not match the prefix, and a built-in job whose extra jars are still rejected; the shadedJarTest smoke test is the valuable addition, since it exercises relocation, class overlap and a real submission through JobExecutorFactory. I did not find a branch introduced here that is unexercised.
I could not execute the tests in this sandbox: Gradle failed to resolve its plugin dependencies (HTTP 429 from repo.maven.apache.org), so every finding below comes from reading the code, not from a run. The findings are all non-blocking.
Review drafted with Claude, requested by @jerryshao.
- Log the jobs jar that a built-in job runs instead of the template's executable, which is what the job records, and the configured `spark.builtinJobsJar` when the executor starts. - Warn when `gravitino.jobExecutor.<name>.class` is set for a built-in job executor name, as it is ignored, and say in the docs that `local` and `k8s` are reserved. - Say which classpath the overlap check of the shaded jar test covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The
k8sjob executor added in #13613 can now be used by Gravitino, and runs the built-in Spark jobs.JobExecutorFactoryregistersk8sby class name, as the executor is a plugin that depends oncore. The name and class name are public constants ofJobExecutorFactory, which the plugin uses, and a plugin test checks the class name againstK8sJobExecutor. If the class can't be loaded, the error names the missinggravitino-k8s-job-executor-<version>.jarand thelibsdirectory.libsof the distribution package.LICENSE.binalready lists everything it bundles (fabric8, zjsonpatch, Jackson, SnakeYAML, SnakeYAML Engine).shadedJarTestsource set and task, part ofcheck, puts the shaded jar on the runtime classpath of:server(with its Jackson 2.15.2), without the module's classes or unshaded dependencies. It checks that Jackson and SnakeYAML are relocated, that the jar bundles no class that is also on the server classpath, and creates the executor throughJobExecutorFactoryto submit a built-in job to a stub API server. It only needsshadowJar, not a distribution build.spark.builtinJobsJaris the URI of the jobs jar that the driver and executors can reach,local:///opt/gravitino/jobs/gravitino-jobs.jarby default: a file in the Spark image or in a mounted volume, orhttps://,s3a://and so on. A template whose name starts withbuiltin-and whose executable is a path on the server runs that jar instead. An executable that the cluster can already reach is kept. The stored templates don't change, and thelocalexecutor keeps using the jar on the server.K8sJobResourceUtils.isClusterReachable, so that the resource check, thespark.builtinJobsJarvalidation and the substitution share it.Why are the changes needed?
Without this,
gravitino.job.executor = k8sfails to load the executor, and the built-in jobs can't run on it.Fix: #13616
Does this PR introduce any user-facing change?
Yes.
gravitino.job.executor = k8sselects thek8sjob executor, configured with thegravitino.jobExecutor.k8s.keys added in [Subtask] Add the k8s job executor to run Spark jobs on Kubernetes #13613.gravitino.jobExecutor.k8s.spark.builtinJobsJar, defaultlocal:///opt/gravitino/jobs/gravitino-jobs.jar.libs/gravitino-k8s-job-executor-<version>.jar.The
gravitino-sparkimage that holds the jobs jar at the default path, and the docs, come with #13618.How was this patch tested?
shadedJarTestsmoke test.http://link throughspark.builtinJobsJar;gravitino.jobExecutor.k8s.spark.conf.spark.jarsorspark.conf.spark.jars.packages(withspark.jars.ivy). Passingspark.jarsin the job'sspark_confargument doesn't work, which [Subtask] Let a job run set submit-time Spark configurations of the built-in job templates #13703 addresses.🤖 Generated with Claude Code