Skip to content

Commit

Permalink
[YUNIKORN-2631] Support canonical labels for queue/applicationId in A…
Browse files Browse the repository at this point in the history 
…dmission Controller (apache#843)

Add canonical representations when Admission Controller try to patch queue/applicationID to pod

Closes: apache#843

Signed-off-by: Yu-Lin Chen <[email protected]>
  • Loading branch information
@chenyulin0719
chenyulin0719 committed Jun 8, 2024
1 parent 8e680af commit 223e1f3
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/admission/admission_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
32 changes: 22 additions & 10 deletions pkg/admission/admission_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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{},
Expand All @@ -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")
Expand All @@ -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{},
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
17 changes: 15 additions & 2 deletions pkg/admission/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,43 @@ 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:
// application ID convention: ${NAMESPACE}-${POD_UID}
// 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
Expand Down
44 changes: 31 additions & 13 deletions pkg/admission/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -157,17 +163,21 @@ 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")
}

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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions pkg/common/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 223e1f3

Please sign in to comment.