chenyulin0719 commented on code in PR #871:
URL: https://github.com/apache/yunikorn-k8shim/pull/871#discussion_r1698754502
##########
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:
> Why will this change across versions?
We are going to fail the task pod if pod have inconsistent metadata. Will
make an anouncement in 1.6.0 and conduct the rejection in 1.7.0
> 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.
Sorry for the unclear explanation, @chia7712 got confused too. So I added
the commented code back to sanityCheckBeforeScheduling(). If the sanity check
failed due to inconsistent metadata, the sanity check should return err and set
the rejectTask to true.
https://github.com/chenyulin0719/yunikorn-k8shim/blob/7f8b8652953cda033a51da17fa1b0a222f24eb8c/pkg/cache/task.go#L587-L589
--
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]
