[GitHub] [spark] ninebigbig commented on a diff in pull request #39884: [SPARK-42344][K8S] Change the default size of the CONFIG_MAP_MAXSIZE

["ninebigbig (via GitHub)" <gi...@apache.org>]

ninebigbig commented on code in PR #39884:
URL: https://github.com/apache/spark/pull/39884#discussion_r1096619128


##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala:
##########
@@ -158,11 +158,10 @@ private[spark] object Config extends Logging {
 
   val CONFIG_MAP_MAXSIZE =
     ConfigBuilder("spark.kubernetes.configMap.maxSize")
-      .doc("Max size limit for a config map. This is configurable as per" +
-        " https://etcd.io/docs/v3.4.0/dev-guide/limit/ on k8s server end.")
-      .version("3.1.0")
+      .doc("Max size limit for a config map. Must have at most 1048576 bytes")

Review Comment:
   Thanks @LuciferYang for review.
   I'm sorry to say there is no relative definition In the official K8S API document. see:
   https://kubernetes.io/docs/reference/kubernetes-api/config-and-storage-resources/config-map-v1/
   But the logic code of validation we can get in func ValidateConfigMap in https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/validation/validation.go
   line 5861 to line 5864.
   the code shows as below:
   // ValidateConfigMap tests whether required fields in the ConfigMap are set.
   func ValidateConfigMap(cfg *core.ConfigMap) field.ErrorList {
   	allErrs := field.ErrorList{}
   	allErrs = append(allErrs, ValidateObjectMeta(&cfg.ObjectMeta, true, ValidateConfigMapName, field.NewPath("metadata"))...)
   
   	totalSize := 0
   
   	for key, value := range cfg.Data {
   		for _, msg := range validation.IsConfigMapKey(key) {
   			allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, msg))
   		}
   		// check if we have a duplicate key in the other bag
   		if _, isValue := cfg.BinaryData[key]; isValue {
   			msg := "duplicate of key present in binaryData"
   			allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, msg))
   		}
   		totalSize += len(value)
   	}
   	for key, value := range cfg.BinaryData {
   		for _, msg := range validation.IsConfigMapKey(key) {
   			allErrs = append(allErrs, field.Invalid(field.NewPath("binaryData").Key(key), key, msg))
   		}
   		totalSize += len(value)
   	}
   	if totalSize > core.MaxSecretSize {
   		// pass back "" to indicate that the error refers to the whole object.
   		allErrs = append(allErrs, field.TooLong(field.NewPath(""), cfg, core.MaxSecretSize))
   	}
   
   	return allErrs
   }
   And the const variable MaxSecretSize that the configmap validation reused is defined in the same file in line 5338.
   
   // MaxSecretSize represents the max secret size.
   const MaxSecretSize = 1 * 1024 * 1024
   
   So may I use the code path instead of the document path？
   Such as:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/validation/validation.go#L5861



-- 
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