Skip to content

fix: declare spec.sparkVersion on SparkApplications that carry a pod template - #7850

Merged
pingsutw merged 8 commits into
mainfrom
fix/spark-version-pod-template
Aug 14, 2026
Merged

fix: declare spec.sparkVersion on SparkApplications that carry a pod template#7850
pingsutw merged 8 commits into
mainfrom
fix/spark-version-pod-template

Conversation

@pingsutw

@pingsutw pingsutw commented Aug 13, 2026

Copy link
Copy Markdown
Member

Tracking issue

Related to #7819

Why are the changes needed?

On a kubeflow v2 spark-operator, every Spark task fails admission as soon as pod templates are in play:

admission webhook "validate-sparkoperator-k8s-io-v1beta2-sparkapplication.sparkoperator.k8s.io"
denied the request: pod template feature requires Spark version 3.0.0 or higher

The check is on the declared field, not the image:

// internal/webhook/sparkapplication_validator.go:179-186
func (v *SparkApplicationValidator) validateSparkVersion(app *v1beta2.SparkApplication) error {
	if app.Spec.Driver.Template != nil || app.Spec.Executor.Template != nil {
		if util.CompareSemanticVersion(app.Spec.SparkVersion, "3.0.0") < 0 {
			return fmt.Errorf("pod template feature requires Spark version 3.0.0 or higher")
		}
	}
	return nil
}

This plugin never set spec.sparkVersion, and an unset value loses that comparison — CompareSemanticVersion prefixes with v and hands "v" to semver.Compare, where an invalid version sorts below every valid one. Verified against the operator's own function:

CompareSemanticVersion("",      "3.0.0") = -1   ← what the plugin submits today
CompareSemanticVersion("3.5.1", "3.0.0") =  1
CompareSemanticVersion("2.4.5", "3.0.0") = -1

So the denial is unconditional once enable-pod-template is on (the default) and the CRD supports templates — no image change can fix it, because no image content is consulted. Legacy GoogleCloudPlatform operators have no such webhook, which is why this only surfaces on clusters moved to the kubeflow 2.x operator.

What changes were proposed in this pull request?

Set spec.sparkVersion to 3.0.0 only when a driver or executor template is actually attached — the one case the operator reads it for. Applications without a template keep today's wire shape exactly.

Two deliberate choices worth reviewing:

Why gated on the template rather than always set. The field is a declaration about the image, which the plugin cannot inspect. Setting it unconditionally would put an unverifiable claim on every SparkApplication, including ones going to operators that never look at it.

Why a constant and not a config knob. The first revision made this configurable; that knob is gone, because it had no reachable effect. The operator compares against the 3.0.0 boundary and reads nothing else from the field — every Spark 3.x value behaves identically. The value is still load-bearing in a second place:

// internal/controller/sparkapplication/submission.go:335
if util.CompareSemanticVersion(app.Spec.SparkVersion, "3.0.0") < 0 || app.Spec.Driver.Template == nil {
	// label the pod for mutation by the operator's webhook
}
// otherwise (submission.go:1136-1143) the template is written to a file and handed to
// spark-submit via spark.kubernetes.driver.podTemplateFile — i.e. Spark applies it

Declaring >= 3.0.0 tells the operator to step back and let spark-submit apply the template. That is correct for any Spark 3.x image and wrong only for a Spark 2.x one, where the conf is ignored and the pod would come up unconfigured — and that case is already served by the existing enable-pod-template: false kill switch, which is the honest lever for it. A lower version string would not help there: it would keep the template on the object and simply move the failure.

How was this patch tested?

Extended TestBuildResourcePodTemplateGating to assert the field is set alongside a template and stays empty without one, and that the object is otherwise byte-identical to the no-template shape.

Added TestDefaultSparkVersionSatisfiesOperator, which pins the declared version against the operator's own CompareSemanticVersion rather than against the literal "3.0.0" — so if kubeflow raises the floor, this fails instead of silently shipping a value that gets denied. It also asserts the empty-string case still loses, documenting the bug being fixed.

$ go test ./flyteplugins/go/tasks/plugins/k8s/spark/... -count=1
ok  	github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/plugins/k8s/spark	0.657s

Mutation-checked: with the assignment disabled, TestBuildResourcePodTemplateGating fails.

Labels

fixed

Check all the applicable boxes

  • I updated the documentation accordingly. (n/a — no new config surface)
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

https://claude.ai/code/session_01UCd3Y6KWUMQRu7bRF5RMZU

…istry

Every other k8s plugin registers its CRD types twice: into client-go's
global scheme.Scheme, and into PluginRegistry() via RegisterScheme. The
clustered plugin only ever did the former, so hosts that build their
scheme from GetSchemeRegisters -- which is what executor/setup.go does,
and what #7819 pushed plugins toward -- never learn about JobSet and fail
at Create with:

  no kind is registered for the type v1alpha2.JobSet in scheme

Add the missing RegisterScheme. The global registration stays: unlike
spark in #7819, JobSet has no competing Go type for its GVK, so it cannot
collide, and downstream binaries still read it.

Claude-Session: https://claude.ai/code/session_01UCd3Y6KWUMQRu7bRF5RMZU
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
check-generate regenerates gen/ts/package-lock.json with the newest
version matching the ^2.10.0 range, so the committed lock goes stale
whenever upstream publishes.

Claude-Session: https://claude.ai/code/session_01XUKAsR2AzZbGBLudAuafg6
Signed-off-by: Kevin Su <pingsutw@apache.org>
…template

The kubeflow operator's validating webhook denies any SparkApplication
that sets a driver or executor pod template while declaring a Spark
version below 3.0.0:

  admission webhook "validate-sparkoperator-k8s-io-v1beta2-sparkapplication...
  denied the request: pod template feature requires Spark version 3.0.0 or higher

The plugin never set spec.sparkVersion, and an unset value loses that
comparison -- CompareSemanticVersion prefixes with "v" and hands "v" to
semver.Compare, where an invalid version sorts below every valid one. So
every spark task fails admission on a v2 operator once pod templates are
enabled, regardless of what Spark the image actually ships.

Declare the version, from a new plugins.spark.spark-version config, only
when a template is attached -- the single case the operator reads it for.
The default is the 3.0.0 floor the feature requires rather than a guess
at the image contents: the operator compares only against that boundary,
and declaring >= 3.0.0 is what tells it to let spark-submit apply the
template instead of mutating the pod itself.

Claude-Session: https://claude.ai/code/session_01UCd3Y6KWUMQRu7bRF5RMZU
Signed-off-by: Kevin Su <pingsutw@apache.org>
Copilot AI lite review requested due to automatic review settings August 13, 2026 22:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the Flyte Spark and clustered k8s plugins to improve compatibility with newer operators/host binaries by (1) declaring spec.sparkVersion on SparkApplications only when pod templates are present, and (2) ensuring the clustered plugin’s CRD types are discoverable via the plugin registry scheme registration mechanism.

Changes:

  • Spark: set SparkApplication.spec.sparkVersion when a driver/executor pod template is attached, driven by new plugins.spark.spark-version config (defaulting to 3.0.0).
  • Spark: extend unit tests to assert the gating behavior and pin the default spark version against the operator’s own semantic-version comparison behavior.
  • Clustered: register JobSet types with PluginRegistry().RegisterScheme(...) and add a unit test verifying scheme registration via the registry path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Show a summary per file
File Description
gen/ts/package-lock.json Bumps @bufbuild/protobuf lockfile entry to 2.14.0.
flyteplugins/go/tasks/plugins/k8s/spark/spark.go Conditionally declares spec.sparkVersion when pod templates are present.
flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go Adds assertions for sparkVersion/template gating and validates the default version behavior.
flyteplugins/go/tasks/plugins/k8s/spark/podtemplate.go Introduces the minPodTemplateSparkVersion constant used as the default spark-version.
flyteplugins/go/tasks/plugins/k8s/spark/config.go Adds SparkVersion config with default minPodTemplateSparkVersion.
flyteplugins/go/tasks/plugins/k8s/clustered/plugin.go Registers JobSet AddToScheme with the plugin registry.
flyteplugins/go/tasks/plugins/k8s/clustered/clustered_test.go Adds a test verifying JobSet kinds are reachable through GetSchemeRegisters().

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings August 13, 2026 22:39
The operator compares spec.sparkVersion against 3.0.0 and reads nothing
else from it, so a configurable value has no reachable effect: any Spark
3.x setting behaves identically, and a Spark 2.x image needs
enable-pod-template=false rather than a lower version string.

Declare minPodTemplateSparkVersion directly and delete the knob.

Claude-Session: https://claude.ai/code/session_01UCd3Y6KWUMQRu7bRF5RMZU
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 13, 2026 22:41
Signed-off-by: Kevin Su <pingsutw@apache.org>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

flyteplugins/go/tasks/plugins/k8s/spark/spark.go:333

  • Comment grammar is a bit awkward/misleading: “Left empty an application…” reads like an imperative rather than a conditional; adding a comma and rephrasing makes it clearer that an unset sparkVersion triggers admission denial when templates are present.
	// Left empty an application with a template is denied at admission, because an unset
	// version parses as invalid semver and sorts below every real one.

@pingsutw
pingsutw merged commit 54f08d1 into main Aug 14, 2026
23 checks passed
@pingsutw
pingsutw deleted the fix/spark-version-pod-template branch August 14, 2026 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants