simplify acc logic

[asm582]

No description provided.

[z103cb]

Small nitpick, looking good otherwise

[tardieu]

I think the PR makes unintended changes to the computation in Dispatcher mode. In dispatcher mode and if strings.Compare(dispatchedAgentId, agentId) == 0 we should add the aggregated AW request to preemptable if we want to preserve the existing behavior.

if strings.Compare(dispatchedAgentId, agentId) != 0 {
					klog.V(10).Infof("[getAggAvaiResPri] %s: Skipping adjustments for %s since it is in cluster %s which is not in the same cluster under evaluation: %s.",
						time.Now().String(), value.Name, dispatchedAgentId, agentId)
					continue
} else {
    for _, resctrl := range qjm.qjobResControls {
        qjv := resctrl.GetAggregatedResources(value)
          preemptable = preemptable.Add(qjv)
     }
    for _, genericItem := range value.Spec.AggrResources.GenericItems {
        qjv, _ := genericresource.GetResources(&genericItem)
        preemptable = preemptable.Add(qjv)
    }
    continue
}

[asm582]

I think the PR makes unintended changes to the computation in Dispatcher mode. In dispatcher mode and if strings.Compare(dispatchedAgentId, agentId) == 0 we should add the aggregated AW request to preemptable if we want to preserve the existing behavior.

if strings.Compare(dispatchedAgentId, agentId) != 0 {
					klog.V(10).Infof("[getAggAvaiResPri] %s: Skipping adjustments for %s since it is in cluster %s which is not in the same cluster under evaluation: %s.",
						time.Now().String(), value.Name, dispatchedAgentId, agentId)
					continue
} else {
    for _, resctrl := range qjm.qjobResControls {
        qjv := resctrl.GetAggregatedResources(value)
          preemptable = preemptable.Add(qjv)
     }
    for _, genericItem := range value.Spec.AggrResources.GenericItems {
        qjv, _ := genericresource.GetResources(&genericItem)
        preemptable = preemptable.Add(qjv)
    }
    continue
}

No changes were done for dispatcher mode and I would avoid doing it as there are no test cases yet AFAIK

[tardieu]

No changes were done for dispatcher mode and I would avoid doing it as there are no test cases yet AFAIK

OK. You are right, the problematic changes were in fact made in PR #415. I was looking at the combination of the two PRs. We'll address these changes separately.