From fd5c08ba523b21f34c1ce4fc83155b81262dbf98 Mon Sep 17 00:00:00 2001 From: Robert Roland Date: Thu, 2 Feb 2017 08:48:05 -0800 Subject: [PATCH 1/2] Fixes #40819 Start looking up the virtual machine by it's UUID in vSphere again. Looking up by IP address is problematic and can either not return a VM entirely, or could return the wrong VM. Retrieves the VM's UUID in one of two methods - either by a `vm-uuid` entry in the cloud config file on the VM, or via sysfs. The sysfs route requires root access, but restores the previous functionality. Multiple VMs in a vCenter cluster can share an IP address - for example, if you have multiple VM networks, but they're all isolated and use the same address range. Additionally, flannel network address ranges can overlap. vSphere seems to have a limitation of reporting no more than 16 interfaces from a virtual machine, so it's possible that the IP address list on a VM is completely untrustworthy anyhow - it can either be empty (because the 16 interfaces it found were veth interfaces with no IP address), or it can report the flannel IP. --- .../providers/vsphere/vsphere.go | 51 ++++++++++--------- .../providers/vsphere/vsphere_test.go | 5 ++ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index e6324c14cd6f..5eff4521bd06 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "io" - "net" "net/url" "path" "path/filepath" @@ -97,12 +96,17 @@ type VSphereConfig struct { Datastore string `gcfg:"datastore"` // WorkingDir is path where VMs can be found. WorkingDir string `gcfg:"working-dir"` + // Soap round tripper count (retries = RoundTripper - 1) + RoundTripperCount uint `gcfg:"soap-roundtrip-count"` + // VMUUID is the virtual machine's UUID. If not set, will be fetched from the machine. + VMUUID string `gcfg:"vm-uuid"` } Network struct { // PublicNetwork is name of the network the VMs are joined to. PublicNetwork string `gcfg:"public-network"` } + Disk struct { // SCSIControllerType defines SCSI controller to be used. SCSIControllerType string `dcfg:"scsicontrollertype"` @@ -158,14 +162,26 @@ func init() { // Returns the name of the VM on which this code is running. // Prerequisite: this code assumes VMWare vmtools or open-vm-tools to be installed in the VM. -func getVMName(cfg *VSphereConfig) (string, error) { - addrs, err := net.InterfaceAddrs() - if err != nil { - return "", err +// Will attempt to determine the machine's name via it's UUID in this precedence order, failing if neither have a UUID: +// * cloud config value VMUUID +// * sysfs entry +func getVMName(client *govmomi.Client, cfg *VSphereConfig) (string, error) { + var vmUUID string + + if cfg.Global.VMUUID != "" { + vmUUID = cfg.Global.VMUUID + } else { + vmUUIDbytes, err := ioutil.ReadFile("/sys/devices/virtual/dmi/id/product_uuid") + if err != nil { + return "", err + } + + vmUUID = string(vmUUIDbytes) + cfg.Global.VMUUID = vmUUID } - if len(addrs) == 0 { - return "", fmt.Errorf("unable to retrieve Instance ID") + if vmUUID == "" { + return "", fmt.Errorf("unable to determine machine ID from cloud configuration or sysfs") } // Create context @@ -191,28 +207,17 @@ func getVMName(cfg *VSphereConfig) (string, error) { s := object.NewSearchIndex(c.Client) - var svm object.Reference - for _, v := range addrs { - ip, _, err := net.ParseCIDR(v.String()) - if err != nil { - return "", fmt.Errorf("unable to parse cidr from ip") - } - - // Finds a virtual machine or host by IP address. - svm, err = s.FindByIp(ctx, dc, ip.String(), true) - if err == nil && svm != nil { - break - } - } - if svm == nil { - return "", fmt.Errorf("unable to retrieve vm reference from vSphere") + svm, err := s.FindByUuid(ctx, dc, strings.ToLower(strings.TrimSpace(vmUUID)), true, nil) + if err != nil { + return "", err } var vm mo.VirtualMachine - err = s.Properties(ctx, svm.Reference(), []string{"name", "resourcePool"}, &vm) + err = s.Properties(ctx, svm.Reference(), []string{"name"}, &vm) if err != nil { return "", err } + return vm.Name, nil } diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_test.go index 9d6a88aa1f8e..5a709dc017fd 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_test.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_test.go @@ -69,6 +69,7 @@ user = user password = password insecure-flag = true datacenter = us-west +vm-uuid = 1234 `)) if err != nil { t.Fatalf("Should succeed when a valid config is provided: %s", err) @@ -85,6 +86,10 @@ datacenter = us-west if cfg.Global.Datacenter != "us-west" { t.Errorf("incorrect datacenter: %s", cfg.Global.Datacenter) } + + if cfg.Global.VMUUID != "1234" { + t.Errorf("incorrect vm-uuid: %s", cfg.Global.VMUUID) + } } func TestNewVSphere(t *testing.T) { From cf00ca6a4ff9ae93a239fcf572a72119ffa772bb Mon Sep 17 00:00:00 2001 From: Robert Roland Date: Fri, 3 Feb 2017 11:19:01 -0800 Subject: [PATCH 2/2] :Addressing code review feedback --- pkg/cloudprovider/providers/vsphere/vsphere.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index 5eff4521bd06..bcc72792fe1a 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "net/url" "path" "path/filepath" @@ -96,9 +97,9 @@ type VSphereConfig struct { Datastore string `gcfg:"datastore"` // WorkingDir is path where VMs can be found. WorkingDir string `gcfg:"working-dir"` - // Soap round tripper count (retries = RoundTripper - 1) - RoundTripperCount uint `gcfg:"soap-roundtrip-count"` - // VMUUID is the virtual machine's UUID. If not set, will be fetched from the machine. + // VMUUID is the VM Instance UUID of virtual machine which can be retrieved from instanceUuid + // property in VmConfigInfo, or also set as vc.uuid in VMX file. + // If not set, will be fetched from the machine via sysfs (requires root) VMUUID string `gcfg:"vm-uuid"` } @@ -165,12 +166,13 @@ func init() { // Will attempt to determine the machine's name via it's UUID in this precedence order, failing if neither have a UUID: // * cloud config value VMUUID // * sysfs entry -func getVMName(client *govmomi.Client, cfg *VSphereConfig) (string, error) { +func getVMName(cfg *VSphereConfig) (string, error) { var vmUUID string if cfg.Global.VMUUID != "" { vmUUID = cfg.Global.VMUUID } else { + // This needs root privileges on the host, and will fail otherwise. vmUUIDbytes, err := ioutil.ReadFile("/sys/devices/virtual/dmi/id/product_uuid") if err != nil { return "", err