[spark] - #7744
Draft
wild-endeavor wants to merge 1 commit into
Draft
Conversation
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates core dependencies (notably Viper and the Spark Operator client) and adapts Flyte’s configuration parsing and Spark K8s plugin implementation to maintain compatibility with the newer libraries.
Changes:
- Upgrade
github.com/spf13/vipertov1.21.0and migratemapstructureimports togithub.com/go-viper/mapstructure/v2. - Migrate the Spark plugin from
GoogleCloudPlatform/spark-on-k8s-operatortokubeflow/spark-operator/v2, updating API types and status handling. - Add SparkApplication CRD capability probing to conditionally pass full pod templates when supported.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| runs/config/config_flags_test.go | Updates generated config decode tests to use github.com/go-viper/mapstructure/v2. |
| go.mod | Removes old Spark operator + mitchellh/mapstructure, upgrades Viper, adds new Spark operator + direct deps for new code. |
| go.sum | Reflects dependency updates/removals from the Viper/Spark operator migration. |
| flytestdlib/config/viper/viper.go | Adjusts map decoding hooks and adds a workaround to restore case-sensitive keys in arrays after Viper upgrade. |
| flyteplugins/go/tasks/plugins/k8s/spark/spark.go | Migrates SparkApplication construction and status handling to kubeflow spark-operator v2 APIs; adds pod-template plumbing. |
| flyteplugins/go/tasks/plugins/k8s/spark/spark_test.go | Updates tests for new Spark operator types/fields and adds pod-template gating coverage. |
| flyteplugins/go/tasks/plugins/k8s/spark/podtemplate.go | Adds CRD schema probing (once-per-process) to detect driver template support. |
| flyteplugins/go/tasks/plugins/k8s/spark/config.go | Adds EnablePodTemplate config flag and enables it by default. |
Files not reviewed (1)
- runs/config/config_flags_test.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
First step in spark upgrade: Swap the spark plugin's client library from the 2020 era
GoogleCloudPlatform/spark-on-k8s-operatortokubeflow/spark-operator/v2 v2.4.0, without requiring any cluster's operator or CRDs to be upgraded first.How it works
On startup, first test to see if the
SparkApplicationCRD has a driver pod field calledtemplate. The old one does not, and the new one does.Then keep writing every
v1beta2field it writes today. If the CRD detected is old (i.e. doesn't have thetemplatefield), the serialized objects are nearly byte-identical to before (see the breaking section below). If the CRD does have that field, then the plugin additionally passes the full flyte-built pod spec through as the driver/executor pod template.On clusters with the old CRDs, the k8s api server just prunes the template field away. A manual kill switch is also provided:
plugins.spark.enable-pod-template: falseWhat's breaking
One field: the pod-level security context. The old struct serialized it as
securityContext; the new one ispodSecurityContext, which old CRDs prune. So on a cluster with old CRDs, a security context coming from platform config or pod templates silently stops reaching spark pods. It's used again (via the newtemplatefield) once that cluster's CRDs & operator are upgraded.Non-breaking changes
The new API moved/removed two fields we set:
DriverSpec.ServiceAccountmoved into the embeddedSparkPodSpecset insidesparkOp.DriverSpec.spec.serviceAccountis gone. We checked the operator version actually deployed with the 1.1.x charts (v1beta2-1.3.8): it never reads that key — the driver SA comes fromspec.driver.serviceAccount, which we still send. Dead field, safe to drop.Side-note from that dig: the old client never set an executor service account (the field didn't exist in its API), so executors have always run as the namespace default SA. On template-capable clusters we now set the task SA on executors too.
Viper
The kubeflow module forces viper 1.11 → 1.21. Two things break and here's how they're handled (straight port of #7013 from master):
go-viper/mapstructure/v2, so flytestdlib's viper wrapper swaps to that import (drop-in for our usage).spark-config-defaultdepends on it). The wrapper now re-reads the config files after viper parses them and restores the original casing for keys inside arrays.Testing
Unit tests cover both gate positions, including an assertion that the with-template object minus the template is deeply equal to the without-template object. Runtime verification (old operator + new client on a live cluster, pod-template disk/security-context checks) is planned via
flyte-sdk/examples/plugins/spark_migrate/.