From fda70d220b4a4c007bad73b2c1f1cb2943512bf3 Mon Sep 17 00:00:00 2001 From: Davide Agnello Date: Wed, 21 Sep 2016 14:49:08 -0700 Subject: [PATCH] ExternalID/InstanceID not returning appropriate error for missing VM Addresses #33215. When vCenter returns error vm not found, this is now being translated to the appropriate error 'cloudprovider.InstanceNotFound' which indicates to Kubernetes node controller that the VM is in fact not found. --- .../providers/vsphere/vsphere.go | 9 +++++++++ .../providers/vsphere/vsphere_test.go | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index e5811fdffc7..09da52cdd42 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -495,6 +495,9 @@ func (i *Instances) ExternalID(name string) (string, error) { vm, err := getVirtualMachineByName(i.cfg, ctx, c, name) if err != nil { + if _, ok := err.(*find.NotFoundError); ok { + return "", cloudprovider.InstanceNotFound + } return "", err } @@ -531,6 +534,12 @@ func (i *Instances) InstanceID(name string) (string, error) { defer c.Logout(ctx) vm, err := getVirtualMachineByName(i.cfg, ctx, c, name) + if err != nil { + if _, ok := err.(*find.NotFoundError); ok { + return "", cloudprovider.InstanceNotFound + } + return "", err + } var mvm mo.VirtualMachine err = getVirtualMachineManagedObjectReference(ctx, c, vm, "summary", &mvm) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere_test.go b/pkg/cloudprovider/providers/vsphere/vsphere_test.go index 1bf98e709bb..5bdded9c75f 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere_test.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere_test.go @@ -24,6 +24,7 @@ import ( "testing" "golang.org/x/net/context" + "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/util/rand" ) @@ -185,12 +186,31 @@ func TestInstances(t *testing.T) { } t.Logf("Found ExternalID(%s) = %s\n", srvs[0], externalId) + nonExistingVM := rand.String(15) + externalId, err = i.ExternalID(nonExistingVM) + if err == cloudprovider.InstanceNotFound { + t.Logf("VM %s was not found as expected\n", nonExistingVM) + } else if err == nil { + t.Fatalf("Instances.ExternalID did not fail as expected, VM %s was found", nonExistingVM) + } else { + t.Fatalf("Instances.ExternalID did not fail as expected, err: %v", err) + } + instanceId, err := i.InstanceID(srvs[0]) if err != nil { t.Fatalf("Instances.InstanceID(%s) failed: %s", srvs[0], err) } t.Logf("Found InstanceID(%s) = %s\n", srvs[0], instanceId) + instanceId, err = i.InstanceID(nonExistingVM) + if err == cloudprovider.InstanceNotFound { + t.Logf("VM %s was not found as expected\n", nonExistingVM) + } else if err == nil { + t.Fatalf("Instances.InstanceID did not fail as expected, VM %s was found", nonExistingVM) + } else { + t.Fatalf("Instances.InstanceID did not fail as expected, err: %v", err) + } + addrs, err := i.NodeAddresses(srvs[0]) if err != nil { t.Fatalf("Instances.NodeAddresses(%s) failed: %s", srvs[0], err)