[K8S] Support to build application state and error from pod container #5711

turboFei · 2023-11-16T06:24:44Z

🔍 Description

This pr is to support to build the application state from pod container instead of pod.

The use case is that:
For spark on k8s, the pod contains two containers.

one is the spark driver main container
the another one is a sidecar container

If the spark driver main container completed but the sidecar container not, the batch application state is always RUNNING as the pod state is NotReady.

It is not expected.

So in this pr, I want to introduce a new option config to support to build application state from the pod container.

Issue References 🔗

This pull request fixes #

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklists

📝 Author Self Checklist

My code follows the style guidelines of this project
I have performed a self-review
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

Be nice. Be informative.

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

zwangsheng

LGTM at all and this is a good idea to check application status by container status!

But have one more question about: we set this kyuubi.kubernetes.application.state.fromContainer as server level conf, what will happen when user submit spark app with custom container name(by setting pod template and spark.kubernetes.driver.podTemplateContainerName)?

turboFei · 2023-11-16T12:16:15Z

But have one more question about: we set this kyuubi.kubernetes.application.state.fromContainer as server level conf, what will happen when user submit spark app with custom container name(by setting pod template and spark.kubernetes.driver.podTemplateContainerName)?

If no container matched with the server level kyuubi.kubernetes.application.state.container, it will fallback to use the pod state to build application state.

So, I think it does not matter.

turboFei · 2023-11-16T12:18:27Z

I can even ignore the user specified spark.kubernetes.driver.podTemplateContainerName in kyuubi server side.

With kyuubi conf:
kyuubi.session.conf.ignore.list and kyuubi.batch.conf.ignore.list.

codecov-commenter · 2023-11-16T14:06:08Z

Codecov Report

Attention: 53 lines in your changes are missing coverage. Please review.

Comparison is base (88fae49) 61.40% compared to head (9e0575b) 61.32%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
...kyuubi/engine/KubernetesApplicationOperation.scala	0.00%	49 Missing ⚠️
...uubi/engine/KubernetesApplicationAuditLogger.scala	0.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5711      +/-   ##
============================================
- Coverage     61.40%   61.32%   -0.09%     
  Complexity       23       23              
============================================
  Files           607      607              
  Lines         35755    35814      +59     
  Branches       4900     4909       +9     
============================================
+ Hits          21957    21963       +6     
- Misses        11413    11467      +54     
+ Partials       2385     2384       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

pan3793 · 2023-11-16T14:25:55Z

If no container matched with the server level kyuubi.kubernetes.application.state.container, it will fallback to use the pod state to build application state.

such behavior should be documented clearly.

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala

turboFei · 2023-11-17T03:09:51Z

cc @pan3793 @zwangsheng

…m pod container # 🔍 Description This pr is to support to build the application state from pod container instead of pod. The use case is that: For spark on k8s, the pod contains two containers. - one is the spark driver main container - the another one is a sidecar container If the spark driver main container completed but the sidecar container not, the batch application state is always RUNNING as the pod state is `NotReady`. It is not expected. So in this pr, I want to introduce a new option config to support to build application state from the pod container. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklists ## 📝 Author Self Checklist - [x] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [x] I have performed a self-review - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [x] Pull request title is okay. - [x] No license issues. - [x] Milestone correctly set? - [ ] Test coverage is ok - [ ] Assignees are selected. - [ ] Minimum number of approvals - [ ] No changes are requested **Be nice. Be informative.** Closes #5711 from turboFei/k8s_pod_state. Closes #5711 9e0575b [fwang12] save eaec5d2 [fwang12] comment 61ae5f7 [fwang12] comment 1eac84f [fwang12] save bca19be [fwang12] nit 38115b7 [fwang12] [K8S] Support to build application state and error from pod container Authored-by: fwang12 <[email protected]> Signed-off-by: fwang12 <[email protected]> (cherry picked from commit 765cdaa) Signed-off-by: fwang12 <[email protected]>

# 🔍 Description Fix typo when checking container name, and audit the pod state and container states as we support to retrieve application state from pod and container states. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklists ## 📝 Author Self Checklist - [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [ ] I have performed a self-review - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [x] Pull request title is okay. - [x] No license issues. - [x] Milestone correctly set? - [ ] Test coverage is ok - [x] Assignees are selected. - [x] Minimum number of approvals - [x] No changes are requested **Be nice. Be informative.** Closes #5763 from turboFei/fix_typo_k8s. Closes #5711 59b6025 [fwang12] container and pod 4ef7fc9 [fwang12] more states 301d44e [fwang12] typo Authored-by: fwang12 <[email protected]> Signed-off-by: fwang12 <[email protected]>

# 🔍 Description Fix typo when checking container name, and audit the pod state and container states as we support to retrieve application state from pod and container states. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklists ## 📝 Author Self Checklist - [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [ ] I have performed a self-review - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [x] Pull request title is okay. - [x] No license issues. - [x] Milestone correctly set? - [ ] Test coverage is ok - [x] Assignees are selected. - [x] Minimum number of approvals - [x] No changes are requested **Be nice. Be informative.** Closes #5763 from turboFei/fix_typo_k8s. Closes #5711 59b6025 [fwang12] container and pod 4ef7fc9 [fwang12] more states 301d44e [fwang12] typo Authored-by: fwang12 <[email protected]> Signed-off-by: fwang12 <[email protected]> (cherry picked from commit ba5cb24) Signed-off-by: fwang12 <[email protected]>

# 🔍 Description Fix typo for container states audit. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklists ## 📝 Author Self Checklist - [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [ ] I have performed a self-review - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [ ] Pull request title is okay. - [ ] No license issues. - [ ] Milestone correctly set? - [ ] Test coverage is ok - [ ] Assignees are selected. - [ ] Minimum number of approvals - [ ] No changes are requested **Be nice. Be informative.** Closes #5770 from turboFei/fix_typo_copilot. Closes #5711 be5181a [fwang12] fix typo Authored-by: fwang12 <[email protected]> Signed-off-by: fwang12 <[email protected]>

# 🔍 Description Fix typo for container states audit. ## Issue References 🔗 This pull request fixes # ## Describe Your Solution 🔧 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. ## Types of changes 🔖 - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request ⚰️ #### Behavior With This Pull Request 🎉 #### Related Unit Tests --- # Checklists ## 📝 Author Self Checklist - [ ] My code follows the [style guidelines](https://kyuubi.readthedocs.io/en/master/contributing/code/style.html) of this project - [ ] I have performed a self-review - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) ## 📝 Committer Pre-Merge Checklist - [ ] Pull request title is okay. - [ ] No license issues. - [ ] Milestone correctly set? - [ ] Test coverage is ok - [ ] Assignees are selected. - [ ] Minimum number of approvals - [ ] No changes are requested **Be nice. Be informative.** Closes #5770 from turboFei/fix_typo_copilot. Closes #5711 be5181a [fwang12] fix typo Authored-by: fwang12 <[email protected]> Signed-off-by: fwang12 <[email protected]> (cherry picked from commit 3ca4b8a) Signed-off-by: fwang12 <[email protected]>

github-actions bot added module:server module:common labels Nov 16, 2023

turboFei self-assigned this Nov 16, 2023

turboFei added this to the v1.8.1 milestone Nov 16, 2023

turboFei commented Nov 16, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala Show resolved Hide resolved

turboFei force-pushed the k8s_pod_state branch from 0faa2af to 252809c Compare November 16, 2023 06:43

github-actions bot added the kind:documentation Documentation is a feature! label Nov 16, 2023

pan3793 requested a review from zwangsheng November 16, 2023 07:01

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Show resolved Hide resolved

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated Show resolved Hide resolved

zwangsheng reviewed Nov 16, 2023

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated Show resolved Hide resolved

turboFei force-pushed the k8s_pod_state branch from 4a3779f to 51b7e01 Compare November 16, 2023 08:21

turboFei requested review from pan3793 and zwangsheng November 16, 2023 08:22

turboFei force-pushed the k8s_pod_state branch from 51b7e01 to fa92fcd Compare November 16, 2023 08:25

turboFei changed the title ~~[KUBERNETES] Support to build application state and error from pod container~~ [K8S] Support to build application state and error from pod container Nov 16, 2023

[K8S] Support to build application state and error from pod container

38115b7

turboFei force-pushed the k8s_pod_state branch from fa92fcd to 38115b7 Compare November 16, 2023 11:46

zwangsheng reviewed Nov 16, 2023

View reviewed changes

nit

bca19be

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala Outdated Show resolved Hide resolved

pan3793 reviewed Nov 16, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala Outdated Show resolved Hide resolved

save

1eac84f

turboFei added 3 commits November 17, 2023 10:51

comment

61ae5f7

comment

eaec5d2

save

9e0575b

turboFei force-pushed the k8s_pod_state branch from b5c8789 to 9e0575b Compare November 17, 2023 03:08

turboFei requested review from pan3793 and zwangsheng November 17, 2023 03:10

pan3793 approved these changes Nov 17, 2023

View reviewed changes

turboFei closed this in 765cdaa Nov 17, 2023

turboFei deleted the k8s_pod_state branch November 17, 2023 06:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[K8S] Support to build application state and error from pod container #5711

[K8S] Support to build application state and error from pod container #5711

turboFei commented Nov 16, 2023 •

edited by pan3793

Loading

zwangsheng left a comment •

edited

Loading

turboFei commented Nov 16, 2023 •

edited

Loading

turboFei commented Nov 16, 2023 •

edited

Loading

codecov-commenter commented Nov 16, 2023 •

edited

Loading

pan3793 commented Nov 16, 2023

turboFei commented Nov 17, 2023

[K8S] Support to build application state and error from pod container #5711

[K8S] Support to build application state and error from pod container #5711

Conversation

turboFei commented Nov 16, 2023 • edited by pan3793 Loading

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklists

📝 Author Self Checklist

📝 Committer Pre-Merge Checklist

zwangsheng left a comment • edited Loading

Choose a reason for hiding this comment

turboFei commented Nov 16, 2023 • edited Loading

turboFei commented Nov 16, 2023 • edited Loading

codecov-commenter commented Nov 16, 2023 • edited Loading

Codecov Report

pan3793 commented Nov 16, 2023

turboFei commented Nov 17, 2023

turboFei commented Nov 16, 2023 •

edited by pan3793

Loading

zwangsheng left a comment •

edited

Loading

turboFei commented Nov 16, 2023 •

edited

Loading

turboFei commented Nov 16, 2023 •

edited

Loading

codecov-commenter commented Nov 16, 2023 •

edited

Loading