diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index 6b299b0b27cb..e4e959411a21 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -122,25 +122,7 @@ func (gce *GceCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N // HasInstance returns whether a given node has a corresponding instance in this cloud provider func (gce *GceCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { - ref, err := GceRefFromProviderId(node.Spec.ProviderID) - if err != nil { - return false, err - } - - mig, err := gce.gceManager.GetMigForInstance(ref) - if err != nil { - return false, err - } - - // Not managed by any registered MIG; there's not enough information to confidently say if this instance exists, - // so err on the side of safety and return an error. ErrNotImplemented is returned (vs. a generic error) to - // avoid repeated warning logging in clusterstate. - if mig == nil { - return true, cloudprovider.ErrNotImplemented - } - - // Instance belongs to a managed MIG, so it's known to be backed by a live instance. - return true, nil + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go index 3917d260539c..3729cadfacab 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go @@ -60,11 +60,7 @@ func (m *gceManagerMock) DeleteInstances(instances []GceRef) error { func (m *gceManagerMock) GetMigForInstance(instance GceRef) (Mig, error) { args := m.Called(instance) - v := args.Get(0) - if v == nil { - return nil, args.Error(1) - } - return v.(*gceMig), args.Error(1) + return args.Get(0).(*gceMig), args.Error(1) } func (m *gceManagerMock) GetMigNodes(mig Mig) ([]GceInstance, error) { @@ -469,86 +465,6 @@ func TestGceRefFromProviderId(t *testing.T) { assert.Equal(t, GceRef{"project1", "us-central1-b", "name1"}, ref) } -func TestHasInstance(t *testing.T) { - migRef := GceRef{Project: "project1", Zone: "us-central1-b", Name: "mig-0"} - mig := &gceMig{gceRef: migRef} - - nodeRef := GceRef{Project: "project1", Zone: "us-central1-b", Name: "mig-0-0001"} - nodeProviderID := "gce://project1/us-central1-b/mig-0-0001" - - for _, tc := range []struct { - name string - node func() *apiv1.Node - setupMocks func(*gceManagerMock) - expectExists bool - expectErr bool - }{ - { - name: "invalid provider ID returns error", - node: func() *apiv1.Node { - n := BuildTestNode("node1", 1000, 1000) - n.Spec.ProviderID = "not-a-valid-gce-provider-id" - return n - }, - setupMocks: func(m *gceManagerMock) {}, - expectErr: true, - }, - { - name: "failure to get mig for instance returns error", - node: func() *apiv1.Node { - n := BuildTestNode("mig-0-0001", 1000, 1000) - n.Spec.ProviderID = nodeProviderID - return n - }, - setupMocks: func(m *gceManagerMock) { - m.On("GetMigForInstance", nodeRef).Return(nil, fmt.Errorf("lookup error")).Once() - }, - expectErr: true, - }, - { - // Instance belongs to either an unregistered MIG, or one that is not in scope of the provider - // (e.g., a node from a non-auto-discovered MIG). - name: "instance in unregistered MIG returns error", - node: func() *apiv1.Node { - n := BuildTestNode("mig-x-0001", 1000, 1000) - n.Spec.ProviderID = nodeProviderID - return n - }, - setupMocks: func(m *gceManagerMock) { - m.On("GetMigForInstance", nodeRef).Return(nil, nil).Once() - }, - expectErr: true, - }, - { - name: "instance in registered MIG returns true", - node: func() *apiv1.Node { - n := BuildTestNode("mig-0-0001", 1000, 1000) - n.Spec.ProviderID = nodeProviderID - return n - }, - setupMocks: func(m *gceManagerMock) { - m.On("GetMigForInstance", nodeRef).Return(mig, nil).Once() - }, - expectExists: true, - }, - } { - t.Run(tc.name, func(t *testing.T) { - gceManagerMock := &gceManagerMock{} - tc.setupMocks(gceManagerMock) - defer mock.AssertExpectationsForObjects(t, gceManagerMock) - gce := &GceCloudProvider{gceManager: gceManagerMock} - - exists, err := gce.HasInstance(tc.node()) - if tc.expectErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tc.expectExists, exists) - } - }) - } -} - func createString(s string) *string { return &s }