[YUNIKORN-2484] Shim: Remove stateaware logic (#802)

Closes: #802

Signed-off-by: Manikandan R <[email protected]>

pkg/admission/admission_controller_test.go

@@ -81,10 +81,9 @@ func TestUpdateLabels(t *testing.T) {
 	assert.Equal(t, patch[0].Op, "add")
 	assert.Equal(t, patch[0].Path, "/metadata/labels")
 	if updatedMap, ok := patch[0].Value.(map[string]string); ok {
-		assert.Equal(t, len(updatedMap), 4)
+		assert.Equal(t, len(updatedMap), 3)
 		assert.Equal(t, updatedMap["random"], "random")
 		assert.Equal(t, updatedMap["queue"], "root.default")
-		assert.Equal(t, updatedMap["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("patch info content is not as expected")
@@ -155,10 +154,9 @@ func TestUpdateLabels(t *testing.T) {
 	assert.Equal(t, patch[0].Op, "add")
 	assert.Equal(t, patch[0].Path, "/metadata/labels")
 	if updatedMap, ok := patch[0].Value.(map[string]string); ok {
-		assert.Equal(t, len(updatedMap), 4)
+		assert.Equal(t, len(updatedMap), 3)
 		assert.Equal(t, updatedMap["random"], "random")
 		assert.Equal(t, updatedMap["queue"], "root.abc")
-		assert.Equal(t, updatedMap["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("patch info content is not as expected")
@@ -188,9 +186,8 @@ func TestUpdateLabels(t *testing.T) {
 	assert.Equal(t, patch[0].Op, "add")
 	assert.Equal(t, patch[0].Path, "/metadata/labels")
 	if updatedMap, ok := patch[0].Value.(map[string]string); ok {
-		assert.Equal(t, len(updatedMap), 3)
+		assert.Equal(t, len(updatedMap), 2)
 		assert.Equal(t, updatedMap["queue"], "root.default")
-		assert.Equal(t, updatedMap["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("patch info content is not as expected")
@@ -217,9 +214,8 @@ func TestUpdateLabels(t *testing.T) {
 	assert.Equal(t, patch[0].Op, "add")
 	assert.Equal(t, patch[0].Path, "/metadata/labels")
 	if updatedMap, ok := patch[0].Value.(map[string]string); ok {
-		assert.Equal(t, len(updatedMap), 3)
+		assert.Equal(t, len(updatedMap), 2)
 		assert.Equal(t, updatedMap["queue"], "root.default")
-		assert.Equal(t, updatedMap["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("patch info content is not as expected")
@@ -244,9 +240,8 @@ func TestUpdateLabels(t *testing.T) {
 	assert.Equal(t, patch[0].Op, "add")
 	assert.Equal(t, patch[0].Path, "/metadata/labels")
 	if updatedMap, ok := patch[0].Value.(map[string]string); ok {
-		assert.Equal(t, len(updatedMap), 3)
+		assert.Equal(t, len(updatedMap), 2)
 		assert.Equal(t, updatedMap["queue"], "root.default")
-		assert.Equal(t, updatedMap["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(updatedMap["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("patch info content is not as expected")
@@ -455,7 +450,6 @@ func TestMutate(t *testing.T) {
 	assert.Check(t, resp.Allowed, "response not allowed for pod")
 	assert.Equal(t, schedulerName(t, resp.Patch), "yunikorn", "yunikorn not set as scheduler for pod")
 	assert.Equal(t, labels(t, resp.Patch)["applicationId"], "yunikorn-default-autogen", "wrong applicationId label")
-	assert.Equal(t, labels(t, resp.Patch)["disableStateAware"], "true", "missing disableStateAware label")
 	assert.Equal(t, labels(t, resp.Patch)["queue"], "root.default", "incorrect queue name")
 
 	// pod without applicationID
@@ -474,7 +468,6 @@ func TestMutate(t *testing.T) {
 	assert.Check(t, resp.Allowed, "response not allowed for pod")
 	assert.Equal(t, schedulerName(t, resp.Patch), "yunikorn", "yunikorn not set as scheduler for pod")
 	assert.Equal(t, labels(t, resp.Patch)["applicationId"], "yunikorn-test-ns-autogen", "wrong applicationId label")
-	assert.Equal(t, labels(t, resp.Patch)["disableStateAware"], "true", "missing disableStateAware label")
 
 	// pod with applicationId
 	pod.ObjectMeta.Labels = map[string]string{"applicationId": "test-app"}

pkg/admission/util.go

@@ -47,11 +47,6 @@ func updatePodLabel(pod *v1.Pod, namespace string, generateUniqueAppIds bool, de
 		// 		application ID convention: ${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
 		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
-		if _, ok := existingLabels[constants.LabelDisableStateAware]; !ok {
-			result[constants.LabelDisableStateAware] = "true"
-		}
 	}
 
 	// if existing label exist, it takes priority over everything else

pkg/admission/util_test.go

@@ -104,10 +104,9 @@ func TestUpdatePodLabelForAdmissionController(t *testing.T) {
 	pod := createTestingPodWithMeta()
 
 	if result := updatePodLabel(pod, "default", false, "root.default"); result != nil {
-		assert.Equal(t, len(result), 4)
+		assert.Equal(t, len(result), 3)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["queue"], "root.default")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(result["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -130,10 +129,9 @@ func TestUpdatePodLabelForAdmissionController(t *testing.T) {
 	// we won't modify it
 	pod = createTestingPodWithQueue()
 	if result := updatePodLabel(pod, "default", false, "root.default"); result != nil {
-		assert.Equal(t, len(result), 4)
+		assert.Equal(t, len(result), 3)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["queue"], "root.abc")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(result["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionControllert is not as expected")
@@ -144,9 +142,8 @@ func TestUpdatePodLabelForAdmissionController(t *testing.T) {
 	pod = createTestingPodNoNamespaceAndLabels()
 
 	if result := updatePodLabel(pod, "default", false, "root.default"); result != nil {
-		assert.Equal(t, len(result), 3)
+		assert.Equal(t, len(result), 2)
 		assert.Equal(t, result["queue"], "root.default")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(result["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -155,19 +152,17 @@ func TestUpdatePodLabelForAdmissionController(t *testing.T) {
 	// pod name might be empty, it can comes from generatedName
 	pod = createTestingPodWithGenerateName()
 	if result := updatePodLabel(pod, "default", false, "root.default"); result != nil {
-		assert.Equal(t, len(result), 3)
+		assert.Equal(t, len(result), 2)
 		assert.Equal(t, result["queue"], "root.default")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(result["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
 	}
 
 	pod = createMinimalTestingPod()
 	if result := updatePodLabel(pod, "default", false, "root.default"); result != nil {
-		assert.Equal(t, len(result), 3)
+		assert.Equal(t, len(result), 2)
 		assert.Equal(t, result["queue"], "root.default")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, strings.HasPrefix(result["applicationId"], constants.AutoGenAppPrefix), true)
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -178,10 +173,9 @@ func TestDefaultQueueName(t *testing.T) {
 	defaultConf := createConfig()
 	pod := createTestingPodWithMeta()
 	if result := updatePodLabel(pod, defaultConf.GetNamespace(), defaultConf.GetGenerateUniqueAppIds(), defaultConf.GetDefaultQueueName()); result != nil {
-		assert.Equal(t, len(result), 4)
+		assert.Equal(t, len(result), 3)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, result["queue"], "root.default")
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -191,10 +185,9 @@ func TestDefaultQueueName(t *testing.T) {
 		conf.AMFilteringDefaultQueueName: "",
 	})
 	if result := updatePodLabel(pod, queueNameEmptyConf.GetNamespace(), queueNameEmptyConf.GetGenerateUniqueAppIds(), queueNameEmptyConf.GetDefaultQueueName()); result != nil {
-		assert.Equal(t, len(result), 3)
+		assert.Equal(t, len(result), 2)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, result["queue"], "")
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -204,10 +197,9 @@ func TestDefaultQueueName(t *testing.T) {
 		conf.AMFilteringDefaultQueueName: "yunikorn",
 	})
 	if result := updatePodLabel(pod, customQueueNameConf.GetNamespace(), customQueueNameConf.GetGenerateUniqueAppIds(), customQueueNameConf.GetDefaultQueueName()); result != nil {
-		assert.Equal(t, len(result), 4)
+		assert.Equal(t, len(result), 3)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Assert(t, result["queue"] != "yunikorn")
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
@@ -218,10 +210,9 @@ func TestDefaultQueueName(t *testing.T) {
 	})
 	if result := updatePodLabel(pod, customValidQueueNameConf.GetNamespace(),
 		customValidQueueNameConf.GetGenerateUniqueAppIds(), customValidQueueNameConf.GetDefaultQueueName()); result != nil {
-		assert.Equal(t, len(result), 4)
+		assert.Equal(t, len(result), 3)
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
-		assert.Equal(t, result["disableStateAware"], "true")
 		assert.Equal(t, result["queue"], "root.yunikorn")
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")

pkg/cache/metadata.go

@@ -19,7 +19,6 @@
 package cache
 
 import (
-	"strconv"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
@@ -32,7 +31,6 @@ import (
 	"github.com/apache/yunikorn-k8shim/pkg/common/utils"
 	"github.com/apache/yunikorn-k8shim/pkg/conf"
 	"github.com/apache/yunikorn-k8shim/pkg/log"
-	siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
 )
 
 func getTaskMetadata(pod *v1.Pod) (TaskMetadata, bool) {
@@ -78,9 +76,6 @@ func getAppMetadata(pod *v1.Pod) (ApplicationMetadata, bool) {
 	} else {
 		tags[constants.AppTagNamespace] = pod.Namespace
 	}
-	if isStateAwareDisabled(pod) {
-		tags[siCommon.AppTagStateAwareDisable] = "true"
-	}
 
 	// attach imagePullSecrets if present
 	secrets := pod.Spec.ImagePullSecrets
@@ -142,20 +137,3 @@ func getOwnerReference(pod *v1.Pod) []metav1.OwnerReference {
 	}
 	return []metav1.OwnerReference{ref}
 }
-
-func isStateAwareDisabled(pod *v1.Pod) bool {
-	value := utils.GetPodLabelValue(pod, constants.LabelDisableStateAware)
-	if value == "" {
-		return false
-	}
-	result, err := strconv.ParseBool(value)
-	if err != nil {
-		log.Log(log.ShimCacheApplication).Debug("unable to parse label for pod",
-			zap.String("namespace", pod.Namespace),
-			zap.String("name", pod.Name),
-			zap.String("label", constants.LabelDisableStateAware),
-			zap.Error(err))
-		return false
-	}
-	return result
-}

pkg/cache/metadata_test.go

@@ -161,7 +161,6 @@ func TestGetAppMetadata(t *testing.T) { //nolint:funlen
 				"applicationId":                   "app00002",
 				"queue":                           "root.b",
 				constants.DomainYuniKorn + "user": "testuser",
-				"disableStateAware":               "true",
 			},
 			Annotations: map[string]string{
 				constants.AnnotationSchedulingPolicyParam: "gangSchedulingStyle=Hard",
@@ -180,7 +179,6 @@ func TestGetAppMetadata(t *testing.T) { //nolint:funlen
 	assert.Equal(t, app.ApplicationID, "app00002")
 	assert.Equal(t, app.QueueName, "root.b")
 	assert.Equal(t, app.User, constants.DefaultUser)
-	assert.Equal(t, app.Tags["application.stateaware.disable"], "true")
 	assert.Equal(t, app.Tags["namespace"], "app-namespace-01")
 	assert.Equal(t, len(app.TaskGroups), 0)
 	assert.Equal(t, app.SchedulingPolicyParameters.GetGangSchedulingStyle(), "Hard")

pkg/common/constants/constants.go

@@ -41,7 +41,6 @@ const LabelQueueName = "queue"
 const RootQueue = "root"
 const AnnotationQueueName = DomainYuniKorn + "queue"
 const AnnotationParentQueue = DomainYuniKorn + "parentqueue"
-const LabelDisableStateAware = "disableStateAware"
 const ApplicationDefaultQueue = "root.default"
 const DefaultPartition = "default"
 const AppTagNamespace = "namespace"

test/e2e/spark_jobs_scheduling/spark_jobs_scheduling_suite_test.go

@@ -45,7 +45,7 @@ var _ = BeforeSuite(func() {
 	suiteName = common.GetSuiteName(filename)
 	annotation = "ann-" + common.RandSeq(10)
 	yunikorn.EnsureYuniKornConfigsPresent()
-	yunikorn.UpdateConfigMapWrapper(oldConfigMap, "stateaware", annotation)
+	yunikorn.UpdateConfigMapWrapper(oldConfigMap, "fifo", annotation)
 })
 
 var _ = AfterSuite(func() {