Delete an success pod, will cause volcano recreate an new pod to do repeat task

[gaopeiliang]

What happened:
when job is running , one pod task successful completed, and delete pod some reason, then another new pod will be create to repeat do same task ....

What you expected to happen:
ignore successful completed delete pod event ,, and not create new same pod....

How to reproduce it (as minimally and precisely as possible):

1. running job , with short running pod successful completed and one long running pod..
2. delete short running pod when completed ....

it will created an new short running pod to do same work

Anything else we need to know?:

Environment:

• Volcano Version:
  release-0.4

• Kubernetes version (use kubectl version):
  1.14

• OS (e.g. from /etc/os-release):
  ubuntu:16

• Kernel (e.g. uname -a):
  Linux 4.9.70-040970-generic

• Install tools:

• Others:

[gaopeiliang]

`
func (cc *Controller) deletePod(obj interface{}) {

    if err := cc.cache.DeletePod(pod); err != nil {
	klog.Errorf("Failed to delete Pod <%s/%s>: %v in cache",
		pod.Namespace, pod.Name, err)
}

key := jobhelpers.GetJobKeyByReq(&req)
queue := cc.getWorkerQueue(key)
queue.Add(req)

}
// here received delete pod event => delete cache ,, (but this pod is an successful completed and deleted some reason）

func (cc *Controller) processNextReq(count uint32) bool {

       cloneJOBINFO
       running => syncJob

}

func (cc *Controller) syncJob(jobInfo *apis.JobInfo, updateStatus state.UpdateStatusFn) error {

      	for i := 0; i < int(ts.Replicas); i++ {
		podName := fmt.Sprintf(jobhelpers.PodNameFmt, job.Name, name, i)
		if pod, found := pods[podName]; !found {
			newPod := createJobPod(job, tc, i)
                           
                          _// HERE will created new , because it not find in cache , even though it has 
                          //  successful completed ....._


			if err := cc.pluginOnPodCreate(job, newPod); err != nil {
				return err
			}
			podToCreate = append(podToCreate, newPod)
		} else {
			delete(pods, podName)
			if pod.DeletionTimestamp != nil {
				klog.Infof("Pod <%s/%s> is terminating", pod.Namespace, pod.Name)
				atomic.AddInt32(&terminating, 1)
				continue
			}

			classifyAndAddUpPodBaseOnPhase(pod, &pending, &running, &succeeded, &failed, &unknown)
		}
	}

}

`

[hzxuzhonghu]

@gaopeiliang Yeah, that's the current behavior, we can cache the completed pods and in the sync handler, we ignore them during reconcile.

Would appreciate it if you can help.

[k82cn]

Good catch ! We did not check task's status in this case :)

[vincent-pli]

So, if check the status of pod, not lunch the request if the status.phase is Succeeded, could this fix the issue?

[k82cn]

hm... I think we should handle this in syncJob func :)

[vincent-pli]

@k82cn
I think handle in syncJob maybe cannot fix the issue:
see the sample:

volcano/example/job.yaml

Lines 8 to 10 in bab40f1

	policies:
	- event: PodEvicted
	action: RestartJob

The PodEvicted event will cause action: RestartJob, cannot get to syncJob.
The "delete after success" is not a Evicted actually.

[hzxuzhonghu]

In this case, we should ignore the delete event.

[k82cn]

The PodEvicted event will cause action: RestartJob, cannot get to syncJob. The "delete after success" is not a Evicted actually.

👍 , yes; so maybe we only record an event and ignore this action :)

[vincent-pli]

@k82cn @hzxuzhonghu
Thanks for clarify, I'm new for this project, maybe I missing something.
So I wonder to know why insist on launching request with PodEvictedEvent in a scenario not Evicted? Is no-launch case issue somewhere, thanks.

[k82cn]

oh, that's ok to me for other options :) Please help to open a PR for that, we can discuss detail there.

[gaopeiliang]

what happen if we ignore all delete pod event ?

func (cc *Controller) deletePod(obj interface{}){
        .........

	//if err := cc.cache.DeletePod(pod); err != nil {
	//	klog.Errorf("Failed to delete Pod <%s/%s>: %v in cache",
	//		pod.Namespace, pod.Name, err)
	//}
      
       ......
}

maybe it would work ......

[vincent-pli]

@gaopeiliang
It will stop create a new pod but will introduce new issue, when delete related job then create again, the previous deleted task(pod) will never create again, I guess it cause by ignore delete cache...

When avoid emit PodEvicted event, the issue not fixed, since we get another request:

<Queue: , Job: default/test-job, Task:, Event:Unknown, ExitCode:0, Action:, JobVersion: 0>

it will trigger syncJob.
The request emit by event handler of podGroup update, that's because some one update PodGroup when delete success pod, I still checking who did this... maybe scheduler? need help.

[gaopeiliang]

what is the meaning delete related job then create again? create an new job? delete job will clean all about in cache ..

[vincent-pli]

@gaocegege
yes, delete then create same job, but based on my test, the previous deleted task gone, never back.

[gaopeiliang]

func (cc *Controller) deletePod(obj interface{}){
       ......
        
	if pod.Status.Phase != v1.PodSucceeded {
		if err := cc.cache.DeletePod(pod); err != nil {
			klog.Errorf("Failed to delete Pod <%s/%s>: %v in cache",
				pod.Namespace, pod.Name, err)
		}
	}
     ......
}

even though it make code not clean

[stale]

Hello 👋 Looks like there was no activity on this issue for last 90 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for 60 days, this issue will be closed (we can always reopen an issue if we need!).

[stale]

Closing for now as there was no activity for last 60 days after marked as stale, let us know if you need this to be reopened! 🤗