From 3528ceb27fddab8b1aae1cb471feda70506b1074 Mon Sep 17 00:00:00 2001 From: Josh Horwitz Date: Fri, 25 Aug 2017 16:15:55 -0400 Subject: [PATCH] address test & doc comments --- pkg/cloudprovider/providers/aws/aws.go | 1 + pkg/cloudprovider/providers/azure/azure_instances.go | 1 + pkg/cloudprovider/providers/fake/fake.go | 1 + pkg/cloudprovider/providers/gce/gce_instances.go | 1 + .../providers/openstack/openstack_instances.go | 1 + pkg/cloudprovider/providers/ovirt/ovirt.go | 1 + pkg/cloudprovider/providers/photon/photon.go | 1 + pkg/cloudprovider/providers/rackspace/rackspace.go | 1 + pkg/cloudprovider/providers/vsphere/vsphere.go | 1 + pkg/controller/cloud/node_controller_test.go | 10 +++++----- 10 files changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 18fbbc3b453..a2132f531a3 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -1102,6 +1102,7 @@ func (c *Cloud) ExternalID(nodeName types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (c *Cloud) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/azure/azure_instances.go b/pkg/cloudprovider/providers/azure/azure_instances.go index e0fb6d0753f..b2d5a9785c4 100644 --- a/pkg/cloudprovider/providers/azure/azure_instances.go +++ b/pkg/cloudprovider/providers/azure/azure_instances.go @@ -88,6 +88,7 @@ func (az *Cloud) ExternalID(name types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (az *Cloud) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/fake/fake.go b/pkg/cloudprovider/providers/fake/fake.go index 633dbba8ae7..78deba48268 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -239,6 +239,7 @@ func (f *FakeCloud) InstanceTypeByProviderID(providerID string) (string, error) } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (f *FakeCloud) InstanceExistsByProviderID(providerID string) (bool, error) { f.addCall("instance-exists-by-provider-id") return f.ExistsByProviderID, f.ErrByProviderID diff --git a/pkg/cloudprovider/providers/gce/gce_instances.go b/pkg/cloudprovider/providers/gce/gce_instances.go index 25f3728bef2..0687dfa5279 100644 --- a/pkg/cloudprovider/providers/gce/gce_instances.go +++ b/pkg/cloudprovider/providers/gce/gce_instances.go @@ -153,6 +153,7 @@ func (gce *GCECloud) ExternalID(nodeName types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (gce *GCECloud) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/openstack/openstack_instances.go b/pkg/cloudprovider/providers/openstack/openstack_instances.go index d8f96061f75..6f817637940 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_instances.go +++ b/pkg/cloudprovider/providers/openstack/openstack_instances.go @@ -111,6 +111,7 @@ func (i *Instances) ExternalID(name types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (i *Instances) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/ovirt/ovirt.go b/pkg/cloudprovider/providers/ovirt/ovirt.go index 38c22e0f1b0..96eed8d9a29 100644 --- a/pkg/cloudprovider/providers/ovirt/ovirt.go +++ b/pkg/cloudprovider/providers/ovirt/ovirt.go @@ -212,6 +212,7 @@ func (v *OVirtCloud) ExternalID(nodeName types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (v *OVirtCloud) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/photon/photon.go b/pkg/cloudprovider/providers/photon/photon.go index 37466bd3468..9612f1b463d 100644 --- a/pkg/cloudprovider/providers/photon/photon.go +++ b/pkg/cloudprovider/providers/photon/photon.go @@ -471,6 +471,7 @@ func (pc *PCCloud) ExternalID(nodeName k8stypes.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (pc *PCCloud) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/rackspace/rackspace.go b/pkg/cloudprovider/providers/rackspace/rackspace.go index 61127aea0e2..e5a84cbc831 100644 --- a/pkg/cloudprovider/providers/rackspace/rackspace.go +++ b/pkg/cloudprovider/providers/rackspace/rackspace.go @@ -437,6 +437,7 @@ func (i *Instances) ExternalID(nodeName types.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (i *Instances) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index 58e1a3020ec..2407d569331 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -377,6 +377,7 @@ func (vs *VSphere) ExternalID(nodeName k8stypes.NodeName) (string, error) { } // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running. +// If false is returned with no error, the instance will be immediately deleted. func (vs *VSphere) InstanceExistsByProviderID(providerID string) (bool, error) { return false, errors.New("unimplemented") } diff --git a/pkg/controller/cloud/node_controller_test.go b/pkg/controller/cloud/node_controller_test.go index 49e956e93fd..9cdebd140d1 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -130,23 +130,23 @@ func TestEnsureNodeExistsByProviderIDOrNodeName(t *testing.T) { instances, _ := fc.Instances() exists, err := ensureNodeExistsByProviderIDOrName(instances, tc.node) if err != nil { - t.Fatal(err) + t.Error(err) } if !reflect.DeepEqual(fc.Calls, tc.expectedCalls) { - t.Fatalf("expected cloud provider methods `%v` to be called but `%v` was called ", tc.expectedCalls, fc.Calls) + t.Errorf("expected cloud provider methods `%v` to be called but `%v` was called ", tc.expectedCalls, fc.Calls) } if tc.existsByProviderID && tc.existsByProviderID != exists { - t.Fatalf("expected exist by provider id to be `%t` but got `%t`", tc.existsByProviderID, exists) + t.Errorf("expected exist by provider id to be `%t` but got `%t`", tc.existsByProviderID, exists) } if tc.existsByNodeName && tc.existsByNodeName != exists { - t.Fatalf("expected exist by node name to be `%t` but got `%t`", tc.existsByNodeName, exists) + t.Errorf("expected exist by node name to be `%t` but got `%t`", tc.existsByNodeName, exists) } if !tc.existsByNodeName && !tc.existsByProviderID && exists { - t.Fatal("node is not supposed to exist") + t.Error("node is not supposed to exist") } })