kvstore: Controllerize stale lock garbage collection

The garbage collection of stale locks has previously been run from an
init() function in the kvstore package. To ensure it is only run when
the kvstore is used, and to provide better visibility into when this GC
runs, convert it into a controller in the daemon.

Signed-off-by: Joe Stringer <joe@cilium.io>

daemon/daemon_main.go

@@ -36,6 +36,7 @@ import (
 	"github.com/cilium/cilium/pkg/cgroups"
 	"github.com/cilium/cilium/pkg/cleanup"
 	"github.com/cilium/cilium/pkg/components"
+	"github.com/cilium/cilium/pkg/controller"
 	linuxdatapath "github.com/cilium/cilium/pkg/datapath/linux"
 	"github.com/cilium/cilium/pkg/datapath/loader"
 	"github.com/cilium/cilium/pkg/datapath/maps"
@@ -1257,6 +1258,16 @@ func (d *Daemon) initKVStore() {
 		ClusterSizeDependantInterval: d.nodeDiscovery.Manager.ClusterSizeDependantInterval,
 	}
 
+	controller.NewManager().UpdateController("kvstore-locks-gc",
+		controller.ControllerParams{
+			DoFunc: func(ctx context.Context) error {
+				kvstore.RunLockGC()
+				return nil
+			},
+			RunInterval: defaults.KVStoreStaleLockTimeout,
+		},
+	)
+
 	// If K8s is enabled we can do the service translation automagically by
 	// looking at services from k8s and retrieve the service IP from that.
 	// This makes cilium to not depend on kube dns to interact with etcd

pkg/defaults/defaults.go

@@ -174,6 +174,10 @@ const (
 	// KVstoreConnectivityTimeout is the timeout when performing kvstore operations
 	KVstoreConnectivityTimeout = 2 * time.Minute
 
+	// KVStoreStaleLockTimeout is the timeout for when a lock is held for
+	// a kvstore path for too long.
+	KVStoreStaleLockTimeout = 30 * time.Second
+
 	// IPAllocationTimeout is the timeout when allocating CIDRs
 	IPAllocationTimeout = 2 * time.Minute
 

pkg/kvstore/lock.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/cilium/cilium/pkg/debug"
+	"github.com/cilium/cilium/pkg/defaults"
 	"github.com/cilium/cilium/pkg/lock"
 	uuidfactor "github.com/cilium/cilium/pkg/uuid"
 
@@ -38,7 +39,7 @@ var (
 	// released lock. The only possibility of concurrent access is if a
 	// consumer is *still* holding the lock but this is highly unlikely
 	// given the duration of this timeout.
-	staleLockTimeout = time.Duration(30) * time.Second
+	staleLockTimeout = defaults.KVStoreStaleLockTimeout
 )
 
 type KVLocker interface {
@@ -65,13 +66,6 @@ type pathLocks struct {
 }
 
 func init() {
-	go func() {
-		for {
-			kvstoreLocks.runGC()
-			time.Sleep(staleLockTimeout)
-		}
-	}()
-
 	debug.RegisterStatusObject("kvstore-locks", &kvstoreLocks)
 }
 
@@ -156,6 +150,13 @@ func LockPath(ctx context.Context, path string) (l *Lock, err error) {
 	return &Lock{kvLock: lock, path: path, id: id}, err
 }
 
+// RunLockGC inspects all local kvstore locks to determine whether they have
+// been held longer than the stale lock timeout, and if so, unlocks them
+// forceably.
+func RunLockGC() {
+	kvstoreLocks.runGC()
+}
+
 // Unlock unlocks a lock
 func (l *Lock) Unlock() error {
 	if l == nil {