Skip to content
This repository was archived by the owner on Jan 12, 2023. It is now read-only.

Commit f68476e

Browse files
alpealpe
committed
Tests and minor refactorings
1 parent ce4fbc1 commit f68476e

File tree

3 files changed

+176
-12
lines changed

3 files changed

+176
-12
lines changed

‎cmd/evicter/controller.go

+8-10
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@ type podProvisioner interface {
1919
}
2020

2121
type Controller struct {
22-
podStore cache.Indexer
22+
podStore cache.Store
2323
queue workqueue.RateLimitingInterface
2424
informer cache.Controller
2525
podProvisioner podProvisioner
2626
incubationPeriodSeconds time.Duration
2727
started time.Time
2828
}
2929

30-
func NewController(queue workqueue.RateLimitingInterface, indexer cache.Indexer, informer cache.Controller, podProvisioner podProvisioner, incubationPeriodSeconds int64) *Controller {
30+
func NewController(queue workqueue.RateLimitingInterface, indexer cache.Store, informer cache.Controller, podProvisioner podProvisioner, incubationPeriodSeconds int64) *Controller {
3131
return &Controller{
3232
informer: informer,
3333
podStore: indexer,
@@ -53,9 +53,9 @@ const (
5353
// annotationPreventEviction is a break-glass annotation to prevent automated eviction
5454
annotationPreventEviction = "k-rail/tainted-prevent-eviction"
5555
// annotationTimestamp stores the unix timestamp when the root event happened
56-
annotationTimestamp = "k-rail/tainted-timestamp"
56+
annotationTimestamp = "k-rail/tainted-timestamp"
5757
// annotationReason is used to define any additional reason in a human readable form
58-
annotationReason = "k-rail/tainted-reason"
58+
annotationReason = "k-rail/tainted-reason"
5959
)
6060

6161
const defaultEvictionReason = "Tainted"
@@ -89,14 +89,12 @@ func canEvict(pod *v1.Pod, incubationPeriod time.Duration) bool {
8989
if pod == nil {
9090
return false
9191
}
92-
val, ok := pod.Annotations[annotationPreventEviction]
93-
if ok {
94-
if val == "yes" || val == "true" {
95-
return false
96-
}
92+
switch pod.Annotations[annotationPreventEviction] {
93+
case "yes", "true", "1", "YES", "TRUE", "Yes", "True":
94+
return false
9795
}
9896

99-
val, ok = pod.Annotations[annotationTimestamp]
97+
val, ok := pod.Annotations[annotationTimestamp]
10098
if ok {
10199
i, err := strconv.ParseInt(val, 10, 64)
102100
if err != nil {

‎cmd/evicter/controller_test.go

+167
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package main
2+
3+
import (
4+
"reflect"
5+
"strconv"
6+
"testing"
7+
"time"
8+
9+
v1 "k8s.io/api/core/v1"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/client-go/tools/cache"
12+
)
13+
14+
func TestCanEvict(t *testing.T) {
15+
now := int(time.Now().Unix())
16+
specs := map[string]struct {
17+
srcAnn map[string]string
18+
expResult bool
19+
}{
20+
"with timestamp after incubation period": {
21+
srcAnn: map[string]string{
22+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
23+
"k-rail/tainted-reason": "test",
24+
},
25+
expResult: true,
26+
},
27+
"with timestamp in incubation period": {
28+
srcAnn: map[string]string{
29+
"k-rail/tainted-timestamp": strconv.Itoa(now),
30+
"k-rail/tainted-reason": "test",
31+
},
32+
expResult: false,
33+
},
34+
"without timestamp annotation": {
35+
srcAnn: map[string]string{
36+
"k-rail/tainted-reason": "test",
37+
},
38+
expResult: true,
39+
},
40+
"with timestamp containing non timestamp string": {
41+
srcAnn: map[string]string{
42+
"k-rail/tainted-timestamp": "",
43+
"k-rail/tainted-reason": "test",
44+
},
45+
expResult: true,
46+
},
47+
"with preventEviction annotation": {
48+
srcAnn: map[string]string{
49+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
50+
"k-rail/tainted-reason": "test",
51+
"k-rail/tainted-prevent-eviction": "true",
52+
},
53+
expResult: false,
54+
},
55+
"with preventEviction annotation - uppercase": {
56+
srcAnn: map[string]string{
57+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
58+
"k-rail/tainted-reason": "test",
59+
"k-rail/tainted-prevent-eviction": "TRUE",
60+
},
61+
expResult: false,
62+
},
63+
"with preventEviction annotation - yes": {
64+
srcAnn: map[string]string{
65+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
66+
"k-rail/tainted-reason": "test",
67+
"k-rail/tainted-prevent-eviction": "yes",
68+
},
69+
expResult: false,
70+
},
71+
"with preventEviction annotation - non bool": {
72+
srcAnn: map[string]string{
73+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
74+
"k-rail/tainted-reason": "test",
75+
"k-rail/tainted-prevent-eviction": "",
76+
},
77+
expResult: true,
78+
},
79+
}
80+
for msg, spec := range specs {
81+
t.Run(msg, func(t *testing.T) {
82+
pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: spec.srcAnn}}
83+
if got := canEvict(&pod, time.Second); spec.expResult != got {
84+
t.Errorf("expected %v but got %v", spec.expResult, got)
85+
}
86+
})
87+
}
88+
}
89+
90+
func TestEvictPod(t *testing.T) {
91+
now := int(time.Now().Unix())
92+
93+
specs := map[string]struct {
94+
srcAnn map[string]string
95+
expReason string
96+
expMsg string
97+
expNoEviction bool
98+
}{
99+
"evicted with custom reason": {
100+
srcAnn: map[string]string{
101+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
102+
"k-rail/tainted-reason": "test",
103+
},
104+
expReason: "Tainted",
105+
expMsg: "test",
106+
},
107+
"evicted with default reason": {
108+
srcAnn: map[string]string{
109+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
110+
},
111+
expReason: "Tainted",
112+
expMsg: noEvictionNote,
113+
},
114+
"not evicted with annotation": {
115+
srcAnn: map[string]string{
116+
"k-rail/tainted-timestamp": strconv.Itoa(now - 1),
117+
"k-rail/tainted-prevent-eviction": "yes",
118+
},
119+
expNoEviction: true,
120+
},
121+
}
122+
for msg, spec := range specs {
123+
t.Run(msg, func(t *testing.T) {
124+
store := cache.NewStore(cache.MetaNamespaceKeyFunc)
125+
pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "myPod", Annotations: spec.srcAnn}}
126+
store.Add(pod)
127+
prov := &recordingPodProvisioner{}
128+
c := NewController(nil, store, nil, prov, 1)
129+
// when
130+
err := c.evictPod("myPod")
131+
// then
132+
if err != nil {
133+
t.Fatalf("unexpected error: %+v", err)
134+
}
135+
if spec.expNoEviction {
136+
if prov.evictedPods != nil {
137+
t.Fatalf("expected no call but got %v", prov.evictedPods)
138+
}
139+
return
140+
}
141+
// there should be 1 call
142+
if exp, got := []*v1.Pod{pod}, prov.evictedPods; !reflect.DeepEqual(exp, got) {
143+
t.Errorf("expected %v but got %v", exp, got)
144+
}
145+
if exp, got := []string{spec.expReason}, prov.reasons; !reflect.DeepEqual(exp, got) {
146+
t.Errorf("expected %v but got %v", exp, got)
147+
}
148+
if exp, got := []string{spec.expMsg}, prov.msgs; !reflect.DeepEqual(exp, got) {
149+
t.Errorf("expected %v but got %v", exp, got)
150+
}
151+
})
152+
}
153+
}
154+
155+
type recordingPodProvisioner struct {
156+
evictedPods []*v1.Pod
157+
reasons []string
158+
msgs []string
159+
result error
160+
}
161+
162+
func (r *recordingPodProvisioner) Evict(pod *v1.Pod, reason, msg string) error {
163+
r.evictedPods = append(r.evictedPods, pod)
164+
r.reasons = append(r.reasons, reason)
165+
r.msgs = append(r.msgs, msg)
166+
return r.result
167+
}

‎cmd/evicter/main.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"k8s.io/klog"
2929
)
3030

31-
3231
func main() {
3332
var (
3433
kubeconfig = flag.String("kubeconfig", "", "absolute path to the kubeconfig file: `<home>/.kube/config`")
@@ -165,7 +164,7 @@ func (p *podEvicter) Evict(pod *v1.Pod, reason, msg string) error {
165164
if err != nil {
166165
return errors.Wrap(err, "eviction")
167166
}
168-
p.eventRecorder.Eventf(pod, v1.EventTypeNormal, reason, msg)
167+
p.eventRecorder.Eventf(pod, v1.EventTypeNormal, reason, msg)
169168
return nil
170169
}
171170

0 commit comments

Comments
 (0)