Skip to content

Commit 2f80131

Browse files
committed
[YUNIKORN-3279] Remove code: clean up utils.go (#1027)
Closes: #1027 Signed-off-by: Manikandan R <manirajv06@gmail.com>
1 parent 0160be2 commit 2f80131

3 files changed

Lines changed: 21 additions & 67 deletions

File tree

pkg/cache/metadata_test.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
apis "k8s.io/apimachinery/pkg/apis/meta/v1"
2828

2929
"github.com/apache/yunikorn-k8shim/pkg/common/constants"
30-
"github.com/apache/yunikorn-k8shim/pkg/common/utils"
3130
"github.com/apache/yunikorn-k8shim/pkg/conf"
3231
"github.com/apache/yunikorn-scheduler-interface/lib/go/common"
3332
)
@@ -107,9 +106,7 @@ func TestGetTaskMetadata(t *testing.T) {
107106
}
108107

109108
func TestGetAppMetadata(t *testing.T) { //nolint:funlen
110-
defer utils.SetPluginMode(false)
111109
defer func() { conf.GetSchedulerConf().GenerateUniqueAppIds = false }()
112-
utils.SetPluginMode(false)
113110
conf.GetSchedulerConf().GenerateUniqueAppIds = false
114111

115112
pod := v1.Pod{
@@ -290,23 +287,16 @@ func TestGetAppMetadata(t *testing.T) { //nolint:funlen
290287
Phase: v1.PodPending,
291288
},
292289
}
293-
294-
utils.SetPluginMode(false)
295290
app, ok = getAppMetadata(&pod)
296291
conf.GetSchedulerConf().GenerateUniqueAppIds = true
297292
assert.Equal(t, ok, true)
298293
assert.Equal(t, app.ApplicationID, "yunikorn-app-namespace-01-autogen")
299294

300-
utils.SetPluginMode(false)
301295
conf.GetSchedulerConf().GenerateUniqueAppIds = true
302296
app, ok = getAppMetadata(&pod)
303297
assert.Equal(t, ok, true)
304298
assert.Equal(t, app.ApplicationID, "app-namespace-01-UID-POD-00001")
305299

306-
utils.SetPluginMode(true)
307-
app, ok = getAppMetadata(&pod)
308-
assert.Equal(t, ok, false)
309-
310300
// case: invalid annotation task groups
311301
pod = v1.Pod{
312302
TypeMeta: apis.TypeMeta{

pkg/common/utils/utils.go

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,11 @@ import (
4444
const userInfoKey = siCommon.DomainYuniKorn + "user.info"
4545
const uniqueAutogenSuffix = "-uniqueautogen"
4646

47-
var pluginMode bool
4847
var (
4948
// ErrorTimeout returned if waiting for a condition times out
5049
ErrorTimeout = errors.New("timeout waiting for condition")
5150
)
5251

53-
func SetPluginMode(value bool) {
54-
pluginMode = value
55-
}
56-
57-
func IsPluginMode() bool {
58-
return pluginMode
59-
}
60-
6152
func Convert2Pod(obj interface{}) (*v1.Pod, error) {
6253
pod, ok := obj.(*v1.Pod)
6354
if !ok {
@@ -146,27 +137,13 @@ func GenerateApplicationID(namespace string, generateUniqueAppIds bool, podUID s
146137
}
147138

148139
// GetApplicationIDFromPod returns the Application for a Pod. If a Pod is marked as schedulable by YuniKorn but is
149-
// missing an ApplicationID, one will be generated here (if YuniKorn is running in standard mode) or an empty string
150-
// will be returned (if YuniKorn is running in plugin mode).
151-
// If an Application ID is returned, the Pod is managed by YuniKorn. Otherwise, it is managed by an external scheduler.
140+
// missing an ApplicationID, one will be generated here
152141
func GetApplicationIDFromPod(pod *v1.Pod) string {
153142
// SchedulerName needs to match
154143
if strings.Compare(pod.Spec.SchedulerName, constants.SchedulerName) != 0 {
155144
return ""
156145
}
157146

158-
// If pod was tagged with ignore-application and plugin mode is active, return
159-
if pluginMode {
160-
if value := GetPodAnnotationValue(pod, constants.AnnotationIgnoreApplication); value != "" {
161-
ignore, err := strconv.ParseBool(value)
162-
if err != nil {
163-
log.Log(log.ShimUtils).Warn("Failed to parse annotation "+constants.AnnotationIgnoreApplication, zap.Error(err))
164-
} else if ignore {
165-
return ""
166-
}
167-
}
168-
}
169-
170147
// Application ID can be defined in multiple places
171148
// The application ID is determined by the following order.
172149
// 1. Label: constants.CanonicalLabelApplicationID
@@ -193,11 +170,6 @@ func GetApplicationIDFromPod(pod *v1.Pod) string {
193170
}
194171
}
195172

196-
// If plugin mode, interpret missing Application ID as a non-YuniKorn pod
197-
if pluginMode && appID == "" {
198-
return ""
199-
}
200-
201173
// does appID end with '-uniqueautogen'?
202174
if strings.HasSuffix(appID, uniqueAutogenSuffix) {
203175
// replace suffix with pod UID

pkg/common/utils/utils_test.go

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -584,58 +584,56 @@ func TestPodUnderCondition(t *testing.T) {
584584

585585
// nolint: funlen
586586
func TestGetApplicationIDFromPod(t *testing.T) {
587-
defer SetPluginMode(false)
588587
defer func() { conf.GetSchedulerConf().GenerateUniqueAppIds = false }()
589588

590589
appIDInCanonicalLabel := "CanonicalLabelAppID"
591590
appIDInAnnotation := "annotationAppID"
592591
appIDInLabel := "labelAppID"
593592
appIDInSelector := "sparkLabelAppID"
594593
testCases := []struct {
595-
name string
596-
pod *v1.Pod
597-
expectedAppID string
598-
expectedAppIDPluginMode string
599-
generateUniqueAppIds bool
594+
name string
595+
pod *v1.Pod
596+
expectedAppID string
597+
generateUniqueAppIds bool
600598
}{
601599
{"AppID defined in canonical label", &v1.Pod{
602600
ObjectMeta: metav1.ObjectMeta{
603601
Labels: map[string]string{constants.CanonicalLabelApplicationID: appIDInCanonicalLabel},
604602
},
605603
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
606-
}, appIDInCanonicalLabel, appIDInCanonicalLabel, false},
604+
}, appIDInCanonicalLabel, false},
607605
{"AppID defined in annotation", &v1.Pod{
608606
ObjectMeta: metav1.ObjectMeta{
609607
Annotations: map[string]string{constants.AnnotationApplicationID: appIDInAnnotation},
610608
},
611609
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
612-
}, appIDInAnnotation, appIDInAnnotation, false},
610+
}, appIDInAnnotation, false},
613611
{"AppID defined in legacy label", &v1.Pod{
614612
ObjectMeta: metav1.ObjectMeta{
615613
Labels: map[string]string{constants.LabelApplicationID: appIDInLabel},
616614
},
617615
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
618-
}, appIDInLabel, appIDInLabel, false},
616+
}, appIDInLabel, false},
619617
{"AppID defined in spark app selector label", &v1.Pod{
620618
ObjectMeta: metav1.ObjectMeta{
621619
Labels: map[string]string{constants.SparkLabelAppID: appIDInSelector},
622620
},
623621
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
624-
}, appIDInSelector, appIDInSelector, false},
622+
}, appIDInSelector, false},
625623
{"AppID defined in canonical label and annotation, canonical label win", &v1.Pod{
626624
ObjectMeta: metav1.ObjectMeta{
627625
Annotations: map[string]string{constants.AnnotationApplicationID: appIDInAnnotation},
628626
Labels: map[string]string{constants.CanonicalLabelApplicationID: appIDInCanonicalLabel},
629627
},
630628
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
631-
}, appIDInCanonicalLabel, appIDInCanonicalLabel, false},
629+
}, appIDInCanonicalLabel, false},
632630
{"AppID defined in annotation and legacy label, annotation win", &v1.Pod{
633631
ObjectMeta: metav1.ObjectMeta{
634632
Annotations: map[string]string{constants.AnnotationApplicationID: appIDInAnnotation},
635633
Labels: map[string]string{constants.LabelApplicationID: appIDInLabel},
636634
},
637635
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
638-
}, appIDInAnnotation, appIDInAnnotation, false},
636+
}, appIDInAnnotation, false},
639637
{"Spark AppID defined in legacy label and spark app selector, legacy label win", &v1.Pod{
640638
ObjectMeta: metav1.ObjectMeta{
641639
Labels: map[string]string{
@@ -644,50 +642,50 @@ func TestGetApplicationIDFromPod(t *testing.T) {
644642
},
645643
},
646644
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
647-
}, appIDInLabel, appIDInLabel, false},
648-
{"No AppID defined", &v1.Pod{}, "", "", false},
645+
}, appIDInLabel, false},
646+
{"No AppID defined", &v1.Pod{}, "", false},
649647
{"No AppID defined but not generateUnique", &v1.Pod{
650648
ObjectMeta: metav1.ObjectMeta{
651649
Namespace: "testns",
652650
UID: "podUid",
653651
},
654652
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
655-
}, "yunikorn-testns-autogen", "", false},
653+
}, "yunikorn-testns-autogen", false},
656654
{"No AppID defined but generateUnique", &v1.Pod{
657655
ObjectMeta: metav1.ObjectMeta{
658656
Namespace: "testns",
659657
UID: "podUid",
660658
},
661659
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
662-
}, "testns-podUid", "", true},
660+
}, "testns-podUid", true},
663661
{"Unique autogen token found with generateUnique in canonical AppId label", &v1.Pod{
664662
ObjectMeta: metav1.ObjectMeta{
665663
Namespace: "testns",
666664
UID: "podUid",
667665
Labels: map[string]string{constants.CanonicalLabelApplicationID: "testns-uniqueautogen"},
668666
},
669667
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
670-
}, "testns-podUid", "testns-podUid", true},
668+
}, "testns-podUid", true},
671669
{"Unique autogen token found with generateUnique in legacy AppId labels", &v1.Pod{
672670
ObjectMeta: metav1.ObjectMeta{
673671
Namespace: "testns",
674672
UID: "podUid",
675673
Labels: map[string]string{constants.LabelApplicationID: "testns-uniqueautogen"},
676674
},
677675
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
678-
}, "testns-podUid", "testns-podUid", true},
676+
}, "testns-podUid", true},
679677
{"Non-yunikorn schedulerName with canonical AppId label", &v1.Pod{
680678
ObjectMeta: metav1.ObjectMeta{
681679
Labels: map[string]string{constants.CanonicalLabelApplicationID: appIDInCanonicalLabel},
682680
},
683681
Spec: v1.PodSpec{SchedulerName: "default"},
684-
}, "", "", false},
682+
}, "", false},
685683
{"Non-yunikorn schedulerName with legacy AppId label", &v1.Pod{
686684
ObjectMeta: metav1.ObjectMeta{
687685
Labels: map[string]string{constants.LabelApplicationID: appIDInLabel},
688686
},
689687
Spec: v1.PodSpec{SchedulerName: "default"},
690-
}, "", "", false},
688+
}, "", false},
691689
{"AppID defined but ignore-application set", &v1.Pod{
692690
ObjectMeta: metav1.ObjectMeta{
693691
Annotations: map[string]string{
@@ -696,7 +694,7 @@ func TestGetApplicationIDFromPod(t *testing.T) {
696694
},
697695
},
698696
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
699-
}, appIDInAnnotation, "", false},
697+
}, appIDInAnnotation, false},
700698
{"AppID defined and ignore-application invalid", &v1.Pod{
701699
ObjectMeta: metav1.ObjectMeta{
702700
Annotations: map[string]string{
@@ -705,20 +703,14 @@ func TestGetApplicationIDFromPod(t *testing.T) {
705703
},
706704
},
707705
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
708-
}, appIDInAnnotation, appIDInAnnotation, false},
706+
}, appIDInAnnotation, false},
709707
}
710708

711709
for _, tc := range testCases {
712710
t.Run(tc.name, func(t *testing.T) {
713711
conf.GetSchedulerConf().GenerateUniqueAppIds = tc.generateUniqueAppIds
714-
SetPluginMode(false)
715-
assert.Equal(t, IsPluginMode(), false)
716712
appID := GetApplicationIDFromPod(tc.pod)
717713
assert.Equal(t, appID, tc.expectedAppID, "Wrong appID (standard mode)")
718-
SetPluginMode(true)
719-
assert.Equal(t, IsPluginMode(), true)
720-
appID2 := GetApplicationIDFromPod(tc.pod)
721-
assert.Equal(t, appID2, tc.expectedAppIDPluginMode, "Wrong appID (plugin mode)")
722714
})
723715
}
724716
}

0 commit comments

Comments
 (0)