code review: adjust log message and emit pod event

pkg/cache/task.go

@@ -536,16 +536,22 @@ func (task *Task) sanityCheckBeforeScheduling() error {
 	// After version 1.7.0, we should reject the task whose pod is unbound and has conflicting metadata.
 	if !utils.PodAlreadyBound(task.pod) {
 		if err := utils.CheckAppIdInPod(task.pod); err != nil {
-			log.Log(log.ShimCacheTask).Warn("The task has conflicting metadata will be rejected after version 1.7.0.",
+			log.Log(log.ShimCacheTask).Warn("Pod has inconsistent application metadata and may be rejected in a future YuniKorn release.",
 				zap.String("appID", task.applicationID),
 				zap.String("podName", task.pod.Name),
 				zap.String("error", err.Error()))
+
+			events.GetRecorder().Eventf(task.pod.DeepCopy(),
+				nil, v1.EventTypeWarning, "Scheduling", "Scheduling", fmt.Sprintf("Pod has inconsistent application metadata and may be rejected in a future YuniKorn release: %s", err.Error()))
 		}
 		if err := utils.CheckQueueNameInPod(task.pod); err != nil {
-			log.Log(log.ShimCacheTask).Warn("The task has conflicting metadata will be rejected after version 1.7.0.",
+			log.Log(log.ShimCacheTask).Warn("Pod has inconsistent queue metadata and may be rejected in a future YuniKorn release.",
 				zap.String("appID", task.applicationID),
 				zap.String("podName", task.pod.Name),
 				zap.String("error", err.Error()))
+
+			events.GetRecorder().Eventf(task.pod.DeepCopy(),
+				nil, v1.EventTypeWarning, "Scheduling", "Scheduling", fmt.Sprintf("Pod has inconsistent queue metadata and may be rejected in a future YuniKorn release: %s", err.Error()))
 		}
 	}
 	return task.checkPodPVCs()

pkg/common/constants/constants.go

@@ -26,6 +26,10 @@ import (
 const True = "true"
 const False = "false"
 
+// Kubernetes
+const Label = "label"
+const Annotation = "annotation"
+
 // Cluster
 const DefaultNodeAttributeHostNameKey = "si.io/hostname"
 const DefaultNodeAttributeRackNameKey = "si.io/rackname"

pkg/common/utils/utils.go

@@ -214,44 +214,46 @@ func GetApplicationIDFromPod(pod *v1.Pod) string {
 }
 
 func CheckAppIdInPod(pod *v1.Pod) error {
-	if err := ValidatePodLabelAnnotation(pod, constants.AppIdLabelKeys, constants.AppIdAnnotationKeys); err != nil {
-		return fmt.Errorf("pod has inconsistent application ID in labels and annotations. %w", err)
-	}
-	return nil
+	return ValidatePodLabelAnnotation(pod, constants.AppIdLabelKeys, constants.AppIdAnnotationKeys)
 }
 
 func CheckQueueNameInPod(pod *v1.Pod) error {
-	if err := ValidatePodLabelAnnotation(pod, constants.QueueLabelKeys, constants.QueueAnnotationKeys); err != nil {
-		return fmt.Errorf("pod has inconsistent queue name in labels and annotations. %w", err)
-	}
-	return nil
+	return ValidatePodLabelAnnotation(pod, constants.QueueLabelKeys, constants.QueueAnnotationKeys)
 }
 
 // return true if all non-empty values are same across all provided label/annotation
 func ValidatePodLabelAnnotation(pod *v1.Pod, labelKeys []string, annotationKeys []string) error {
+	var referenceKey string
 	var referenceValue string
+	var referenceType string
 
+	checkingType := constants.Label
 	for _, key := range labelKeys {
 		value := GetPodLabelValue(pod, key)
 		if value == "" {
 			continue
 		}
 		if referenceValue == "" {
+			referenceKey = key
 			referenceValue = value
+			referenceType = checkingType
 		} else if referenceValue != value {
-			return fmt.Errorf("inconsistent values: %s, %s", referenceValue, value)
+			return fmt.Errorf("%s %s: \"%s\" doesn't match %s %s: \"%s\"", checkingType, key, value, referenceType, referenceKey, referenceValue)
 		}
 	}
 
+	checkingType = constants.Annotation
 	for _, key := range annotationKeys {
 		value := GetPodAnnotationValue(pod, key)
 		if value == "" {
 			continue
 		}
 		if referenceValue == "" {
+			referenceKey = key
 			referenceValue = value
+			referenceType = checkingType
 		} else if referenceValue != value {
-			return fmt.Errorf("inconsistent values: %s, %s", referenceValue, value)
+			return fmt.Errorf("%s %s: \"%s\" doesn't match %s %s: \"%s\"", checkingType, key, value, referenceType, referenceKey, referenceValue)
 		}
 	}
 

pkg/common/utils/utils_test.go

@@ -725,7 +725,6 @@ func TestGetApplicationIDFromPod(t *testing.T) {
 }
 
 func TestCheckAppIdInPod(t *testing.T) {
-	mismatchError := errors.New("pod has inconsistent application ID")
 	testCases := []struct {
 		name     string
 		pod      *v1.Pod
@@ -757,7 +756,7 @@ func TestCheckAppIdInPod(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("label spark-app-selector: \"app-456\" doesn't match label yunikorn.apache.org/app-id: \"app-123\""),
 		},
 		{
 			name: "inconsistent app ID between label and annotation",
@@ -771,7 +770,7 @@ func TestCheckAppIdInPod(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("annotation yunikorn.apache.org/app-id: \"app-456\" doesn't match label yunikorn.apache.org/app-id: \"app-123\""),
 		},
 	}
 	for _, tc := range testCases {
@@ -787,7 +786,6 @@ func TestCheckAppIdInPod(t *testing.T) {
 }
 
 func TestCheckQueueNameInPod(t *testing.T) {
-	mismatchError := errors.New("pod has inconsistent queue name")
 	testCases := []struct {
 		name     string
 		pod      *v1.Pod
@@ -818,7 +816,7 @@ func TestCheckQueueNameInPod(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("label queue: \"root.b\" doesn't match label yunikorn.apache.org/queue: \"root.a\""),
 		},
 		{
 			name: "inconsistent app ID between label and annotation",
@@ -832,7 +830,7 @@ func TestCheckQueueNameInPod(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("annotation yunikorn.apache.org/queue: \"root.b\" doesn't match label yunikorn.apache.org/queue: \"root.a\""),
 		},
 	}
 	for _, tc := range testCases {
@@ -850,7 +848,6 @@ func TestCheckQueueNameInPod(t *testing.T) {
 func TestValidatePodLabelAnnotation(t *testing.T) {
 	labelKeys := []string{"labelKey1", "labelKey2"}
 	annotationKeys := []string{"annotationKey1", "annotationKey2"}
-	mismatchError := errors.New("inconsistent values: ")
 
 	testCases := []struct {
 		name     string
@@ -888,7 +885,7 @@ func TestValidatePodLabelAnnotation(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("label labelKey2: \"value2\" doesn't match label labelKey1: \"value1\""),
 		},
 		{
 			name: "pod with inconsistent value between label and annotation",
@@ -902,7 +899,7 @@ func TestValidatePodLabelAnnotation(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("annotation annotationKey1: \"value2\" doesn't match label labelKey1: \"value1\""),
 		},
 		{
 			name: "pod with inconsistent value in annotations",
@@ -914,7 +911,7 @@ func TestValidatePodLabelAnnotation(t *testing.T) {
 					},
 				},
 			},
-			expected: mismatchError,
+			expected: errors.New("annotation annotationKey2: \"value2\" doesn't match annotation annotationKey1: \"value1\""),
 		},
 	}