vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 #3469

jichangyun · 2024-05-15T08:42:38Z

代码位置：

volcano/pkg/scheduler/plugins/task-topology/topology.go

Lines 243 to 275 in f6e0a52

    
           func affinityCheck(job *api.JobInfo, affinity [][]string) error { 
        
           	if job == nil || affinity == nil { 
        
           		return fmt.Errorf("empty input, job: %v, affinity: %v", job, affinity) 
        
           	} 
        
           	var taskNumber = len(job.Tasks) 
        
           	var taskRef = make(map[string]bool, taskNumber) 
        
           	for _, task := range job.Tasks { 
        
           		tmpStrings := strings.Split(task.Name, "-") 
        
           		if _, exist := taskRef[tmpStrings[len(tmpStrings)-2]]; !exist { 
        
           			taskRef[tmpStrings[len(tmpStrings)-2]] = true 
        
           		} 
        
           	} 
        
           	for _, aff := range affinity { 
        
           		affTasks := make(map[string]bool, len(aff)) 
        
           		for _, task := range aff { 
        
           			if len(task) == 0 { 
        
           				continue 
        
           			} 
        
           			if _, exist := taskRef[task]; !exist { 
        
           				return fmt.Errorf("task %s do not exist in job <%s/%s>", task, job.Namespace, job.Name) 
        
           			} 
        
           			if _, exist := affTasks[task]; exist { 
        
           				return fmt.Errorf("task %s is duplicated in job <%s/%s>", task, job.Namespace, job.Name) 
        
           			} 
        
           			affTasks[task] = true 
        
           		} 
        
           	} 
        
           	return nil 
        
           }

问题点：

volcano/pkg/scheduler/plugins/task-topology/topology.go

Line 251 in f6e0a52

tmpStrings := strings.Split(task.Name, "-")

task 名称包含 - 时， tmpStrings := strings.Split(task.Name, "-") 切割的结果会非期望，放进 taskRef 的 key 就不对了，下面 check 的时候就会报错 “task %s do not exist in job <%s/%s>”

Monokaix · 2024-05-15T09:51:56Z

It has been fixed by #2940 :）

jichangyun · 2024-05-16T02:27:07Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。

打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 Job 的 uid
看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

    
           func (ji *JobInfo) SetPodGroup(pg *PodGroup) { 
        
           	ji.Name = pg.Name 
        
           	ji.Namespace = pg.Namespace 
        
           	ji.MinAvailable = pg.Spec.MinMember 
        
           	ji.Queue = QueueID(pg.Spec.Queue) 
        
           	ji.CreationTimestamp = pg.GetCreationTimestamp()

Monokaix · 2024-05-16T03:01:40Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。

打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 podgroup 的 uid 看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

func (ji *JobInfo) SetPodGroup(pg *PodGroup) {

ji.Name = pg.Name

ji.Namespace = pg.Namespace

ji.MinAvailable = pg.Spec.MinMember

ji.Queue = QueueID(pg.Spec.Queue)

ji.CreationTimestamp = pg.GetCreationTimestamp()

Is it a vcjob or deplopyemnt?

jichangyun · 2024-05-16T03:20:12Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。
打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 podgroup 的 uid 看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

func (ji *JobInfo) SetPodGroup(pg *PodGroup) {

ji.Name = pg.Name

ji.Namespace = pg.Namespace

ji.MinAvailable = pg.Spec.MinMember

ji.Queue = QueueID(pg.Spec.Queue)

ji.CreationTimestamp = pg.GetCreationTimestamp()

Is it a vcjob or deplopyemnt?

vcjob

jichangyun · 2024-05-16T03:51:19Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。
打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 podgroup 的 uid 看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

func (ji *JobInfo) SetPodGroup(pg *PodGroup) {

ji.Name = pg.Name

ji.Namespace = pg.Namespace

ji.MinAvailable = pg.Spec.MinMember

ji.Queue = QueueID(pg.Spec.Queue)

ji.CreationTimestamp = pg.GetCreationTimestamp()

Is it a vcjob or deplopyemnt?

vcjob

前面写错了， jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，后面的 uuid 是 vcjob 的UID。

Monokaix · 2024-05-16T04:17:30Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。
打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 podgroup 的 uid 看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

func (ji *JobInfo) SetPodGroup(pg *PodGroup) {

ji.Name = pg.Name

ji.Namespace = pg.Namespace

ji.MinAvailable = pg.Spec.MinMember

ji.Queue = QueueID(pg.Spec.Queue)

ji.CreationTimestamp = pg.GetCreationTimestamp()

Is it a vcjob or deplopyemnt?

vcjob

前面写错了， jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，后面的 uuid 是 vcjob 的UID。

can you paste the whole job name and
pod name and job yaml？

jichangyun · 2024-05-16T04:24:28Z

It has been fixed by #2940 :）

改动好像有点问题，把改动合入到 1.7.0 版本后仍有问题。
打日志看到 jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，有 uuid 后缀，这个 uuid 是 podgroup 的 uid 看了 master 分支的代码， SetPodGroup 时，会置 ji.Name = pg.Name 。

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

func (ji *JobInfo) SetPodGroup(pg *PodGroup) {

ji.Name = pg.Name

ji.Namespace = pg.Namespace

ji.MinAvailable = pg.Spec.MinMember

ji.Queue = QueueID(pg.Spec.Queue)

ji.CreationTimestamp = pg.GetCreationTimestamp()

Is it a vcjob or deplopyemnt?

vcjob

前面写错了， jobNamePrefix 为 test-xytsyh2-d6d4a2e5-1ddb-4d1d-98ec-8eb97b054e63- ，后面的 uuid 是 vcjob 的UID。

can you paste the whole job name and pod name and job yaml？

# kubectl get pods
NAME                            READY   STATUS                 RESTARTS         AGE
test-binpack-xytsyh2-task-1-0   0/1     CreateContainerError   0                9m1s
test-binpack-xytsyh2-task-2-0   0/1     CreateContainerError   0                9m1s

# kubectl get podgroup
NAME                                                        STATUS    MINMEMBER   RUNNINGS   AGE
test-binpack-xytsyh2-d7e821cc-2e6c-4b5a-9818-f3efca39e5da   Running   2                      59m

# 
apiVersion: batch.volcano.sh/v1alpha1
kind: Job
metadata:
  creationTimestamp: "2024-05-16T03:21:00Z"
  generation: 1
  name: test-binpack-xytsyh2
  namespace: default
  resourceVersion: "1025579"
  uid: d7e821cc-2e6c-4b5a-9818-f3efca39e5da
spec:
  maxRetry: 3
  minAvailable: 2
  queue: default
  schedulerName: volcano
  tasks:
  - maxRetry: 3
    minAvailable: 1
    name: task-1
    replicas: 1
    template:
      metadata: {}
      spec:
        containers:
        - image: alpine
          imagePullPolicy: IfNotPresent
          name: test
        restartPolicy: OnFailure
  - maxRetry: 3
    minAvailable: 1
    name: task-2
    replicas: 1
    template:
      metadata: {}
      spec:
        containers:
        - image: alpine
          imagePullPolicy: IfNotPresent
          name: test
        restartPolicy: OnFailure
status:
  conditions:
  - lastTransitionTime: "2024-05-16T03:21:02Z"
    status: Pending
  minAvailable: 2
  pending: 2
  state:
    lastTransitionTime: "2024-05-16T03:21:02Z"
    phase: Pending
  taskStatusCount:
    task-1:
      phase:
        Pending: 1
    task-2:
      phase:
        Pending: 1

Monokaix · 2024-05-16T07:59:10Z

/good-first-issue

volcano-sh-bot · 2024-05-16T07:59:13Z

@Monokaix:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Monokaix · 2024-05-17T04:18:03Z

@loheagn can you take a look?

Monokaix · 2024-05-17T04:18:59Z

@jichangyun Have you set affinity rules in job's annotation?

jichangyun · 2024-05-17T04:52:07Z

@jichangyun Have you set affinity rules in job's annotation?

恩，是的，就是 task-1 和 task-2 反亲和。

大概看了下，问题点应该在这里， SetPodGroup 时，会置 ji.Name = pg.Name ，而 pg.Name 是 job.Name + "-" + job.UID .

volcano/pkg/scheduler/api/job_info.go

Lines 366 to 372 in 016d215

    
           func (ji *JobInfo) SetPodGroup(pg *PodGroup) { 
        
           	ji.Name = pg.Name 
        
           	ji.Namespace = pg.Namespace 
        
           	ji.MinAvailable = pg.Spec.MinMember 
        
           	ji.Queue = QueueID(pg.Spec.Queue) 
        
           	ji.CreationTimestamp = pg.GetCreationTimestamp()

私有项目就先把 topology.go 文件中的 var jobNamePrefix = job.Name + "-" 改成 var jobNamePrefix = job.Name[:len(job.Name)-36] 了，自测没问题。
但是，个人认为 job_info.go 中的置 ji.Name = pg.Name 不太合适，但是目前研究还不够，不敢直接改 job_info.go ，所以先不提 PR 过来了。

lowang-bh · 2024-05-17T13:28:36Z

个人认为 job_info.go 中的置 ji.Name = pg.Name 不太合适

ji.Name = pg.Name没有问题，本来这俩就是相同的。问题在于controller自动生成的podgroup为啥还要带一个UID，有的又没有。这个UID生成我没找到在哪，我认为没必要有。

loheagn · 2024-05-17T13:39:39Z

So the current issue is that the full task name in topology.go is not necessarily formatted as ${job name}-${task name}-${index}, which might lead to the patch in #2940 not being able to retrieve the correct task name?

jichangyun · 2024-05-20T08:56:28Z

个人认为 job_info.go 中的置 ji.Name = pg.Name 不太合适

ji.Name = pg.Name没有问题，本来这俩就是相同的。问题在于controller自动生成的podgroup为啥还要带一个UID，有的又没有。这个UID生成我没找到在哪，我认为没必要有。

恩，如果这两个名称是一样的，那应该没问题。
我在 1.7.0 版本上自测，似乎都是 pg.name = job.Name + "-" + job.UID

翻了下 master 分支的代码，好像是在这里。

volcano/pkg/controllers/job/job_controller_actions.go

Lines 647 to 659 in 3520a0f

    
           func (cc *jobcontroller) createOrUpdatePodGroup(job *batch.Job) error { 
        
           	// If PodGroup does not exist, create one for Job. 
        
           	pgName := job.Name + "-" + string(job.UID) 
        
           	var pg *scheduling.PodGroup 
        
           	var err error 
        
           	pg, err = cc.pgLister.PodGroups(job.Namespace).Get(pgName) 
        
           	if err != nil { 
        
           		if !apierrors.IsNotFound(err) { 
        
           			klog.Errorf("Failed to get PodGroup for Job <%s/%s>: %v", 
        
           				job.Namespace, job.Name, err) 
        
           			return err 
        
           		} 
        
           		// try to get old pg if new pg not exist

JesseStutler · 2024-09-06T03:56:02Z

It's a bug definitely. As @jichangyun said, the name of podgroup created by vc-controller isjob.Name - job.UID, but the job name in task.Name has no uid. It means that the annotation configured does not work now, but in old code #2940 , if the task does not contain "-", we will get the right task name no matter what the jobNamePrefix is.

JesseStutler · 2024-09-09T12:18:19Z

/assign

jichangyun changed the title ~~vsjob的task名称包含 - 时， task-topolopy 插件会误判而失效。~~ vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 May 15, 2024

jichangyun closed this as completed May 15, 2024

jichangyun reopened this May 16, 2024

volcano-sh-bot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels May 16, 2024

JesseStutler mentioned this issue Sep 9, 2024

fix: not remove podgroup uid will cause topology annotation to be useless #3711

Merged

volcano-sh-bot assigned JesseStutler Sep 9, 2024

volcano-sh-bot closed this as completed in #3711 Sep 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 #3469

vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 #3469

jichangyun commented May 15, 2024

Monokaix commented May 15, 2024 •

edited

Loading

jichangyun commented May 16, 2024 •

edited

Loading

Monokaix commented May 16, 2024

jichangyun commented May 16, 2024

jichangyun commented May 16, 2024

Monokaix commented May 16, 2024

jichangyun commented May 16, 2024

Monokaix commented May 16, 2024

volcano-sh-bot commented May 16, 2024

Monokaix commented May 17, 2024

Monokaix commented May 17, 2024

jichangyun commented May 17, 2024

lowang-bh commented May 17, 2024

loheagn commented May 17, 2024

jichangyun commented May 20, 2024

JesseStutler commented Sep 6, 2024

JesseStutler commented Sep 9, 2024

vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 #3469

vcjob的task名称包含 - 时， task-topolopy 插件会误判而失效。 #3469

Comments

jichangyun commented May 15, 2024

Monokaix commented May 15, 2024 • edited Loading

jichangyun commented May 16, 2024 • edited Loading

Monokaix commented May 16, 2024

jichangyun commented May 16, 2024

jichangyun commented May 16, 2024

Monokaix commented May 16, 2024

jichangyun commented May 16, 2024

Monokaix commented May 16, 2024

volcano-sh-bot commented May 16, 2024

Monokaix commented May 17, 2024

Monokaix commented May 17, 2024

jichangyun commented May 17, 2024

lowang-bh commented May 17, 2024

loheagn commented May 17, 2024

jichangyun commented May 20, 2024

JesseStutler commented Sep 6, 2024

JesseStutler commented Sep 9, 2024

Monokaix commented May 15, 2024 •

edited

Loading

jichangyun commented May 16, 2024 •

edited

Loading