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/07/12 06:51:12 UTC

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 opened a new pull request #281: Yunikorn 706

0yukali0 opened a new pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281


   ### What is this PR for?
   Original function will miss initcontainers requirement in podspec when pod has initcontainers and containers.
   The reason is that resources calculation function only check resources requirement of containers in pod.
   I modify resource calculation in task which use function in common package to confirm it.
   
   
   ### What type of PR is it?
   * [x] - Bug Fix
   * [ ] - Improvement
   * [ ] - Feature
   * [ ] - Documentation
   * [ ] - Hot Fix
   * [ ] - Refactoring
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/YUNIKORN-706
   
   ### How should this be tested?
   I add test case to check calculation correctly.
   Use make test to check.
   ### Screenshots (if appropriate)
   
   ### Questions:
   * [ ] - The licenses files need update.
   * [ ] - There is breaking changes for older versions.
   * [ ] - It needs documentation.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676649645



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]

Review comment:
       Got 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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d785616) 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 `3.34%`.
   > The diff coverage is `76.35%`.
   
   > :exclamation: Current head d785616 differs from pull request most recent head 1eae00a. Consider uploading reports for the commit 1eae00a to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.09%   +3.34%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3447     +314     
   ==========================================
   + Hits         1872     2175     +303     
   - Misses       1180     1186       +6     
   - Partials       81       86       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...1eae00a](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686495224



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       Does this mean we skip a request that is not defined on the container resources? That does not look right. 
   cont = 1GB, 1cpu
   init cont = 2GB, 1GPU
   the result should be 2GB, 1cpu, 1GPU a merge of all requests.

##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {

Review comment:
       Can we rename this function? something like `checkInitContainerRequest()`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1eae00a) 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 `3.41%`.
   > The diff coverage is `76.65%`.
   
   > :exclamation: Current head 1eae00a differs from pull request most recent head 314c6ca. Consider uploading reports for the commit 314c6ca to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.16%   +3.41%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3453     +320     
   ==========================================
   + Hits         1872     2181     +309     
   - Misses       1180     1187       +7     
   - Partials       81       85       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...314c6ca](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d785616) 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 `3.34%`.
   > The diff coverage is `76.35%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.09%   +3.34%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3447     +314     
   ==========================================
   + Hits         1872     2175     +303     
   - Misses       1180     1186       +6     
   - Partials       81       86       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d785616](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676621858



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       v1 is the value of thee init resource. If the pod has init resource is it ok to just replace it, or we should add its value to the pods resources. How is the resource calculated in this case? For me it seems more logical to add them up instead of just replacing it. Please double check this.

##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]

Review comment:
       Please use more self explanatory parameter naming than v1 and v2. 

##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {

Review comment:
       We can keep this method private (`isNeedMoreResourceAndSet`) since it is used only from this place




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676701331



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       Thank you for advise, i will check object information in cache of core-scheduler to make sure it right.
   This code idea is from k8s default scheduler.
   k8s default scheduler make assigned node has enough allocated resource to handle max resource requirement in pod.
   Calculation function: podResourceRequest = max(sum(podSpec.Containers), podSpec.InitContainers) + overHead
   
   k8s doc of initcontainer describe requirement of resource usage.
   related doc:
   https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resources
   
   It show up in default scheduler predicate and score.
   prefilter and filter use the calculation function to check if node has enough allocated resource for max requirement in pod.
   related code:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/framework/plugins/noderesources/fit.go#L135-L179
   https://github.com/kubernetes/kubernetes/blob/eae87bfe7e9636694a5cb45a9863715b7b25435c/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go#L108-L135




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676701331



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       Thank you for advise, i will modify it with replace.
   This code idea is from k8s default scheduler.
   k8s default scheduler make assigned node has enough allocated resource to handle max resource requirement in pod.
   Calculation function: podResourceRequest = max(sum(podSpec.Containers), podSpec.InitContainers) + overHead
   
   k8s doc of initcontainer describe requirement of resource usage.
   related doc:
   https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resources
   
   It show up in default scheduler predicate and score.
   prefilter and filter use the calculation function to check if node has enough allocated resource for max requirement in pod.
   related code:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/framework/plugins/noderesources/fit.go#L135-L179
   https://github.com/kubernetes/kubernetes/blob/eae87bfe7e9636694a5cb45a9863715b7b25435c/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go#L108-L135




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686504783



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       Ok




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] codecov[bot] commented on pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (287407e) 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 `3.31%`.
   > The diff coverage is `76.15%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.06%   +3.31%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3441     +308     
   ==========================================
   + Hits         1872     2170     +298     
   - Misses       1180     1185       +5     
   - Partials       81       86       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.78% <0.00%> (-0.56%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [26 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...287407e](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-891050726


   Thanks, i will fix it.
   
   Wilfred Spiegelenburg ***@***.***> 於 2021年8月2日 週一 下午7:24寫道：
   
   > Looking at the code and the tests I think there is a problem with the
   > calculation.
   > This is the base you gave:
   >
   > 	// pod
   > 	// initcontainers
   > 	// IC1{500mi, 1000m, 1}
   > 	// IC2{1024mi, 2000m, 4}
   > 	// containers
   > 	// C1{4096mi, 2000m, 2}
   > 	// C2{1024mi, 5000m, 2}
   > 	// result is {4096mi, 5000m, 5}
   >
   > There is a three step process:
   >
   >    1. Get the maximum for the init containers. They real serially which
   >    means that I just need to make sure that I can accomodate the largest
   >    request for each resource type.
   >    That gives me in this case cpu: 1024mi, memory 2000m and gpu 4. In
   >    this specific case it all maps to IC2. IC1 has all types smaller than IC2.
   >    2. Get the maximum usage for the real containers. They run in parallel
   >    which means I need to sum up all the resources used for all containers.
   >    The total usage of all regular containers is: cpu: 5120mi, memory
   >    7000m and gpu 4.
   >    3. Calculate the maximum number of resources needed in the startup
   >    phase for the init containers and the normal process:
   >
   > init cpu: 1024mi, normal cpu: 5120mi --> cpu: 5120mi
   > init memory 2000m, normal memory 7000m --> memory 7000m
   > init gpu 4, normal gpu 4 --> gpu 4
   >
   > The request for this specific pod passed on by the shim to the core is
   > cpu: 5120mi, memory 7000m, gpu 4
   >
   > If for example the init container IC2 would request 10000m for memory and
   > 0 (zero) gpu the request would become: 5120mi, memory 10000m, gpu 4. CPU
   > does not change: sum of C1 and C2, memory is now based on IC2 max, gpu
   > remains at 4 as init and regular containers used the same.
   >
   > —
   > You are receiving this because you were assigned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-890948140>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AK6DJMDCEY63K6V6J6DFPTTT2Z6ARANCNFSM5AGGMH5A>
   > .
   >
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676649383



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {

Review comment:
       Ok




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686502850



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       I will add test for this situation in next commit.
   In my thought, it will be like following
   loop1 men:2GB > 1GB replace.
   loop2 GPU is not found in cont request, additional request add.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] wilfred-s closed pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686676341



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       Thanks @wilfred-s.
   Additional resource from init cont  also add into request in next commit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1eae00a) 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 `3.41%`.
   > The diff coverage is `76.65%`.
   
   > :exclamation: Current head 1eae00a differs from pull request most recent head d785616. Consider uploading reports for the commit d785616 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.16%   +3.41%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3453     +320     
   ==========================================
   + Hits         1872     2181     +309     
   - Misses       1180     1187       +7     
   - Partials       81       85       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d785616](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1eae00a) 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 `3.41%`.
   > The diff coverage is `76.65%`.
   
   > :exclamation: Current head 1eae00a differs from pull request most recent head d785616. Consider uploading reports for the commit d785616 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.16%   +3.41%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3453     +320     
   ==========================================
   + Hits         1872     2181     +309     
   - Misses       1180     1187       +7     
   - Partials       81       85       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d785616](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] wilfred-s edited a comment on pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
wilfred-s edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-890948140


   Looking at the code and the tests I think there is a problem with the calculation.
   This is the base you gave:
   ```
   	// pod
   	// initcontainers
   	// IC1{500mi, 1000m, 1}
   	// IC2{1024mi, 2000m, 4}
   	// containers
   	// C1{4096mi, 2000m, 2}
   	// C2{1024mi, 5000m, 2}
   	// result is {4096mi, 5000m, 5}
   ```
   There is a three step process:
   1) Get the maximum for the init containers. They real serially which means that I just need to make sure that I can accomodate the largest request for each resource type.
   That gives me in this case cpu: 1024mi, memory 2000m and gpu 4. In this specific case it all maps to IC2. IC1 has all types smaller than IC2.
   2) Get the maximum usage for the real containers. They run in parallel which means I need to sum up all the resources used for all containers.
   The total usage of all regular containers is:  cpu: 5120mi, memory 7000m and gpu 4.
   3) Calculate the maximum number of resources needed in the startup phase for the init containers and the normal process:
   ```
   init cpu: 1024mi, normal cpu: 5120mi --> cpu: 5120mi
   init memory 2000m, normal memory 7000m --> memory 7000m
   init gpu 4, normal gpu 4 --> gpu 4
   ```
   
   The request for this specific pod passed on by the shim to the core is cpu: 5120mi, memory 7000m, gpu 4
   
   If for example the init container IC2 would request 10000m for memory and 0 (zero) gpu the request would become: cpu 5120mi, memory 10000m, gpu 4. CPU does not change: sum of C1 and C2, memory is now based on IC2 max, gpu remains at 4 as init and regular containers used the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1eae00a) 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 `3.41%`.
   > The diff coverage is `76.65%`.
   
   > :exclamation: Current head 1eae00a differs from pull request most recent head d4bcbad. Consider uploading reports for the commit d4bcbad to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.16%   +3.41%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3453     +320     
   ==========================================
   + Hits         1872     2181     +309     
   - Misses       1180     1187       +7     
   - Partials       81       85       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d4bcbad](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (287407e) 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 `3.31%`.
   > The diff coverage is `76.15%`.
   
   > :exclamation: Current head 287407e differs from pull request most recent head d785616. Consider uploading reports for the commit d785616 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.06%   +3.31%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3441     +308     
   ==========================================
   + Hits         1872     2170     +298     
   - Misses       1180     1185       +5     
   - Partials       81       86       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.78% <0.00%> (-0.56%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [26 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d785616](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686502850



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       I will add test for this situation in next commit.
   In my thought, it should be like following
   loop1 men:2GB > 1GB replace.
   loop2 GPU is not found in cont request, additional request add.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676701331



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       Thank you for advise, i will modify it via replacing.
   This code idea is from k8s default scheduler.
   k8s default scheduler make assigned node has enough allocated resource to handle max resource requirement in pod.
   Calculation function: podResourceRequest = max(sum(podSpec.Containers), podSpec.InitContainers) + overHead
   in this case, Result: CPU: 3, Memory: 3G
    Pod:
      InitContainers
        IC1:
          CPU: 2
          Memory: 1G
        IC2:
          CPU: 2
          Memory: 3G
      Containers
        C1:
          CPU: 2
          Memory: 1G
        C2:
          CPU: 1
          Memory: 1G
   
   k8s doc of initcontainer describe requirement of resource usage.
   related doc:
   https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resources
   
   It show up in default scheduler predicate and score.
   prefilter and filter use the calculation function to check if node has enough allocated resource for max requirement in pod.
   related code:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/framework/plugins/noderesources/fit.go#L135-L179
   https://github.com/kubernetes/kubernetes/blob/eae87bfe7e9636694a5cb45a9863715b7b25435c/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go#L108-L135




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4bcbad) 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 `3.27%`.
   > The diff coverage is `76.35%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.02%   +3.27%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3448     +315     
   ==========================================
   + Hits         1872     2173     +301     
   - Misses       1180     1187       +7     
   - Partials       81       88       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [29 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...d4bcbad](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d4bcbad) 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 `3.27%`.
   > The diff coverage is `76.35%`.
   
   > :exclamation: Current head d4bcbad differs from pull request most recent head 8b400f6. Consider uploading reports for the commit 8b400f6 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.02%   +3.27%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3448     +315     
   ==========================================
   + Hits         1872     2173     +301     
   - Misses       1180     1187       +7     
   - Partials       81       88       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/resource.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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==) | `89.62% <38.88%> (-10.38%)` | :arrow_down: |
   | ... and [29 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...8b400f6](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686498066



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {

Review comment:
       ok, i wiil rename it in next commit




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676701331



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       Thank you for advise, i will modify it via replacing.
   This code idea is from k8s default scheduler.
   k8s default scheduler make assigned node has enough allocated resource to handle max resource requirement in pod.
   Calculation function: podResourceRequest = max(sum(podSpec.Containers), podSpec.InitContainers) + overHead
   
   k8s doc of initcontainer describe requirement of resource usage.
   related doc:
   https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resources
   
   It show up in default scheduler predicate and score.
   prefilter and filter use the calculation function to check if node has enough allocated resource for max requirement in pod.
   related code:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/framework/plugins/noderesources/fit.go#L135-L179
   https://github.com/kubernetes/kubernetes/blob/eae87bfe7e9636694a5cb45a9863715b7b25435c/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go#L108-L135




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r676701331



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,31 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace(choose bigger one)
+	if pod.Spec.InitContainers != nil {
+		IsNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func IsNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		initCResource := getResource(resourceList)
+		for resouceName, v1 := range initCResource.Resources {
+			v2, exist := containersResources.Resources[resouceName]
+			if !exist {
+				continue
+			}
+			if v1.GetValue() > v2.GetValue() {
+				containersResources.Resources[resouceName] = v1

Review comment:
       Thank you for advise, i will check object information in cache of core-scheduler to make sure it right.
   This code idea is from k8s default scheduler.
   k8s default scheduler make assigned node has enough allocated resource to handle max resource requirement in pod.
   Calculation function: podResourceRequest = max(sum(podSpec.Containers), podSpec.InitContainers) + overHead
   
   k8s doc of initcontainer describe requirement of resource usage.
   related doc:
   https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resources
   
   It show up in default scheduler predicate and score.
   prefilter and filter use the calculation function to check if node has enough allocated resource for maximum of pod requirement.
   related code:
   https://github.com/kubernetes/kubernetes/blob/master/pkg/scheduler/framework/plugins/noderesources/fit.go#L135-L179
   https://github.com/kubernetes/kubernetes/blob/eae87bfe7e9636694a5cb45a9863715b7b25435c/pkg/scheduler/framework/plugins/noderesources/resource_allocation.go#L108-L135




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8b400f6) 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 `3.33%`.
   > The diff coverage is `76.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.09%   +3.33%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3449     +316     
   ==========================================
   + Hits         1872     2176     +304     
   - Misses       1180     1186       +6     
   - Partials       81       87       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [29 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...8b400f6](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
wilfred-s commented on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-890948140


   Looking at the code and the tests I think there is a problem with the calculation.
   This is the base you gave:
   ```
   	// pod
   	// initcontainers
   	// IC1{500mi, 1000m, 1}
   	// IC2{1024mi, 2000m, 4}
   	// containers
   	// C1{4096mi, 2000m, 2}
   	// C2{1024mi, 5000m, 2}
   	// result is {4096mi, 5000m, 5}
   ```
   There is a three step process:
   1) Get the maximum for the init containers. They real serially which means that I just need to make sure that I can accomodate the largest request for each resource type.
   That gives me in this case cpu: 1024mi, memory 2000m and gpu 4. In this specific case it all maps to IC2. IC1 has all types smaller than IC2.
   2) Get the maximum usage for the real containers. They run in parallel which means I need to sum up all the resources used for all containers.
   The total usage of all regular containers is:  cpu: 5120mi, memory 7000m and gpu 4.
   3) Calculate the maximum number of resources needed in the startup phase for the init containers and the normal process:
   ```
   init cpu: 1024mi, normal cpu: 5120mi --> cpu: 5120mi
   init memory 2000m, normal memory 7000m --> memory 7000m
   init gpu 4, normal gpu 4 --> gpu 4
   ```
   
   The request for this specific pod passed on by the shim to the core is cpu: 5120mi, memory 7000m, gpu 4
   
   If for example the init container IC2 would request 10000m for memory and 0 (zero) gpu the request would become: 5120mi, memory 10000m, gpu 4. CPU does not change: sum of C1 and C2, memory is now based on IC2 max, gpu remains at 4 as init and regular containers used the same.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-yunikorn-k8shim] 0yukali0 commented on a change in pull request #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
0yukali0 commented on a change in pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#discussion_r686502850



##########
File path: pkg/common/resource.go
##########
@@ -72,9 +72,32 @@ func GetPodResource(pod *v1.Pod) (resource *si.Resource) {
 		containerResource := getResource(resourceList)
 		podResource = Add(podResource, containerResource)
 	}
+
+	// vcore, mem compare between initcontainer and containers and replace
+	// For each resource, podResourceRequest = max(sum(Containers requirement), InitContainers requirement)
+	if pod.Spec.InitContainers != nil {
+		isNeedMoreResourceAndSet(pod, podResource)
+	}
+
 	return podResource
 }
 
+func isNeedMoreResourceAndSet(pod *v1.Pod, containersResources *si.Resource) {
+	for _, c := range pod.Spec.InitContainers {
+		resourceList := c.Resources.Requests
+		ICResource := getResource(resourceList)
+		for resourceName, ICRequest := range ICResource.Resources {
+			containersRequests, exist := containersResources.Resources[resourceName]

Review comment:
       I will add test for this situation in next commit.
   In my thought, it should be like following
   loop1 men:2GB > 1GB replace.
   loop2 GPU is not found in cont request, additional request add.
   the result should be 2GB,1 cpu, 1GPU




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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 #281: [YUNIKONR-706] Fix the resource calculation when the pod has init-containers

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #281:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/281#issuecomment-878248140


   # [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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 [#281](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1eae00a) 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 `3.41%`.
   > The diff coverage is `76.65%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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     #281      +/-   ##
   ==========================================
   + Coverage   59.75%   63.16%   +3.41%     
   ==========================================
     Files          35       37       +2     
     Lines        3133     3453     +320     
   ==========================================
   + Hits         1872     2181     +309     
   - Misses       1180     1187       +7     
   - Partials       81       85       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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/281/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/281/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/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `45.23% <0.00%> (-1.11%)` | :arrow_down: |
   | [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NhY2hlL25vZGVzLmdv) | `79.80% <ø> (ø)` | |
   | [pkg/common/events/recorder\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/events/states.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL2NvbW1vbi9ldmVudHMvc3RhdGVzLmdv) | `0.00% <0.00%> (ø)` | |
   | [pkg/controller/application/app\_controller.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281/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/shim/main.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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-cGtnL3NoaW0vbWFpbi5nbw==) | `0.00% <ø> (ø)` | |
   | [pkg/common/events/recorder.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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: |
   | ... and [28 more](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281/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/281?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/281?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 [5658ce3...1eae00a](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/281?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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org