Fix: pod's deadline shoud be nil when cache ttl is 0

Signed-off-by: kerthcet <kerthcet@gmail.com>
This commit is contained in:
kerthcet 2022-07-05 23:29:31 +08:00
parent 47ad357c52
commit 72e8fc1d87
3 changed files with 72 additions and 38 deletions

View File

@ -77,6 +77,7 @@ type cacheImpl struct {
type podState struct { type podState struct {
pod *v1.Pod pod *v1.Pod
// Used by assumedPod to determinate expiration. // Used by assumedPod to determinate expiration.
// If deadline is nil, assumedPod will never expire.
deadline *time.Time deadline *time.Time
// Used to block cache from expiring assumedPod if binding still runs // Used to block cache from expiring assumedPod if binding still runs
bindingFinished bool bindingFinished bool
@ -401,10 +402,14 @@ func (cache *cacheImpl) finishBinding(pod *v1.Pod, now time.Time) error {
klog.V(5).InfoS("Finished binding for pod, can be expired", "pod", klog.KObj(pod)) klog.V(5).InfoS("Finished binding for pod, can be expired", "pod", klog.KObj(pod))
currState, ok := cache.podStates[key] currState, ok := cache.podStates[key]
if ok && cache.assumedPods.Has(key) { if ok && cache.assumedPods.Has(key) {
if cache.ttl == time.Duration(0) {
currState.deadline = nil
} else {
dl := now.Add(cache.ttl) dl := now.Add(cache.ttl)
currState.bindingFinished = true
currState.deadline = &dl currState.deadline = &dl
} }
currState.bindingFinished = true
}
return nil return nil
} }

View File

@ -253,25 +253,31 @@ func TestExpirePod(t *testing.T) {
makeBasePod(t, nodeName, "test-3", "200m", "1Ki", "", []v1.ContainerPort{{HostIP: "127.0.0.1", HostPort: 8080, Protocol: "TCP"}}), makeBasePod(t, nodeName, "test-3", "200m", "1Ki", "", []v1.ContainerPort{{HostIP: "127.0.0.1", HostPort: 8080, Protocol: "TCP"}}),
} }
now := time.Now() now := time.Now()
ttl := 10 * time.Second defaultTTL := 10 * time.Second
tests := []struct { tests := []struct {
name string
pods []*testExpirePodStruct pods []*testExpirePodStruct
cleanupTime time.Time cleanupTime time.Time
ttl time.Duration
wNodeInfo *framework.NodeInfo wNodeInfo *framework.NodeInfo
}{{ // assumed pod would expires }{
{
name: "assumed pod would expire",
pods: []*testExpirePodStruct{ pods: []*testExpirePodStruct{
{pod: testPods[0], finishBind: true, assumedTime: now}, {pod: testPods[0], finishBind: true, assumedTime: now},
}, },
cleanupTime: now.Add(2 * ttl), cleanupTime: now.Add(2 * defaultTTL),
wNodeInfo: nil, wNodeInfo: nil,
}, { // first one would expire, second and third would not. ttl: defaultTTL,
},
{
name: "first one would expire, second and third would not",
pods: []*testExpirePodStruct{ pods: []*testExpirePodStruct{
{pod: testPods[0], finishBind: true, assumedTime: now}, {pod: testPods[0], finishBind: true, assumedTime: now},
{pod: testPods[1], finishBind: true, assumedTime: now.Add(3 * ttl / 2)}, {pod: testPods[1], finishBind: true, assumedTime: now.Add(3 * defaultTTL / 2)},
{pod: testPods[2]}, {pod: testPods[2]},
}, },
cleanupTime: now.Add(2 * ttl), cleanupTime: now.Add(2 * defaultTTL),
wNodeInfo: newNodeInfo( wNodeInfo: newNodeInfo(
&framework.Resource{ &framework.Resource{
MilliCPU: 400, MilliCPU: 400,
@ -286,13 +292,36 @@ func TestExpirePod(t *testing.T) {
newHostPortInfoBuilder().add("TCP", "127.0.0.1", 8080).build(), newHostPortInfoBuilder().add("TCP", "127.0.0.1", 8080).build(),
make(map[string]*framework.ImageStateSummary), make(map[string]*framework.ImageStateSummary),
), ),
}} ttl: defaultTTL,
},
{
name: "assumed pod would never expire",
pods: []*testExpirePodStruct{
{pod: testPods[0], finishBind: true, assumedTime: now},
},
cleanupTime: now.Add(3 * defaultTTL),
wNodeInfo: newNodeInfo(
&framework.Resource{
MilliCPU: 100,
Memory: 500,
},
&framework.Resource{
MilliCPU: 100,
Memory: 500,
},
[]*v1.Pod{testPods[0]},
newHostPortInfoBuilder().add("TCP", "127.0.0.1", 80).build(),
make(map[string]*framework.ImageStateSummary),
),
ttl: time.Duration(0),
},
}
for i, tt := range tests { for _, tc := range tests {
t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
cache := newCache(ttl, time.Second, nil) cache := newCache(tc.ttl, time.Second, nil)
for _, pod := range tt.pods { for _, pod := range tc.pods {
if err := cache.AssumePod(pod.pod); err != nil { if err := cache.AssumePod(pod.pod); err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -305,9 +334,9 @@ func TestExpirePod(t *testing.T) {
} }
// pods that got bound and have assumedTime + ttl < cleanupTime will get // pods that got bound and have assumedTime + ttl < cleanupTime will get
// expired and removed // expired and removed
cache.cleanupAssumedPods(tt.cleanupTime) cache.cleanupAssumedPods(tc.cleanupTime)
n := cache.nodes[nodeName] n := cache.nodes[nodeName]
if err := deepEqualWithoutGeneration(n, tt.wNodeInfo); err != nil { if err := deepEqualWithoutGeneration(n, tc.wNodeInfo); err != nil {
t.Error(err) t.Error(err)
} }
}) })

View File

@ -53,7 +53,7 @@ import (
const ( const (
// Duration the scheduler will wait before expiring an assumed pod. // Duration the scheduler will wait before expiring an assumed pod.
// See issue #106361 for more details about this parameter and its value. // See issue #106361 for more details about this parameter and its value.
durationToExpireAssumedPod = 0 * time.Minute durationToExpireAssumedPod time.Duration = 0
) )
// ErrNoNodesAvailable is used to describe the error that no nodes available to schedule pods. // ErrNoNodesAvailable is used to describe the error that no nodes available to schedule pods.