You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2021/04/14 01:13:03 UTC
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 opened a new pull request #253: [YUNIKORN-558] Integration with Spark operator plugin
yuchaoran2011 opened a new pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253
### What is this PR for?
Improved the integration with Spark operator plugin. The areas of improvement as as follows:
* Make sure that individual pods fall under the responsibility of the general plugin. Make the Spark operator plugin manage the SparkApplication CRD only
* Make sure that the general and operator plugins see the same application ID for the same SparkApplication. The ID is the string generated by the Spark K8s backend that starts with "spark-"
* Make sure that when a SparkApplication completes or fails, cleanup is performed for all placeholders if there's any that remain at that time
* When cleaning up placeholders, there are often cases where the placeholder doesn't exist anymore and that will cause the deletion to fail. Do not add the placeholder to the orphan pod list in this case because there's no need to retry deletion.
* Added and updated some logging to aid monitoring and debugging
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* This is only part of the work for the referenced JIRA. A separate PR will be made on https://github.com/GoogleCloudPlatform/spark-on-k8s-operator to complete the integration.
### What is the Jira issue?
* https://issues.apache.org/jira/browse/YUNIKORN-558 (Integrate YuniKorn with Spark K8S Operator)
### How should this be tested?
All the existing unit tests passed. Not quite sure how to unit test this feature specifically. But probably we can enhance the integration tests to cover it. We have been running this patch for relatively large Spark operator workloads for a few weeks and it has been stable for us.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613310459
##########
File path: pkg/cache/placeholder_manager.go
##########
@@ -46,10 +47,13 @@ type PlaceholderManager struct {
}
var placeholderMgr *PlaceholderManager
+var placeholderManagerLock sync.Mutex
Review comment:
You are exactly correct. I have fixed the `TestFailApplication` following the example of `TestTryReserve`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613668645
##########
File path: deployments/image/configmap/Dockerfile
##########
@@ -46,8 +46,8 @@ ENV DISPATCHER_TIMEOUT "300s"
ENV KUBE_CLIENT_QPS "1000"
ENV KUBE_CLIENT_BURST "1000"
ENV PREDICATES ""
-ENV OPERATOR_PLUGINS "general"
ENV ENABLE_CONFIG_HOT_REFRESH "true"
+ENV OPERATOR_PLUGINS "general,spark-k8s-operator"
Review comment:
Sure, I'll roll back this line to include only the general plugin by default. I'll create another JIRA to track the work on exposing it in the Helm chart
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] commented on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d0905d7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.45%`.
> The diff coverage is `61.13%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.20% +0.45%
==========================================
Files 35 36 +1
Lines 3133 3302 +169
==========================================
+ Hits 1872 1988 +116
- Misses 1180 1233 +53
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...d0905d7](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7b814eb) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.41%`.
> The diff coverage is `61.29%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.17% +0.41%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1982 +110
- Misses 1180 1232 +52
+ Partials 81 80 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...7b814eb](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7b814eb) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.41%`.
> The diff coverage is `61.29%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.17% +0.41%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1982 +110
- Misses 1180 1232 +52
+ Partials 81 80 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...7b814eb](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819949571
It's strange why the latest commit caused e2e test failures. It only contained changes to the comments and formatting, as well as roll back the plugins that are enabled by default to the "general" only
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613678388
##########
File path: pkg/common/events/events.go
##########
@@ -33,20 +33,20 @@ type SchedulingEvent interface {
type ApplicationEventType string
const (
- SubmitApplication ApplicationEventType = "SubmitApplication"
- RecoverApplication ApplicationEventType = "RecoverApplication"
- AcceptApplication ApplicationEventType = "AcceptApplication"
- TryReserve ApplicationEventType = "TryReserve"
- UpdateReservation ApplicationEventType = "UpdateReservation"
- RunApplication ApplicationEventType = "RunApplication"
- RejectApplication ApplicationEventType = "RejectApplication"
- CompleteApplication ApplicationEventType = "CompleteApplication"
- FailApplication ApplicationEventType = "FailApplication"
- KillApplication ApplicationEventType = "KillApplication"
- KilledApplication ApplicationEventType = "KilledApplication"
- ReleaseAppAllocation ApplicationEventType = "ReleaseAppAllocation"
+ SubmitApplication ApplicationEventType = "SubmitApplication"
+ RecoverApplication ApplicationEventType = "RecoverApplication"
+ AcceptApplication ApplicationEventType = "AcceptApplication"
+ TryReserve ApplicationEventType = "TryReserve"
+ UpdateReservation ApplicationEventType = "UpdateReservation"
+ RunApplication ApplicationEventType = "RunApplication"
+ RejectApplication ApplicationEventType = "RejectApplication"
+ CompleteApplication ApplicationEventType = "CompleteApplication"
+ FailApplication ApplicationEventType = "FailApplication"
+ KillApplication ApplicationEventType = "KillApplication"
+ KilledApplication ApplicationEventType = "KilledApplication"
+ ReleaseAppAllocation ApplicationEventType = "ReleaseAppAllocation"
ReleaseAppAllocationAsk ApplicationEventType = "ReleaseAppAllocationAsk"
- AppStateChange ApplicationEventType = "ApplicationStateChange"
+ AppStateChange ApplicationEventType = "ApplicationStateChange"
Review comment:
Ok. I'll roll back this change. It was introduced when I ran `go fmt`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613311340
##########
File path: go.mod
##########
@@ -18,20 +18,20 @@
module github.com/apache/incubator-yunikorn-k8shim
-go 1.12
+go 1.14
Review comment:
Agreed. Rolled back
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7b814eb) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.41%`.
> The diff coverage is `61.29%`.
> :exclamation: Current head 7b814eb differs from pull request most recent head 554dc46. Consider uploading reports for the commit 554dc46 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.17% +0.41%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1982 +110
- Misses 1180 1232 +52
+ Partials 81 80 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...554dc46](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613312078
##########
File path: pkg/appmgmt/general/general.go
##########
@@ -34,6 +33,8 @@ import (
"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
"github.com/apache/incubator-yunikorn-scheduler-interface/lib/go/si"
+
+ "go.uber.org/zap"
Review comment:
This is due to the usage of logging in this file. If I removed this import, `make lint` would complain
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613312467
##########
File path: go.mod
##########
@@ -18,20 +18,20 @@
module github.com/apache/incubator-yunikorn-k8shim
-go 1.12
+go 1.14
require (
- github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20200817155620-c19d2b8660d8
- github.com/apache/incubator-yunikorn-core v0.0.0-20210308173435-79a60272f12f
- github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20210226143918-19a5cca5e428
+ github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20201215015655-2e8b733f5ad0
+ github.com/apache/incubator-yunikorn-core v0.0.0-20210412120202-73fdc11674a5
+ github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20210412034924-44e8abeff79e
github.com/google/uuid v1.1.1
github.com/gorilla/mux v1.7.3
github.com/looplab/fsm v0.1.0
- github.com/onsi/ginkgo v1.11.0
- github.com/onsi/gomega v1.7.0
- go.uber.org/zap v1.13.0
+ github.com/onsi/ginkgo v1.15.1
+ github.com/onsi/gomega v1.11.0
+ go.uber.org/zap v1.16.0
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
- gopkg.in/yaml.v2 v2.2.8
+ gopkg.in/yaml.v2 v2.4.0
Review comment:
Agreed. Rolled back
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819174253
The data race issue that happened in the unit tests has been fixed. Now ready for review
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613683141
##########
File path: deployments/image/configmap/Dockerfile
##########
@@ -46,8 +46,8 @@ ENV DISPATCHER_TIMEOUT "300s"
ENV KUBE_CLIENT_QPS "1000"
ENV KUBE_CLIENT_BURST "1000"
ENV PREDICATES ""
-ENV OPERATOR_PLUGINS "general"
ENV ENABLE_CONFIG_HOT_REFRESH "true"
+ENV OPERATOR_PLUGINS "general,spark-k8s-operator"
Review comment:
Created https://issues.apache.org/jira/browse/YUNIKORN-642 to track the follow-up work
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613668061
##########
File path: go.mod
##########
@@ -21,7 +21,7 @@ module github.com/apache/incubator-yunikorn-k8shim
go 1.12
require (
- github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20200817155620-c19d2b8660d8
+ github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20201215015655-2e8b733f5ad0
Review comment:
It's this version: https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/releases/tag/v1beta2-1.2.2-3.0.0. When I manually entered `v1beta2-1.2.2-3.0.0`, the go module system automatically changed it to the string in the PR
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613316554
##########
File path: pkg/appmgmt/sparkoperator/spark.go
##########
@@ -51,6 +40,7 @@ type Manager struct {
}
func NewManager(amProtocol interfaces.ApplicationManagementProtocol, apiProvider client.APIProvider) *Manager {
+ log.Logger().Info("New AppMgr initialized")
Review comment:
Yeah this is redundant. Removed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (554dc46) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.11%`.
> The diff coverage is `61.29%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 59.86% +0.11%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1972 +100
- Misses 1180 1241 +61
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...554dc46](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
yangwwei commented on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-820909726
> It's strange why the latest commit caused e2e test failures. It only contained changes to the comments and formatting, as well as roll back the plugins that are enabled by default to the "general" only
Looks like some intermittent issue, I have rerun the tests and it looks good now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613313168
##########
File path: .gitignore
##########
@@ -1,3 +1,4 @@
.idea
_output/
+vendor
Review comment:
Yeah this is mostly for my own convenience when looking at the code of the dependencies. Removed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819174253
Looks like there's a potential data race that happened in one of the unit tests. I'll look into it
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yangwwei merged pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
yangwwei merged pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613668896
##########
File path: pkg/cache/application.go
##########
@@ -460,9 +461,14 @@ func (app *Application) postAppAccepted() {
// app could have allocated tasks upon a recovery, and in that case,
// the reserving phase has already passed, no need to trigger that again.
var ev events.SchedulingEvent
+ log.Logger().Info("postAppAccepted on cached app",
+ zap.String("appID", app.applicationID),
+ zap.Int("numTaskGroups", len(app.taskGroups)),
+ zap.Int("numAllocatedTasks", len(app.getTasks(events.States().Task.Allocated))))
Review comment:
Yeah makes sense. I'll update to DEBUG level
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d0905d7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.45%`.
> The diff coverage is `61.13%`.
> :exclamation: Current head d0905d7 differs from pull request most recent head 39090bd. Consider uploading reports for the commit 39090bd to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.20% +0.45%
==========================================
Files 35 36 +1
Lines 3133 3302 +169
==========================================
+ Hits 1872 1988 +116
- Misses 1180 1233 +53
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...39090bd](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613313358
##########
File path: deployments/image/configmap/Dockerfile
##########
@@ -21,7 +21,7 @@ FROM alpine:latest
RUN apk add curl
RUN apk add jq
RUN apk add --update openssl
-RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.15.12/bin/linux/amd64/kubectl
+RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.18.16/bin/linux/amd64/kubectl
Review comment:
Sure. Rolled back
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-643] Improve the spark-k8s-operator app mgmt plugin to better handling Spark CRDs
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (554dc46) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.41%`.
> The diff coverage is `61.29%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.17% +0.41%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1982 +110
- Misses 1180 1232 +52
+ Partials 81 80 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...554dc46](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d0905d7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.45%`.
> The diff coverage is `61.13%`.
> :exclamation: Current head d0905d7 differs from pull request most recent head 105b043. Consider uploading reports for the commit 105b043 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 60.20% +0.45%
==========================================
Files 35 36 +1
Lines 3133 3302 +169
==========================================
+ Hits 1872 1988 +116
- Misses 1180 1233 +53
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...105b043](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yuchaoran2011 commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yuchaoran2011 commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613679694
##########
File path: pkg/appmgmt/sparkoperator/spark.go
##########
@@ -23,25 +23,14 @@ import (
crcClientSet "github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/client/clientset/versioned"
crInformers "github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/client/informers/externalversions"
"go.uber.org/zap"
- "k8s.io/api/core/v1"
- "k8s.io/apimachinery/pkg/labels"
k8sCache "k8s.io/client-go/tools/cache"
"github.com/apache/incubator-yunikorn-k8shim/pkg/appmgmt/interfaces"
"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
- "github.com/apache/incubator-yunikorn-scheduler-interface/lib/go/si"
)
-const (
- LabelAnnotationPrefix = "sparkoperator.k8s.io/"
- SparkAppNameLabel = LabelAnnotationPrefix + "app-name"
-)
-
-// implements interfaces#Recoverable, interfaces#AppManager
+// Manager implements interfaces#Recoverable, interfaces#AppManager
Review comment:
I added more comments in this file to explain what the plugin does
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r613648898
##########
File path: deployments/image/configmap/Dockerfile
##########
@@ -46,8 +46,8 @@ ENV DISPATCHER_TIMEOUT "300s"
ENV KUBE_CLIENT_QPS "1000"
ENV KUBE_CLIENT_BURST "1000"
ENV PREDICATES ""
-ENV OPERATOR_PLUGINS "general"
ENV ENABLE_CONFIG_HOT_REFRESH "true"
+ENV OPERATOR_PLUGINS "general,spark-k8s-operator"
Review comment:
this change will build the docker image by default with spark-k8s-operator plugin enabled.
We might want to provide an option instead of doing this by default. I think we can add the env var `OPERATOR_PLUGINS` in the helm chart template: https://github.com/apache/incubator-yunikorn-release/blob/2ade62ac43f9f62f804739f418fcdcb256b9703b/helm-charts/yunikorn/templates/deployment.yaml#L55. And expose this in https://github.com/apache/incubator-yunikorn-release/blob/master/helm-charts/yunikorn/values.yaml.
##########
File path: go.mod
##########
@@ -21,7 +21,7 @@ module github.com/apache/incubator-yunikorn-k8shim
go 1.12
require (
- github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20200817155620-c19d2b8660d8
+ github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20201215015655-2e8b733f5ad0
Review comment:
which version of spark-k8s-operator this line point to?
I think we need to point to the most stable released version
##########
File path: pkg/cache/application.go
##########
@@ -460,9 +461,14 @@ func (app *Application) postAppAccepted() {
// app could have allocated tasks upon a recovery, and in that case,
// the reserving phase has already passed, no need to trigger that again.
var ev events.SchedulingEvent
+ log.Logger().Info("postAppAccepted on cached app",
+ zap.String("appID", app.applicationID),
+ zap.Int("numTaskGroups", len(app.taskGroups)),
+ zap.Int("numAllocatedTasks", len(app.getTasks(events.States().Task.Allocated))))
if len(app.taskGroups) != 0 &&
len(app.getTasks(events.States().Task.Allocated)) == 0 {
ev = NewSimpleApplicationEvent(app.applicationID, events.TryReserve)
+ log.Logger().Info("tryReserve")
Review comment:
minor: pls improve this line of logging
something like: "app <appID> has taskGroups defined, trying to reserve resources for the gang members"
##########
File path: pkg/cache/application.go
##########
@@ -460,9 +461,14 @@ func (app *Application) postAppAccepted() {
// app could have allocated tasks upon a recovery, and in that case,
// the reserving phase has already passed, no need to trigger that again.
var ev events.SchedulingEvent
+ log.Logger().Info("postAppAccepted on cached app",
+ zap.String("appID", app.applicationID),
+ zap.Int("numTaskGroups", len(app.taskGroups)),
+ zap.Int("numAllocatedTasks", len(app.getTasks(events.States().Task.Allocated))))
Review comment:
this seems more like a DEBUG level logging
any reason why we want to expose this in INFO level?
##########
File path: pkg/appmgmt/sparkoperator/spark.go
##########
@@ -23,25 +23,14 @@ import (
crcClientSet "github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/client/clientset/versioned"
crInformers "github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/client/informers/externalversions"
"go.uber.org/zap"
- "k8s.io/api/core/v1"
- "k8s.io/apimachinery/pkg/labels"
k8sCache "k8s.io/client-go/tools/cache"
"github.com/apache/incubator-yunikorn-k8shim/pkg/appmgmt/interfaces"
"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
- "github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
- "github.com/apache/incubator-yunikorn-scheduler-interface/lib/go/si"
)
-const (
- LabelAnnotationPrefix = "sparkoperator.k8s.io/"
- SparkAppNameLabel = LabelAnnotationPrefix + "app-name"
-)
-
-// implements interfaces#Recoverable, interfaces#AppManager
+// Manager implements interfaces#Recoverable, interfaces#AppManager
Review comment:
minor: suggest to add some more code comment to explain what has been done by this plugin
##########
File path: pkg/common/events/events.go
##########
@@ -33,20 +33,20 @@ type SchedulingEvent interface {
type ApplicationEventType string
const (
- SubmitApplication ApplicationEventType = "SubmitApplication"
- RecoverApplication ApplicationEventType = "RecoverApplication"
- AcceptApplication ApplicationEventType = "AcceptApplication"
- TryReserve ApplicationEventType = "TryReserve"
- UpdateReservation ApplicationEventType = "UpdateReservation"
- RunApplication ApplicationEventType = "RunApplication"
- RejectApplication ApplicationEventType = "RejectApplication"
- CompleteApplication ApplicationEventType = "CompleteApplication"
- FailApplication ApplicationEventType = "FailApplication"
- KillApplication ApplicationEventType = "KillApplication"
- KilledApplication ApplicationEventType = "KilledApplication"
- ReleaseAppAllocation ApplicationEventType = "ReleaseAppAllocation"
+ SubmitApplication ApplicationEventType = "SubmitApplication"
+ RecoverApplication ApplicationEventType = "RecoverApplication"
+ AcceptApplication ApplicationEventType = "AcceptApplication"
+ TryReserve ApplicationEventType = "TryReserve"
+ UpdateReservation ApplicationEventType = "UpdateReservation"
+ RunApplication ApplicationEventType = "RunApplication"
+ RejectApplication ApplicationEventType = "RejectApplication"
+ CompleteApplication ApplicationEventType = "CompleteApplication"
+ FailApplication ApplicationEventType = "FailApplication"
+ KillApplication ApplicationEventType = "KillApplication"
+ KilledApplication ApplicationEventType = "KilledApplication"
+ ReleaseAppAllocation ApplicationEventType = "ReleaseAppAllocation"
ReleaseAppAllocationAsk ApplicationEventType = "ReleaseAppAllocationAsk"
- AppStateChange ApplicationEventType = "ApplicationStateChange"
+ AppStateChange ApplicationEventType = "ApplicationStateChange"
Review comment:
do we have any change here other than formating?
if not, prefer not to change this in this PR
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (105b043) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.11%`.
> The diff coverage is `61.29%`.
> :exclamation: Current head 105b043 differs from pull request most recent head 7b814eb. Consider uploading reports for the commit 7b814eb to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 59.86% +0.11%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1972 +100
- Misses 1180 1241 +61
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...7b814eb](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#discussion_r612945681
##########
File path: go.mod
##########
@@ -18,20 +18,20 @@
module github.com/apache/incubator-yunikorn-k8shim
-go 1.12
+go 1.14
require (
- github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20200817155620-c19d2b8660d8
- github.com/apache/incubator-yunikorn-core v0.0.0-20210308173435-79a60272f12f
- github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20210226143918-19a5cca5e428
+ github.com/GoogleCloudPlatform/spark-on-k8s-operator v0.0.0-20201215015655-2e8b733f5ad0
+ github.com/apache/incubator-yunikorn-core v0.0.0-20210412120202-73fdc11674a5
+ github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20210412034924-44e8abeff79e
github.com/google/uuid v1.1.1
github.com/gorilla/mux v1.7.3
github.com/looplab/fsm v0.1.0
- github.com/onsi/ginkgo v1.11.0
- github.com/onsi/gomega v1.7.0
- go.uber.org/zap v1.13.0
+ github.com/onsi/ginkgo v1.15.1
+ github.com/onsi/gomega v1.11.0
+ go.uber.org/zap v1.16.0
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
- gopkg.in/yaml.v2 v2.2.8
+ gopkg.in/yaml.v2 v2.4.0
Review comment:
We should not just move dependencies unless we require these changes to be made for the code to work.
I can understand the spark operator update but we should leave the rest in this change
##########
File path: deployments/image/configmap/Dockerfile
##########
@@ -21,7 +21,7 @@ FROM alpine:latest
RUN apk add curl
RUN apk add jq
RUN apk add --update openssl
-RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.15.12/bin/linux/amd64/kubectl
+RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/v1.18.16/bin/linux/amd64/kubectl
Review comment:
We need to be careful with this change. It might break 1.15 support which we have not removed yet.
Please roll back.
##########
File path: go.mod
##########
@@ -18,20 +18,20 @@
module github.com/apache/incubator-yunikorn-k8shim
-go 1.12
+go 1.14
Review comment:
Changing to `go 1.14` means we require at least that version to compile. The makefile checks this. We need to announce that before we push this and make sure we have fully tested it.
Moving the version has side effects, see my comments in [YUNIKORN-397](https://issues.apache.org/jira/browse/YUNIKORN-397)
##########
File path: pkg/cache/application.go
##########
@@ -460,9 +461,14 @@ func (app *Application) postAppAccepted() {
// app could have allocated tasks upon a recovery, and in that case,
// the reserving phase has already passed, no need to trigger that again.
var ev events.SchedulingEvent
+ log.Logger().Info("postAppAccepted",
Review comment:
Please clarify that this is from a cached app while post processing or something like that.
##########
File path: .gitignore
##########
@@ -1,3 +1,4 @@
.idea
_output/
+vendor
Review comment:
Nothing generates or should generate a vendor directory in the build. Why do we need this?
##########
File path: pkg/appmgmt/general/general.go
##########
@@ -34,6 +33,8 @@ import (
"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
"github.com/apache/incubator-yunikorn-scheduler-interface/lib/go/si"
+
+ "go.uber.org/zap"
Review comment:
Not sure why we need this change...
##########
File path: pkg/appmgmt/appmgmt.go
##########
@@ -47,7 +47,10 @@ func NewAMService(amProtocol interfaces.ApplicationManagementProtocol,
managers: make([]interfaces.AppManager, 0),
}
+ log.Logger().Info("Initializing new AppMgmt service")
+
if !apiProvider.IsTestingMode() {
+ log.Logger().Info("Registered Spark operator with the AppMgmt service")
Review comment:
Looking at the register call below we do more than the Spark operator.
NIT: until we return from the register call the work has not been done yet (registered vs registering).
##########
File path: pkg/appmgmt/sparkoperator/spark.go
##########
@@ -51,6 +40,7 @@ type Manager struct {
}
func NewManager(amProtocol interfaces.ApplicationManagementProtocol, apiProvider client.APIProvider) *Manager {
+ log.Logger().Info("New AppMgr initialized")
Review comment:
Isn't the app manager initialised in `ServiceInit()` below?
You log almost the same message in that call.
##########
File path: pkg/appmgmt/general/general.go
##########
@@ -301,12 +311,14 @@ func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata
// get existing apps
existingApps := make(map[string]interfaces.ApplicationMetadata)
- if appPods != nil && len(appPods) > 0 {
+ if len(appPods) > 0 {
Review comment:
range can handle a nil input so there is no need to check at all.
##########
File path: pkg/appmgmt/sparkoperator/spark.go
##########
@@ -105,125 +87,26 @@ func (os *Manager) Stop() {
os.stopCh <- struct{}{}
}
-func (os *Manager) getTaskMetadata(pod *v1.Pod) (interfaces.TaskMetadata, bool) {
- // spark executors are having a common label
- if appName, ok := pod.Labels[SparkAppNameLabel]; ok {
- return interfaces.TaskMetadata{
- ApplicationID: appName,
- TaskID: string(pod.UID),
- Pod: pod,
- }, true
- }
- return interfaces.TaskMetadata{}, false
-}
-
-func (os *Manager) getAppMetadata(sparkApp *v1beta2.SparkApplication) interfaces.ApplicationMetadata {
- // extract tags from annotations
- tags := make(map[string]string)
- for annotationKey, annotationValue := range sparkApp.GetAnnotations() {
- tags[annotationKey] = annotationValue
- }
-
- // set queue name if app labels it
- queueName := constants.ApplicationDefaultQueue
- if an, ok := sparkApp.Labels[constants.LabelQueueName]; ok {
- queueName = an
- }
-
- // retrieve the namespace info from the CRD
- tags[constants.AppTagNamespace] = sparkApp.Namespace
-
- return interfaces.ApplicationMetadata{
- ApplicationID: sparkApp.Name,
- QueueName: queueName,
- User: "default",
- Tags: tags,
- }
-}
-
-// list all existing applications
-func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata, error) {
- lister := os.crdInformerFactory.Sparkoperator().V1beta2().SparkApplications().Lister()
- sparkApps, err := lister.List(labels.Everything())
- if err != nil {
- return nil, err
- }
-
- existingApps := make(map[string]interfaces.ApplicationMetadata)
- for _, sparkApp := range sparkApps {
- existingApps[sparkApp.Name] = os.getAppMetadata(sparkApp)
- }
- return existingApps, nil
-}
-
-func (os *Manager) GetExistingAllocation(pod *v1.Pod) *si.Allocation {
- if meta, valid := os.getTaskMetadata(pod); valid {
- return &si.Allocation{
- AllocationKey: string(pod.UID),
- AllocationTags: nil,
- UUID: string(pod.UID),
- ResourcePerAlloc: common.GetPodResource(pod),
- QueueName: utils.GetQueueNameFromPod(pod),
- NodeID: pod.Spec.NodeName,
- ApplicationID: meta.ApplicationID,
- PartitionName: constants.DefaultPartition,
- }
- }
- return nil
-}
-
-// callbacks for SparkApplication CRD
-func (os *Manager) addApplication(obj interface{}) {
- app := obj.(*v1beta2.SparkApplication)
- log.Logger().Info("spark app added", zap.Any("SparkApplication", app))
- os.amProtocol.AddApplication(&interfaces.AddApplicationRequest{
- Metadata: os.getAppMetadata(app),
- })
-}
-
func (os *Manager) updateApplication(old, new interface{}) {
appOld := old.(*v1beta2.SparkApplication)
appNew := new.(*v1beta2.SparkApplication)
+ currState := appNew.Status.AppState.State
log.Logger().Debug("spark app updated",
zap.Any("old", appOld),
- zap.Any("old", appNew))
+ zap.Any("new", appNew))
+ log.Logger().Debug("spark app state",
+ zap.String("current state", string(currState)))
Review comment:
please combine the two log entries into one
##########
File path: pkg/cache/placeholder_manager.go
##########
@@ -46,10 +47,13 @@ type PlaceholderManager struct {
}
var placeholderMgr *PlaceholderManager
+var placeholderManagerLock sync.Mutex
Review comment:
The init and usage of the manager has not changed. Having a lock like this seems weird because we should only ever the one that was created by the current test, or when the context is created in production.
This probably also explains why you needed the nil check in the app code before.
This is a test problem. By adding the manager usage to the fail application test we are re-using the manager from a previous test. We should not be doing that. Please check the way `TestTryReserve()` and `TestTryReservePostRestart()` initiate the manager.
It looks like the test context does not create a manager and we might not clean up correctly either
##########
File path: pkg/cache/application.go
##########
@@ -511,11 +517,24 @@ func (app *Application) handleRejectApplicationEvent(event *fsm.Event) {
fmt.Sprintf("application %s is rejected by scheduler", app.applicationID)))
}
+func placeholderCleanup(app *Application) {
+ placeholderManager := getPlaceholderManager()
+ if placeholderManager != nil {
+ placeholderManager.cleanUp(app)
+ }
+}
+
Review comment:
`getPlaceholderManager()` cannot return a nil we can simplify this by directly calling this inside the go function while handling the events:
```
go func() {
getPlaceholderManager().cleanup(app)
}
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #253: [YUNIKORN-558] Integration with Spark K8s Operator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #253:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/253#issuecomment-819214180
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#253](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (105b043) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/c47ed51f075c5af5910f71da40e7e68699a9abae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c47ed51) will **increase** coverage by `0.11%`.
> The diff coverage is `61.29%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/graphs/tree.svg?width=650&height=150&src=pr&token=LZImIuvleR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 59.75% 59.86% +0.11%
==========================================
Files 35 36 +1
Lines 3133 3294 +161
==========================================
+ Hits 1872 1972 +100
- Misses 1180 1241 +61
Partials 81 81
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/appmgmt/appmgmt\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2FwcG1nbXQvYXBwbWdtdF9yZWNvdmVyeS5nbw==) | `67.50% <0.00%> (-8.18%)` | :arrow_down: |
| [pkg/cache/amprotocol\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FtcHJvdG9jb2xfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXJfbW9jay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9yZXNvdXJjZS5nbw==) | `90.72% <0.00%> (-9.28%)` | :arrow_down: |
| [pkg/common/utils/gang\_utils.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi91dGlscy9nYW5nX3V0aWxzLmdv) | `65.43% <0.00%> (-16.11%)` | :arrow_down: |
| [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbnRyb2xsZXIvYXBwbGljYXRpb24vYXBwX2NvbnRyb2xsZXIuZ28=) | `71.05% <ø> (-0.26%)` | :arrow_down: |
| [...missioncontrollers/webhook/admission\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3BsdWdpbi9hZG1pc3Npb25jb250cm9sbGVycy93ZWJob29rL2FkbWlzc2lvbl9jb250cm9sbGVyLmdv) | `33.74% <0.00%> (+1.00%)` | :arrow_up: |
| [pkg/cache/application\_events.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2V2ZW50cy5nbw==) | `55.55% <31.25%> (+2.49%)` | :arrow_up: |
| [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbW1vbi9ldmVudHMvcmVjb3JkZXIuZ28=) | `33.33% <40.00%> (+3.33%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `39.72% <45.45%> (-1.36%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [adf66f9...105b043](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/253?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org