mirror of
https://github.com/kubereboot/kured.git
synced 2026-05-20 07:12:58 +00:00
Implementation details of lock should not leak into the calling
methods.
Without this path, calls are a bit more complex
and error handling is harder to find.
This is a problem for long term maintenance, as it
is tougher to refactor the locks without impacting the main.
Decoupling the two (main usage of the lock, and the lock
themselves) will allow us to introduce other kinds of locks
easily.
I solve this by inlining into the daemonsetlock package:
- including all the methods for managing locks from the main.go
functions. Those were mostly doing error handling
where code became no-op by introducing multiple
daemonsetlock types
- adding the lock release delay part of lock info
I also did not like the pattern include in Test method,
which added a reference to nodeMeta: It was not very clear
that Test was storing the current metadata of the node,
or was returning the current state. (Metadata here only means unschedulable).
The problem I saw was that the metadata was silently
mutated from a lock Test method, which was very not obvious.
Instead, I picked to explicitly return the lock data instead.
I also made it explicit that the Acquire lock method
is passing the node metadata as structured information,
rather than an interface{}. This is a bit more fragile
at runtime, but I prefer having very explicit errors if
the locks are incorrect, rather than having to deal with
unvalidated data.
For the lock release delay, it was part of the rebootasrequired
loop, where I believe it makes more sense to be part of the
Release method itself, for readability. Yet, it hides the
delay into the implementation detail, but it keeps the
reboot as required goroutine more readable.
Instead of passing the argument rebootDelay as parameter of the
rebootasrequired method, this refactor took creation of the lock
object in the main loop, close to all the variables, and then
pass the lock object to the rebootasrequired. This makes the
call for rebootasrequired more clear, and lock is now
encompassing everything needed to acquire, release, or get
info about the lock.
Signed-off-by: Jean-Philippe Evrard <open-source@a.spamming.party>
209 lines
4.6 KiB
Go
209 lines
4.6 KiB
Go
package daemonsetlock
|
|
|
|
import (
|
|
"reflect"
|
|
"sort"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func TestTtlExpired(t *testing.T) {
|
|
d := time.Date(2020, 05, 05, 14, 15, 0, 0, time.UTC)
|
|
second, _ := time.ParseDuration("1s")
|
|
zero, _ := time.ParseDuration("0m")
|
|
|
|
tests := []struct {
|
|
created time.Time
|
|
ttl time.Duration
|
|
result bool
|
|
}{
|
|
{d, second, true},
|
|
{time.Now(), second, false},
|
|
{d, zero, false},
|
|
}
|
|
|
|
for i, tst := range tests {
|
|
if ttlExpired(tst.created, tst.ttl) != tst.result {
|
|
t.Errorf("Test %d failed, expected %v but got %v", i, tst.result, !tst.result)
|
|
}
|
|
}
|
|
}
|
|
|
|
func multiLockAnnotationsAreEqualByNodes(src, dst multiLockAnnotationValue) bool {
|
|
srcNodes := []string{}
|
|
for _, srcLock := range src.LockAnnotations {
|
|
srcNodes = append(srcNodes, srcLock.NodeID)
|
|
}
|
|
sort.Strings(srcNodes)
|
|
|
|
dstNodes := []string{}
|
|
for _, dstLock := range dst.LockAnnotations {
|
|
dstNodes = append(dstNodes, dstLock.NodeID)
|
|
}
|
|
sort.Strings(dstNodes)
|
|
|
|
return reflect.DeepEqual(srcNodes, dstNodes)
|
|
}
|
|
|
|
func TestCanAcquireMultiple(t *testing.T) {
|
|
node1Name := "n1"
|
|
node2Name := "n2"
|
|
node3Name := "n3"
|
|
testCases := []struct {
|
|
name string
|
|
daemonSetLock DaemonSetLock
|
|
maxOwners int
|
|
current multiLockAnnotationValue
|
|
desired multiLockAnnotationValue
|
|
lockPossible bool
|
|
}{
|
|
{
|
|
name: "empty_lock",
|
|
daemonSetLock: DaemonSetLock{
|
|
nodeID: node1Name,
|
|
},
|
|
maxOwners: 2,
|
|
current: multiLockAnnotationValue{},
|
|
desired: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node1Name},
|
|
},
|
|
},
|
|
lockPossible: true,
|
|
},
|
|
{
|
|
name: "partial_lock",
|
|
daemonSetLock: DaemonSetLock{
|
|
nodeID: node1Name,
|
|
},
|
|
maxOwners: 2,
|
|
current: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node2Name},
|
|
},
|
|
},
|
|
desired: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node1Name},
|
|
{NodeID: node2Name},
|
|
},
|
|
},
|
|
lockPossible: true,
|
|
},
|
|
{
|
|
name: "full_lock",
|
|
daemonSetLock: DaemonSetLock{
|
|
nodeID: node1Name,
|
|
},
|
|
maxOwners: 2,
|
|
current: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{
|
|
NodeID: node2Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Minute),
|
|
TTL: time.Hour,
|
|
},
|
|
{
|
|
NodeID: node3Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Minute),
|
|
TTL: time.Hour,
|
|
},
|
|
},
|
|
},
|
|
desired: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node2Name},
|
|
{NodeID: node3Name},
|
|
},
|
|
},
|
|
lockPossible: false,
|
|
},
|
|
{
|
|
name: "full_with_one_expired_lock",
|
|
daemonSetLock: DaemonSetLock{
|
|
nodeID: node1Name,
|
|
},
|
|
maxOwners: 2,
|
|
current: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{
|
|
NodeID: node2Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Hour),
|
|
TTL: time.Minute,
|
|
},
|
|
{
|
|
NodeID: node3Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Minute),
|
|
TTL: time.Hour,
|
|
},
|
|
},
|
|
},
|
|
desired: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node1Name},
|
|
{NodeID: node3Name},
|
|
},
|
|
},
|
|
lockPossible: true,
|
|
},
|
|
{
|
|
name: "full_with_all_expired_locks",
|
|
daemonSetLock: DaemonSetLock{
|
|
nodeID: node1Name,
|
|
},
|
|
maxOwners: 2,
|
|
current: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{
|
|
NodeID: node2Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Hour),
|
|
TTL: time.Minute,
|
|
},
|
|
{
|
|
NodeID: node3Name,
|
|
Created: time.Now().UTC().Add(-1 * time.Hour),
|
|
TTL: time.Minute,
|
|
},
|
|
},
|
|
},
|
|
desired: multiLockAnnotationValue{
|
|
MaxOwners: 2,
|
|
LockAnnotations: []LockAnnotationValue{
|
|
{NodeID: node1Name},
|
|
},
|
|
},
|
|
lockPossible: true,
|
|
},
|
|
}
|
|
nm := NodeMeta{Unschedulable: false}
|
|
for _, testCase := range testCases {
|
|
t.Run(testCase.name, func(t *testing.T) {
|
|
lockPossible, actual := testCase.daemonSetLock.canAcquireMultiple(testCase.current, nm, time.Minute, testCase.maxOwners)
|
|
if lockPossible != testCase.lockPossible {
|
|
t.Fatalf(
|
|
"unexpected result for lock possible (got %t expected %t new annotation %v",
|
|
lockPossible,
|
|
testCase.lockPossible,
|
|
actual,
|
|
)
|
|
}
|
|
|
|
if lockPossible && (!multiLockAnnotationsAreEqualByNodes(actual, testCase.desired) || testCase.desired.MaxOwners != actual.MaxOwners) {
|
|
t.Fatalf(
|
|
"expected lock %v but got %v",
|
|
testCase.desired,
|
|
actual,
|
|
)
|
|
}
|
|
})
|
|
}
|
|
}
|