[sweb]

Description

Closes #774

Adds the field cleanup to set how long the spark-submit Job is kept around. This is useful, because the operator now moves the ownership of driver pod, serviceaccount, role binding and config maps that were created for this SparkApplication to the Job, once the driver pod finishes (either with Succeeded or Failed). There are two subfields that govern how to handle succeeded and failed jobs, as it is quite often to require longer TTLs for failed jobs.

apiVersion: spark.stackable.tech/v1alpha1
kind: SparkApplication
metadata:
  name: example
spec:
  cleanup:
    afterSucceeded: 10m
    afterFailed: 1d

There are three two possible values for afterSucceeded and afterFailed:

• never: The Job never gets deleted and sticks around until the SparkApplication (its owner) gets deleted.
• a duration like 10m (which is the default): The job sticks around for the specified duration and then gets deleted - this then also deletes the mentioned resources that have the job as owner.
• 0s: Or any other variant that results in a duration of 0s - this deletes the Job and the other resources right away.

Before this change, driver pods get deleted right after the driver finishes. Users want to change this have enough time to look at driver logs of failed applications before these are removed. This also enables interactions with other products, e.g. Airflow where the logs can be copied into the task logs for a better user experience.

The pod-driver controller needs two more permissions for this: patch on Pods and get on Jobs.

Definition of Done Checklist

• Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
• Please make sure all these things are done and tick the boxes

Author

• Changes are OpenShift compatible
• CRD changes approved
• CRD documentation for all fields, following the style guide.
• Helm chart can be installed and deployed operator works
• Integration tests passed (for non trivial changes)
• Changes need to be "offline" compatible
• Links to generated (nightly) docs added
• Release note snippet added

Reviewer

• Code contains useful comments
• Code contains useful logging statements
• (Integration-)Test cases added
• Documentation added or updated. Follows the style guide.
• Changelog updated
• Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

• Feature Tracker has been updated
• Proper release label has been added
• Links to generated (nightly) docs added
• Release note snippet added
• Add type/deprecation label & add to the deprecation schedule
• Add type/experimental label & add to the experimental features tracker

[sbernauer]

Your CRD docs state

How long the spark-submit Job, the driver Pod and the objects the operator created for the application are kept after the Job finished

With that argumentation, I would have expected this to be a "clusterConfig" and not on the job "role". As there is no "clusterConfig" on the SparkApp, I would propse putting it below .spec.config (normally it's .spec.clusterConfig). One additional argument is that we likely get rid of the Job in the future (e.g. for streaming applications) and still want a TTL.

I also left two comments about not supporting spark.kubernetes.submission.waitAppCompletion and spark.kubernetes.driver.pod.name to make our lives easier.

[sweb]

Your CRD docs state

How long the spark-submit Job, the driver Pod and the objects the operator created for the application are kept after the Job finished

With that argumentation, I would have expected this to be a "clusterConfig" and not on the job "role". As there is no "clusterConfig" on the SparkApp, I would propse putting it below .spec.config (normally it's .spec.clusterConfig). One additional argument is that we likely get rid of the Job in the future (e.g. for streaming applications) and still want a TTL.

I also left two comments about not supporting spark.kubernetes.submission.waitAppCompletion and spark.kubernetes.driver.pod.name to make our lives easier.

@sbernauer I moved it in 7b2771a

I put it into spec.ttlAfterFinished to not open up a new namespace right now, as we also configure other elements directly in spec. Happy to change it though.

Regarding the two spark conf entries: I think stopping to support them is the right direction, however I would avoid doing it right now as we do not have a good default configuration for streaming applications and my guess is that both settings might be used in streaming workloads. I would suggest to get rid of them when we tackle a redesign for Spark streaming applications. If we are confident that support for those will be removed in the future, we can add a deprecation warning right now.

[sbernauer]

Thanks for addressing the change! Putting it top level below .spec is also ok for me.
As for the two spark conf entries I trust the experts and leave it to others to decide ;)
Just an idea, we could make it a bit more explanatory by calling it .spec.cleanupTtlAfterFinished, but no strong opinion.

[sbernauer]

Just a random brain fart: Shouldn't we have different TTLs for successful and failed Jobs? A successful Job can be typically cleaned up fast, logs for a failed one are much valuable. Or at least design the CRD in such a way that we can add that later?
E.g. cleanupTtlAfterSuccessful: 10m and cleanupTtlAfterfailed: 24h

[sbernauer]

General change was approved in the daily today, there was only a wish for expressing the default in the CRD.
edit: We can't do that because of the SparkAppTemplate.
Sensible defaults mentioned were 10m for successful and 24h for failed.