-
Notifications
You must be signed in to change notification settings - Fork 914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[KYUUBI #5237] ConfigMaps deletion on Kubernetes #6700
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6700 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 684 684
Lines 42279 42315 +36
Branches 5765 5774 +9
======================================
- Misses 42279 42315 +36 ☔ View full report in Codecov by Sentry. |
The overall design here is, in Spark on K8s cluster mode, all resources created by the Spark app should set the owner reference to the Spark driver pod, which means all resources will be deleted automatically after deleting the driver Pod, is this not sufficient? Can you elaborate more about your use cases and problems? |
Hi @pan3793, As per my understanding I was expecting the same behaviour, cm to be deleted by spark. While launching the kyuubi-server I have defined, spark.submit.deployMode=cluster This is the engine launch submit command: /opt/spark/bin/spark-submit |
@@ -320,6 +320,39 @@ class KubernetesApplicationOperation extends ApplicationOperation with Logging { | |||
appStateSource, | |||
appStateContainer) | |||
} | |||
|
|||
try { | |||
val volumes = pod.getSpec.getVolumes.asScala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might take time to make this action, could you use executor service/thread pool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @turboFei , I have implemented it using ThreadPool, could you please review. Was it the expectation?
340c3d7
to
c729e2d
Compare
@@ -320,6 +365,8 @@ class KubernetesApplicationOperation extends ApplicationOperation with Logging { | |||
appStateSource, | |||
appStateContainer) | |||
} | |||
|
|||
runConfigMapDeletion(pod, kubernetesInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions:
Do the executors belong to the same app share the configmap?
If that, on executor pod deleted, you will delete the shared the configmap.
🔍 Description
Issue References 🔗
This pull request fixes #5237
Describe Your Solution 🔧
Extended the implementation of pod deletion method and which deletes the config maps associated with the pod, if the config map contains spark-exec keyword in it. Here I used specific matching to avoid the deletion of any other important config maps
Types of changes 🔖
Test Plan 🧪
Locally, attaching the results
Checklist 📝
Be nice. Be informative.