pbacsko commented on code in PR #871:
URL: https://github.com/apache/yunikorn-k8shim/pull/871#discussion_r1698401082
##########
pkg/cache/application.go:
##########
@@ -390,8 +390,13 @@ func (app *Application) Schedule() bool {
func (app *Application) scheduleTasks(taskScheduleCondition func(t *Task)
bool) {
for _, task := range app.GetNewTasks() {
if taskScheduleCondition(task) {
- // for each new task, we do a sanity check before
moving the state to Pending_Schedule
- if err := task.sanityCheckBeforeScheduling(); err ==
nil {
+ // for each new task, we do a sanity check before
moving the state to "Pending"
+ // if sanity check fails due to PVC check, it means the
task is not ready for scheduling, we keep it in "New" state
+ // if sanity check fails due to an unbounded pod having
conflicting metadata, we move the task to "Rejected" state
+ // Before version 1.7.0, the sanity check would pass
even if the unbounded pod having conflicting metadata.
Review Comment:
I don't have enough context, so I have to ask:
1. Why will this change across versions?
2. What will change inversion 1.7.0? I can't see any hard coded variable,
but something has to change in 1.7.0 in order for this comment to make sense.
##########
pkg/cache/application.go:
##########
@@ -390,8 +390,13 @@ func (app *Application) Schedule() bool {
func (app *Application) scheduleTasks(taskScheduleCondition func(t *Task)
bool) {
for _, task := range app.GetNewTasks() {
if taskScheduleCondition(task) {
- // for each new task, we do a sanity check before
moving the state to Pending_Schedule
- if err := task.sanityCheckBeforeScheduling(); err ==
nil {
+ // for each new task, we do a sanity check before
moving the state to "Pending"
+ // if sanity check fails due to PVC check, it means the
task is not ready for scheduling, we keep it in "New" state
+ // if sanity check fails due to an unbounded pod having
conflicting metadata, we move the task to "Rejected" state
+ // Before version 1.7.0, the sanity check would pass
even if the unbounded pod having conflicting metadata.
Review Comment:
I don't have enough context, so I have to ask:
1. Why will this change across versions?
2. What will change in version 1.7.0? I can't see any hard coded variable,
but something has to change in 1.7.0 in order for this comment to make sense.
--
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]
