Introduce staggered exit when caps change

This patch changes the behavior of how the pod is exited when the
capabilities have changed. Instead of all the replicas exiting at
the same time, the exits are staggered. If the pod is the leader
or no leader election is configured, the pod is exited after a
brief delay. Otherwise the pod is exited immediately.

This should allow the non-leaders to come back online before the
leader is exited, enabling one of the restarted pods to take over
as the leader when it exits.

.golangci.yml

@@ -85,7 +85,9 @@ linters-settings:
       - alias: pkgctx
         pkg: github.com/vmware-tanzu/vm-operator/pkg/context
       - alias: pkgerr
-        pkg: github.com/vmware-tanzu/vm-operator/pkg/pkgerr
+        pkg: github.com/vmware-tanzu/vm-operator/pkg/errors
+      - alias: pkgexit
+        pkg: github.com/vmware-tanzu/vm-operator/pkg/exit
       - alias: ctxop
         pkg: github.com/vmware-tanzu/vm-operator/pkg/context/operation
       - alias: pkgmgr

config/local/vmoperator/local_env_var_patch.yaml

@@ -13,6 +13,8 @@ spec:
           value: "true"
         - name: ASYNC_CREATE_ENABLED
           value: "true"
+        - name: EXIT_DELAY
+          value: "3s"
         - name: MEM_STATS_PERIOD
           value: "10m"
         - name: VSPHERE_NETWORKING

config/wcp/vmoperator/manager_env_var_patch.yaml

@@ -40,6 +40,12 @@
     name: PRIVILEGED_USERS
     value: "<COMMA_SEPARATED_LIST_OF_USERS>"
 
+- op: add
+  path: /spec/template/spec/containers/0/env/-
+  value:
+    name: EXIT_DELAY
+    value: "3s"
+
 - op: add
   path: /spec/template/spec/containers/0/env/-
   value:

controllers/infra/capability/configmap/configmap_capability_controller.go

@@ -20,10 +20,10 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
-	"github.com/vmware-tanzu/vm-operator/controllers/infra/capability/exit"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/config/capabilities"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
+	pkgexit "github.com/vmware-tanzu/vm-operator/pkg/exit"
 	pkgmgr "github.com/vmware-tanzu/vm-operator/pkg/manager"
 	"github.com/vmware-tanzu/vm-operator/pkg/record"
 	kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube"
@@ -53,6 +53,7 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 		cache,
 		ctrl.Log.WithName("controllers").WithName(controllerName),
 		record.New(mgr.GetEventRecorderFor(controllerNameLong)),
+		mgr.Elected(),
 	)
 
 	// This controller is also run on the non-leaders (webhooks) pods too
@@ -89,13 +90,15 @@ func NewReconciler(
 	ctx context.Context,
 	client ctrlclient.Reader,
 	logger logr.Logger,
-	recorder record.Recorder) *Reconciler {
+	recorder record.Recorder,
+	elected <-chan struct{}) *Reconciler {
 
 	return &Reconciler{
 		Context:  ctx,
 		Client:   client,
 		Logger:   logger,
 		Recorder: recorder,
+		Elected:  elected,
 	}
 }
 
@@ -104,6 +107,7 @@ type Reconciler struct {
 	Client   ctrlclient.Reader
 	Logger   logr.Logger
 	Recorder record.Recorder
+	Elected  <-chan struct{}
 }
 
 // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch
@@ -120,8 +124,10 @@ func (r *Reconciler) Reconcile(
 	}
 
 	if capabilities.UpdateCapabilitiesFeatures(ctx, obj) {
-		r.Logger.Info("killing pod due to changed capabilities")
-		exit.Exit()
+		pkgexit.Exit(
+			logr.NewContext(ctx, r.Logger),
+			"capabilities have changed",
+			r.Elected)
 	}
 
 	return ctrl.Result{}, nil

controllers/infra/capability/configmap/configmap_capability_controller_suite_test.go

@@ -7,20 +7,21 @@ package capability_test
 import (
 	"sync/atomic"
 	"testing"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 
 	capability "github.com/vmware-tanzu/vm-operator/controllers/infra/capability/configmap"
-	"github.com/vmware-tanzu/vm-operator/controllers/infra/capability/exit"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
+	pkgexit "github.com/vmware-tanzu/vm-operator/pkg/exit"
 	"github.com/vmware-tanzu/vm-operator/pkg/manager"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
 var numExits int32
 
 func init() {
-	exit.Exit = func() {
+	pkgexit.ExitFn = func(_ time.Duration) {
 		atomic.AddInt32(&numExits, 1)
 	}
 }

controllers/infra/capability/crd/crd_capability_controller.go

@@ -16,11 +16,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
-	"github.com/vmware-tanzu/vm-operator/controllers/infra/capability/exit"
 	capv1 "github.com/vmware-tanzu/vm-operator/external/capabilities/api/v1alpha1"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/config/capabilities"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
+	pkgexit "github.com/vmware-tanzu/vm-operator/pkg/exit"
 	"github.com/vmware-tanzu/vm-operator/pkg/record"
 	"github.com/vmware-tanzu/vm-operator/pkg/util/ptr"
 )
@@ -39,6 +39,7 @@ func AddToManager(ctx *pkgctx.ControllerManagerContext, mgr manager.Manager) err
 		mgr.GetClient(),
 		ctrl.Log.WithName("controllers").WithName(controllerName),
 		record.New(mgr.GetEventRecorderFor(controllerNameLong)),
+		mgr.Elected(),
 	)
 
 	return ctrl.NewControllerManagedBy(mgr).
@@ -66,13 +67,15 @@ func NewReconciler(
 	ctx context.Context,
 	client ctrlclient.Client,
 	logger logr.Logger,
-	recorder record.Recorder) *Reconciler {
+	recorder record.Recorder,
+	elected <-chan struct{}) *Reconciler {
 
 	return &Reconciler{
 		Context:  ctx,
 		Client:   client,
 		Logger:   logger,
 		Recorder: recorder,
+		Elected:  elected,
 	}
 }
 
@@ -81,6 +84,7 @@ type Reconciler struct {
 	Client   ctrlclient.Client
 	Logger   logr.Logger
 	Recorder record.Recorder
+	Elected  <-chan struct{}
 }
 
 // +kubebuilder:rbac:groups=iaas.vmware.com,resources=capabilities,verbs=get;list;watch
@@ -98,8 +102,10 @@ func (r *Reconciler) Reconcile(
 	}
 
 	if capabilities.UpdateCapabilitiesFeatures(ctx, obj) {
-		r.Logger.Info("killing pod due to changed capabilities")
-		exit.Exit()
+		pkgexit.Exit(
+			logr.NewContext(ctx, r.Logger),
+			"capabilities have changed",
+			r.Elected)
 	}
 
 	return ctrl.Result{}, nil

controllers/infra/capability/crd/crd_capability_controller_suite_test.go

@@ -7,20 +7,21 @@ package capability_test
 import (
 	"sync/atomic"
 	"testing"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 
 	capability "github.com/vmware-tanzu/vm-operator/controllers/infra/capability/crd"
-	"github.com/vmware-tanzu/vm-operator/controllers/infra/capability/exit"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
+	pkgexit "github.com/vmware-tanzu/vm-operator/pkg/exit"
 	"github.com/vmware-tanzu/vm-operator/pkg/manager"
 	"github.com/vmware-tanzu/vm-operator/test/builder"
 )
 
 var numExits int32
 
 func init() {
-	exit.Exit = func() {
+	pkgexit.ExitFn = func(_ time.Duration) {
 		atomic.AddInt32(&numExits, 1)
 	}
 }

main.go

@@ -34,6 +34,7 @@ import (
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/config/capabilities"
 	pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
+	pkgexit "github.com/vmware-tanzu/vm-operator/pkg/exit"
 	pkgmgr "github.com/vmware-tanzu/vm-operator/pkg/manager"
 	pkgmgrinit "github.com/vmware-tanzu/vm-operator/pkg/manager/init"
 	"github.com/vmware-tanzu/vm-operator/pkg/mem"
@@ -72,6 +73,8 @@ func main() {
 		"buildtype", pkg.BuildType,
 		"commit", pkg.BuildCommit)
 
+	initExitFn()
+
 	initContext()
 
 	initFlags()
@@ -128,6 +131,10 @@ func initMemStats() {
 		metrics.Registry.MustRegister)
 }
 
+func initExitFn() {
+	pkgexit.ExitFn = func(_ time.Duration) { os.Exit(1) }
+}
+
 func initContext() {
 	ctx = pkgcfg.WithConfig(defaultConfig)
 	ctx = cource.WithContext(ctx)

pkg/config/config.go

@@ -106,6 +106,14 @@ type Config struct {
 	// such as passwords. Defaults to false.
 	LogSensitiveData bool
 
+	// ExitDelay may be set to a time.Duration value and is the maximum delay
+	// used when exiting the pod due to an event that requires the pod be
+	// restarted, ex. the capabilities have changed. The exit will be delayed
+	// by some duration between 0-ExitDelay.
+	//
+	// Defaults to 3s.
+	ExitDelay time.Duration
+
 	// AsyncSignalEnabled may be set to false to disable the vm-watcher service
 	// used to reconcile VirtualMachine objects if their backend state has
 	// changed.

pkg/config/default.go

@@ -39,6 +39,7 @@ func Default() Config {
 		LeaderElectionID:             defaultPrefix + "controller-manager-runtime",
 		MaxCreateVMsOnProvider:       80,
 		MaxConcurrentReconciles:      1,
+		ExitDelay:                    3 * time.Second,
 		AsyncSignalEnabled:           true,
 		AsyncCreateEnabled:           true,
 		MemStatsPeriod:               10 * time.Minute,

pkg/config/env.go

@@ -29,6 +29,7 @@ func FromEnv() Config {
 	setBool(env.VSphereNetworking, &config.VSphereNetworking)
 	setStringSlice(env.PrivilegedUsers, &config.PrivilegedUsers)
 	setBool(env.LogSensitiveData, &config.LogSensitiveData)
+	setDuration(env.ExitDelay, &config.ExitDelay)
 	setBool(env.AsyncSignalEnabled, &config.AsyncSignalEnabled)
 	setBool(env.AsyncCreateEnabled, &config.AsyncCreateEnabled)
 	setDuration(env.MemStatsPeriod, &config.MemStatsPeriod)

pkg/config/env/env.go

@@ -14,6 +14,7 @@ type VarName uint8
 const (
 	_varNameBegin VarName = iota
 
+	ExitDelay
 	DefaultVMClassControllerName
 	MaxCreateVMsOnProvider
 	CreateVMRequeueDelay
@@ -89,6 +90,8 @@ func All() []VarName {
 //nolint:gocyclo
 func (n VarName) String() string {
 	switch n {
+	case ExitDelay:
+		return "EXIT_DELAY"
 	case DefaultVMClassControllerName:
 		return "DEFAULT_VM_CLASS_CONTROLLER_NAME"
 	case MaxCreateVMsOnProvider:

pkg/config/env_test.go

@@ -108,6 +108,7 @@ var _ = Describe(
 					Expect(os.Setenv("POWERED_ON_VM_HAS_IP_REQUEUE_DELAY", "126h")).To(Succeed())
 					Expect(os.Setenv("MEM_STATS_PERIOD", "127h")).To(Succeed())
 					Expect(os.Setenv("SYNC_IMAGE_REQUEUE_DELAY", "128h")).To(Succeed())
+					Expect(os.Setenv("EXIT_DELAY", "129h")).To(Succeed())
 				})
 				It("Should return a default config overridden by the environment", func() {
 					Expect(config).To(BeComparableTo(pkgcfg.Config{
@@ -162,6 +163,7 @@ var _ = Describe(
 						PoweredOnVMHasIPRequeueDelay: 126 * time.Hour,
 						MemStatsPeriod:               127 * time.Hour,
 						SyncImageRequeueDelay:        128 * time.Hour,
+						ExitDelay:                    129 * time.Hour,
 					}))
 				})
 			})

pkg/exit/exit.go

@@ -0,0 +1,53 @@
+// Copyright (c) 2024 Broadcom. All Rights Reserved.
+// Broadcom Confidential. The term "Broadcom" refers to Broadcom Inc.
+// and/or its subsidiaries.
+
+package exit
+
+import (
+	"context"
+	"math/rand/v2"
+	"time"
+
+	"github.com/go-logr/logr"
+
+	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
+)
+
+// ExitFn is used in testing to assert the Exit() function is called.
+// The default value is a no-op.
+var ExitFn func(exitDelay time.Duration)
+
+func init() {
+	ExitFn = func(_ time.Duration) {}
+}
+
+// Exit will exit the pod immediately if it is not the leader, otherwise if
+// the pod is the leader or leader election is not enabled, the pod will exit
+// after a brief delay.
+func Exit(
+	ctx context.Context,
+	reason string,
+	elected <-chan struct{}) {
+
+	var exitDelay time.Duration
+	select {
+	case <-elected:
+		// When the leader or when there is no leader election, delay the
+		// exit up to some random amount of time.
+		n := rand.Float64() //nolint:gosec
+		exitDelay = time.Duration(n * float64(pkgcfg.FromContext(ctx).ExitDelay))
+	default:
+		// When not the leader, exit immediately. This should allow one of
+		// the non-leader pods to come back online to become the leader when
+		// the leader pod exits.
+	}
+	if exitDelay > 0 {
+		time.Sleep(exitDelay)
+	}
+
+	logr.FromContextOrDiscard(ctx).
+		Info("Exiting pod", "exitDelay", exitDelay, "reason", reason)
+
+	ExitFn(exitDelay)
+}

pkg/exit/exit_suite_test.go

@@ -0,0 +1,17 @@
+// © Broadcom. All Rights Reserved.
+// The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries.
+// SPDX-License-Identifier: Apache-2.0
+
+package exit_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestExit(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Exit Suite")
+}