Issue deprecated warning if applicationInfo is from labels

apache · Nov 20, 2023 · cf71eef · cf71eef
1 parent d417aef
commit cf71eef
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 22 deletions.
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
@@ -33,12 +33,19 @@ func getNewApplicationInfo(pod *v1.Pod, namespace string, generateUniqueAppIds b
 	newLabels := make(map[string]string)
 	newAnnotations := make(map[string]string)
 
-	appID := getApplicationIDFromPod(pod)
-	disableStateAware := getDisableStateAwareFromPod(pod)
+	appID, isAppIdFromLabel := getApplicationIDFromPod(pod)
+	disableStateAware, isDisableStateAwareFromLabel := getDisableStateAwareFromPod(pod)
 
 	// for undefined configuration, am_conf will add 'root.default' as default queue name
 	// if a custom name is configured for default queue, it will be used instead of root.default
-	queueName := utils.GetQueueNameFromPod(pod, defaultQueueName)
+	queueName, isQueueNameFromLabel := utils.GetQueueNameFromPod(pod, defaultQueueName)
+
+	if isAppIdFromLabel || isDisableStateAwareFromLabel || isQueueNameFromLabel {
+		log.Log(log.Admission).Warn("Pod contains label for Yunikorn configuration. This is deprecated and will be ignored in a future release. Please migrate to annotation-based configuration.",
+			zap.Bool("applicationId from label", isAppIdFromLabel),
+			zap.Bool("disableStateAware from label", isDisableStateAwareFromLabel),
+			zap.Bool("queue name from label", isQueueNameFromLabel))
+	}
 
 	if appID == "" {
 		// if app id not exist, generate one
@@ -122,26 +129,26 @@ func convert2Namespace(obj interface{}) *v1.Namespace {
 	return nil
 }
 
-func getApplicationIDFromPod(pod *v1.Pod) string {
+func getApplicationIDFromPod(pod *v1.Pod) (value string, isFromLabel bool) {
 	// if existing annotation exist, it takes priority over everything else
 	if value := utils.GetPodAnnotationValue(pod, constants.AnnotationApplicationID); value != "" {
-		return value
+		return value, false
 	} else if value := utils.GetPodLabelValue(pod, constants.LabelApplicationID); value != "" {
-		return value
+		return value, true
 	}
 	// application ID can be defined in Spark label
 	if value := utils.GetPodLabelValue(pod, constants.SparkLabelAppID); value != "" {
-		return value
+		return value, true
 	}
-	return ""
+	return "", false
 }
 
-func getDisableStateAwareFromPod(pod *v1.Pod) string {
+func getDisableStateAwareFromPod(pod *v1.Pod) (value string, isFromLabel bool) {
 	// if existing annotation exist, it takes priority over everything else
 	if value := utils.GetPodAnnotationValue(pod, constants.AnnotationDisableStateAware); value != "" {
-		return value
+		return value, false
 	} else if value := utils.GetPodLabelValue(pod, constants.LabelDisableStateAware); value != "" {
-		return value
+		return value, true
 	}
-	return ""
+	return "", false
 }
diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go
@@ -351,7 +351,9 @@ func TestGetApplicationIDFromPod(t *testing.T) {
 
 	// test empty pod
 	pod := &v1.Pod{}
-	assert.Equal(t, getApplicationIDFromPod(pod), "")
+	appId, isFromLabel := getApplicationIDFromPod(pod)
+	assert.Equal(t, appId, "")
+	assert.Equal(t, isFromLabel, false)
 
 	// test annotation take precedence over label
 	pod = &v1.Pod{
@@ -364,7 +366,9 @@ func TestGetApplicationIDFromPod(t *testing.T) {
 			},
 		},
 	}
-	assert.Equal(t, getApplicationIDFromPod(pod), appName[1])
+	appId, isFromLabel = getApplicationIDFromPod(pod)
+	assert.Equal(t, appId, appName[1])
+	assert.Equal(t, isFromLabel, false)
 
 	// test pod with label only
 	pod = &v1.Pod{
@@ -374,13 +378,17 @@ func TestGetApplicationIDFromPod(t *testing.T) {
 			},
 		},
 	}
-	assert.Equal(t, getApplicationIDFromPod(pod), appName[0])
+	appId, isFromLabel = getApplicationIDFromPod(pod)
+	assert.Equal(t, appId, appName[0])
+	assert.Equal(t, isFromLabel, true)
 }
 
 func TestGetDisableStateAwareFromPod(t *testing.T) {
 	// test empty pod
 	pod := &v1.Pod{}
-	assert.Equal(t, getDisableStateAwareFromPod(pod), "")
+	disableStateAware, isFromLabel := getDisableStateAwareFromPod(pod)
+	assert.Equal(t, disableStateAware, "")
+	assert.Equal(t, isFromLabel, false)
 
 	// test annotation take precedence over label
 	pod = &v1.Pod{
@@ -393,7 +401,9 @@ func TestGetDisableStateAwareFromPod(t *testing.T) {
 			},
 		},
 	}
-	assert.Equal(t, getDisableStateAwareFromPod(pod), "false")
+	disableStateAware, isFromLabel = getDisableStateAwareFromPod(pod)
+	assert.Equal(t, disableStateAware, "false")
+	assert.Equal(t, isFromLabel, false)
 
 	// test pod with label only
 	pod = &v1.Pod{
@@ -403,5 +413,7 @@ func TestGetDisableStateAwareFromPod(t *testing.T) {
 			},
 		},
 	}
-	assert.Equal(t, getDisableStateAwareFromPod(pod), "true")
+	disableStateAware, isFromLabel = getDisableStateAwareFromPod(pod)
+	assert.Equal(t, disableStateAware, "true")
+	assert.Equal(t, isFromLabel, true)
 }
diff --git a/pkg/appmgmt/general/metadata.go b/pkg/appmgmt/general/metadata.go
@@ -67,6 +67,7 @@ func getAppMetadata(pod *v1.Pod, recovery bool) (interfaces.ApplicationMetadata,
 			zap.String("name", pod.Name))
 		return interfaces.ApplicationMetadata{}, false
 	}
+	queueName, _ := utils.GetQueueNameFromPod(pod, constants.ApplicationDefaultQueue)
 
 	// tags will at least have namespace info
 	// labels or annotations from the pod can be added when needed
@@ -120,7 +121,7 @@ func getAppMetadata(pod *v1.Pod, recovery bool) (interfaces.ApplicationMetadata,
 
 	return interfaces.ApplicationMetadata{
 		ApplicationID:              appID,
-		QueueName:                  utils.GetQueueNameFromPod(pod, constants.ApplicationDefaultQueue),
+		QueueName:                  queueName,
 		User:                       user,
 		Groups:                     groups,
 		Tags:                       tags,

diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go
@@ -102,14 +102,16 @@ func IsAssignedPod(pod *v1.Pod) bool {
 	return len(pod.Spec.NodeName) != 0
 }
 
-func GetQueueNameFromPod(pod *v1.Pod, defaultQueueName string) string {
+func GetQueueNameFromPod(pod *v1.Pod, defaultQueueName string) (value string, isFromLabel bool) {
 	queueName := defaultQueueName
+	isFromLabel = false
 	if value := GetPodAnnotationValue(pod, constants.AnnotationQueueName); value != "" {
 		queueName = value
 	} else if value := GetPodLabelValue(pod, constants.LabelQueueName); value != "" {
+		isFromLabel = true
 		queueName = value
 	}
-	return queueName
+	return queueName, isFromLabel
 }
 
 // GenerateApplicationID generates an appID based on the namespace value

diff --git a/pkg/common/utils/utils_test.go b/pkg/common/utils/utils_test.go
@@ -765,6 +765,7 @@ func TestGetQueueNameFromPod(t *testing.T) {
 		name          string
 		pod           *v1.Pod
 		expectedQueue string
+		isFromLabel   bool
 	}{
 		{
 			name: "With queue label",
@@ -774,6 +775,7 @@ func TestGetQueueNameFromPod(t *testing.T) {
 				},
 			},
 			expectedQueue: queueInLabel,
+			isFromLabel:   true,
 		},
 		{
 			name: "With queue annotation",
@@ -783,6 +785,7 @@ func TestGetQueueNameFromPod(t *testing.T) {
 				},
 			},
 			expectedQueue: queueInAnnotation,
+			isFromLabel:   false,
 		},
 		{
 			name: "With queue label and annotation",
@@ -793,20 +796,23 @@ func TestGetQueueNameFromPod(t *testing.T) {
 				},
 			},
 			expectedQueue: queueInAnnotation,
+			isFromLabel:   false,
 		},
 		{
 			name: "Without queue label and annotation",
 			pod: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{},
 			},
 			expectedQueue: constants.ApplicationDefaultQueue,
+			isFromLabel:   false,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			queue := GetQueueNameFromPod(tc.pod, constants.ApplicationDefaultQueue)
+			queue, isFromLabel := GetQueueNameFromPod(tc.pod, constants.ApplicationDefaultQueue)
 			assert.Equal(t, queue, tc.expectedQueue)
+			assert.Equal(t, isFromLabel, tc.isFromLabel)
 		})
 	}
 }