From 64601373f11c062ed1146addc48dd40e5042a5ea Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Sat, 5 May 2018 11:33:53 -0700 Subject: [PATCH 1/4] vsphere: use vim25.Client directly to support token authentication This refactor is in support of SAML token authentication: #63209 Avoid use of govmomi.Client as it only supports username+password authentication via SessionManager.Login(). Using vim25.Client directly will allow VCP to add other authentication methods, such as SessionManager.LoginByToken(). --- .../providers/vsphere/vclib/connection.go | 35 ++++++++++++++----- .../providers/vsphere/vclib/datacenter.go | 4 +-- .../vsphere/vclib/datacenter_test.go | 2 +- .../providers/vsphere/vclib/datastore_test.go | 2 +- .../providers/vsphere/vclib/folder_test.go | 2 +- .../providers/vsphere/vclib/utils_test.go | 2 +- .../providers/vsphere/vclib/virtualmachine.go | 8 ++--- .../vsphere/vclib/virtualmachine_test.go | 2 +- .../providers/vsphere/vsphere.go | 2 +- .../providers/vsphere/vsphere_test.go | 2 +- 10 files changed, 39 insertions(+), 22 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/connection.go b/pkg/cloudprovider/providers/vsphere/vclib/connection.go index dda185ab0a3..097fbd4937e 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/connection.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/connection.go @@ -18,19 +18,19 @@ package vclib import ( "context" - "fmt" + "net" neturl "net/url" "sync" "github.com/golang/glog" - "github.com/vmware/govmomi" "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/vim25" + "github.com/vmware/govmomi/vim25/soap" ) // VSphereConnection contains information for connecting to vCenter type VSphereConnection struct { - GoVmomiClient *govmomi.Client + GoVmomiClient *vim25.Client Username string Password string Hostname string @@ -59,7 +59,7 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error { } return nil } - m := session.NewManager(connection.GoVmomiClient.Client) + m := session.NewManager(connection.GoVmomiClient) userSession, err := m.UserSession(ctx) if err != nil { glog.Errorf("Error while obtaining user session. err: %+v", err) @@ -69,7 +69,7 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error { return nil } glog.Warningf("Creating new client session since the existing session is not valid or not authenticated") - connection.GoVmomiClient.Logout(ctx) + connection.GoVmomiClient, err = connection.NewClient(ctx) if err != nil { glog.Errorf("Failed to create govmomi client. err: %+v", err) @@ -78,19 +78,36 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error { return nil } +// Logout calls SessionManager.Logout for the given connection. +func (connection *VSphereConnection) Logout(ctx context.Context) { + m := session.NewManager(connection.GoVmomiClient) + if err := m.Logout(ctx); err != nil { + glog.Errorf("Logout failed: %s", err) + } +} + // NewClient creates a new govmomi client for the VSphereConnection obj -func (connection *VSphereConnection) NewClient(ctx context.Context) (*govmomi.Client, error) { - url, err := neturl.Parse(fmt.Sprintf("https://%s:%s/sdk", connection.Hostname, connection.Port)) +func (connection *VSphereConnection) NewClient(ctx context.Context) (*vim25.Client, error) { + url, err := soap.ParseURL(net.JoinHostPort(connection.Hostname, connection.Port)) if err != nil { glog.Errorf("Failed to parse URL: %s. err: %+v", url, err) return nil, err } - url.User = neturl.UserPassword(connection.Username, connection.Password) - client, err := govmomi.NewClient(ctx, url, connection.Insecure) + + sc := soap.NewClient(url, connection.Insecure) + client, err := vim25.NewClient(ctx, sc) if err != nil { glog.Errorf("Failed to create new client. err: %+v", err) return nil, err } + + m := session.NewManager(client) + + err = m.Login(ctx, neturl.UserPassword(connection.Username, connection.Password)) + if err != nil { + return nil, err + } + if connection.RoundTripperCount == 0 { connection.RoundTripperCount = RoundTripperDefaultCount } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go index fa4f1c3d76a..2c744116e9a 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go @@ -39,7 +39,7 @@ type Datacenter struct { // GetDatacenter returns the DataCenter Object for the given datacenterPath // If datacenter is located in a folder, include full path to datacenter else just provide the datacenter name func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacenterPath string) (*Datacenter, error) { - finder := find.NewFinder(connection.GoVmomiClient.Client, false) + finder := find.NewFinder(connection.GoVmomiClient, false) datacenter, err := finder.Datacenter(ctx, datacenterPath) if err != nil { glog.Errorf("Failed to find the datacenter: %s. err: %+v", datacenterPath, err) @@ -52,7 +52,7 @@ func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacente // GetAllDatacenter returns all the DataCenter Objects func GetAllDatacenter(ctx context.Context, connection *VSphereConnection) ([]*Datacenter, error) { var dc []*Datacenter - finder := find.NewFinder(connection.GoVmomiClient.Client, false) + finder := find.NewFinder(connection.GoVmomiClient, false) datacenters, err := finder.DatacenterList(ctx, "*") if err != nil { glog.Errorf("Failed to find the datacenter. err: %+v", err) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go index 284a3edafc9..65b70a76b24 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go @@ -47,7 +47,7 @@ func TestDatacenter(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c} + vc := &VSphereConnection{GoVmomiClient: c.Client} _, err = GetDatacenter(ctx, vc, testNameNotFound) if err == nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go index 4300e4d6f8c..3ecd99c0a1a 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go @@ -45,7 +45,7 @@ func TestDatastore(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c} + vc := &VSphereConnection{GoVmomiClient: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go index 315d008dc09..b83ea7f0da5 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go @@ -47,7 +47,7 @@ func TestFolder(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c} + vc := &VSphereConnection{GoVmomiClient: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go index ab4bd89df42..786da2f1a33 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go @@ -46,7 +46,7 @@ func TestUtils(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c} + vc := &VSphereConnection{GoVmomiClient: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine.go b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine.go index 679d827adc3..01654b3d1ef 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine.go @@ -23,9 +23,9 @@ import ( "time" "github.com/golang/glog" - "github.com/vmware/govmomi" "github.com/vmware/govmomi/object" "github.com/vmware/govmomi/property" + "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/mo" "github.com/vmware/govmomi/vim25/types" ) @@ -403,8 +403,8 @@ func (vm *VirtualMachine) deleteController(ctx context.Context, controllerDevice } // RenewVM renews this virtual machine with new client connection. -func (vm *VirtualMachine) RenewVM(client *govmomi.Client) VirtualMachine { - dc := Datacenter{Datacenter: object.NewDatacenter(client.Client, vm.Datacenter.Reference())} - newVM := object.NewVirtualMachine(client.Client, vm.VirtualMachine.Reference()) +func (vm *VirtualMachine) RenewVM(client *vim25.Client) VirtualMachine { + dc := Datacenter{Datacenter: object.NewDatacenter(client, vm.Datacenter.Reference())} + newVM := object.NewVirtualMachine(client, vm.VirtualMachine.Reference()) return VirtualMachine{VirtualMachine: newVM, Datacenter: &dc} } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go index 0b38fd1be4f..35b7c5a51e9 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go @@ -43,7 +43,7 @@ func TestVirtualMachine(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c} + vc := &VSphereConnection{GoVmomiClient: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index cac821eb556..87a08234ea3 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -411,7 +411,7 @@ func newControllerNode(cfg VSphereConfig) (*VSphere, error) { func logout(vs *VSphere) { for _, vsphereIns := range vs.vsphereInstanceMap { if vsphereIns.conn.GoVmomiClient != nil { - vsphereIns.conn.GoVmomiClient.Logout(context.TODO()) + vsphereIns.conn.Logout(context.TODO()) } } diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_test.go index 84b96644134..3e57ba54239 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_test.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_test.go @@ -135,7 +135,7 @@ func TestVSphereLogin(t *testing.T) { if err != nil { t.Errorf("Failed to connect to vSphere: %s", err) } - defer vcInstance.conn.GoVmomiClient.Logout(ctx) + defer vcInstance.conn.Logout(ctx) } func TestZones(t *testing.T) { From e7f74d83c6d2265f8aaec60fcf103025241c4c56 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Sat, 5 May 2018 16:16:24 -0700 Subject: [PATCH 2/4] Rename VSphereConnection.GoVmomiClient -> Client --- .../providers/vsphere/nodemanager.go | 2 +- .../providers/vsphere/vclib/connection.go | 18 +++++++++--------- .../providers/vsphere/vclib/datacenter.go | 4 ++-- .../providers/vsphere/vclib/datacenter_test.go | 2 +- .../providers/vsphere/vclib/datastore_test.go | 2 +- .../providers/vsphere/vclib/folder_test.go | 2 +- .../providers/vsphere/vclib/utils_test.go | 2 +- .../vsphere/vclib/virtualmachine_test.go | 2 +- pkg/cloudprovider/providers/vsphere/vsphere.go | 2 +- 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/nodemanager.go b/pkg/cloudprovider/providers/vsphere/nodemanager.go index 4979d605871..25f8d58c9e2 100644 --- a/pkg/cloudprovider/providers/vsphere/nodemanager.go +++ b/pkg/cloudprovider/providers/vsphere/nodemanager.go @@ -360,7 +360,7 @@ func (nm *NodeManager) renewNodeInfo(nodeInfo *NodeInfo, reconnect bool) (*NodeI return nil, err } } - vm := nodeInfo.vm.RenewVM(vsphereInstance.conn.GoVmomiClient) + vm := nodeInfo.vm.RenewVM(vsphereInstance.conn.Client) return &NodeInfo{vm: &vm, dataCenter: vm.Datacenter, vcServer: nodeInfo.vcServer}, nil } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/connection.go b/pkg/cloudprovider/providers/vsphere/vclib/connection.go index 097fbd4937e..6181eba3f72 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/connection.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/connection.go @@ -30,7 +30,7 @@ import ( // VSphereConnection contains information for connecting to vCenter type VSphereConnection struct { - GoVmomiClient *vim25.Client + Client *vim25.Client Username string Password string Hostname string @@ -43,23 +43,23 @@ var ( clientLock sync.Mutex ) -// Connect makes connection to vCenter and sets VSphereConnection.GoVmomiClient. -// If connection.GoVmomiClient is already set, it obtains the existing user session. -// if user session is not valid, connection.GoVmomiClient will be set to the new client. +// Connect makes connection to vCenter and sets VSphereConnection.Client. +// If connection.Client is already set, it obtains the existing user session. +// if user session is not valid, connection.Client will be set to the new client. func (connection *VSphereConnection) Connect(ctx context.Context) error { var err error clientLock.Lock() defer clientLock.Unlock() - if connection.GoVmomiClient == nil { - connection.GoVmomiClient, err = connection.NewClient(ctx) + if connection.Client == nil { + connection.Client, err = connection.NewClient(ctx) if err != nil { glog.Errorf("Failed to create govmomi client. err: %+v", err) return err } return nil } - m := session.NewManager(connection.GoVmomiClient) + m := session.NewManager(connection.Client) userSession, err := m.UserSession(ctx) if err != nil { glog.Errorf("Error while obtaining user session. err: %+v", err) @@ -70,7 +70,7 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error { } glog.Warningf("Creating new client session since the existing session is not valid or not authenticated") - connection.GoVmomiClient, err = connection.NewClient(ctx) + connection.Client, err = connection.NewClient(ctx) if err != nil { glog.Errorf("Failed to create govmomi client. err: %+v", err) return err @@ -80,7 +80,7 @@ func (connection *VSphereConnection) Connect(ctx context.Context) error { // Logout calls SessionManager.Logout for the given connection. func (connection *VSphereConnection) Logout(ctx context.Context) { - m := session.NewManager(connection.GoVmomiClient) + m := session.NewManager(connection.Client) if err := m.Logout(ctx); err != nil { glog.Errorf("Logout failed: %s", err) } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go index 2c744116e9a..d60448d1ccf 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter.go @@ -39,7 +39,7 @@ type Datacenter struct { // GetDatacenter returns the DataCenter Object for the given datacenterPath // If datacenter is located in a folder, include full path to datacenter else just provide the datacenter name func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacenterPath string) (*Datacenter, error) { - finder := find.NewFinder(connection.GoVmomiClient, false) + finder := find.NewFinder(connection.Client, false) datacenter, err := finder.Datacenter(ctx, datacenterPath) if err != nil { glog.Errorf("Failed to find the datacenter: %s. err: %+v", datacenterPath, err) @@ -52,7 +52,7 @@ func GetDatacenter(ctx context.Context, connection *VSphereConnection, datacente // GetAllDatacenter returns all the DataCenter Objects func GetAllDatacenter(ctx context.Context, connection *VSphereConnection) ([]*Datacenter, error) { var dc []*Datacenter - finder := find.NewFinder(connection.GoVmomiClient, false) + finder := find.NewFinder(connection.Client, false) datacenters, err := finder.DatacenterList(ctx, "*") if err != nil { glog.Errorf("Failed to find the datacenter. err: %+v", err) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go index 65b70a76b24..95c89b70112 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go @@ -47,7 +47,7 @@ func TestDatacenter(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c.Client} + vc := &VSphereConnection{Client: c.Client} _, err = GetDatacenter(ctx, vc, testNameNotFound) if err == nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go index 3ecd99c0a1a..7e9b2b7367b 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go @@ -45,7 +45,7 @@ func TestDatastore(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c.Client} + vc := &VSphereConnection{Client: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go index b83ea7f0da5..a8b39a8671e 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go @@ -47,7 +47,7 @@ func TestFolder(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c.Client} + vc := &VSphereConnection{Client: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go index 786da2f1a33..df4bba3a341 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go @@ -46,7 +46,7 @@ func TestUtils(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c.Client} + vc := &VSphereConnection{Client: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go index 35b7c5a51e9..c366c396146 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go @@ -43,7 +43,7 @@ func TestVirtualMachine(t *testing.T) { t.Fatal(err) } - vc := &VSphereConnection{GoVmomiClient: c.Client} + vc := &VSphereConnection{Client: c.Client} dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) if err != nil { diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index 87a08234ea3..14ecfed732c 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -410,7 +410,7 @@ func newControllerNode(cfg VSphereConfig) (*VSphere, error) { func logout(vs *VSphere) { for _, vsphereIns := range vs.vsphereInstanceMap { - if vsphereIns.conn.GoVmomiClient != nil { + if vsphereIns.conn.Client != nil { vsphereIns.conn.Logout(context.TODO()) } } From e22f9ca4ae7ddac1a035312e371eb60ae319fef3 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Sat, 5 May 2018 17:13:53 -0700 Subject: [PATCH 3/4] vsphere: fallback to vcsim for testing authentication The TestVSphereLogin method still defaults to testing against a real vCenter, but if the required environment variables are not set, it can test against vcsim. More tests can be converted to use configFromEnvOrSim(), but can be in follow up PRs. --- .../providers/vsphere/vclib/constants.go | 5 +- .../vsphere/vclib/datacenter_test.go | 8 +-- .../providers/vsphere/vclib/datastore_test.go | 2 +- .../providers/vsphere/vclib/folder_test.go | 2 +- .../providers/vsphere/vclib/utils_test.go | 2 +- .../vsphere/vclib/virtualmachine_test.go | 2 +- .../providers/vsphere/vsphere.go | 7 ++- .../providers/vsphere/vsphere_test.go | 56 +++++++++++++++++-- 8 files changed, 66 insertions(+), 18 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/constants.go b/pkg/cloudprovider/providers/vsphere/vclib/constants.go index 451d9241180..522f308b8b3 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/constants.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/constants.go @@ -53,7 +53,8 @@ const ( // Test Constants const ( - testDefaultDatacenter = "DC0" - testDefaultDatastore = "LocalDS_0" + TestDefaultDatacenter = "DC0" + TestDefaultDatastore = "LocalDS_0" + TestDefaultNetwork = "VM Network" testNameNotFound = "enoent" ) diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go index 95c89b70112..ad7e13921ae 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datacenter_test.go @@ -54,7 +54,7 @@ func TestDatacenter(t *testing.T) { t.Error("expected error") } - dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter) if err != nil { t.Error(err) } @@ -74,7 +74,7 @@ func TestDatacenter(t *testing.T) { t.Error("expected error") } - vm, err := dc.GetVMByPath(ctx, testDefaultDatacenter+"/vm/"+avm.Name) + vm, err := dc.GetVMByPath(ctx, TestDefaultDatacenter+"/vm/"+avm.Name) if err != nil { t.Error(err) } @@ -103,7 +103,7 @@ func TestDatacenter(t *testing.T) { t.Error("expected error") } - ds, err := dc.GetDatastoreByName(ctx, testDefaultDatastore) + ds, err := dc.GetDatastoreByName(ctx, TestDefaultDatastore) if err != nil { t.Error(err) } @@ -113,7 +113,7 @@ func TestDatacenter(t *testing.T) { t.Error("expected error") } - _, err = dc.GetFolderByPath(ctx, testDefaultDatacenter+"/vm") + _, err = dc.GetFolderByPath(ctx, TestDefaultDatacenter+"/vm") if err != nil { t.Error(err) } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go index 7e9b2b7367b..6c6e888cb3e 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/datastore_test.go @@ -47,7 +47,7 @@ func TestDatastore(t *testing.T) { vc := &VSphereConnection{Client: c.Client} - dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter) if err != nil { t.Error(err) } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go index a8b39a8671e..be570d80dcd 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/folder_test.go @@ -49,7 +49,7 @@ func TestFolder(t *testing.T) { vc := &VSphereConnection{Client: c.Client} - dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter) if err != nil { t.Error(err) } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go index df4bba3a341..436055cf172 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/utils_test.go @@ -48,7 +48,7 @@ func TestUtils(t *testing.T) { vc := &VSphereConnection{Client: c.Client} - dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter) if err != nil { t.Error(err) } diff --git a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go index c366c396146..ca2ef9665e4 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go +++ b/pkg/cloudprovider/providers/vsphere/vclib/virtualmachine_test.go @@ -45,7 +45,7 @@ func TestVirtualMachine(t *testing.T) { vc := &VSphereConnection{Client: c.Client} - dc, err := GetDatacenter(ctx, vc, testDefaultDatacenter) + dc, err := GetDatacenter(ctx, vc, TestDefaultDatacenter) if err != nil { t.Error(err) } diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index 14ecfed732c..c9390077849 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -360,7 +360,10 @@ func populateVsphereInstanceMap(cfg *VSphereConfig) (map[string]*VSphereInstance return vsphereInstanceMap, nil } -// Creates new Contreoller node interface and returns +// getVMUUID allows tests to override GetVMUUID +var getVMUUID = GetVMUUID + +// Creates new Controller node interface and returns func newControllerNode(cfg VSphereConfig) (*VSphere, error) { var err error @@ -399,7 +402,7 @@ func newControllerNode(cfg VSphereConfig) (*VSphere, error) { glog.Errorf("Failed to get hostname. err: %+v", err) return nil, err } - vs.vmUUID, err = GetVMUUID() + vs.vmUUID, err = getVMUUID() if err != nil { glog.Errorf("Failed to get uuid. err: %+v", err) return nil, err diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_test.go index 3e57ba54239..35e1903aea5 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_test.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_test.go @@ -18,12 +18,14 @@ package vsphere import ( "context" + "crypto/tls" "log" "os" "strconv" "strings" "testing" + "github.com/vmware/govmomi/simulator" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/kubernetes/pkg/cloudprovider" @@ -59,6 +61,50 @@ func configFromEnv() (cfg VSphereConfig, ok bool) { return } +// configFromEnvOrSim returns config from configFromEnv if set, +// otherwise starts a vcsim instance and returns config for use against the vcsim instance. +func configFromEnvOrSim() (VSphereConfig, func()) { + cfg, ok := configFromEnv() + if ok { + return cfg, func() {} + } + + model := simulator.VPX() + + err := model.Create() + if err != nil { + log.Fatal(err) + } + + model.Service.TLS = new(tls.Config) + s := model.Service.NewServer() + + cfg.Global.InsecureFlag = true + cfg.Global.VCenterIP = s.URL.Hostname() + cfg.Global.VCenterPort = s.URL.Port() + cfg.Global.User = s.URL.User.Username() + cfg.Global.Password, _ = s.URL.User.Password() + cfg.Global.Datacenter = vclib.TestDefaultDatacenter + cfg.Network.PublicNetwork = vclib.TestDefaultNetwork + cfg.Global.DefaultDatastore = vclib.TestDefaultDatastore + cfg.Disk.SCSIControllerType = os.Getenv("VSPHERE_SCSICONTROLLER_TYPE") + cfg.Global.WorkingDir = os.Getenv("VSPHERE_WORKING_DIR") + cfg.Global.VMName = os.Getenv("VSPHERE_VM_NAME") + + if cfg.Global.WorkingDir == "" { + cfg.Global.WorkingDir = "vm" // top-level Datacenter.VmFolder + } + + uuid := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine).Config.Uuid + getVMUUID = func() (string, error) { return uuid, nil } + + return cfg, func() { + getVMUUID = GetVMUUID + s.Close() + model.Remove() + } +} + func TestReadConfig(t *testing.T) { _, err := readConfig(nil) if err == nil { @@ -110,10 +156,8 @@ func TestNewVSphere(t *testing.T) { } func TestVSphereLogin(t *testing.T) { - cfg, ok := configFromEnv() - if !ok { - t.Skipf("No config found in environment") - } + cfg, cleanup := configFromEnvOrSim() + defer cleanup() // Create vSphere configuration object vs, err := newControllerNode(cfg) @@ -126,8 +170,8 @@ func TestVSphereLogin(t *testing.T) { defer cancel() // Create vSphere client - var vcInstance *VSphereInstance - if vcInstance, ok = vs.vsphereInstanceMap[cfg.Global.VCenterIP]; !ok { + vcInstance, ok := vs.vsphereInstanceMap[cfg.Global.VCenterIP] + if !ok { t.Fatalf("Couldn't get vSphere instance: %s", cfg.Global.VCenterIP) } From cc1552c072a052b8adc684bd133035225422a2c0 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Mon, 7 May 2018 07:30:39 -0700 Subject: [PATCH 4/4] vsphere: update bazel --- pkg/cloudprovider/providers/vsphere/BUILD | 1 + pkg/cloudprovider/providers/vsphere/vclib/BUILD | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/vsphere/BUILD b/pkg/cloudprovider/providers/vsphere/BUILD index 0566d806411..ef7c3435c11 100644 --- a/pkg/cloudprovider/providers/vsphere/BUILD +++ b/pkg/cloudprovider/providers/vsphere/BUILD @@ -39,6 +39,7 @@ go_test( deps = [ "//pkg/cloudprovider:go_default_library", "//pkg/cloudprovider/providers/vsphere/vclib:go_default_library", + "//vendor/github.com/vmware/govmomi/simulator:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library", ], diff --git a/pkg/cloudprovider/providers/vsphere/vclib/BUILD b/pkg/cloudprovider/providers/vsphere/vclib/BUILD index ca4eb530d7d..28efd2b3818 100644 --- a/pkg/cloudprovider/providers/vsphere/vclib/BUILD +++ b/pkg/cloudprovider/providers/vsphere/vclib/BUILD @@ -26,7 +26,6 @@ go_library( deps = [ "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", - "//vendor/github.com/vmware/govmomi:go_default_library", "//vendor/github.com/vmware/govmomi/find:go_default_library", "//vendor/github.com/vmware/govmomi/object:go_default_library", "//vendor/github.com/vmware/govmomi/pbm:go_default_library",