From 3a9b6bd50aa90ad7a23ca7969c78b16c5278e79f Mon Sep 17 00:00:00 2001 From: lubronzhan Date: Mon, 29 Jun 2020 21:22:15 +0800 Subject: [PATCH 1/3] Cherry pick the fix https://github.com/kubernetes/kubernetes/pull/70291 --- .../vsphere/vclib/virtualmachine.go | 23 ++++++++++++++++ .../vsphere/vclib/virtualmachine_test.go | 26 +++++++++++++++++++ .../legacy-cloud-providers/vsphere/vsphere.go | 8 +++--- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go index 4837b7db373..03e60df1477 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go @@ -214,6 +214,29 @@ func (vm *VirtualMachine) IsActive(ctx context.Context) (bool, error) { return false, nil } +// Exists checks if VM exists and is not terminated +func (vm *VirtualMachine) Exists(ctx context.Context) (bool, error) { + vmMoList, err := vm.Datacenter.GetVMMoList(ctx, []*VirtualMachine{vm}, []string{"summary.runtime.powerState"}) + if err != nil { + klog.Errorf("Failed to get VM Managed object with property summary. err: +%v", err) + return false, err + } + // We check for VMs which are still available in vcenter and has not been terminated/removed from + // disk and hence we consider PoweredOn,PoweredOff and Suspended as alive states. + aliveStates := []types.VirtualMachinePowerState{ + types.VirtualMachinePowerStatePoweredOff, + types.VirtualMachinePowerStatePoweredOn, + types.VirtualMachinePowerStateSuspended, + } + currentState := vmMoList[0].Summary.Runtime.PowerState + for _, state := range aliveStates { + if state == currentState { + return true, nil + } + } + return false, nil +} + // GetAllAccessibleDatastores gets the list of accessible Datastores for the given Virtual Machine func (vm *VirtualMachine) GetAllAccessibleDatastores(ctx context.Context) ([]*DatastoreInfo, error) { host, err := vm.HostSystem(ctx) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine_test.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine_test.go index ca2ef9665e4..e92514d8a4c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine_test.go @@ -114,6 +114,26 @@ func TestVirtualMachine(t *testing.T) { } } + for _, turnOff := range []bool{true, false} { + // Turn off for checking if exist return true + if turnOff { + _, _ = vm.PowerOff(ctx) + } + + exist, err := vm.Exists(ctx) + if err != nil { + t.Error(err) + } + if !exist { + t.Errorf("exist=%t, expected=%t", exist, true) + } + + // Turn back on + if turnOff { + _, _ = vm.PowerOn(ctx) + } + } + for _, expect := range []bool{true, false} { active, err := vm.IsActive(ctx) if err != nil { @@ -140,5 +160,11 @@ func TestVirtualMachine(t *testing.T) { t.Error(err) } } + + // Expecting Exists func to throw error if VM deleted + _, err = vm.Exists(ctx) + if err == nil { + t.Error("expected error") + } } } diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 2ef2a30833c..0f00a8fa32a 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -857,14 +857,16 @@ func (vs *VSphere) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) ( klog.Errorf("Failed to get VM object for node: %q. err: +%v", convertToString(nodeName), err) return "", err } - isActive, err := vm.IsActive(ctx) + + exists, err := vm.Exists(ctx) if err != nil { - klog.Errorf("Failed to check whether node %q is active. err: %+v.", convertToString(nodeName), err) + klog.Errorf("Failed to check whether node %q still exists. err: %+v.", convertToString(nodeName), err) return "", err } - if isActive { + if exists { return vs.vmUUID, nil } + klog.Warningf("The VM: %s is not in %s state", convertToString(nodeName), vclib.ActivePowerState) return "", cloudprovider.InstanceNotFound } From 1ebea3ad9d70e527d6781ae1cda9fb17b7a99e0d Mon Sep 17 00:00:00 2001 From: lubronzhan Date: Mon, 29 Jun 2020 23:33:39 +0800 Subject: [PATCH 2/3] Return nil as error when instance is not found so that node_controller could delete the node --- .../src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 0f00a8fa32a..3031442d117 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -756,11 +756,14 @@ func (vs *VSphere) InstanceExistsByProviderID(ctx context.Context, providerID st return false, err } _, err = vs.InstanceID(ctx, convertToK8sType(nodeName)) - if err == nil { - return true, nil + if err != nil { + if err == cloudprovider.InstanceNotFound { + return false, nil + } + return false, err } - return false, err + return true, nil } // InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes From 566c0d735a377f827f370c54035de3945e3e2bf3 Mon Sep 17 00:00:00 2001 From: lubronzhan Date: Mon, 17 Aug 2020 12:50:42 +0800 Subject: [PATCH 3/3] Fix the logging message --- staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 3031442d117..076e8c7432e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -870,7 +870,7 @@ func (vs *VSphere) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) ( return vs.vmUUID, nil } - klog.Warningf("The VM: %s is not in %s state", convertToString(nodeName), vclib.ActivePowerState) + klog.Warningf("The VM: %s doesn't exist", convertToString(nodeName)) return "", cloudprovider.InstanceNotFound }