wilfred-s commented on code in PR #1073:
URL: https://github.com/apache/yunikorn-k8shim/pull/1073#discussion_r4163399869
##########
pkg/cache/application.go:
##########
@@ -14,6 +14,7 @@
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
+ limitations under the License.
Review Comment:
line duplication, please revert
##########
pkg/cache/context.go:
##########
@@ -741,6 +742,87 @@ func (ctx *Context) IsPodFitNodeViaPreemption(name, node
string, allocations []s
return -1, false
}
+func (ctx *Context) doPluginRequisiteChecks(name, node string) (*v1.Pod,
*framework.NodeInfo, error) {
+ ctx.lock.RLock()
+ defer ctx.lock.RUnlock()
+ pod := ctx.schedulerCache.GetPod(name)
+ if pod == nil {
+ return nil, nil, ErrorPodNotFound
+ }
+ // if pod exists in cache, try to run predicates
+ targetNode := ctx.schedulerCache.GetNode(node)
+ if targetNode == nil {
+ return nil, nil, ErrorNodeNotFound
+ }
+ return pod, targetNode, nil
+}
+
+// Reserve Binding Cycle - Reserve the node for binding process
+func (ctx *Context) Reserve(name, node string) bool {
+ pod, targetNode, err := ctx.doPluginRequisiteChecks(name, node)
+ if err != nil {
+ log.Log(log.ShimContext).Error("reserve failed",
+ zap.Error(err),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ // need to lock cache here as predicates need a stable view into the
cache
+ ctx.schedulerCache.LockForReads()
+ defer ctx.schedulerCache.UnlockForReads()
+ plugin, err := ctx.predManager.Reserve(pod, nil, targetNode)
+ if err != nil {
+ log.Log(log.ShimContext).Error("reserve failed",
+ zap.Error(err),
+ zap.String("plugin", plugin),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ return true
+}
+
+// PreBind Binding Cycle - PreBind the node for binding process
+func (ctx *Context) PreBind(name, node string) bool {
+ pod, targetNode, err := ctx.doPluginRequisiteChecks(name, node)
+ if err != nil {
+ log.Log(log.ShimContext).Error("prebind failed",
+ zap.Error(err),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ // need to lock cache here as predicates need a stable view into the
cache
+ ctx.schedulerCache.LockForReads()
+ defer ctx.schedulerCache.UnlockForReads()
+ plugin, err := ctx.predManager.PreBind(pod, nil, targetNode)
+ if err != nil {
+ log.Log(log.ShimContext).Error("prebind failed",
+ zap.Error(err),
+ zap.String("plugin", plugin),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ return true
+}
+
+// Unreserve Binding Cycle - Unreserve the node to clear out the binding work
+func (ctx *Context) Unreserve(name, node string) {
Review Comment:
Don't export
##########
pkg/cache/task_test.go:
##########
@@ -1085,6 +1085,7 @@ func TestPostTaskAllocated_BindRetrySucceeds(t
*testing.T) {
events.SetRecorder(events.NewMockedRecorder())
defer events.SetRecorder(events.NewMockedRecorder())
defer setShortBindBackoff(5)()
+ dispatcher.Start()
Review Comment:
Should also shutdown the dispatcher again at the end of the test
##########
pkg/cache/context.go:
##########
@@ -741,6 +742,87 @@ func (ctx *Context) IsPodFitNodeViaPreemption(name, node
string, allocations []s
return -1, false
}
+func (ctx *Context) doPluginRequisiteChecks(name, node string) (*v1.Pod,
*framework.NodeInfo, error) {
+ ctx.lock.RLock()
+ defer ctx.lock.RUnlock()
+ pod := ctx.schedulerCache.GetPod(name)
+ if pod == nil {
+ return nil, nil, ErrorPodNotFound
+ }
+ // if pod exists in cache, try to run predicates
+ targetNode := ctx.schedulerCache.GetNode(node)
+ if targetNode == nil {
+ return nil, nil, ErrorNodeNotFound
+ }
+ return pod, targetNode, nil
+}
+
+// Reserve Binding Cycle - Reserve the node for binding process
+func (ctx *Context) Reserve(name, node string) bool {
+ pod, targetNode, err := ctx.doPluginRequisiteChecks(name, node)
+ if err != nil {
+ log.Log(log.ShimContext).Error("reserve failed",
+ zap.Error(err),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ // need to lock cache here as predicates need a stable view into the
cache
+ ctx.schedulerCache.LockForReads()
+ defer ctx.schedulerCache.UnlockForReads()
+ plugin, err := ctx.predManager.Reserve(pod, nil, targetNode)
+ if err != nil {
+ log.Log(log.ShimContext).Error("reserve failed",
+ zap.Error(err),
+ zap.String("plugin", plugin),
+ zap.String("pod", name),
+ zap.String("node", node))
+ return false
+ }
+ return true
+}
+
+// PreBind Binding Cycle - PreBind the node for binding process
+func (ctx *Context) PreBind(name, node string) bool {
Review Comment:
There should be no need to export this
--
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]
