This is an automated email from the ASF dual-hosted git repository.
ccondit 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 fc54037e [YUNIKORN-2082] Make autogenerated applicationIDs
reproducible (#707)
fc54037e is described below
commit fc54037e35af3e7dd40f57c3975c9421f153d21a
Author: Craig Condit <[email protected]>
AuthorDate: Fri Oct 27 15:38:08 2023 -0500
[YUNIKORN-2082] Make autogenerated applicationIDs reproducible (#707)
Moved the admission-controller private function generateAppID() to
the shim utils package and renamed to GenerateApplicationID() to
allow for re-use by the shim. Updated implementation to use the pod
UID instead of a random UUID so that the same identifier will always
be created for the same Pod.
Closes: #707
---
pkg/admission/util.go | 26 ++-------------------
pkg/admission/util_test.go | 52 ------------------------------------------
pkg/common/utils/utils.go | 14 ++++++++++++
pkg/common/utils/utils_test.go | 21 +++++++++++++++++
4 files changed, 37 insertions(+), 76 deletions(-)
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
index 346a82dc..5c957648 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>-<pod-uid> namespace capped at 26chars
-// if not set or configured as false, appID generated as
<autogen-prefix>-<namespace>-<autogen-suffix>
-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 c85554ed..1b768631 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 df69b9d0..dc627ab8 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>-<pod-uid> namespace capped at 26chars
+// if not set or configured as false, appID generated as
<autogen-prefix>-<namespace>-<autogen-suffix>
+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 2e2fd02e..712d36b1 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)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
