This is an automated email from the ASF dual-hosted git repository.
chenyulin0719 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/master by this push:
new 223e1f3b [YUNIKORN-2631] Support canonical labels for
queue/applicationId in Admission Controller (#843)
223e1f3b is described below
commit 223e1f3b69262a379a2f19efd2294cd7597d8f77
Author: Yu-Lin Chen <[email protected]>
AuthorDate: Sat Jun 8 05:50:55 2024 +0000
[YUNIKORN-2631] Support canonical labels for queue/applicationId in
Admission Controller (#843)
Add canonical representations when Admission Controller try to patch
queue/applicationID to pod
Closes: #843
Signed-off-by: Yu-Lin Chen <[email protected]>
---
pkg/admission/admission_controller.go | 2 +-
pkg/admission/admission_controller_test.go | 32 +++++++++++++++-------
pkg/admission/util.go | 17 ++++++++++--
pkg/admission/util_test.go | 44 +++++++++++++++++++++---------
pkg/common/constants/constants.go | 2 ++
5 files changed, 71 insertions(+), 26 deletions(-)
diff --git a/pkg/admission/admission_controller.go
b/pkg/admission/admission_controller.go
index 3b058da6..35c3fecb 100644
--- a/pkg/admission/admission_controller.go
+++ b/pkg/admission/admission_controller.go
@@ -322,7 +322,7 @@ func (c *AdmissionController) processPodUpdate(req
*admissionv1.AdmissionRequest
func (c *AdmissionController) shouldProcessAdmissionReview(namespace string,
labels map[string]string) bool {
if c.shouldProcessNamespace(namespace) &&
- (labels[constants.LabelApplicationID] != "" ||
labels[constants.SparkLabelAppID] != "" || c.shouldLabelNamespace(namespace)) {
+ (labels[constants.CanonicalLabelApplicationID] != "" ||
labels[constants.LabelApplicationID] != "" || labels[constants.SparkLabelAppID]
!= "" || c.shouldLabelNamespace(namespace)) {
return true
}
diff --git a/pkg/admission/admission_controller_test.go
b/pkg/admission/admission_controller_test.go
index 88bf8d6a..e5c66189 100644
--- a/pkg/admission/admission_controller_test.go
+++ b/pkg/admission/admission_controller_test.go
@@ -81,9 +81,11 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 3)
+ assert.Equal(t, len(updatedMap), 5)
assert.Equal(t, updatedMap["random"], "random")
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.default")
+ assert.Equal(t,
strings.HasPrefix(updatedMap[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(updatedMap[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("patch info content is not as expected")
@@ -104,8 +106,8 @@ func TestUpdateLabels(t *testing.T) {
UID: "7f5fd6c5d5",
ResourceVersion: "10654",
Labels: map[string]string{
- "random": "random",
- constants.LabelApplicationID: "app-0001",
+ "random": "random",
+ constants.CanonicalLabelApplicationID:
"app-0001",
},
},
Spec: v1.PodSpec{},
@@ -117,9 +119,11 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 3)
+ assert.Equal(t, len(updatedMap), 5)
assert.Equal(t, updatedMap["random"], "random")
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.default")
+ assert.Equal(t,
updatedMap[constants.CanonicalLabelApplicationID], "app-0001")
assert.Equal(t, updatedMap[constants.LabelApplicationID],
"app-0001")
} else {
t.Fatal("patch info content is not as expected")
@@ -140,8 +144,8 @@ func TestUpdateLabels(t *testing.T) {
UID: "7f5fd6c5d5",
ResourceVersion: "10654",
Labels: map[string]string{
- "random": "random",
- constants.LabelQueueName: "root.abc",
+ "random": "random",
+ constants.CanonicalLabelQueueName: "root.abc",
},
},
Spec: v1.PodSpec{},
@@ -154,9 +158,11 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 3)
+ assert.Equal(t, len(updatedMap), 5)
assert.Equal(t, updatedMap["random"], "random")
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.abc")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.abc")
+ assert.Equal(t,
strings.HasPrefix(updatedMap[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(updatedMap[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("patch info content is not as expected")
@@ -186,8 +192,10 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 2)
+ assert.Equal(t, len(updatedMap), 4)
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.default")
+ assert.Equal(t,
strings.HasPrefix(updatedMap[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(updatedMap[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("patch info content is not as expected")
@@ -214,8 +222,10 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 2)
+ assert.Equal(t, len(updatedMap), 4)
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.default")
+ assert.Equal(t,
strings.HasPrefix(updatedMap[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(updatedMap[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("patch info content is not as expected")
@@ -240,8 +250,10 @@ func TestUpdateLabels(t *testing.T) {
assert.Equal(t, patch[0].Op, "add")
assert.Equal(t, patch[0].Path, "/metadata/labels")
if updatedMap, ok := patch[0].Value.(map[string]string); ok {
- assert.Equal(t, len(updatedMap), 2)
+ assert.Equal(t, len(updatedMap), 4)
+ assert.Equal(t, updatedMap[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, updatedMap[constants.LabelQueueName],
"root.default")
+ assert.Equal(t,
strings.HasPrefix(updatedMap[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(updatedMap[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("patch info content is not as expected")
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
index fe994da7..5276b5b6 100644
--- a/pkg/admission/util.go
+++ b/pkg/admission/util.go
@@ -35,10 +35,11 @@ func updatePodLabel(pod *v1.Pod, namespace string,
generateUniqueAppIds bool, de
result[k] = v
}
+ canonicalAppID := utils.GetPodLabelValue(pod,
constants.CanonicalLabelApplicationID)
sparkAppID := utils.GetPodLabelValue(pod, constants.SparkLabelAppID)
labelAppID := utils.GetPodLabelValue(pod, constants.LabelApplicationID)
annotationAppID := utils.GetPodAnnotationValue(pod,
constants.AnnotationApplicationID)
- if sparkAppID == "" && labelAppID == "" && annotationAppID == "" {
+ if canonicalAppID == "" && sparkAppID == "" && labelAppID == "" &&
annotationAppID == "" {
// if app id not exist, generate one
// for each namespace, we group unnamed pods to one single app
- if GenerateUniqueAppId is not set
// if GenerateUniqueAppId:
@@ -46,19 +47,31 @@ func updatePodLabel(pod *v1.Pod, namespace string,
generateUniqueAppIds bool, de
// else
// application ID convention:
${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
generatedID := utils.GenerateApplicationID(namespace,
generateUniqueAppIds, string(pod.UID))
+
+ result[constants.CanonicalLabelApplicationID] = generatedID
+ // Deprecated: After 1.7.0, admission controller will only add
canonical label if application ID was not set
result[constants.LabelApplicationID] = generatedID
+ } else if canonicalAppID != "" {
+ // Deprecated: Added in 1.6.0 for backward compatibility, in
case the prior shim version can't handle canonical label
+ result[constants.LabelApplicationID] = canonicalAppID
}
+ canonicalQueueName := utils.GetPodLabelValue(pod,
constants.CanonicalLabelQueueName)
labelQueueName := utils.GetPodLabelValue(pod, constants.LabelQueueName)
annotationQueueName := utils.GetPodAnnotationValue(pod,
constants.AnnotationQueueName)
- if labelQueueName == "" && annotationQueueName == "" {
+ if canonicalQueueName == "" && labelQueueName == "" &&
annotationQueueName == "" {
// if queueName not exist, generate one
// if defaultQueueName is "", skip adding default queue name to
the pod labels
if defaultQueueName != "" {
// for undefined configuration, am_conf will add
'root.default' to retain existing behavior
// if a custom name is configured for default queue, it
will be used instead of root.default
+ result[constants.CanonicalLabelQueueName] =
defaultQueueName
+ // Deprecated: After 1.7.0, admission controller will
only add canonical label if queue was not set
result[constants.LabelQueueName] = defaultQueueName
}
+ } else if canonicalQueueName != "" {
+ // Deprecated: Added in 1.6.0 for backward compatibility, in
case the prior shim version can't handle canonical label
+ result[constants.LabelQueueName] = canonicalQueueName
}
return result
diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go
index fa28c824..2181bd5a 100644
--- a/pkg/admission/util_test.go
+++ b/pkg/admission/util_test.go
@@ -70,8 +70,8 @@ func createTestingPodWithMeta() *v1.Pod {
func createTestingPodWithLabels(appId string, queue string) *v1.Pod {
pod := createTestingPodWithMeta()
- pod.ObjectMeta.Labels[constants.LabelApplicationID] = appId
- pod.ObjectMeta.Labels[constants.LabelQueueName] = queue
+ pod.ObjectMeta.Labels[constants.CanonicalLabelApplicationID] = appId
+ pod.ObjectMeta.Labels[constants.CanonicalLabelQueueName] = queue
return pod
}
@@ -111,21 +111,25 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
// we generate new appId/queue labels
pod := createTestingPodWithMeta()
if result := updatePodLabel(pod, "default", false, defaultQueueName);
result != nil {
- assert.Equal(t, len(result), 3)
+ assert.Equal(t, len(result), 5)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t,
strings.HasPrefix(result[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(result[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
defaultQueueName)
assert.Equal(t, result[constants.LabelQueueName],
defaultQueueName)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
}
- // verify if applicationId and queue is given in the labels,
- // we won't modify it
+ // verify if appId/queue is given in the canonical labels
+ // we won't modify the value and will add it to non-canonical label for
backward compatibility
pod = createTestingPodWithLabels(dummyAppId, dummyQueueName)
if result := updatePodLabel(pod, "default", false, defaultQueueName);
result != nil {
- assert.Equal(t, len(result), 3)
+ assert.Equal(t, len(result), 5)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t, result[constants.CanonicalLabelApplicationID],
dummyAppId)
assert.Equal(t, result[constants.LabelApplicationID],
dummyAppId)
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
dummyQueueName)
assert.Equal(t, result[constants.LabelQueueName],
dummyQueueName)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -147,8 +151,10 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
pod = createTestingPodNoNamespaceAndLabels()
if result := updatePodLabel(pod, "default", false, defaultQueueName);
result != nil {
- assert.Equal(t, len(result), 2)
+ assert.Equal(t, len(result), 4)
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
defaultQueueName)
assert.Equal(t, result[constants.LabelQueueName],
defaultQueueName)
+ assert.Equal(t,
strings.HasPrefix(result[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(result[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -157,8 +163,10 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
// pod name might be empty, it can comes from generatedName
pod = createTestingPodWithGenerateName()
if result := updatePodLabel(pod, "default", false, defaultQueueName);
result != nil {
- assert.Equal(t, len(result), 2)
+ assert.Equal(t, len(result), 4)
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
defaultQueueName)
assert.Equal(t, result[constants.LabelQueueName],
defaultQueueName)
+ assert.Equal(t,
strings.HasPrefix(result[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(result[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -166,8 +174,10 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
pod = createMinimalTestingPod()
if result := updatePodLabel(pod, "default", false, defaultQueueName);
result != nil {
- assert.Equal(t, len(result), 2)
+ assert.Equal(t, len(result), 4)
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
defaultQueueName)
assert.Equal(t, result[constants.LabelQueueName],
defaultQueueName)
+ assert.Equal(t,
strings.HasPrefix(result[constants.CanonicalLabelApplicationID],
constants.AutoGenAppPrefix), true)
assert.Equal(t,
strings.HasPrefix(result[constants.LabelApplicationID],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -178,9 +188,11 @@ func TestDefaultQueueName(t *testing.T) {
defaultConf := createConfig()
pod := createTestingPodWithMeta()
if result := updatePodLabel(pod, defaultConf.GetNamespace(),
defaultConf.GetGenerateUniqueAppIds(), defaultConf.GetDefaultQueueName());
result != nil {
- assert.Equal(t, len(result), 3)
+ assert.Equal(t, len(result), 5)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t, result[constants.CanonicalLabelApplicationID],
"yunikorn-default-autogen")
assert.Equal(t, result[constants.LabelApplicationID],
"yunikorn-default-autogen")
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
"root.default")
assert.Equal(t, result[constants.LabelQueueName],
"root.default")
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -190,9 +202,11 @@ func TestDefaultQueueName(t *testing.T) {
conf.AMFilteringDefaultQueueName: "",
})
if result := updatePodLabel(pod, queueNameEmptyConf.GetNamespace(),
queueNameEmptyConf.GetGenerateUniqueAppIds(),
queueNameEmptyConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 2)
+ assert.Equal(t, len(result), 3)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t, result[constants.CanonicalLabelApplicationID],
"yunikorn-default-autogen")
assert.Equal(t, result[constants.LabelApplicationID],
"yunikorn-default-autogen")
+ assert.Equal(t, result[constants.CanonicalLabelQueueName], "")
assert.Equal(t, result[constants.LabelQueueName], "")
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -202,9 +216,11 @@ func TestDefaultQueueName(t *testing.T) {
conf.AMFilteringDefaultQueueName: "yunikorn",
})
if result := updatePodLabel(pod, customQueueNameConf.GetNamespace(),
customQueueNameConf.GetGenerateUniqueAppIds(),
customQueueNameConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 3)
+ assert.Equal(t, len(result), 5)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t, result[constants.CanonicalLabelApplicationID],
"yunikorn-default-autogen")
assert.Equal(t, result[constants.LabelApplicationID],
"yunikorn-default-autogen")
+ assert.Assert(t, result[constants.CanonicalLabelQueueName] !=
"yunikorn")
assert.Assert(t, result[constants.LabelQueueName] != "yunikorn")
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -215,9 +231,11 @@ func TestDefaultQueueName(t *testing.T) {
})
if result := updatePodLabel(pod,
customValidQueueNameConf.GetNamespace(),
customValidQueueNameConf.GetGenerateUniqueAppIds(),
customValidQueueNameConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 3)
+ assert.Equal(t, len(result), 5)
assert.Equal(t, result["random"], "random")
+ assert.Equal(t, result[constants.CanonicalLabelApplicationID],
"yunikorn-default-autogen")
assert.Equal(t, result[constants.LabelApplicationID],
"yunikorn-default-autogen")
+ assert.Equal(t, result[constants.CanonicalLabelQueueName],
"root.yunikorn")
assert.Equal(t, result[constants.LabelQueueName],
"root.yunikorn")
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
diff --git a/pkg/common/constants/constants.go
b/pkg/common/constants/constants.go
index 7a4c415d..bdee70a8 100644
--- a/pkg/common/constants/constants.go
+++ b/pkg/common/constants/constants.go
@@ -38,9 +38,11 @@ const DomainYuniKornInternal =
siCommon.DomainYuniKornInternal
const LabelApp = "app"
const LabelApplicationID = "applicationId"
const AnnotationApplicationID = DomainYuniKorn + "app-id"
+const CanonicalLabelApplicationID = DomainYuniKorn + "app-id"
const LabelQueueName = "queue"
const RootQueue = "root"
const AnnotationQueueName = DomainYuniKorn + "queue"
+const CanonicalLabelQueueName = DomainYuniKorn + "queue"
const AnnotationParentQueue = DomainYuniKorn + "parentqueue"
const ApplicationDefaultQueue = "root.default"
const DefaultPartition = "default"
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
