peter-toth commented on code in PR #947:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/947#discussion_r4204342231
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptor.java:
##########
@@ -217,14 +221,14 @@ private Meter getMeterByResponseCode(int code) {
* Parses the given path to extract namespace-scoped resource information.
*
* @param path The request path.
- * @return An Optional containing a Pair of namespace and resource name, or
empty if not found.
+ * @return An Optional containing a Pair of namespace and resource name, or
empty if the path has
+ * no {@code namespaces/<namespace>/<resource>} segments.
*/
public Optional<Pair<String, String>> parseNamespaceScopedResource(String
path) {
- if (path.contains(NAMESPACES)) {
- int index = path.indexOf(NAMESPACES) + NAMESPACES.length();
- String namespaceAndResources = path.substring(index + 1);
- String[] parts = namespaceAndResources.split("/");
- return Optional.of(Pair.of(parts[0], parts[1]));
+ List<String> segments = Arrays.asList(path.split("/"));
Review Comment:
**Finding 1.** The description says "The operator does not call these APIs
today, so this bug is latent." On `main` it does, for the third row of the
table. With `spark.kubernetes.operator.kueue.enabled`, these calls GET a
cluster-scoped object by a name which the user picks:
- `KueueWorkloadPriority.java:95` reads the `WorkloadPriorityClass` of the
`kueue.x-k8s.io/priority-class` label.
- `KueueWorkloadPriority.java:141` reads the `PriorityClass` of a pod
template.
- `KueueWorkloadUtils.java:591` reads each `ResourceFlavor` which Kueue
assigned.
The client metrics are on by default. So a flavor named `all-namespaces`
gives the path `/apis/kueue.x-k8s.io/v1beta2/resourceflavors/all-namespaces`,
and the old code throws `StringIndexOutOfBoundsException` for it, like for
`/api/v1/namespaces`. Neither `setPriority` nor `holdForAdmission` catches that
type. So it reaches the `catch (Exception e)` at `AppInitStep.java:154` and
`ClusterInitStep.java:183`, and the application or cluster ends in
`SchedulingFailure`. I traced this by reading and didn't run it.
Please say in the description that the fix also covers these Kueue lookups
on `main`. 1.0.0 has no Kueue integration, so the bug stays latent there.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
