You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by wi...@apache.org on 2023/07/07 08:18:17 UTC
[yunikorn-k8shim] branch master updated: [YUNIKORN-1856] generateUniqueAppId must not use pod UID (#632)
This is an automated email from the ASF dual-hosted git repository.
wilfreds 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 597b12d8 [YUNIKORN-1856] generateUniqueAppId must not use pod UID (#632)
597b12d8 is described below
commit 597b12d87cd8b0214f46ab09020e62708526dfe1
Author: Mit Desai <mi...@gmail.com>
AuthorDate: Fri Jul 7 18:14:38 2023 +1000
[YUNIKORN-1856] generateUniqueAppId must not use pod UID (#632)
Pod creation fails in the case that generateUniqueAppId is enabled. In
that case a pod is expected to have a UID which is not the case. Move to
using a generated UID for uniqueness.
Closes: #632
Signed-off-by: Wilfred Spiegelenburg <wi...@apache.org>
---
pkg/admission/util.go | 26 +++++++++++++++++---------
pkg/admission/util_test.go | 35 +++++++++++------------------------
2 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
index cc92310c..346a82dc 100644
--- a/pkg/admission/util.go
+++ b/pkg/admission/util.go
@@ -22,6 +22,7 @@ import (
"fmt"
"reflect"
+ "github.com/google/uuid"
"go.uber.org/zap"
v1 "k8s.io/api/core/v1"
@@ -41,9 +42,12 @@ func updatePodLabel(pod *v1.Pod, namespace string, generateUniqueAppIds bool, de
appID := utils.GetPodLabelValue(pod, constants.LabelApplicationID)
if sparkAppID == "" && appID == "" {
// if app id not exist, generate one
- // for each namespace, we group unnamed pods to one single app
- // application ID convention: ${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
- generatedID := generateAppID(namespace, pod, generateUniqueAppIds)
+ // for each namespace, we group unnamed pods to one single app - if GenerateUniqueAppId is not set
+ // if GenerateUniqueAppId:
+ // application ID convention: ${NAMESPACE}-${GENERATED_UUID}
+ // else
+ // application ID convention: ${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
+ generatedID := generateAppID(namespace, generateUniqueAppIds)
result[constants.LabelApplicationID] = generatedID
// if we generate an app ID, disable state-aware scheduling for this app
@@ -83,18 +87,22 @@ func convert2Namespace(obj interface{}) *v1.Namespace {
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, pod *v1.Pod, generateUniqueAppIds bool) string {
- podUid := string(pod.UID)
-
+func generateAppID(namespace string, generateUniqueAppIds bool) string {
var generatedID string
if generateUniqueAppIds {
- generatedID = fmt.Sprintf("%.26s-%s", namespace, string(podUid))
+ uuid := GetNewUUID()
+ generatedID = fmt.Sprintf("%.26s-%s", namespace, uuid)
} else {
generatedID = fmt.Sprintf("%s-%s-%s", constants.AutoGenAppPrefix, namespace, constants.AutoGenAppSuffix)
}
- appID := fmt.Sprintf("%.63s", generatedID)
- return appID
+
+ return fmt.Sprintf("%.63s", generatedID)
}
diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go
index 95903765..9a92221c 100644
--- a/pkg/admission/util_test.go
+++ b/pkg/admission/util_test.go
@@ -68,17 +68,6 @@ func createTestingPodWithMeta() *v1.Pod {
return pod
}
-func createTestingPodWithCustomFields(ns string) *v1.Pod {
- pod := createMinimalTestingPod()
- pod.ObjectMeta =
- metav1.ObjectMeta{
- Namespace: ns,
- UID: "abcd1234-5678-efgh-90ij-klmnopqrstuv",
- }
-
- return pod
-}
-
func createTestingPodWithAppId() *v1.Pod {
pod := createTestingPodWithMeta()
pod.ObjectMeta.Labels["applicationId"] = "app-0001"
@@ -229,15 +218,15 @@ func TestDefaultQueueName(t *testing.T) {
func TestGenerateAppID(t *testing.T) {
defaultConf := createConfig()
- appID := generateAppID("this-is-a-namespace", createTestingPodWithMeta(), defaultConf.GetGenerateUniqueAppIds())
+ 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", createTestingPodWithMeta(), defaultConf.GetGenerateUniqueAppIds())
+ 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), createTestingPodWithMeta(), defaultConf.GetGenerateUniqueAppIds())
+ 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)
@@ -246,15 +235,15 @@ func TestGenerateAppID(t *testing.T) {
conf.AMFilteringGenerateUniqueAppIds: fmt.Sprintf("%t", false),
})
- appID = generateAppID("this-is-a-namespace", createTestingPodWithMeta(), uniqueDisabled.GetGenerateUniqueAppIds())
+ 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", createTestingPodWithMeta(), uniqueDisabled.GetGenerateUniqueAppIds())
+ 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), createTestingPodWithMeta(), uniqueDisabled.GetGenerateUniqueAppIds())
+ 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)
@@ -263,18 +252,16 @@ func TestGenerateAppID(t *testing.T) {
conf.AMFilteringGenerateUniqueAppIds: fmt.Sprintf("%t", true),
})
- podUid := "abcd1234-5678-efgh-90ij-klmnopqrstuv"
-
// short namespace name
ns := "short"
- appID = generateAppID(ns, createTestingPodWithCustomFields(ns), uniqueEnabled.GetGenerateUniqueAppIds())
- assert.Equal(t, strings.HasSuffix(appID, fmt.Sprintf("short-%s", podUid)), true)
- assert.Equal(t, len(appID), len("short")+len("-")+len(podUid))
+ 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, createTestingPodWithCustomFields(ns), uniqueEnabled.GetGenerateUniqueAppIds())
+ appID = generateAppID(ns, uniqueEnabled.GetGenerateUniqueAppIds())
assert.Equal(t, strings.HasPrefix(appID, "long"), true)
- assert.Equal(t, strings.HasSuffix(appID, podUid), true)
+ assert.Equal(t, strings.HasPrefix(appID, ns[0:26]+"-"), true)
assert.Equal(t, len(appID), 63)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org