From a61fd8cb44ad42dfaa6621c7959f65b2f3765cea Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Thu, 27 Apr 2023 12:43:55 -0700 Subject: [PATCH] [Release 1.6] Re-introduce the keep alive flag Thie patch re-introduces the keep alive flag on the CAPV manager. The flag is by enabled default so there are no breaking changes after upgrades. --- config/manager/manager.yaml | 1 + controllers/vspherecluster_reconciler.go | 1 + .../vspheredeploymentzone_controller.go | 1 + controllers/vspherevm_controller.go | 1 + main.go | 2 +- pkg/clustermodule/session.go | 1 + pkg/constants/constants.go | 4 +- pkg/session/session.go | 54 +++++++++++-------- 8 files changed, 39 insertions(+), 26 deletions(-) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 61ccd72267..ae60f172ab 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -20,6 +20,7 @@ spec: - --leader-elect - --logtostderr - --v=4 + - --enable-keep-alive - "--feature-gates=NodeAntiAffinity=${EXP_NODE_ANTI_AFFINITY:=false},NodeLabeling=${EXP_NODE_LABELING:=false}" image: gcr.io/cluster-api-provider-vsphere/release/manager:latest imagePullPolicy: IfNotPresent diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go index 56fdbfc29a..a1cb7cbf19 100644 --- a/controllers/vspherecluster_reconciler.go +++ b/controllers/vspherecluster_reconciler.go @@ -325,6 +325,7 @@ func (r clusterReconciler) reconcileVCenterConnectivity(ctx *context.ClusterCont WithServer(ctx.VSphereCluster.Spec.Server). WithThumbprint(ctx.VSphereCluster.Spec.Thumbprint). WithFeatures(session.Feature{ + EnableKeepAlive: r.EnableKeepAlive, KeepAliveDuration: r.KeepAliveDuration, }) diff --git a/controllers/vspheredeploymentzone_controller.go b/controllers/vspheredeploymentzone_controller.go index b347031c68..ce555162b0 100644 --- a/controllers/vspheredeploymentzone_controller.go +++ b/controllers/vspheredeploymentzone_controller.go @@ -231,6 +231,7 @@ func (r vsphereDeploymentZoneReconciler) getVCenterSession(ctx *context.VSphereD WithDatacenter(ctx.VSphereFailureDomain.Spec.Topology.Datacenter). WithUserInfo(r.ControllerContext.Username, r.ControllerContext.Password). WithFeatures(session.Feature{ + EnableKeepAlive: r.EnableKeepAlive, KeepAliveDuration: r.KeepAliveDuration, }) diff --git a/controllers/vspherevm_controller.go b/controllers/vspherevm_controller.go index 3bde50cf4a..ed9e0b3682 100644 --- a/controllers/vspherevm_controller.go +++ b/controllers/vspherevm_controller.go @@ -536,6 +536,7 @@ func (r vmReconciler) retrieveVcenterSession(ctx goctx.Context, vsphereVM *infra WithUserInfo(r.ControllerContext.Username, r.ControllerContext.Password). WithThumbprint(vsphereVM.Spec.Thumbprint). WithFeatures(session.Feature{ + EnableKeepAlive: r.EnableKeepAlive, KeepAliveDuration: r.KeepAliveDuration, }) cluster, err := clusterutilv1.GetClusterFromMetadata(r.ControllerContext, r.Client, vsphereVM.ObjectMeta) diff --git a/main.go b/main.go index 36c3e2bb9f..9f4195c4ba 100644 --- a/main.go +++ b/main.go @@ -135,7 +135,7 @@ func InitFlags(fs *pflag.FlagSet) { &managerOpts.EnableKeepAlive, "enable-keep-alive", defaultEnableKeepAlive, - "DEPRECATED: feature to enable keep alive handler in vsphere sessions. This functionality is enabled by default now") + "feature to enable keep alive handler in vsphere sessions. This functionality is enabled by default.") flag.DurationVar( &managerOpts.KeepAliveDuration, "keep-alive-duration", diff --git a/pkg/clustermodule/session.go b/pkg/clustermodule/session.go index 73d6aa2712..9aa3480231 100644 --- a/pkg/clustermodule/session.go +++ b/pkg/clustermodule/session.go @@ -44,6 +44,7 @@ func newParams(ctx context.ClusterContext) *session.Params { WithServer(ctx.VSphereCluster.Spec.Server). WithThumbprint(ctx.VSphereCluster.Spec.Thumbprint). WithFeatures(session.Feature{ + EnableKeepAlive: ctx.EnableKeepAlive, KeepAliveDuration: ctx.KeepAliveDuration, }) } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 136d009a3a..24c5ca7b74 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -51,8 +51,8 @@ const ( // cluster are in maintenance mode. MaintenanceAnnotationLabel = "capv." + v1alpha3.GroupName + "/maintenance" - // DefaultEnableKeepAlive is false by default. - DefaultEnableKeepAlive = false + // DefaultEnableKeepAlive is true by default. + DefaultEnableKeepAlive = true // KeepaliveDuration unit minutes. DefaultKeepAliveDuration = time.Minute * 5 diff --git a/pkg/session/session.go b/pkg/session/session.go index f17cd03a7f..4361148fd2 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -41,6 +41,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/pkg/constants" ) var ( @@ -61,11 +62,14 @@ type Session struct { } type Feature struct { + EnableKeepAlive bool KeepAliveDuration time.Duration } func DefaultFeature() Feature { - return Feature{} + return Feature{ + EnableKeepAlive: constants.DefaultEnableKeepAlive, + } } type Params struct { @@ -220,15 +224,17 @@ func newClient(ctx context.Context, logger logr.Logger, sessionKey string, url * SessionManager: session.NewManager(vimClient), } - vimClient.RoundTripper = session.KeepAliveHandler(vimClient.RoundTripper, feature.KeepAliveDuration, func(tripper soap.RoundTripper) error { - _, err := methods.GetCurrentTime(ctx, tripper) - if err != nil { - logger.Error(err, "failed to keep alive govmomi client") - logger.Info("clearing the session") - sessionCache.Delete(sessionKey) - } - return err - }) + if feature.EnableKeepAlive { + vimClient.RoundTripper = session.KeepAliveHandler(vimClient.RoundTripper, feature.KeepAliveDuration, func(tripper soap.RoundTripper) error { + _, err := methods.GetCurrentTime(ctx, tripper) + if err != nil { + logger.Error(err, "failed to keep alive govmomi client") + logger.Info("clearing the session") + sessionCache.Delete(sessionKey) + } + return err + }) + } if err := c.Login(ctx, url.User); err != nil { return nil, err @@ -240,19 +246,21 @@ func newClient(ctx context.Context, logger logr.Logger, sessionKey string, url * // newManager creates a Manager that encompasses the REST Client for the VSphere tagging API. func newManager(ctx context.Context, logger logr.Logger, sessionKey string, client *vim25.Client, user *url.Userinfo, feature Feature) (*tags.Manager, error) { rc := rest.NewClient(client) - rc.Transport = keepalive.NewHandlerREST(rc, feature.KeepAliveDuration, func() error { - s, err := rc.Session(ctx) - if err != nil { - return err - } - if s != nil { - return nil - } - - logger.Info("rest client session expired, clearing session") - sessionCache.Delete(sessionKey) - return errors.New("rest client session expired") - }) + if feature.EnableKeepAlive { + rc.Transport = keepalive.NewHandlerREST(rc, feature.KeepAliveDuration, func() error { + s, err := rc.Session(ctx) + if err != nil { + return err + } + if s != nil { + return nil + } + + logger.Info("rest client session expired, clearing session") + sessionCache.Delete(sessionKey) + return errors.New("rest client session expired") + }) + } if err := rc.Login(ctx, user); err != nil { return nil, err }