[YUNIKORN-1655] Use constant DomainYuniKorn (apache#680)

Use DomainYuniKorn constant instead of hardcoded string in the code. Create a local constant based on the SI value instead of importing the SI in all files that use it. Closes: apache#680 Signed-off-by: Wilfred Spiegelenburg <[email protected]>
chenyulin0719 · Oct 26, 2023 · 4f82e93 · 4f82e93
1 parent 30d8d2e
commit 4f82e93
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 66 deletions.
diff --git a/pkg/appmgmt/general/metadata_test.go b/pkg/appmgmt/general/metadata_test.go
@@ -138,7 +138,7 @@ func TestGetAppMetadata(t *testing.T) { //nolint:funlen
 			Labels: map[string]string{
 				"applicationId":            "app00002",
 				"queue":                    "root.b",
-				"yunikorn.apache.org/user": "testuser",
+				constants.DomainYuniKorn + "user": "testuser",
 				"disableStateAware":        "true",
 			},
 			Annotations: map[string]string{
@@ -202,7 +202,7 @@ func TestGetAppMetadata(t *testing.T) { //nolint:funlen
 			Labels: map[string]string{
 				"applicationId":            "app00002",
 				"queue":                    "root.b",
-				"yunikorn.apache.org/user": "testuser",
+				constants.DomainYuniKorn + "user": "testuser",
 			},
 			Annotations: map[string]string{
 				constants.AnnotationSchedulingPolicyParam: "gangSchedulingStyle=Hard=Soft",

diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go
@@ -1165,7 +1165,7 @@ func TestAddApplicationsWithTags(t *testing.T) {
 		ObjectMeta: apis.ObjectMeta{
 			Name: "test1",
 			Annotations: map[string]string{
-				"yunikorn.apache.org/namespace.max.memory": "256M",
+				constants.DomainYuniKorn + "namespace.max.memory": "256M",
 			},
 		},
 	}
@@ -1175,7 +1175,7 @@ func TestAddApplicationsWithTags(t *testing.T) {
 			Name: "test2",
 			Annotations: map[string]string{
 				constants.NamespaceQuota:          "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}",
-				"yunikorn.apache.org/parentqueue": "root.test",
+				constants.DomainYuniKorn + "parentqueue": "root.test",
 				constants.NamespaceGuaranteed:     "{\"cpu\": \"1\", \"memory\": \"256M\", \"nvidia.com/gpu\": \"1\"}",
 			},
 		},

diff --git a/pkg/cache/placeholder_test.go b/pkg/cache/placeholder_test.go
@@ -149,7 +149,7 @@ func TestNewPlaceholderWithLabelsAndAnnotations(t *testing.T) {
 	assert.Equal(t, holder.pod.Annotations["annotationKey1"], "annotationValue1")
 	assert.Equal(t, holder.pod.Annotations["annotationKey2"], "annotationValue2")
 	var taskGroupsDef []interfaces.TaskGroup
-	err = json.Unmarshal([]byte(holder.pod.Annotations["yunikorn.apache.org/task-groups"]), &taskGroupsDef)
+	err = json.Unmarshal([]byte(holder.pod.Annotations[siCommon.DomainYuniKorn+"task-groups"]), &taskGroupsDef)
 	assert.NilError(t, err, "taskGroupsDef unmarshal failed")
 	var priority *int32
 	assert.Equal(t, priority, holder.pod.Spec.Priority)

diff --git a/pkg/cache/task.go b/pkg/cache/task.go
@@ -435,7 +435,7 @@ func (task *Task) postTaskBound() {
 			if pod.Annotations == nil {
 				pod.Annotations = make(map[string]string)
 			}
-			pod.Annotations["yunikorn.apache.org/scheduled-at"] = strconv.FormatInt(time.Now().UnixNano(), 10)
+			pod.Annotations[constants.DomainYuniKorn+"scheduled-at"] = strconv.FormatInt(time.Now().UnixNano(), 10)
 		}); err != nil {
 			log.Log(log.ShimCacheTask).Warn("failed to update pod status", zap.Error(err))
 		}

diff --git a/pkg/common/constants/constants.go b/pkg/common/constants/constants.go
@@ -18,6 +18,10 @@
 
 package constants
 
+import (
+	siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
+)
+
 // Common
 const True = "true"
 const False = "false"
@@ -27,23 +31,24 @@ const DefaultNodeAttributeHostNameKey = "si.io/hostname"
 const DefaultNodeAttributeRackNameKey = "si.io/rackname"
 const DefaultNodeInstanceTypeNodeLabelKey = "node.kubernetes.io/instance-type"
 const DefaultRackName = "/rack-default"
+const DomainYuniKorn = siCommon.DomainYuniKorn
 
 // Application
 const LabelApp = "app"
 const LabelApplicationID = "applicationId"
-const AnnotationApplicationID = "yunikorn.apache.org/app-id"
+const AnnotationApplicationID = DomainYuniKorn + "app-id"
 const LabelQueueName = "queue"
 const RootQueue = "root"
-const AnnotationQueueName = "yunikorn.apache.org/queue"
-const AnnotationParentQueue = "yunikorn.apache.org/parentqueue"
+const AnnotationQueueName = DomainYuniKorn + "queue"
+const AnnotationParentQueue = DomainYuniKorn + "parentqueue"
 const LabelDisableStateAware = "disableStateAware"
 const ApplicationDefaultQueue = "root.sandbox"
 const DefaultPartition = "default"
 const AppTagNamespace = "namespace"
 const AppTagNamespaceParentQueue = "namespace.parentqueue"
 const AppTagImagePullSecrets = "imagePullSecrets"
 const DefaultAppNamespace = "default"
-const DefaultUserLabel = "yunikorn.apache.org/username"
+const DefaultUserLabel = DomainYuniKorn + "username"
 const DefaultUser = "nobody"
 
 // Spark
@@ -67,10 +72,10 @@ const PlaceholderContainerImage = "registry.k8s.io/pause:3.7"
 const PlaceholderContainerName = "pause"
 const PlaceholderPodRestartPolicy = "Never"
 const LabelPlaceholderFlag = "placeholder"
-const AnnotationPlaceholderFlag = "yunikorn.apache.org/placeholder"
-const AnnotationTaskGroupName = "yunikorn.apache.org/task-group-name"
-const AnnotationTaskGroups = "yunikorn.apache.org/task-groups"
-const AnnotationSchedulingPolicyParam = "yunikorn.apache.org/schedulingPolicyParameters"
+const AnnotationPlaceholderFlag = DomainYuniKorn + "placeholder"
+const AnnotationTaskGroupName = DomainYuniKorn + "task-group-name"
+const AnnotationTaskGroups = DomainYuniKorn + "task-groups"
+const AnnotationSchedulingPolicyParam = DomainYuniKorn + "schedulingPolicyParameters"
 const SchedulingPolicyTimeoutParam = "placeholderTimeoutInSeconds"
 const SchedulingPolicyParamDelimiter = " "
 const SchedulingPolicyStyleParam = "gangSchedulingStyle"
@@ -82,32 +87,32 @@ const ApplicationInsufficientResourcesFailure = "ResourceReservationTimeout"
 const ApplicationRejectedFailure = "ApplicationRejected"
 
 // namespace.max.* (Retaining for backwards compatibility. Need to be removed in next major release)
-const CPUQuota = "yunikorn.apache.org/namespace.max.cpu"
-const MemQuota = "yunikorn.apache.org/namespace.max.memory"
+const CPUQuota = DomainYuniKorn + "namespace.max.cpu"
+const MemQuota = DomainYuniKorn + "namespace.max.memory"
 
 // NamespaceQuota Namespace Quota
-const NamespaceQuota = "yunikorn.apache.org/namespace.quota"
+const NamespaceQuota = DomainYuniKorn + "namespace.quota"
 
 // NamespaceGuaranteed Namespace Guaranteed
-const NamespaceGuaranteed = "yunikorn.apache.org/namespace.guaranteed"
+const NamespaceGuaranteed = DomainYuniKorn + "namespace.guaranteed"
 
 // AnnotationAllowPreemption set on PriorityClass, opt out of preemption for pods with this priority class
-const AnnotationAllowPreemption = "yunikorn.apache.org/allow-preemption"
+const AnnotationAllowPreemption = DomainYuniKorn + "allow-preemption"
 
 // AnnotationIgnoreApplication set on Pod prevents by admission controller, prevents YuniKorn from honoring application ID
-const AnnotationIgnoreApplication = "yunikorn.apache.org/ignore-application"
+const AnnotationIgnoreApplication = DomainYuniKorn + "ignore-application"
 
 // AnnotationGenerateAppID adds application ID to workloads in the namespace even if not set in the admission config.
 // Overrides the regexp behaviour if set, checked before the regexp is evaluated.
 // true: add an application ID label
 // false: do not add an application ID
-const AnnotationGenerateAppID = "yunikorn.apache.org/namespace.generateAppId"
+const AnnotationGenerateAppID = DomainYuniKorn + "namespace.generateAppId"
 
 // AnnotationEnableYuniKorn sets the scheduler name to YuniKorn for workloads in the namespace even if not set in the admission config.
 // Overrides the regexp behaviour if set, checked before the regexp is evaluated.
 // true: set the scheduler name to YuniKorn
 // false: do not do anything
-const AnnotationEnableYuniKorn = "yunikorn.apache.org/namespace.enableYuniKorn"
+const AnnotationEnableYuniKorn = DomainYuniKorn + "namespace.enableYuniKorn"
 
 // Admission Controller pod label update constants
 const AutoGenAppPrefix = "yunikorn"

diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go
@@ -83,7 +83,7 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu": "1",
+					constants.DomainYuniKorn + "namespace.max.cpu": "1",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -94,7 +94,7 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.memory": "128M",
+					constants.DomainYuniKorn + "namespace.max.memory": "128M",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -105,8 +105,8 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "error",
-					"yunikorn.apache.org/namespace.max.memory": "128M",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "error",
+					constants.DomainYuniKorn + "namespace.max.memory": "128M",
 				},
 			},
 		}, nil},
@@ -115,8 +115,8 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "1",
-					"yunikorn.apache.org/namespace.max.memory": "error",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "1",
+					constants.DomainYuniKorn + "namespace.max.memory": "error",
 				},
 			},
 		}, nil},
@@ -125,8 +125,8 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "error",
-					"yunikorn.apache.org/namespace.max.memory": "error",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "error",
+					constants.DomainYuniKorn + "namespace.max.memory": "error",
 				},
 			},
 		}, nil},
@@ -135,8 +135,8 @@ func TestGetNamespaceQuotaFromAnnotation(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "1",
-					"yunikorn.apache.org/namespace.max.memory": "64M",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "1",
+					constants.DomainYuniKorn + "namespace.max.memory": "64M",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -162,7 +162,7 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.quota": "{\"cpu\": \"5\"}",
+					constants.DomainYuniKorn + "namespace.quota": "{\"cpu\": \"5\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -173,7 +173,7 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.quota": "{\"memory\": \"256M\"}",
+					constants.DomainYuniKorn + "namespace.quota": "{\"memory\": \"256M\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -184,7 +184,7 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.quota": "{\"cpu\": \"1\", \"memory\": \"64M\"}",
+					constants.DomainYuniKorn + "namespace.quota": "{\"cpu\": \"1\", \"memory\": \"64M\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -196,7 +196,7 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.quota": "{\"cpu\": \"1\", \"memory\": \"64M\", \"nvidia.com/gpu\": \"1\"}",
+					constants.DomainYuniKorn + "namespace.quota": "{\"cpu\": \"1\", \"memory\": \"64M\", \"nvidia.com/gpu\": \"1\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -209,7 +209,7 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAnnotations(t *testing.T) {
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.quota": "{\"cpu\": \"error\", \"memory\": \"error\"}",
+					constants.DomainYuniKorn + "namespace.quota": "{\"cpu\": \"error\", \"memory\": \"error\"}",
 				},
 			},
 		}, nil},
@@ -302,8 +302,8 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T)
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu": "1",
-					"yunikorn.apache.org/namespace.quota":   "{\"cpu\": \"5\"}",
+					constants.DomainYuniKorn + "namespace.max.cpu": "1",
+					constants.DomainYuniKorn + "namespace.quota":   "{\"cpu\": \"5\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -314,8 +314,8 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T)
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.memory": "128M",
-					"yunikorn.apache.org/namespace.quota":      "{\"memory\": \"256M\"}",
+					constants.DomainYuniKorn + "namespace.max.memory": "128M",
+					constants.DomainYuniKorn + "namespace.quota":      "{\"memory\": \"256M\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -326,9 +326,9 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T)
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "5",
-					"yunikorn.apache.org/namespace.max.memory": "32M",
-					"yunikorn.apache.org/namespace.quota":      "{\"cpu\": \"1\", \"memory\": \"64M\"}",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "5",
+					constants.DomainYuniKorn + "namespace.max.memory": "32M",
+					constants.DomainYuniKorn + "namespace.quota":      "{\"cpu\": \"1\", \"memory\": \"64M\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -340,9 +340,9 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T)
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "1",
-					"yunikorn.apache.org/namespace.max.memory": "64M",
-					"yunikorn.apache.org/namespace.quota":      "{\"cpu\": \"1\", \"memory\": \"64M\", \"nvidia.com/gpu\": \"1\"}",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "1",
+					constants.DomainYuniKorn + "namespace.max.memory": "64M",
+					constants.DomainYuniKorn + "namespace.quota":      "{\"cpu\": \"1\", \"memory\": \"64M\", \"nvidia.com/gpu\": \"1\"}",
 				},
 			},
 		}, common.NewResourceBuilder().
@@ -355,9 +355,9 @@ func TestGetNamespaceQuotaFromAnnotationUsingNewAndOldAnnotations(t *testing.T)
 				Name:      "test",
 				Namespace: "test",
 				Annotations: map[string]string{
-					"yunikorn.apache.org/namespace.max.cpu":    "1",
-					"yunikorn.apache.org/namespace.max.memory": "64M",
-					"yunikorn.apache.org/namespace.quota":      "{\"cpu\": \"error\", \"memory\": \"error\"}",
+					constants.DomainYuniKorn + "namespace.max.cpu":    "1",
+					constants.DomainYuniKorn + "namespace.max.memory": "64M",
+					constants.DomainYuniKorn + "namespace.quota":      "{\"cpu\": \"error\", \"memory\": \"error\"}",
 				},
 			},
 		}, nil},

diff --git a/test/e2e/admission_controller/admission_controller_test.go b/test/e2e/admission_controller/admission_controller_test.go
@@ -36,7 +36,7 @@ import (
 	"github.com/apache/yunikorn-k8shim/test/e2e/framework/helpers/yunikorn"
 )
 
-const userInfoAnnotation = "yunikorn.apache.org/user.info"
+const userInfoAnnotation = constants.DomainYuniKorn + "user.info"
 const nonExistentNode = "non-existent-node"
 const defaultPodTimeout = 10 * time.Second
 const cronJobPodTimeout = 65 * time.Second
@@ -222,11 +222,11 @@ var _ = ginkgo.Describe("AdmissionController", func() {
 		gomega.Ω(err).ShouldNot(gomega.HaveOccurred())
 		pod, err = kubeClient.GetPod(pod.Name, ns)
 		gomega.Ω(err).ShouldNot(gomega.HaveOccurred())
-		userinfo := pod.Annotations["yunikorn.apache.org/user.info"]
+		userinfo := pod.Annotations[constants.DomainYuniKorn+"user.info"]
 		gomega.Ω(userinfo).Should(gomega.Not(gomega.BeNil()))
 
 		ginkgo.By("Attempt to update userinfo annotation")
-		_, err = kubeClient.UpdatePodWithAnnotation(pod, ns, "yunikorn.apache.org/user.info", "shouldnotsucceed")
+		_, err = kubeClient.UpdatePodWithAnnotation(pod, ns, constants.DomainYuniKorn+"user.info", "shouldnotsucceed")
 		gomega.Ω(err).Should(gomega.HaveOccurred())
 	})
 
@@ -514,7 +514,7 @@ func runWorkloadTest(workloadType k8s.WorkloadType, create func() (string, error
 	fmt.Fprintf(ginkgo.GinkgoWriter, "Running pod is %s\n", pods.Items[0].Name)
 	pod, err2 := kubeClient.GetPod(pods.Items[0].Name, ns)
 	gomega.Ω(err2).ShouldNot(gomega.HaveOccurred())
-	userinfo := pod.Annotations["yunikorn.apache.org/user.info"]
+	userinfo := pod.Annotations[constants.DomainYuniKorn+"user.info"]
 	gomega.Ω(userinfo).Should(gomega.Not(gomega.BeNil()))
 }
 

diff --git a/test/e2e/framework/helpers/k8s/gang_job.go b/test/e2e/framework/helpers/k8s/gang_job.go
@@ -28,6 +28,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/apache/yunikorn-k8shim/pkg/appmgmt/interfaces"
+	"github.com/apache/yunikorn-k8shim/pkg/common/constants"
 	"github.com/apache/yunikorn-k8shim/pkg/common/utils"
 )
 
@@ -75,15 +76,15 @@ func getGangSchedulingAnnotations(placeholderTimeout int,
 	}
 
 	if schedulingParams != "" {
-		annotations["yunikorn.apache.org/schedulingPolicyParameters"] = schedulingParams
+		annotations[constants.DomainYuniKorn+"schedulingPolicyParameters"] = schedulingParams
 	}
 
-	annotations["yunikorn.apache.org/task-group-name"] = taskGroupName
+	annotations[constants.DomainYuniKorn+"task-group-name"] = taskGroupName
 	taskGroupJSON, err := json.Marshal(taskGroups)
 	if err != nil {
 		panic("Unable to marshal taskGroups")
 	}
-	annotations["yunikorn.apache.org/task-groups"] = string(taskGroupJSON)
+	annotations[constants.DomainYuniKorn+"task-groups"] = string(taskGroupJSON)
 
 	return annotations
 }