Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
86 changes: 1 addition & 85 deletions cluster-autoscaler/cloudprovider/gce/gce_cloud_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}