HwangDongJun opened a new pull request, #58156:
URL: https://github.com/apache/spark/pull/58156

   ### What changes were proposed in this pull request?
   
   Add `KubernetesClientUtils.validateConfigMapSize`, a single validation point 
invoked right before every Spark-managed Kubernetes ConfigMap is built (driver 
base conf map, Hadoop conf map, Kerberos conf map, pod template conf map, and 
executor conf map). It fails fast with an `IllegalArgumentException` when the 
combined size of a ConfigMap's data would exceed 
`spark.kubernetes.configMap.maxSize`.
   
   `KubernetesClientUtils.loadSparkConfDirFiles` no longer silently 
truncates/skips conf dir files to stay under the size limit; all files are 
loaded as-is, and enforcing the limit is now the responsibility of 
`validateConfigMapSize`.
   
   ### Why are the changes needed?
   
   Currently, when the combined size of configuration files exceeds the 
Kubernetes ConfigMap size limit, Spark silently skips loading some files. This 
can leave an application running with missing or unexpected configuration.
   
   The existing validation is also incomplete and inconsistent:
   1. Only the driver's base conf map is validated against the limit; the 
Hadoop conf, Kerberos conf, pod template, and executor conf maps are never 
checked, so Kubernetes itself ends up rejecting an oversized ConfigMap with a 
raw API error instead of Spark failing with a clear message.
   2. The resolved `spark.properties` file is added to the driver conf map 
*after* the old size check ran, so its size was never counted toward the limit.
   
   `validateConfigMapSize` is invoked once the full data map (including 
resolved `spark.properties`, where applicable) has been assembled for each of 
the five ConfigMap call sites, so both gaps are addressed by a single, 
uniformly-applied check.
   
   The existing public `buildConfigMap`/`buildConfigMapJava` APIs are left 
unchanged so that external callers (e.g. the Spark Kubernetes Operator) relying 
on their current signature and behavior are not affected.
   
   There was a previous attempt at this issue in #53455, which went stale 
without any review and was closed by stale-bot (`Closed with unmerged 
commits`). This PR independently reproduces the underlying bug and takes a 
smaller, source- and binary-compatible approach: rather than changing the 
signature of the existing `@Stable` `buildConfigMap`/`buildConfigMapJava` APIs, 
this PR adds a separate `validateConfigMapSize` check invoked at each call site.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. An application will now fail fast with a clear error when the combined 
size of Spark's generated Kubernetes ConfigMap(s) exceeds 
`spark.kubernetes.configMap.maxSize`, instead of silently continuing with 
missing configuration.
   
   ### How was this patch tested?
   
   * Reverted the fix locally and reran the updated 
`KubernetesClientUtilsSuite` against the unmodified code to confirm the new 
tests fail against the original behavior (conf files were silently truncated 
from 10,000 down to 2), then confirmed they pass with the fix applied.
   * Added `KubernetesClientUtilsSuite` cases covering: no more silent 
truncation, `validateConfigMapSize` throwing when the limit is exceeded, 
allowing data exactly at the limit (safety/no regression for normal-sized 
configs), and accounting for the resolved `spark.properties` size.
   * Ran the full `kubernetes/core` module test suite: 386/386 passed.
   * `scalastyle` and `checkstyle`: no violations.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Sonnet 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: [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]

Reply via email to