Skip to content

Commit 35c9c24

Browse files
author
Pushkar Acharya
committed
Handle the case of missing instances when checking for shutdown instances
1 parent 5c3cadf commit 35c9c24

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed

Diff for: internal/instances/instances.go

+18
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ var ErrAssertTimeTypeFailed = errors.New("failed to assert type time.Time for fi
2525

2626
type Instances struct {
2727
nodeFirstSeen sync.Map
28+
nodeShutdown sync.Map
2829
apiClient client.APIClient
2930
}
3031

@@ -100,6 +101,23 @@ func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID
100101
defer responseBody.Body.Close()
101102
}
102103
if err != nil {
104+
if err == client.ErrInstanceNotFound {
105+
attempt, ok := i.nodeShutdown.Load(providerID)
106+
if !ok {
107+
attempt = 1
108+
i.nodeShutdown.Store(providerID, attempt)
109+
110+
return false, err
111+
}
112+
if attempt.(int) >= 3 {
113+
114+
return true, nil
115+
}
116+
intAttempt := attempt.(int)
117+
i.nodeShutdown.Store(providerID, (intAttempt + 1))
118+
119+
return false, err
120+
}
103121
return false, fmt.Errorf("failed to get instance by provider ID %s: %w", providerID, err)
104122
}
105123
if currInstance == nil || currInstance.State == "STATE_SHUTOFF" || currInstance.State == "STATE_SHUTDOWN" {

Diff for: internal/instances/instances_test.go

+32
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
v1alpha5 "github.com/crusoecloud/client-go/swagger/v1alpha5"
10+
"github.com/crusoecloud/crusoe-cloud-controller-manager/internal/client"
1011
mock_client "github.com/crusoecloud/crusoe-cloud-controller-manager/internal/client/mock"
1112
"github.com/crusoecloud/crusoe-cloud-controller-manager/internal/instances"
1213
"github.com/golang/mock/gomock"
@@ -109,6 +110,37 @@ func TestInstanceShutdownByProviderID(t *testing.T) {
109110
require.True(t, shutdown)
110111
}
111112

113+
func TestInstanceShutdownByProviderIDInstanceMissingFromCloudProvider(t *testing.T) {
114+
t.Parallel()
115+
ctrl := gomock.NewController(t)
116+
defer ctrl.Finish()
117+
118+
mockClient := mock_client.NewMockApiClient(ctrl)
119+
mockClient.EXPECT().GetInstanceByID(gomock.Any(), TESTInstanceID).Return(nil, nil, client.ErrInstanceNotFound).AnyTimes()
120+
instanceService := instances.NewCrusoeInstances(mockClient)
121+
122+
// Instance shutdown by ID should return true on third attempt
123+
// attempt #1
124+
shutdown, err := instanceService.InstanceShutdownByProviderID(context.Background(), ProviderIDPrefix+TESTInstanceID)
125+
require.ErrorIs(t, err, client.ErrInstanceNotFound)
126+
require.False(t, shutdown)
127+
128+
// attempt #2
129+
shutdown, err = instanceService.InstanceShutdownByProviderID(context.Background(), ProviderIDPrefix+TESTInstanceID)
130+
require.ErrorIs(t, err, client.ErrInstanceNotFound)
131+
require.False(t, shutdown)
132+
133+
// attempt #3
134+
shutdown, err = instanceService.InstanceShutdownByProviderID(context.Background(), ProviderIDPrefix+TESTInstanceID)
135+
require.ErrorIs(t, err, client.ErrInstanceNotFound)
136+
require.False(t, shutdown)
137+
138+
// attempt #4
139+
shutdown, err = instanceService.InstanceShutdownByProviderID(context.Background(), ProviderIDPrefix+TESTInstanceID)
140+
require.NoError(t, err)
141+
require.True(t, shutdown)
142+
}
143+
112144
func TestInstanceMetadata(t *testing.T) {
113145
t.Parallel()
114146
ctrl := gomock.NewController(t)

0 commit comments

Comments
 (0)