Move DefaultLeaseDuration... parameters to controller pkg and flesh out code doc comments

Author: Matthew Wong

aws/efs/cmd/efs-provisioner/efs-provisioner.go

@@ -33,7 +33,6 @@ import (
 	"github.com/golang/glog"
 	"github.com/kubernetes-incubator/external-storage/aws/efs/pkg/gidallocator"
 	"github.com/kubernetes-incubator/external-storage/lib/controller"
-	"github.com/kubernetes-incubator/external-storage/lib/leaderelection"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
@@ -49,10 +48,10 @@ const (
 	resyncPeriod              = 15 * time.Second
 	exponentialBackOffOnError = true
 	failedRetryThreshold      = 5
-	leasePeriod               = leaderelection.DefaultLeaseDuration
-	retryPeriod               = leaderelection.DefaultRetryPeriod
-	renewDeadline             = leaderelection.DefaultRenewDeadline
-	termLimit                 = leaderelection.DefaultTermLimit
+	leasePeriod               = controller.DefaultLeaseDuration
+	retryPeriod               = controller.DefaultRetryPeriod
+	renewDeadline             = controller.DefaultRenewDeadline
+	termLimit                 = controller.DefaultTermLimit
 )
 
 type efsProvisioner struct {

gluster/block/glusterblock-provisioner.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/kubernetes-incubator/external-storage/lib/controller"
-	"github.com/kubernetes-incubator/external-storage/lib/leaderelection"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/uuid"
@@ -47,10 +46,10 @@ const (
 	failedRetryThreshold      = 5
 	defaultExecPath           = "./createiscsi"
 	secretKeyName             = "key"
-	leasePeriod               = leaderelection.DefaultLeaseDuration
-	retryPeriod               = leaderelection.DefaultRetryPeriod
-	renewDeadline             = leaderelection.DefaultRenewDeadline
-	termLimit                 = leaderelection.DefaultTermLimit
+	leasePeriod               = controller.DefaultLeaseDuration
+	retryPeriod               = controller.DefaultRetryPeriod
+	renewDeadline             = controller.DefaultRenewDeadline
+	termLimit                 = controller.DefaultTermLimit
 )
 
 type glusterBlockProvisioner struct {

lib/controller/controller.go

@@ -68,6 +68,27 @@ const createProvisionedPVRetryCount = 5
 // Interval between retries when we create a PV object for a provisioned volume.
 const createProvisionedPVInterval = 10 * time.Second
 
+const (
+	// DefaultLeaseDuration is a suggested controller LeaseDuration:
+	// LeaseDuration is the duration that non-leader candidates will
+	// wait to force acquire leadership. This is measured against time of
+	// last observed ack.
+	DefaultLeaseDuration = 15 * time.Second
+	// DefaultRenewDeadline is a suggested controller RenewDeadline:
+	// RenewDeadline is the duration that the acting master will retry
+	// refreshing leadership before giving up.
+	DefaultRenewDeadline = 10 * time.Second
+	// DefaultRetryPeriod is a suggested controller RetryPeriod:
+	// RetryPeriod is the duration the LeaderElector clients should wait
+	// between tries of actions.
+	DefaultRetryPeriod = 2 * time.Second
+	// DefaultTermLimit is a suggested controller TermLimit:
+	// TermLimit is the maximum duration that a leader may remain the leader
+	// to complete the task before it must give up its leadership. 0 for forever
+	// or indefinite.
+	DefaultTermLimit = 30 * time.Second
+)
+
 // ProvisionController is a controller that provisions PersistentVolumes for
 // PersistentVolumeClaims.
 type ProvisionController struct {
@@ -119,24 +140,29 @@ type ProvisionController struct {
 	// Interval between retries when we create a PV object for a provisioned volume.
 	createProvisionedPVInterval time.Duration
 
-	// Identity of this controller, generated at creation time.
+	// Identity of this controller, generated at creation time and not persisted
+	// across restarts. Useful only for debugging, for seeing the source of
+	// events. controller.provisioner may have its own, different notion of
+	// identity which may/may not persist across restarts
 	identity types.UID
 
-	// Parameters of LeaderElectionConfig: set to defaults except in tests
+	// Parameters of leaderelection.LeaderElectionConfig. Leader election is for
+	// when multiple controllers are running: they race to lock (lead) every PVC
+	// so that only one calls Provision for it (saving API calls, CPU cycles...)
+	// Descriptions of each can be found in the leaderelection package and
+	// reproduced above (DefaultLeaseDuration...)
 	leaseDuration, renewDeadline, retryPeriod, termLimit time.Duration
 
 	// Map of claim UID to LeaderElector: for checking if this controller
 	// is the leader of a given claim
 	leaderElectors map[types.UID]*leaderelection.LeaderElector
+	mapMutex       *sync.Mutex
 
-	mapMutex *sync.Mutex
-
-	// threshold for max number of retries on failure of provisioner
+	// Threshold for max number of retries on failure of provisioner
 	failedRetryThreshold int
 
-	// map of failed claims
-	failedClaimsStats map[types.UID]int
-
+	// Map of failed claims
+	failedClaimsStats      map[types.UID]int
 	failedClaimsStatsMutex *sync.Mutex
 }
 

lib/controller/controller_test.go

@@ -25,7 +25,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/kubernetes-incubator/external-storage/lib/leaderelection"
 	rl "github.com/kubernetes-incubator/external-storage/lib/leaderelection/resourcelock"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -246,7 +245,7 @@ func TestMultipleControllers(t *testing.T) {
 		ctrls := make([]*ProvisionController, test.numControllers)
 		stopChs := make([]chan struct{}, test.numControllers)
 		for i := 0; i < test.numControllers; i++ {
-			ctrls[i] = NewProvisionController(client, 15*time.Second, test.provisionerName, provisioner, "v1.5.0", false, failedRetryThreshold, leaderelection.DefaultLeaseDuration, leaderelection.DefaultRenewDeadline, leaderelection.DefaultRetryPeriod, leaderelection.DefaultTermLimit)
+			ctrls[i] = NewProvisionController(client, 15*time.Second, test.provisionerName, provisioner, "v1.5.0", false, failedRetryThreshold, DefaultLeaseDuration, DefaultRenewDeadline, DefaultRetryPeriod, DefaultTermLimit)
 			ctrls[i].createProvisionedPVInterval = 10 * time.Millisecond
 			ctrls[i].claimSource = claimSource
 			ctrls[i].claims.Add(newClaim("claim-1", "uid-1-1", "class-1", "", nil))

lib/leaderelection/leaderelection.go

@@ -39,11 +39,7 @@ import (
 )
 
 const (
-	JitterFactor         = 1.2
-	DefaultLeaseDuration = 15 * time.Second
-	DefaultRenewDeadline = 10 * time.Second
-	DefaultRetryPeriod   = 2 * time.Second
-	DefaultTermLimit     = 30 * time.Second
+	JitterFactor = 1.2
 )
 
 // NewLeaderElector creates a LeaderElector from a LeaderElecitionConfig

nfs/cmd/nfs-provisioner/main.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/kubernetes-incubator/external-storage/lib/controller"
-	"github.com/kubernetes-incubator/external-storage/lib/leaderelection"
 	"github.com/kubernetes-incubator/external-storage/nfs/pkg/server"
 	vol "github.com/kubernetes-incubator/external-storage/nfs/pkg/volume"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -50,10 +49,10 @@ var (
 const (
 	exportDir     = "/export"
 	ganeshaConfig = "/export/vfs.conf"
-	leasePeriod   = leaderelection.DefaultLeaseDuration
-	retryPeriod   = leaderelection.DefaultRetryPeriod
-	renewDeadline = leaderelection.DefaultRenewDeadline
-	termLimit     = leaderelection.DefaultTermLimit
+	leasePeriod   = controller.DefaultLeaseDuration
+	retryPeriod   = controller.DefaultRetryPeriod
+	renewDeadline = controller.DefaultRenewDeadline
+	termLimit     = controller.DefaultTermLimit
 )
 
 func main() {