diff --git a/go.mod b/go.mod index d7535c60b..43dbc9940 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 0562a61b8..edb53ce59 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 3b058da6b..dd0292e19 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 56f9b0b33..c42f0c890 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 154df9ced..e55c0f3dc 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 0849647c7..b6b40a850 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 5c9576482..adf93cd8a 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 1b768631c..21b8b6aae 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 ab4dbd1c8..053d548aa 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 b0a844709..0d8652e8d 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 9bebae2bb..d2c1b1fc8 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: "", }, }