From 7dd26a39af6055ffbac6e371105629260e771607 Mon Sep 17 00:00:00 2001 From: qzhu Date: Fri, 26 Jul 2024 11:11:36 +0800 Subject: [PATCH 1/4] [YUNIKORN-1697] [shim] Make namespace annotation to support max applications update. --- pkg/cache/context.go | 6 +++++ pkg/cache/context_test.go | 7 ++++++ pkg/common/constants/constants.go | 3 +++ pkg/common/utils/utils.go | 14 ++++++++++++ pkg/common/utils/utils_test.go | 38 +++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+) diff --git a/pkg/cache/context.go b/pkg/cache/context.go index f9abaed63..34b8e2379 100644 --- a/pkg/cache/context.go +++ b/pkg/cache/context.go @@ -951,6 +951,12 @@ func (ctx *Context) updateApplicationTags(request *AddApplicationRequest, namesp } } + // add maxApps resource info as an app tag + maxApps := utils.GetNamespaceMaxAppsFromAnnotation(namespaceObj) + if maxApps != "" { + request.Metadata.Tags[siCommon.AppTagNamespaceResourceMaxApps] = maxApps + } + // add parent queue info as an app tag parentQueue := utils.GetNameSpaceAnnotationValue(namespaceObj, constants.AnnotationParentQueue) if parentQueue != "" { diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go index 23f68f2dc..396680a10 100644 --- a/pkg/cache/context_test.go +++ b/pkg/cache/context_test.go @@ -1519,6 +1519,7 @@ func TestAddApplicationsWithTags(t *testing.T) { constants.NamespaceQuota: "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}", constants.DomainYuniKorn + "parentqueue": "root.test", constants.NamespaceGuaranteed: "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}", + constants.NamespaceMaxApps: "1000", }, }, } @@ -1607,6 +1608,12 @@ func TestAddApplicationsWithTags(t *testing.T) { t.Fatalf("resource parsing failed") } + maxApps, ok := request.Metadata.Tags[siCommon.AppTagNamespaceResourceMaxApps] + if !ok { + t.Fatalf("max apps tag is not updated from the namespace") + } + assert.Equal(t, maxApps, "1000") + parentQueue, ok := request.Metadata.Tags[constants.AppTagNamespaceParentQueue] if !ok { t.Fatalf("parent queue tag is not updated from the namespace") diff --git a/pkg/common/constants/constants.go b/pkg/common/constants/constants.go index a6968e582..0e6f00bab 100644 --- a/pkg/common/constants/constants.go +++ b/pkg/common/constants/constants.go @@ -97,6 +97,9 @@ const NamespaceQuota = DomainYuniKorn + "namespace.quota" // NamespaceGuaranteed Namespace Guaranteed const NamespaceGuaranteed = DomainYuniKorn + "namespace.guaranteed" +// NamespaceMaxApps Namespace Max Apps +const NamespaceMaxApps = DomainYuniKorn + "namespace.maxApps" + // AnnotationAllowPreemption set on PriorityClass, opt out of preemption for pods with this priority class const AnnotationAllowPreemption = DomainYuniKorn + "allow-preemption" diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index eb9f71208..1f3de4237 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -214,6 +214,20 @@ func GetNamespaceGuaranteedFromAnnotation(namespaceObj *v1.Namespace) *si.Resour return nil } +// get namespace max apps from namespace annotation +func GetNamespaceMaxAppsFromAnnotation(namespaceObj *v1.Namespace) string { + if maxApps := GetNameSpaceAnnotationValue(namespaceObj, constants.NamespaceMaxApps); maxApps != "" { + if _, err := strconv.Atoi(maxApps); err != nil { + log.Log(log.ShimUtils).Warn("Unable to process namespace.maxApps annotation", + zap.String("namespace", namespaceObj.Name), + zap.String("namespace.maxApps is", maxApps)) + return "" + } + return maxApps + } + return "" +} + func GetNamespaceQuotaFromAnnotation(namespaceObj *v1.Namespace) *si.Resource { // retrieve resource quota info from annotations cpuQuota := GetNameSpaceAnnotationValue(namespaceObj, constants.CPUQuota) diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index adb072142..3a1ec87e1 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -331,6 +331,44 @@ func TestGetNamespaceGuaranteedFromAnnotation(t *testing.T) { } } +func TestGetNamespaceMaxAppsFromAnnotation(t *testing.T) { + testCases := []struct { + namespace *v1.Namespace + expectedMaxApp string + }{ + {&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + }, ""}, + {&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Annotations: map[string]string{ + constants.NamespaceMaxApps: "5", + }, + }, + }, "5"}, + {&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Annotations: map[string]string{ + constants.NamespaceMaxApps: "error", + }, + }, + }, ""}, + } + for _, tc := range testCases { + t.Run(fmt.Sprintf("namespace: %v", tc.namespace), func(t *testing.T) { + maxApp := GetNamespaceMaxAppsFromAnnotation(tc.namespace) + assert.Equal(t, maxApp, tc.expectedMaxApp) + }) + } +} + func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T) { testCases := []struct { namespace *v1.Namespace From df3002acb92972d0111bfa7282e1eeb1a9b374c3 Mon Sep 17 00:00:00 2001 From: qzhu Date: Fri, 26 Jul 2024 11:16:44 +0800 Subject: [PATCH 2/4] fix lint --- pkg/cache/context_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go index 396680a10..20ddf9435 100644 --- a/pkg/cache/context_test.go +++ b/pkg/cache/context_test.go @@ -1519,7 +1519,7 @@ func TestAddApplicationsWithTags(t *testing.T) { constants.NamespaceQuota: "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}", constants.DomainYuniKorn + "parentqueue": "root.test", constants.NamespaceGuaranteed: "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}", - constants.NamespaceMaxApps: "1000", + constants.NamespaceMaxApps: "1000", }, }, } From f43415803c19ef28a6bec4404781089ddab54507 Mon Sep 17 00:00:00 2001 From: qzhu Date: Wed, 31 Jul 2024 09:47:42 +0800 Subject: [PATCH 3/4] Address comments --- pkg/cache/context.go | 2 ++ pkg/common/utils/utils.go | 9 ++++++++- pkg/common/utils/utils_test.go | 9 +++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/cache/context.go b/pkg/cache/context.go index 34b8e2379..59ade857f 100644 --- a/pkg/cache/context.go +++ b/pkg/cache/context.go @@ -930,6 +930,8 @@ func (ctx *Context) notifyTaskComplete(appID, taskID string) { // adds the following tags to the request based on annotations (if exist): // - namespace.resourcequota // - namespace.parentqueue +// - namespace.resourceguaranteed +// - namespace.resourcemaxapps func (ctx *Context) updateApplicationTags(request *AddApplicationRequest, namespace string) { namespaceObj := ctx.getNamespaceObject(namespace) if namespaceObj == nil { diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index 1f3de4237..a6132ebfe 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -217,12 +217,19 @@ func GetNamespaceGuaranteedFromAnnotation(namespaceObj *v1.Namespace) *si.Resour // get namespace max apps from namespace annotation func GetNamespaceMaxAppsFromAnnotation(namespaceObj *v1.Namespace) string { if maxApps := GetNameSpaceAnnotationValue(namespaceObj, constants.NamespaceMaxApps); maxApps != "" { - if _, err := strconv.Atoi(maxApps); err != nil { + numMaxApp, err := strconv.Atoi(maxApps); + if err != nil { log.Log(log.ShimUtils).Warn("Unable to process namespace.maxApps annotation", zap.String("namespace", namespaceObj.Name), zap.String("namespace.maxApps is", maxApps)) return "" } + if numMaxApp < 0 { + log.Log(log.ShimUtils).Warn("Invalid value for namespace.maxApps annotation", + zap.String("namespace", namespaceObj.Name), + zap.String("namespace.maxApps is", maxApps)) + return "" + } return maxApps } return "" diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index 3a1ec87e1..0b20f1a95 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -351,6 +351,15 @@ func TestGetNamespaceMaxAppsFromAnnotation(t *testing.T) { }, }, }, "5"}, + {&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Annotations: map[string]string{ + constants.NamespaceMaxApps: "-5", + }, + }, + }, ""}, {&v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "test", From 5e02645289bd2d251abfaf57fc3c65a11ce2373c Mon Sep 17 00:00:00 2001 From: qzhu Date: Wed, 31 Jul 2024 10:53:02 +0800 Subject: [PATCH 4/4] Fix golint --- pkg/common/utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index a6132ebfe..281189972 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -217,7 +217,7 @@ func GetNamespaceGuaranteedFromAnnotation(namespaceObj *v1.Namespace) *si.Resour // get namespace max apps from namespace annotation func GetNamespaceMaxAppsFromAnnotation(namespaceObj *v1.Namespace) string { if maxApps := GetNameSpaceAnnotationValue(namespaceObj, constants.NamespaceMaxApps); maxApps != "" { - numMaxApp, err := strconv.Atoi(maxApps); + numMaxApp, err := strconv.Atoi(maxApps) if err != nil { log.Log(log.ShimUtils).Warn("Unable to process namespace.maxApps annotation", zap.String("namespace", namespaceObj.Name),