diff --git a/pkg/admission/util.go b/pkg/admission/util.go index 346a82dc7..5c9576482 100644 --- a/pkg/admission/util.go +++ b/pkg/admission/util.go @@ -19,10 +19,8 @@ package admission import ( - "fmt" "reflect" - "github.com/google/uuid" "go.uber.org/zap" v1 "k8s.io/api/core/v1" @@ -44,10 +42,10 @@ func updatePodLabel(pod *v1.Pod, namespace string, generateUniqueAppIds bool, de // 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}-${GENERATED_UUID} + // application ID convention: ${NAMESPACE}-${POD_UID} // else // application ID convention: ${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX} - generatedID := generateAppID(namespace, generateUniqueAppIds) + generatedID := utils.GenerateApplicationID(namespace, generateUniqueAppIds, string(pod.UID)) result[constants.LabelApplicationID] = generatedID // if we generate an app ID, disable state-aware scheduling for this app @@ -86,23 +84,3 @@ func convert2Namespace(obj interface{}) *v1.Namespace { log.Log(log.AdmissionUtils).Warn("cannot convert to *v1.Namespace", zap.Stringer("type", reflect.TypeOf(obj))) return nil } - -// Generate a new uuid. The chance of getting duplicate are very small -func GetNewUUID() string { - return uuid.NewString() -} - -// generate appID based on the namespace value -// if configured to generate unique appID, generate appID as - namespace capped at 26chars -// if not set or configured as false, appID generated as -- -func generateAppID(namespace string, generateUniqueAppIds bool) string { - var generatedID string - if generateUniqueAppIds { - uuid := GetNewUUID() - generatedID = fmt.Sprintf("%.26s-%s", namespace, uuid) - } else { - generatedID = fmt.Sprintf("%s-%s-%s", constants.AutoGenAppPrefix, namespace, constants.AutoGenAppSuffix) - } - - return fmt.Sprintf("%.63s", generatedID) -} diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go index c85554edc..1b768631c 100644 --- a/pkg/admission/util_test.go +++ b/pkg/admission/util_test.go @@ -19,7 +19,6 @@ package admission import ( - "fmt" "strings" "testing" @@ -228,54 +227,3 @@ func TestDefaultQueueName(t *testing.T) { t.Fatal("UpdatePodLabelForAdmissionController is not as expected") } } - -func TestGenerateAppID(t *testing.T) { - defaultConf := createConfig() - - appID := generateAppID("this-is-a-namespace", defaultConf.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-this-is-a-namespace", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 36) - - appID = generateAppID("short", defaultConf.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-short", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 22) - - appID = generateAppID(strings.Repeat("long", 100), defaultConf.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-long", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 63) - - // explicitly disable autogen config - uniqueDisabled := createConfigWithOverrides(map[string]string{ - conf.AMFilteringGenerateUniqueAppIds: fmt.Sprintf("%t", false), - }) - - appID = generateAppID("this-is-a-namespace", uniqueDisabled.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-this-is-a-namespace", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 36) - - appID = generateAppID("short", uniqueDisabled.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-short", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 22) - - appID = generateAppID(strings.Repeat("long", 100), uniqueDisabled.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, fmt.Sprintf("%s-long", constants.AutoGenAppPrefix)), true) - assert.Equal(t, len(appID), 63) - - // enabled autogen config - uniqueEnabled := createConfigWithOverrides(map[string]string{ - conf.AMFilteringGenerateUniqueAppIds: fmt.Sprintf("%t", true), - }) - - // short namespace name - ns := "short" - appID = generateAppID(ns, uniqueEnabled.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, "short-"), true) - assert.Equal(t, len(appID), len("short")+len("-")+len(GetNewUUID())) - - // long namespace name - ns = strings.Repeat("long", 100) - appID = generateAppID(ns, uniqueEnabled.GetGenerateUniqueAppIds()) - assert.Equal(t, strings.HasPrefix(appID, "long"), true) - assert.Equal(t, strings.HasPrefix(appID, ns[0:26]+"-"), true) - assert.Equal(t, len(appID), 63) -} diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index df69b9d07..dc627ab85 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -102,6 +102,20 @@ func GetQueueNameFromPod(pod *v1.Pod) string { return queueName } +// GenerateApplicationID generates an appID based on the namespace value +// if configured to generate unique appID, generate appID as - namespace capped at 26chars +// if not set or configured as false, appID generated as -- +func GenerateApplicationID(namespace string, generateUniqueAppIds bool, podUID string) string { + var generatedID string + if generateUniqueAppIds { + generatedID = fmt.Sprintf("%.26s-%s", namespace, podUID) + } else { + generatedID = fmt.Sprintf("%s-%s-%s", constants.AutoGenAppPrefix, namespace, constants.AutoGenAppSuffix) + } + + return fmt.Sprintf("%.63s", generatedID) +} + // GetApplicationIDFromPod returns the applicationID (if present) from a Pod or an empty string if not present. // If an applicationID is present, the Pod is managed by YuniKorn. Otherwise, it is managed by an external scheduler. func GetApplicationIDFromPod(pod *v1.Pod) string { diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go index 2e2fd02ef..712d36b15 100644 --- a/pkg/common/utils/utils_test.go +++ b/pkg/common/utils/utils_test.go @@ -23,6 +23,7 @@ import ( "compress/gzip" "encoding/base64" "fmt" + "strings" "testing" "time" @@ -587,6 +588,26 @@ func TestGetApplicationIDFromPod(t *testing.T) { } } +func TestGenerateApplicationID(t *testing.T) { + assert.Equal(t, "yunikorn-this-is-a-namespace-autogen", + GenerateApplicationID("this-is-a-namespace", false, "pod-uid")) + + assert.Equal(t, "this-is-a-namespace-pod-uid", + GenerateApplicationID("this-is-a-namespace", true, "pod-uid")) + + assert.Equal(t, "yunikorn-short-autogen", + GenerateApplicationID("short", false, "pod-uid")) + + assert.Equal(t, "short-pod-uid", + GenerateApplicationID("short", true, "pod-uid")) + + assert.Equal(t, "yunikorn-longlonglonglonglonglonglonglonglonglonglonglonglonglo", + GenerateApplicationID(strings.Repeat("long", 100), false, "pod-uid")) + + assert.Equal(t, "longlonglonglonglonglonglo-pod-uid", + GenerateApplicationID(strings.Repeat("long", 100), true, "pod-uid")) +} + func TestMergeMaps(t *testing.T) { result := MergeMaps(nil, nil) assert.Assert(t, result == nil)