This is an automated email from the ASF dual-hosted git repository.
ccondit pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/branch-1.5 by this push:
new 4f8d48a3 [YUNIKORN-2711] Shim: Skip setting the queue name to default
queue
4f8d48a3 is described below
commit 4f8d48a30e711ec56b59eba7ccfa1d82026b2dac
Author: Mit Desai <[email protected]>
AuthorDate: Thu Jul 11 12:49:27 2024 -0500
[YUNIKORN-2711] Shim: Skip setting the queue name to default queue
Closes: #874
Signed-off-by: Craig Condit <[email protected]>
---
go.mod | 2 +-
go.sum | 4 +-
pkg/admission/admission_controller.go | 2 +-
pkg/admission/admission_controller_test.go | 16 ++----
pkg/admission/conf/am_conf.go | 17 ------
pkg/admission/conf/am_conf_test.go | 9 ----
pkg/admission/util.go | 12 +----
pkg/admission/util_test.go | 85 +++++-------------------------
pkg/common/constants/constants.go | 1 -
pkg/common/utils/utils.go | 2 +-
pkg/common/utils/utils_test.go | 2 +-
11 files changed, 25 insertions(+), 127 deletions(-)
diff --git a/go.mod b/go.mod
index d7535c60..43dbc994 100644
--- a/go.mod
+++ b/go.mod
@@ -21,7 +21,7 @@ module github.com/apache/yunikorn-k8shim
go 1.21
require (
- github.com/apache/yunikorn-core v1.5.1-1
+ github.com/apache/yunikorn-core v0.0.0-20240711174509-5454d7d32663
github.com/apache/yunikorn-scheduler-interface v1.5.1-1
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
diff --git a/go.sum b/go.sum
index 0562a61b..edb53ce5 100644
--- a/go.sum
+++ b/go.sum
@@ -9,8 +9,8 @@ github.com/NYTimes/gziphandler v1.1.1
h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cq
github.com/NYTimes/gziphandler v1.1.1/go.mod
h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c=
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df
h1:7RFfzj4SSt6nnvCPbCqijJi1nWCd+TqAT3bYCStRC18=
github.com/antlr/antlr4/runtime/Go/antlr/v4
v4.0.0-20230305170008-8188dc5388df/go.mod
h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM=
-github.com/apache/yunikorn-core v1.5.1-1
h1:BmPAoMd+ovH/NAG+VA2Nkb51fVGz2DuO5xoSqNn6lZ8=
-github.com/apache/yunikorn-core v1.5.1-1/go.mod
h1:49Kd4+44XRAWVdXzhtphnH6ctf5NthrHfRElSZNIiJo=
+github.com/apache/yunikorn-core v0.0.0-20240711174509-5454d7d32663
h1:1ENu93SCfvLcIMpRVANLg+L4hHItTevv8E97F4jw3lU=
+github.com/apache/yunikorn-core v0.0.0-20240711174509-5454d7d32663/go.mod
h1:49Kd4+44XRAWVdXzhtphnH6ctf5NthrHfRElSZNIiJo=
github.com/apache/yunikorn-scheduler-interface v1.5.1-1
h1:8NXsCFSrQ6yBodZQYcJWfyQW7CoYn3MlODYUDdiLmpc=
github.com/apache/yunikorn-scheduler-interface v1.5.1-1/go.mod
h1:gk3BtbzoUH7T5lhNoLxp/8g9pw25S1/d7NbiypV/30Q=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5
h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
diff --git a/pkg/admission/admission_controller.go
b/pkg/admission/admission_controller.go
index 3b058da6..dd0292e1 100644
--- a/pkg/admission/admission_controller.go
+++ b/pkg/admission/admission_controller.go
@@ -422,7 +422,7 @@ func (c *AdmissionController) updateLabels(namespace
string, pod *v1.Pod, patch
zap.String("namespace", namespace),
zap.Any("labels", pod.Labels))
- result := updatePodLabel(pod, namespace,
c.conf.GetGenerateUniqueAppIds(), c.conf.GetDefaultQueueName())
+ result := updatePodLabel(pod, namespace,
c.conf.GetGenerateUniqueAppIds())
patch = append(patch, common.PatchOperation{
Op: "add",
diff --git a/pkg/admission/admission_controller_test.go
b/pkg/admission/admission_controller_test.go
index 56f9b0b3..c42f0c89 100644
--- a/pkg/admission/admission_controller_test.go
+++ b/pkg/admission/admission_controller_test.go
@@ -81,9 +81,8 @@ 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), 4)
+ assert.Equal(t, len(updatedMap), 3)
assert.Equal(t, updatedMap["random"], "random")
- assert.Equal(t, updatedMap["queue"], "root.default")
assert.Equal(t, updatedMap["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -118,9 +117,8 @@ 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), 2)
assert.Equal(t, updatedMap["random"], "random")
- assert.Equal(t, updatedMap["queue"], "root.default")
assert.Equal(t, updatedMap["applicationId"], "app-0001")
} else {
t.Fatal("patch info content is not as expected")
@@ -188,8 +186,7 @@ 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, updatedMap["queue"], "root.default")
+ assert.Equal(t, len(updatedMap), 2)
assert.Equal(t, updatedMap["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -217,8 +214,7 @@ 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, updatedMap["queue"], "root.default")
+ assert.Equal(t, len(updatedMap), 2)
assert.Equal(t, updatedMap["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -244,8 +240,7 @@ 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, updatedMap["queue"], "root.default")
+ assert.Equal(t, len(updatedMap), 2)
assert.Equal(t, updatedMap["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -456,7 +451,6 @@ func TestMutate(t *testing.T) {
assert.Equal(t, schedulerName(t, resp.Patch), "yunikorn", "yunikorn not
set as scheduler for pod")
assert.Equal(t, labels(t, resp.Patch)["applicationId"],
"yunikorn-default-autogen", "wrong applicationId label")
assert.Equal(t, labels(t, resp.Patch)["disableStateAware"], "true",
"missing disableStateAware label")
- assert.Equal(t, labels(t, resp.Patch)["queue"], "root.default",
"incorrect queue name")
// pod without applicationID
pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{
diff --git a/pkg/admission/conf/am_conf.go b/pkg/admission/conf/am_conf.go
index 154df9ce..e55c0f3d 100644
--- a/pkg/admission/conf/am_conf.go
+++ b/pkg/admission/conf/am_conf.go
@@ -52,7 +52,6 @@ const (
AMFilteringLabelNamespaces = FilteringPrefix + "labelNamespaces"
AMFilteringNoLabelNamespaces = FilteringPrefix + "noLabelNamespaces"
AMFilteringGenerateUniqueAppIds = FilteringPrefix +
"generateUniqueAppId"
- AMFilteringDefaultQueueName = FilteringPrefix + "defaultQueue"
// access control configuration
AMAccessControlBypassAuth = AccessControlPrefix + "bypassAuth"
@@ -73,7 +72,6 @@ const (
DefaultFilteringLabelNamespaces = ""
DefaultFilteringNoLabelNamespaces = ""
DefaultFilteringGenerateUniqueAppIds = false
- DefaultFilteringQueueName = constants.ApplicationDefaultQueue
// access control defaults
DefaultAccessControlBypassAuth = false
@@ -102,7 +100,6 @@ type AdmissionControllerConf struct {
systemUsers []*regexp.Regexp
externalUsers []*regexp.Regexp
externalGroups []*regexp.Regexp
- defaultQueueName string
configMaps []*v1.ConfigMap
lock locking.RWMutex
@@ -217,12 +214,6 @@ func (acc *AdmissionControllerConf) GetExternalGroups()
[]*regexp.Regexp {
return acc.externalGroups
}
-func (acc *AdmissionControllerConf) GetDefaultQueueName() string {
- acc.lock.RLock()
- defer acc.lock.RUnlock()
- return acc.defaultQueueName
-}
-
type configMapUpdateHandler struct {
conf *AdmissionControllerConf
}
@@ -326,14 +317,6 @@ func (acc *AdmissionControllerConf)
updateConfigMaps(configMaps []*v1.ConfigMap,
acc.externalUsers = parseConfigRegexps(configs,
AMAccessControlExternalUsers, DefaultAccessControlExternalUsers)
acc.externalGroups = parseConfigRegexps(configs,
AMAccessControlExternalGroups, DefaultAccessControlExternalGroups)
- // labeling
- acc.defaultQueueName = parseConfigString(configs,
AMFilteringDefaultQueueName, DefaultFilteringQueueName)
- if acc.defaultQueueName != "" &&
!strings.HasPrefix(acc.defaultQueueName, constants.RootQueue) {
- log.Log(log.AdmissionConf).Warn("invalid default queue.
defaultQueue must be fully qualified. Resetting to "+DefaultFilteringQueueName,
- zap.String(AMFilteringDefaultQueueName,
acc.defaultQueueName))
- acc.defaultQueueName = DefaultFilteringQueueName
- }
-
// logging
log.UpdateLoggingConfig(configs)
diff --git a/pkg/admission/conf/am_conf_test.go
b/pkg/admission/conf/am_conf_test.go
index 0849647c..b6b40a85 100644
--- a/pkg/admission/conf/am_conf_test.go
+++ b/pkg/admission/conf/am_conf_test.go
@@ -43,7 +43,6 @@ func TestConfigMapVars(t *testing.T) {
AMAccessControlExternalUsers: "^yunikorn$",
AMAccessControlExternalGroups: "^devs$",
AMAccessControlTrustControllers: "false",
- AMFilteringDefaultQueueName: "root.default.queue",
}}})
assert.Equal(t, conf.GetPolicyGroup(), "testPolicyGroup")
assert.Equal(t, conf.GetAmServiceName(), "testYunikornService")
@@ -58,7 +57,6 @@ func TestConfigMapVars(t *testing.T) {
assert.Equal(t, conf.GetExternalUsers()[0].String(), "^yunikorn$")
assert.Equal(t, conf.GetExternalGroups()[0].String(), "^devs$")
assert.Equal(t, conf.GetTrustControllers(), false)
- assert.Equal(t, conf.GetDefaultQueueName(), "root.default.queue")
// test missing settings
conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, nil})
@@ -75,7 +73,6 @@ func TestConfigMapVars(t *testing.T) {
assert.Equal(t, 0, len(conf.GetExternalUsers()))
assert.Equal(t, 0, len(conf.GetExternalGroups()))
assert.Equal(t, conf.GetTrustControllers(),
DefaultAccessControlTrustControllers)
- assert.Equal(t, conf.GetDefaultQueueName(), DefaultFilteringQueueName)
// test faulty settings for boolean values
conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, {Data:
map[string]string{
@@ -93,12 +90,6 @@ func TestConfigMapVars(t *testing.T) {
}}})
assert.Equal(t, len(conf.GetProcessNamespaces()), 0)
- // test invalid defaultQueue name
- conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, {Data:
map[string]string{
- AMFilteringDefaultQueueName: "default.queue",
- }}})
- assert.Equal(t, conf.GetDefaultQueueName(), DefaultFilteringQueueName)
-
// test disable / enable of config hot refresh
conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, nil})
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
index 5c957648..adf93cd8 100644
--- a/pkg/admission/util.go
+++ b/pkg/admission/util.go
@@ -29,7 +29,7 @@ import (
"github.com/apache/yunikorn-k8shim/pkg/log"
)
-func updatePodLabel(pod *v1.Pod, namespace string, generateUniqueAppIds bool,
defaultQueueName string) map[string]string {
+func updatePodLabel(pod *v1.Pod, namespace string, generateUniqueAppIds bool)
map[string]string {
existingLabels := pod.Labels
result := make(map[string]string)
for k, v := range existingLabels {
@@ -54,16 +54,6 @@ func updatePodLabel(pod *v1.Pod, namespace string,
generateUniqueAppIds bool, de
}
}
- // if existing label exist, it takes priority over everything else
- if _, ok := existingLabels[constants.LabelQueueName]; !ok {
- // 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.LabelQueueName] = defaultQueueName
- }
- }
-
return result
}
diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go
index 1b768631..21b8b6aa 100644
--- a/pkg/admission/util_test.go
+++ b/pkg/admission/util_test.go
@@ -103,10 +103,9 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
// verify when appId/queue are not given,
pod := createTestingPodWithMeta()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
- assert.Equal(t, len(result), 4)
+ if result := updatePodLabel(pod, "default", false); result != nil {
+ assert.Equal(t, len(result), 3)
assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["queue"], "root.default")
assert.Equal(t, result["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(result["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -117,10 +116,9 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
// we won't modify it
pod = createTestingPodWithAppId()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
- assert.Equal(t, len(result), 3)
+ if result := updatePodLabel(pod, "default", false); result != nil {
+ assert.Equal(t, len(result), 2)
assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["queue"], "root.default")
assert.Equal(t, result["applicationId"], "app-0001")
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
@@ -129,23 +127,22 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
// verify if queue is given in the labels,
// we won't modify it
pod = createTestingPodWithQueue()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
+ if result := updatePodLabel(pod, "default", false); result != nil {
assert.Equal(t, len(result), 4)
assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["queue"], "root.abc")
assert.Equal(t, result["disableStateAware"], "true")
+ assert.Equal(t, result["queue"], "root.abc")
assert.Equal(t, strings.HasPrefix(result["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
- t.Fatal("UpdatePodLabelForAdmissionControllert is not as
expected")
+ t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
}
// namespace might be empty
// labels might be empty
pod = createTestingPodNoNamespaceAndLabels()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
- assert.Equal(t, len(result), 3)
- assert.Equal(t, result["queue"], "root.default")
+ if result := updatePodLabel(pod, "default", false); result != nil {
+ assert.Equal(t, len(result), 2)
assert.Equal(t, result["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(result["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -154,9 +151,8 @@ func TestUpdatePodLabelForAdmissionController(t *testing.T)
{
// pod name might be empty, it can comes from generatedName
pod = createTestingPodWithGenerateName()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
- assert.Equal(t, len(result), 3)
- assert.Equal(t, result["queue"], "root.default")
+ if result := updatePodLabel(pod, "default", false); result != nil {
+ assert.Equal(t, len(result), 2)
assert.Equal(t, result["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(result["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
@@ -164,66 +160,11 @@ func TestUpdatePodLabelForAdmissionController(t
*testing.T) {
}
pod = createMinimalTestingPod()
- if result := updatePodLabel(pod, "default", false, "root.default");
result != nil {
- assert.Equal(t, len(result), 3)
- assert.Equal(t, result["queue"], "root.default")
+ if result := updatePodLabel(pod, "default", false); result != nil {
+ assert.Equal(t, len(result), 2)
assert.Equal(t, result["disableStateAware"], "true")
assert.Equal(t, strings.HasPrefix(result["applicationId"],
constants.AutoGenAppPrefix), true)
} else {
t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
}
}
-
-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), 4)
- assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["applicationId"],
"yunikorn-default-autogen")
- assert.Equal(t, result["disableStateAware"], "true")
- assert.Equal(t, result["queue"], "root.default")
- } else {
- t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
- }
-
- queueNameEmptyConf := createConfigWithOverrides(map[string]string{
- conf.AMFilteringDefaultQueueName: "",
- })
- if result := updatePodLabel(pod, queueNameEmptyConf.GetNamespace(),
queueNameEmptyConf.GetGenerateUniqueAppIds(),
queueNameEmptyConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 3)
- assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["applicationId"],
"yunikorn-default-autogen")
- assert.Equal(t, result["disableStateAware"], "true")
- assert.Equal(t, result["queue"], "")
- } else {
- t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
- }
-
- customQueueNameConf := createConfigWithOverrides(map[string]string{
- conf.AMFilteringDefaultQueueName: "yunikorn",
- })
- if result := updatePodLabel(pod, customQueueNameConf.GetNamespace(),
customQueueNameConf.GetGenerateUniqueAppIds(),
customQueueNameConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 4)
- assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["applicationId"],
"yunikorn-default-autogen")
- assert.Equal(t, result["disableStateAware"], "true")
- assert.Assert(t, result["queue"] != "yunikorn")
- } else {
- t.Fatal("UpdatePodLabelForAdmissionController is not as
expected")
- }
-
- customValidQueueNameConf := createConfigWithOverrides(map[string]string{
- conf.AMFilteringDefaultQueueName: "root.yunikorn",
- })
- if result := updatePodLabel(pod,
customValidQueueNameConf.GetNamespace(),
- customValidQueueNameConf.GetGenerateUniqueAppIds(),
customValidQueueNameConf.GetDefaultQueueName()); result != nil {
- assert.Equal(t, len(result), 4)
- assert.Equal(t, result["random"], "random")
- assert.Equal(t, result["applicationId"],
"yunikorn-default-autogen")
- assert.Equal(t, result["disableStateAware"], "true")
- assert.Equal(t, result["queue"], "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 ab4dbd1c..053d548a 100644
--- a/pkg/common/constants/constants.go
+++ b/pkg/common/constants/constants.go
@@ -42,7 +42,6 @@ const RootQueue = "root"
const AnnotationQueueName = DomainYuniKorn + "queue"
const AnnotationParentQueue = DomainYuniKorn + "parentqueue"
const LabelDisableStateAware = "disableStateAware"
-const ApplicationDefaultQueue = "root.default"
const DefaultPartition = "default"
const AppTagNamespace = "namespace"
const AppTagNamespaceParentQueue = "namespace.parentqueue"
diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go
index b0a84470..0d8652e8 100644
--- a/pkg/common/utils/utils.go
+++ b/pkg/common/utils/utils.go
@@ -104,7 +104,7 @@ func IsAssignedPod(pod *v1.Pod) bool {
}
func GetQueueNameFromPod(pod *v1.Pod) string {
- queueName := constants.ApplicationDefaultQueue
+ queueName := ""
if an := GetPodLabelValue(pod, constants.LabelQueueName); an != "" {
queueName = an
} else if qu := GetPodAnnotationValue(pod,
constants.AnnotationQueueName); qu != "" {
diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go
index 9bebae2b..d2c1b1fc 100644
--- a/pkg/common/utils/utils_test.go
+++ b/pkg/common/utils/utils_test.go
@@ -857,7 +857,7 @@ func TestGetQueueNameFromPod(t *testing.T) {
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{},
},
- expectedQueue: constants.ApplicationDefaultQueue,
+ expectedQueue: "",
},
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
