wilfred-s commented on code in PR #810:
URL: https://github.com/apache/yunikorn-k8shim/pull/810#discussion_r1550882867
##########
pkg/cache/context.go:
##########
@@ -792,45 +792,43 @@ func (ctx *Context) AssumePod(name string, node string)
error {
// assume pod volumes before assuming the pod
// this will update scheduler cache with essential
PV/PVC binding info
var allBound = true
- // volume builder might be null in UTs
- if ctx.apiProvider.GetAPIs().VolumeBinder != nil {
- var err error
- // retrieve the volume claims
- podVolumeClaims, err :=
ctx.apiProvider.GetAPIs().VolumeBinder.GetPodVolumeClaims(ctx.klogger, pod)
- if err != nil {
- log.Log(log.ShimContext).Error("Failed
to get pod volume claims",
- zap.String("podName",
assumedPod.Name),
- zap.Error(err))
- return err
- }
+ var err error
+ // retrieve the volume claims
+ podVolumeClaims, err :=
ctx.apiProvider.GetAPIs().VolumeBinder.GetPodVolumeClaims(ctx.klogger, pod)
+ if err != nil {
+ log.Log(log.ShimContext).Error("Failed to get
pod volume claims",
+ zap.String("podName", assumedPod.Name),
+ zap.Error(err))
+ return err
+ }
- // retrieve volumes
- volumes, reasons, err :=
ctx.apiProvider.GetAPIs().VolumeBinder.FindPodVolumes(ctx.klogger, pod,
podVolumeClaims, targetNode.Node())
- if err != nil {
- log.Log(log.ShimContext).Error("Failed
to find pod volumes",
- zap.String("podName",
assumedPod.Name),
- zap.String("nodeName",
assumedPod.Spec.NodeName),
- zap.Error(err))
- return err
- }
- if len(reasons) > 0 {
- sReasons := make([]string, 0)
- for _, reason := range reasons {
- sReasons = append(sReasons,
string(reason))
- }
- sReason := strings.Join(sReasons, ", ")
- err = fmt.Errorf("pod %s has
conflicting volume claims: %s", pod.Name, sReason)
- log.Log(log.ShimContext).Error("Pod has
conflicting volume claims",
- zap.String("podName",
assumedPod.Name),
- zap.String("nodeName",
assumedPod.Spec.NodeName),
- zap.Error(err))
- return err
- }
- allBound, err =
ctx.apiProvider.GetAPIs().VolumeBinder.AssumePodVolumes(ctx.klogger, pod, node,
volumes)
- if err != nil {
- return err
+ // retrieve volumes
+ volumes, reasons, err :=
ctx.apiProvider.GetAPIs().VolumeBinder.FindPodVolumes(ctx.klogger, pod,
podVolumeClaims, targetNode.Node())
+ if err != nil {
+ log.Log(log.ShimContext).Error("Failed to find
pod volumes",
+ zap.String("podName", assumedPod.Name),
+ zap.String("nodeName",
assumedPod.Spec.NodeName),
+ zap.Error(err))
+ return err
+ }
+ if len(reasons) > 0 {
+ sReasons := make([]string, 0)
+ for _, reason := range reasons {
+ sReasons = append(sReasons,
string(reason))
Review Comment:
This should be optimised to:
```
sReasons := make([]string, len(reasons))
for i, reason := range reasons {
sReasons[i] = string(reason)
}
```
--
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]
