You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "peter-toth (via GitHub)" <gi...@apache.org> on 2026/09/18 12:47:34 UTC
[PR] [SPARK-59578] Clarify that Kueue `integrations.externalFrameworks` is not needed in `operations.md` [spark-kubernetes-operator]
peter-toth opened a new pull request, #844:
URL: https://github.com/apache/spark-kubernetes-operator/pull/844
### What changes were proposed in this pull request?
This PR replaces the instruction to register `SparkApplication.v1.spark.apache.org` and `SparkCluster.v1.spark.apache.org` in Kueue's `integrations.externalFrameworks` in the Kueue entry of `docs/operations.md` with a sentence saying it is not needed, and why.
### Why are the changes needed?
The registration has no effect for this operator, so the instruction asks users to configure something that does nothing.
Traced against Kueue `v0.19.4`:
- `integrations.externalFrameworks` only populates a lookup table. `registerExternal` (`pkg/controller/jobframework/integrationmanager.go:149-169`) stores a `PartialObjectMetadata` for the kind in `m.externalIntegrations`, and `setupControllers` (`pkg/controller/jobframework/setup.go:66-70`) calls `RegisterExternalJobType` for external frameworks and nothing else — no controller and no webhook is built for them.
- The only owner-related consumer of that table is `getJobTypeForOwner` (`integrationmanager.go:230-244`).
- Its caller `IsOwnerManagedByKueueForObject` (`integrationmanager.go:358-363`) starts with `metav1.GetControllerOf(obj)`, which returns the owner reference whose `controller` field is true.
- The operator does not set a controller owner reference on the pods it creates: `ModelUtils.buildOwnerReferenceTo` sets `blockOwnerDeletion` only, and the single place the operator sets `controller: true` is `KueueWorkloadFactory`, on the `Workload` itself.
So `GetControllerOf` returns nil for a driver or executor pod and the registry is never consulted. The operator also does not need Kueue to manage `Workload`s on its behalf: it builds them with `KueueWorkloadFactory` and deletes them in `AppCleanUpStep`.
The same reasoning rules out the one case where the registration might have looked load-bearing. `defaultLocalQueueApplies` (`pkg/controller/jobframework/defaults.go:80-88`) has no feature gate, so in a namespace holding a `LocalQueue` named `default` Kueue can add `queue-name: default` to an object whose owner it does not manage. But that branch returns `!IsOwnerManagedByKueueForObject(jobObj)`, which evaluates the same way with or without the registration, again because there is no controller reference to resolve.
### Does this PR introduce _any_ user-facing change?
No, documentation only.
### How was this patch tested?
Manual review against the Kueue `v0.19.4` sources cited above. `integrations.externalFrameworks` had no other mention in the repository, so nothing else goes stale.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59578] Clarify that Kueue `integrations.externalFrameworks` is not needed in `operations.md` [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #844: [SPARK-59578] Clarify that Kueue `integrations.externalFrameworks` is not needed in `operations.md`
URL: https://github.com/apache/spark-kubernetes-operator/pull/844
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59578] Clarify that Kueue `integrations.externalFrameworks` is not needed in `operations.md` [spark-kubernetes-operator]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #844:
URL: https://github.com/apache/spark-kubernetes-operator/pull/844#issuecomment-5740022056
Thank you @dongjoon-hyun.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org