Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pod template for Spark 3.x applications #2141

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ChenYi015
Copy link
Contributor

@ChenYi015 ChenYi015 commented Aug 22, 2024

Purpose of this PR

Close #2101
Close #1690

Proposed changes:

  • Add fields spec.driver.template and spec.executor.template to SparkApplication CRD
  • Webhook will only mutate Spark pods with label sparkoperator.k8s.io/mutated-by-spark-operator="true", and this label will be added by controller as needed.
  • For Spark 2.x applications, the webhook will be used to mutate Spark pods.
  • For Spark 3.x applications, the webhook will be used to mutate Spark driver/executor pods only if driver/executor pod template is not defined.
  • Add example spark-pi-pod-template.yaml

Change Category

Indicate the type of change by marking the applicable boxes:

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that could affect existing functionality)
  • Documentation update

Rationale

Checklist

Before submitting your PR, please review the following:

  • I have conducted a self-review of my own code.
  • I have updated documentation accordingly.
  • I have added tests that prove my changes are effective or that my feature works.
  • Existing unit tests pass locally with my changes.

Additional Notes

@ChenYi015
Copy link
Contributor Author

/hold
/assign @yuchaoran2011 @vara-bonthu @jacobsalway

// Spark version >= 3.0.0 is required.
// Ref: https://spark.apache.org/docs/latest/running-on-kubernetes.html#pod-template.
// +optional
Template *corev1.PodTemplateSpec `json:"template,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Will review the rest of the PR as well, but adding this as a struct rather than a binary/string field increases the number of properties in the resulting CRD by quite a bit. Is there any downside/considerations on the K8s side for very large CRDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I added a kubebuilder marker +kubebuilder:validation:Schemaless to mark the pod template field as schemaless to reduce the size of CRD.

@@ -302,6 +304,12 @@ func driverConfOption(app *v1beta2.SparkApplication) ([]string, error) {
property = fmt.Sprintf(common.SparkKubernetesDriverLabelTemplate, common.LabelLaunchedBySparkOperator)
args = append(args, "--conf", fmt.Sprintf("%s=%s", property, "true"))

// If Spark version is less than 3.0.0 or driver pod template is not defined, then the driver pod needs to be mutated by the webhook.
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed during the community call, we can think about a future release (maybe 2.1) where we announce the deprecation of web hook and support for Spark 2.x

Copy link
Member

Choose a reason for hiding this comment

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

Would we take the approach of moving to users only being able to specify a pod template, or map the existing struct fields and construct a pod template on the fly during submission?

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuchaoran2011

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

@yuchaoran2011 yuchaoran2011 left a comment

Choose a reason for hiding this comment

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

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants