From e5f4c0f854b07af0bcfe76a2583965bc4ae5b286 Mon Sep 17 00:00:00 2001 From: jimoosciuc Date: Sun, 20 Nov 2022 12:18:14 +0800 Subject: [PATCH] Fixed issue on PodGroup's minResource: extended resource(such as nvidia.com/gpu) is missing Signed-off-by: jimoosciuc --- pkg/controllers/job/job_controller_actions.go | 10 ++-- .../podgroup/pg_controller_handler.go | 16 ++---- pkg/controllers/util/util.go | 20 +++++++ pkg/controllers/util/util_test.go | 55 +++++++++++++++++++ test/e2e/jobp/job_controlled_resource.go | 8 +-- 5 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 pkg/controllers/util/util.go create mode 100644 pkg/controllers/util/util_test.go diff --git a/pkg/controllers/job/job_controller_actions.go b/pkg/controllers/job/job_controller_actions.go index 97521ba4f1..53132c8e4f 100644 --- a/pkg/controllers/job/job_controller_actions.go +++ b/pkg/controllers/job/job_controller_actions.go @@ -31,15 +31,14 @@ import ( "k8s.io/apimachinery/pkg/util/wait" quotav1 "k8s.io/apiserver/pkg/quota/v1" "k8s.io/klog" - quotacore "k8s.io/kubernetes/pkg/quota/v1/evaluator/core" - "k8s.io/utils/clock" - batch "volcano.sh/apis/pkg/apis/batch/v1alpha1" "volcano.sh/apis/pkg/apis/helpers" scheduling "volcano.sh/apis/pkg/apis/scheduling/v1beta1" + "volcano.sh/volcano/pkg/controllers/apis" jobhelpers "volcano.sh/volcano/pkg/controllers/job/helpers" "volcano.sh/volcano/pkg/controllers/job/state" + "volcano.sh/volcano/pkg/controllers/util" ) var calMutex sync.Mutex @@ -671,7 +670,7 @@ func (cc *jobcontroller) createOrUpdatePodGroup(job *batch.Job) error { pg := &scheduling.PodGroup{ ObjectMeta: metav1.ObjectMeta{ Namespace: job.Namespace, - //add job.UID into its name when create new PodGroup + // add job.UID into its name when create new PodGroup Name: pgName, Annotations: job.Annotations, Labels: job.Labels, @@ -791,8 +790,7 @@ func (cc *jobcontroller) calcPGMinResources(job *batch.Job) *v1.ResourceList { pod := &v1.Pod{ Spec: task.Template.Spec, } - res, _ := quotacore.PodUsageFunc(pod, clock.RealClock{}) - minReq = quotav1.Add(minReq, res) + minReq = quotav1.Add(minReq, *util.GetPodQuotaUsage(pod)) } } diff --git a/pkg/controllers/podgroup/pg_controller_handler.go b/pkg/controllers/podgroup/pg_controller_handler.go index 0af19957ca..16166529cb 100644 --- a/pkg/controllers/podgroup/pg_controller_handler.go +++ b/pkg/controllers/podgroup/pg_controller_handler.go @@ -25,13 +25,12 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/klog" - quotacore "k8s.io/kubernetes/pkg/quota/v1/evaluator/core" - "k8s.io/utils/clock" - "k8s.io/apimachinery/pkg/types" + "k8s.io/klog" "volcano.sh/apis/pkg/apis/helpers" scheduling "volcano.sh/apis/pkg/apis/scheduling/v1beta1" + + "volcano.sh/volcano/pkg/controllers/util" ) type podRequest struct { @@ -166,7 +165,7 @@ func (pg *pgcontroller) createNormalPodPGIfNotExist(pod *v1.Pod) error { Spec: scheduling.PodGroupSpec{ MinMember: 1, PriorityClassName: pod.Spec.PriorityClassName, - MinResources: calcPGMinResources(pod), + MinResources: util.GetPodQuotaUsage(pod), }, Status: scheduling.PodGroupStatus{ Phase: scheduling.PodGroupPending, @@ -228,10 +227,3 @@ func newPGOwnerReferences(pod *v1.Pod) []metav1.OwnerReference { ref := metav1.NewControllerRef(pod, gvk) return []metav1.OwnerReference{*ref} } - -// calcPGMinResources calculate podgroup minimum resource -func calcPGMinResources(pod *v1.Pod) *v1.ResourceList { - pgMinRes, _ := quotacore.PodUsageFunc(pod, clock.RealClock{}) - - return &pgMinRes -} diff --git a/pkg/controllers/util/util.go b/pkg/controllers/util/util.go new file mode 100644 index 0000000000..ebac161b9c --- /dev/null +++ b/pkg/controllers/util/util.go @@ -0,0 +1,20 @@ +package util + +import ( + "strings" + + v1 "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/apis/core/v1/helper" + quotacore "k8s.io/kubernetes/pkg/quota/v1/evaluator/core" + "k8s.io/utils/clock" +) + +func GetPodQuotaUsage(pod *v1.Pod) *v1.ResourceList { + res, _ := quotacore.PodUsageFunc(pod, clock.RealClock{}) + for name, quantity := range res { + if !helper.IsNativeResource(name) && strings.HasPrefix(string(name), v1.DefaultResourceRequestsPrefix) { + res[v1.ResourceName(strings.TrimPrefix(string(name), v1.DefaultResourceRequestsPrefix))] = quantity + } + } + return &res +} diff --git a/pkg/controllers/util/util_test.go b/pkg/controllers/util/util_test.go new file mode 100644 index 0000000000..ba20b6ed43 --- /dev/null +++ b/pkg/controllers/util/util_test.go @@ -0,0 +1,55 @@ +package util + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestGetPodQuotaUsage(t *testing.T) { + resList := corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1000m"), + corev1.ResourceMemory: resource.MustParse("1000Mi"), + "nvidia.com/gpu": resource.MustParse("1"), + "hugepages-test": resource.MustParse("2000"), + } + + container := corev1.Container{ + Resources: corev1.ResourceRequirements{ + Requests: resList, + Limits: resList, + }, + } + + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{container, container}, + }, + } + + expected := map[string]int64{ + "count/pods": 1, + "cpu": 2, + "memory": 1024 * 1024 * 2000, + "nvidia.com/gpu": 2, + "hugepages-test": 4000, + "limits.cpu": 2, + "limits.memory": 1024 * 1024 * 2000, + "requests.memory": 1024 * 1024 * 2000, + "requests.nvidia.com/gpu": 2, + "requests.hugepages-test": 4000, + "pods": 1, + "requests.cpu": 2, + } + + res := *GetPodQuotaUsage(pod) + for name, quantity := range expected { + value, ok := res[corev1.ResourceName(name)] + if !ok { + t.Errorf("Resource %s should exists in pod resources", name) + } else if quantity != value.Value() { + t.Errorf("Resource %s 's value %d should equal to %d", name, quantity, value.Value()) + } + } +} diff --git a/test/e2e/jobp/job_controlled_resource.go b/test/e2e/jobp/job_controlled_resource.go index 8bdb26eb77..52c37c5855 100644 --- a/test/e2e/jobp/job_controlled_resource.go +++ b/test/e2e/jobp/job_controlled_resource.go @@ -189,11 +189,11 @@ var _ = Describe("Job E2E Test: Test Job PVCs", func() { pGroup, err := ctx.Vcclient.SchedulingV1beta1().PodGroups(ctx.Namespace).Get(context.TODO(), pgName, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) - for name, q := range *pGroup.Spec.MinResources { - value, ok := expected[string(name)] + minReq := *pGroup.Spec.MinResources + for name, q := range expected { + value, ok := minReq[v12.ResourceName(name)] Expect(ok).To(Equal(true), "Resource %s should exists in PodGroup", name) - Expect(q.Value()).To(Equal(value), "Resource %s 's value should equal to %d", name, value) + Expect(q).To(Equal(value.Value()), "Resource %s 's value should equal to %d", name, value) } - }) })