spark-operator icon indicating copy to clipboard operation
spark-operator copied to clipboard

Spark not reading default conf provided by sparkConfigMap

Open pradeeppathak9 opened this issue 3 years ago • 14 comments

I have installed spark-operator using Helm Chart with webhook enabled. It's working when I set spark configuration using sparkConf, but not when specifying via sparkConfigMap.

spark.yaml

apiVersion: "sparkoperator.k8s.io/v1beta2"
kind: SparkApplication
metadata:
  name: consumer-pipeline-v2-consumer-process-2  
  namespace: pipelines
spec:
  type: Python
  pythonVersion: "3"
  mode: cluster
  image: registry.digitalocean.com/spoonshot/consumer-pipeline-v2/consumer_process:v0.0.5.test
  imagePullPolicy: Always
  mainApplicationFile: local:///main/main.py
  sparkVersion: "2.4.7"
  sparkConfigMap: consumer-pipeline-v2-config
  
  restartPolicy:
    type: Always

  driver:
    cores: 1
    memory: "1G"
    serviceAccount: spark
    labels:
      version: 2.4.7

  executor:
    cores: 1
    memory: "2G"
    memoryOverhead: "2G"
    instances: 1
    labels:
      version: 2.4.7

configmap.yaml

apiVersion: v1
kind: ConfigMap
metadata:
  name: consumer-pipeline-v2-config
  namespace: pipelines
data:
  spark-defaults.conf: |
    spark.ui.port=4041
    spark.hadoop.fs.s3a.access.key=cxascascascsacsdcsdc
    spark.hadoop.fs.s3a.secret.key=acsacsacsdacsd+casscasasc
    spark.hadoop.fs.s3a.multipart.size=104857600

pradeeppathak9 avatar Mar 01 '21 13:03 pradeeppathak9

I have the same issue, have you found a workaround?

karpoftea avatar Oct 19 '21 14:10 karpoftea

Did you find any solution?

rubenssoto avatar Nov 10 '21 18:11 rubenssoto

I think this is likely related to #216. I have been running into the same issue. @liyinan926 I think this issue is a duplicate of #216.

tafaust avatar May 24 '22 08:05 tafaust

@pradeeppathak9 See my previous comment. I have the same issue. The problem is that while SPARK_CONF_DIR is set to /etc/spark/conf and the config map gets successfully mounted, spark-submit doesn't pick up the config.

Also, when you set your configmap, make sure to have spaces or tabs in your data instead of the = between your key and value.

tafaust avatar May 24 '22 21:05 tafaust

I think it's not only a problem with SPARK_CONF_DIR, I already resolve this problem, and added a spark-default.conf file to the configmap, the file are mounted to SPARK_CONF_DIR/spark-default.conf in driver/executor pod, but never loaded by spark, because spark already creates a spark.properties file with all of sparkConf found on spark-submit arguments, I think the problem is here, spark ignore spark-default.conf file when a spark.properties file found in SPARK_CONF_DIR, according to :

https://github.com/apache/spark/blob/0494dc90af48ce7da0625485a4dc6917a244d580/core/src/main/scala/org/apache/spark/deploy/master/MasterArguments.scala#L55

and

https://github.com/apache/spark/blob/5412688cd69792d804ba04ef43104e58c208e26c/core/src/main/scala/org/apache/spark/util/Utils.scala#L2148

` * Load default Spark properties from the given file. If no file is provided,

  • use the common defaults file. This mutates state in the given SparkConf and
  • in this JVM's system properties if the config specified in the file is not
  • already set. Return the path of the properties file used. `

maknihamdi avatar Aug 16 '22 10:08 maknihamdi

@maknihamdi Closely read the progression of #216. You have no control of the /etc/spark/conf (set in SPARK_CONF_DIR) mountpoint without the patch introduced by @bbenzikry and me. Our patch does not subpath mount the auto-generated (by upstream spark k8s resource manager) spark.properties to not break spark when run in k8s. @bbenzikry and I have not settled for the appropriate strategy here because we want to have feedback on the current PR https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/pull/1568. I have created a small hotfix on a separate branch in my forked repo (link) to patch the auto-generated spark.properties on properties that will not interfere with the auto-generated keys in the spark.properties. The hotfix is very experimental though.

Looking at https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Constants.scala#L74-L75 you can already see that there is a desync between spark k8s resource manager upstream and spark operator upstream.

tafaust avatar Aug 16 '22 11:08 tafaust

@tahesse You spoke about problem with spark-defaults.conf in #216 , my remark is about using spark-defaults.conf when spark already set a properties file and then with precedences rules will ignore spark-defaults.conf here https://github.com/apache/spark/blob/5412688cd69792d804ba04ef43104e58c208e26c/core/src/main/scala/org/apache/spark/util/Utils.scala#L2148 . I think your solution (in your forked repo) can be also used with spark-defaults.conf by merging generated spark.properties file and custom user spark-defaults.conf, but we should care about precedence rule between files content. Or we can update doc and say explicitly we shouldn't use spark-defaults.conf and suggest to use custom spark.properties file (this is after fixing the issue with sparkConfigMap)

maknihamdi avatar Aug 16 '22 13:08 maknihamdi

@tahesse You spoke about problem with spark-defaults.conf in #216 , my remark is about using spark-defaults.conf when spark already set a properties file and then with precedences rules will ignore spark-defaults.conf here https://github.com/apache/spark/blob/5412688cd69792d804ba04ef43104e58c208e26c/core/src/main/scala/org/apache/spark/util/Utils.scala#L2148 .

@maknihamdi spark k8s resource manager always generates a spark.properties file. Describe your driver pods, there'll always be a spark.properties.

I think your solution (in your forked repo) can be also used with spark-defaults.conf by merging generated spark.properties file and custom user spark-defaults.conf, but we should care about precedence rule between files content. Or we can update doc and say explicitly we shouldn't use spark-defaults.conf and suggest to use custom spark.properties file (this is after fixing the issue with sparkConfigMap)

The solution and upstream spark operator always implicitly ignores spark-defaults.conf. Upstream operator overwrites spark.properties through the mount and the subpath mount overwrites spark.properties by the autogenerated one.

tafaust avatar Aug 16 '22 13:08 tafaust

spark k8s resource manager always generates a spark.properties file. Describe your driver pods, there'll always be a spark.properties.

this is what I'm saying above, spark k8s resource manager always generates a spark.properties, and it's the reason why the spark-defaults.conf is (will be) ignored. If it's always implicitly ignored, we should explicitly say it. because in the doc we have: mounting a special Kubernetes ConfigMap storing Spark configuration files (e.g. spark-defaults.conf, spark-env.sh, log4j.properties) using the optional field .spec.sparkConfigMap https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/blob/master/docs/user-guide.md#specifying-spark-configuration

maknihamdi avatar Aug 16 '22 13:08 maknihamdi

Hi @maknihamdi you are correct in asserting that docs are problematic when it gets to this topic - once #1568 is pushed it'll be great to get a discussion going around this.

I will say that @tahesse and I have discussed some of those issues at length. Some of the suggested implementations require decisions around where the responsibility lies for generation and/or config validation ( e.g. spark/k8s or the operator ) and may make the operator exceed its boundaries, at least IMHO ( create a tighter coupling to upstream minor versions, etc )

bbenzikry avatar Aug 16 '22 17:08 bbenzikry

Sorry folks. It's been a while on this!

I wasn't able to fix it directly. I did a workaround. I was using Jenkins CI/CD, so my workaround was templating the yaml file and updating the values during the Jenkins deployment using variables references

something like below

"spark.hadoop.fs.azure.account.key.spoonshotlrsstorage.blob.core.windows.net": "${AZURE_ACCOUNT_KEY}"

pradeeppathak9 avatar Aug 16 '22 18:08 pradeeppathak9

Some of the suggested implementations require decisions around where the responsibility lies for generation and/or config validation ( e.g. spark/k8s or the operator ) and may make the operator exceed its boundaries, at least IMHO ( create a tighter coupling to upstream minor versions, etc )

I think operator should continue to have only one way to pass spark configurations to spark: add configs one by one on spark-submit, and keep spark/k8s do what he should do with it (generate spark.properties, mount the file, set the env var)

We may add an explicit exclude of spark-defaults.conf and spark.properties files from configmap created by user, user can use sparkConf array field to add specific spark conf, and for commun configurations, like spark.eventLog.dir, we can add new field (ex sparkCommunConfigMap), another configmap, but a literal one (no files), with a list of key/value commun spark conf we can share between jobs

maknihamdi avatar Aug 17 '22 16:08 maknihamdi

I think operator should continue to have only one way to pass spark configurations to spark: add configs one by one on spark-submit, and keep spark/k8s do what he should do with it (generate spark.properties, mount the file, set the env var)

spark-operator CRDs already have a field to provide a custom configMap here: https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/blob/master/manifest/crds/sparkoperator.k8s.io_sparkapplications.yaml#L3701-L3702

We may add an explicit exclude of spark-defaults.conf and spark.properties files from configmap created by user, user can use sparkConf array field to add specific spark conf, and for commun configurations, like spark.eventLog.dir, we can add new field (ex sparkCommunConfigMap), another configmap, but a literal one (no files), with a list of key/value commun spark conf we can share between jobs

Beni's PR ignores spark.properties as of now.

tafaust avatar Aug 18 '22 07:08 tafaust

spark-operator CRDs already have a field to provide a custom configMap here:

sparkConfigMap is for files, not literal

Beni's PR ignores spark.properties as of now

I know, "We may add an explicit exclude of spark-defaults.conf and spark.properties files" not only spark.properties file

maknihamdi avatar Aug 18 '22 08:08 maknihamdi