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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
