diff --git a/controllers/vspherecluster_controller.go b/controllers/vspherecluster_controller.go index d502088c78..fc0d963236 100644 --- a/controllers/vspherecluster_controller.go +++ b/controllers/vspherecluster_controller.go @@ -330,16 +330,17 @@ func cloudProviderConfigurationAvailable(ctx *context.ClusterContext) bool { } func (r clusterReconciler) reconcileVCenterConnectivity(ctx *context.ClusterContext) error { - _, err := session.GetOrCreate(session.NewGetOrCreateContext(ctx.Context, ctx.Logger), - ctx.VSphereCluster.Spec.Server, - ctx.VSphereCluster.Spec.CloudProviderConfiguration.Workspace.Datacenter, - ctx.Username, - ctx.Password, - ctx.VSphereCluster.Spec.Thumbprint, - session.Feature{ + params := session.NewParams(). + WithServer(ctx.VSphereCluster.Spec.Server). + WithDatacenter(ctx.VSphereCluster.Spec.CloudProviderConfiguration.Workspace.Datacenter). + WithUserInfo(ctx.Username, ctx.Password). + WithThumbprint(ctx.VSphereCluster.Spec.Thumbprint). + WithFeatures(session.Feature{ EnableKeepAlive: r.EnableKeepAlive, KeepAliveDuration: r.KeepAliveDuration, }) + _, err := session.GetOrCreate(session.NewSessionContext(ctx.Context, ctx.Logger), + params) return err } diff --git a/controllers/vspherevm_controller.go b/controllers/vspherevm_controller.go index 04629c0209..12ec06c737 100644 --- a/controllers/vspherevm_controller.go +++ b/controllers/vspherevm_controller.go @@ -134,17 +134,18 @@ func (r vmReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_ ctrl.Res return reconcile.Result{}, err } - // Get or create an authenticated session to the vSphere endpoint. - authSession, err := session.GetOrCreate(session.NewGetOrCreateContext(r.Context, r.Logger), - vsphereVM.Spec.Server, - vsphereVM.Spec.Datacenter, - r.ControllerManagerContext.Username, - r.ControllerManagerContext.Password, - vsphereVM.Spec.Thumbprint, - session.Feature{ + params := session.NewParams(). + WithServer(vsphereVM.Spec.Server). + WithDatacenter(vsphereVM.Spec.Datacenter). + WithUserInfo(r.ControllerContext.Username, r.ControllerContext.Password). + WithThumbprint(vsphereVM.Spec.Thumbprint). + WithFeatures(session.Feature{ EnableKeepAlive: r.EnableKeepAlive, KeepAliveDuration: r.KeepAliveDuration, }) + // Get or create an authenticated session to the vSphere endpoint. + authSession, err := session.GetOrCreate(session.NewSessionContext(r.Context, r.Logger), + params) if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to create vSphere session") } diff --git a/pkg/services/govmomi/create_test.go b/pkg/services/govmomi/create_test.go index eef4bac252..d894496c67 100644 --- a/pkg/services/govmomi/create_test.go +++ b/pkg/services/govmomi/create_test.go @@ -47,9 +47,12 @@ func TestCreate(t *testing.T) { vmContext.VSphereVM.Spec.Server = s.URL.Host authSession, err := session.GetOrCreate( - session.NewGetOrCreateContext(vmContext.Context, vmContext.Logger), - vmContext.VSphereVM.Spec.Server, "", - s.URL.User.Username(), pass, "", session.DefaultFeature()) + session.NewSessionContext(vmContext.Context, vmContext.Logger), + session.NewParams(). + WithServer(vmContext.VSphereVM.Spec.Server). + WithDatacenter(""). + WithUserInfo(s.URL.User.Username(), pass). + WithThumbprint("")) if err != nil { t.Fatal(err) } diff --git a/pkg/services/govmomi/vcenter/clone_test.go b/pkg/services/govmomi/vcenter/clone_test.go index 8acfeb54cc..ae225ccfce 100644 --- a/pkg/services/govmomi/vcenter/clone_test.go +++ b/pkg/services/govmomi/vcenter/clone_test.go @@ -142,11 +142,18 @@ func initSimulator(t *testing.T) (*simulator.Model, *session.Session, *simulator server := model.Service.NewServer() pass, _ := server.URL.User.Password() + //authSession, err := session.GetOrCreate( + // session.NewSessionContext(ctx.TODO(), klogr.New()), + // server.URL.Host, "", + // server.URL.User.Username(), pass, "", + // session.DefaultFeature()) authSession, err := session.GetOrCreate( - session.NewGetOrCreateContext(ctx.TODO(), klogr.New()), - server.URL.Host, "", - server.URL.User.Username(), pass, "", - session.DefaultFeature()) + session.NewSessionContext(ctx.TODO(), klogr.New()), + session.NewParams(). + WithServer(server.URL.Host). + WithDatacenter(""). + WithUserInfo(server.URL.User.Username(), pass). + WithThumbprint("")) if err != nil { t.Fatal(err) } diff --git a/pkg/session/session.go b/pkg/session/session.go index adb50b0fe2..d193a1064a 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -45,13 +45,15 @@ type Session struct { datacenter *object.Datacenter } -type GetOrCreateContext struct { +// wrapper around session context +// TODO: sadysnaat replace with controller-manager context logger +type Context struct { context context.Context logger logr.Logger } -func NewGetOrCreateContext(ctx context.Context, logger logr.Logger) GetOrCreateContext { - return GetOrCreateContext{ +func NewSessionContext(ctx context.Context, logger logr.Logger) Context { + return Context{ context: ctx, logger: logger.WithName("session"), } @@ -69,20 +71,57 @@ func DefaultFeature() Feature { } } +type Params struct { + server string + datacenter string + userinfo *url.Userinfo + thumbprint string + feature Feature +} + +func NewParams() *Params { + return &Params{ + feature: DefaultFeature(), + } +} + +func (p *Params) WithServer(server string) *Params { + p.server = server + return p +} + +func (p *Params) WithDatacenter(datacenter string) *Params { + p.datacenter = datacenter + return p +} + +func (p *Params) WithUserInfo(username, password string) *Params { + p.userinfo = url.UserPassword(username, password) + return p +} + +func (p *Params) WithThumbprint(thumbprint string) *Params { + p.thumbprint = thumbprint + return p +} + +func (p *Params) WithFeatures(feature Feature) *Params { + p.feature = feature + return p +} + // GetOrCreate gets a cached session or creates a new one if one does not // already exist. -func GetOrCreate( - ctx GetOrCreateContext, - server, datacenter, username, password string, thumbprint string, feature Feature) (*Session, error) { +func GetOrCreate(ctx Context, params *Params) (*Session, error) { sessionMU.Lock() defer sessionMU.Unlock() - sessionKey := server + username + datacenter + sessionKey := params.server + params.userinfo.Username() + params.datacenter if session, ok := sessionCache[sessionKey]; ok { // if keepalive is enabled we depend upon roundtripper to reestablish the connection // and remove the key if it could not - if feature.EnableKeepAlive { + if params.feature.EnableKeepAlive { return &session, nil } if ok, _ := session.SessionManager.SessionIsActive(ctx.context); ok { @@ -90,16 +129,16 @@ func GetOrCreate( } } - soapURL, err := soap.ParseURL(server) + soapURL, err := soap.ParseURL(params.server) if err != nil { - return nil, errors.Wrapf(err, "error parsing vSphere URL %q", server) + return nil, errors.Wrapf(err, "error parsing vSphere URL %q", params.server) } if soapURL == nil { - return nil, errors.Errorf("error parsing vSphere URL %q", server) + return nil, errors.Errorf("error parsing vSphere URL %q", params.server) } - soapURL.User = url.UserPassword(username, password) - client, err := newClient(ctx, sessionKey, soapURL, thumbprint, feature) + soapURL.User = params.userinfo + client, err := newClient(ctx, sessionKey, soapURL, params.thumbprint, params.feature) if err != nil { return nil, err } @@ -111,9 +150,9 @@ func GetOrCreate( session.Finder = find.NewFinder(session.Client.Client, false) // Assign the datacenter if one was specified. - dc, err := session.Finder.DatacenterOrDefault(ctx.context, datacenter) + dc, err := session.Finder.DatacenterOrDefault(ctx.context, params.datacenter) if err != nil { - return nil, errors.Wrapf(err, "unable to find datacenter %q", datacenter) + return nil, errors.Wrapf(err, "unable to find datacenter %q", params.datacenter) } session.datacenter = dc session.Finder.SetDatacenter(dc) @@ -127,7 +166,7 @@ func GetOrCreate( return &session, nil } -func newClient(ctx GetOrCreateContext, sessionKey string, url *url.URL, thumprint string, feature Feature) (*govmomi.Client, error) { +func newClient(ctx Context, sessionKey string, url *url.URL, thumprint string, feature Feature) (*govmomi.Client, error) { insecure := thumprint == "" soapClient := soap.NewClient(url, insecure) if !insecure { @@ -155,7 +194,7 @@ func newClient(ctx GetOrCreateContext, sessionKey string, url *url.URL, thumprin _, err := methods.GetCurrentTime(ctx.context, tripper) if err != nil { ctx.logger.Error(err, "failed to keep alive govmomi client") - ClearCache(sessionKey) + clearCache(sessionKey) } return err }) @@ -168,7 +207,7 @@ func newClient(ctx GetOrCreateContext, sessionKey string, url *url.URL, thumprin return c, nil } -func ClearCache(sessionKey string) { +func clearCache(sessionKey string) { sessionMU.Lock() defer sessionMU.Unlock() delete(sessionCache, sessionKey)