Skip to content

Commit e846994

Browse files
jharrodjharrod
authored
add getSvmState timeout
Add a 20sec timeout for GetSVMState calls.
1 parent 8d8892e commit e846994

12 files changed

+60
-29
lines changed

‎storage_drivers/ontap/api/azgo/common.go‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,17 @@ func (o *ZapiRunner) GetOntapApiVersion() string {
171171

172172
// SendZapi sends the provided ZAPIRequest to the Ontap system
173173
func (o *ZapiRunner) SendZapi(r ZAPIRequest) (*http.Response, error) {
174+
return o.SendZapiWithContext(context.Background(), r)
175+
}
176+
177+
// SendZapiWithContext sends the provided ZAPIRequest to the Ontap system using the provided context
178+
func (o *ZapiRunner) SendZapiWithContext(ctx context.Context, r ZAPIRequest) (*http.Response, error) {
174179
startTime := time.Now()
175180

176181
if o.DebugTraceFlags["method"] {
177-
fields := log.Fields{"Method": "SendZapi", "Type": "ZapiRunner"}
178-
log.WithFields(fields).Debug(">>>> SendZapi")
179-
defer log.WithFields(fields).Debug("<<<< SendZapi")
182+
fields := log.Fields{"Method": "SendZapiWithContext", "Type": "ZapiRunner"}
183+
log.WithFields(fields).Debug(">>>> SendZapiWithContext")
184+
defer log.WithFields(fields).Debug("<<<< SendZapiWithContext")
180185
}
181186

182187
zapiCommand, err := r.ToXML()
@@ -226,8 +231,12 @@ func (o *ZapiRunner) SendZapi(r ZAPIRequest) (*http.Response, error) {
226231
}
227232

228233
b := []byte(s)
229-
ctx, cancel := context.WithTimeout(context.Background(), tridentconfig.StorageAPITimeout)
234+
235+
// Add default timeout
236+
var cancel context.CancelFunc
237+
ctx, cancel = context.WithTimeout(ctx, tridentconfig.StorageAPITimeout)
230238
defer cancel()
239+
231240
req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(b))
232241
if err != nil {
233242
return nil, err
@@ -254,6 +263,11 @@ func (o *ZapiRunner) SendZapi(r ZAPIRequest) (*http.Response, error) {
254263

255264
// ExecuteUsing converts this object to a ZAPI XML representation and uses the supplied ZapiRunner to send to a filer
256265
func (o *ZapiRunner) ExecuteUsing(z ZAPIRequest, requestType string, v interface{}) (interface{}, error) {
266+
return o.ExecuteUsingWithContext(context.Background(), z, requestType, v)
267+
}
268+
269+
// ExecuteUsingWithContext converts this object to a ZAPI XML representation and uses the supplied ZapiRunner to send to a filer with context support
270+
func (o *ZapiRunner) ExecuteUsingWithContext(ctx context.Context, z ZAPIRequest, requestType string, v interface{}) (interface{}, error) {
257271
// Copy the v interface, in case we need a clean version for a retry
258272
o.m.RLock()
259273
var vCopy interface{}
@@ -266,7 +280,7 @@ func (o *ZapiRunner) ExecuteUsing(z ZAPIRequest, requestType string, v interface
266280
svm := o.svm
267281

268282
// Try API call as-is first
269-
response, err := o.executeWithoutIteration(z, requestType, v)
283+
response, err := o.executeWithoutIterationWithContext(ctx, z, requestType, v)
270284
o.m.RUnlock()
271285
if err != nil {
272286
// Always return an error if the call itself failed
@@ -289,18 +303,23 @@ func (o *ZapiRunner) ExecuteUsing(z ZAPIRequest, requestType string, v interface
289303
o.svm = fmt.Sprintf("%s-mc", svm)
290304
}
291305

292-
return o.executeWithoutIteration(z, requestType, vCopy)
306+
return o.executeWithoutIterationWithContext(ctx, z, requestType, vCopy)
293307
}
294308

295309
// executeWithoutIteration does not attempt to perform any nextTag style iteration
296310
func (o *ZapiRunner) executeWithoutIteration(z ZAPIRequest, requestType string, v interface{}) (interface{}, error) {
311+
return o.executeWithoutIterationWithContext(context.Background(), z, requestType, v)
312+
}
313+
314+
// executeWithoutIterationWithContext does not attempt to perform any nextTag style iteration, with context support
315+
func (o *ZapiRunner) executeWithoutIterationWithContext(ctx context.Context, z ZAPIRequest, requestType string, v interface{}) (interface{}, error) {
297316
if o.DebugTraceFlags["method"] {
298-
fields := log.Fields{"Method": "ExecuteUsing", "Type": requestType}
299-
log.WithFields(fields).Debug(">>>> ExecuteUsing")
300-
defer log.WithFields(fields).Debug("<<<< ExecuteUsing")
317+
fields := log.Fields{"Method": "executeWithoutIterationWithContext", "Type": requestType}
318+
log.WithFields(fields).Debug(">>>> executeWithoutIterationWithContext")
319+
defer log.WithFields(fields).Debug("<<<< executeWithoutIterationWithContext")
301320
}
302321

303-
resp, err := o.SendZapi(z)
322+
resp, err := o.SendZapiWithContext(ctx, z)
304323
if err != nil {
305324
log.Errorf("API invocation failed. %v", err.Error())
306325
return nil, err

‎storage_drivers/ontap/api/ontap_zapi.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,14 +2404,21 @@ func (c Client) GetSVMState(ctx context.Context) (string, error) {
24042404
info := azgo.NewVserverInfoType().SetVserverName(c.SVMName())
24052405
query.SetVserverInfo(*info)
24062406

2407-
response, err := azgo.NewVserverGetIterRequest().
2407+
request := azgo.NewVserverGetIterRequest().
24082408
SetMaxRecords(DefaultZapiRecords).
2409-
SetQuery(*query).
2410-
ExecuteUsing(c.zr)
2409+
SetQuery(*query)
2410+
2411+
// Use context-aware execution to respect timeout
2412+
result, err := c.zr.ExecuteUsingWithContext(ctx, request, "VserverGetIterRequest", azgo.NewVserverGetIterResponse())
24112413
if err != nil {
24122414
return "", err
24132415
}
24142416

2417+
response, ok := result.(*azgo.VserverGetIterResponse)
2418+
if !ok {
2419+
return "", fmt.Errorf("unexpected response type from VserverGetIterRequest: %T", result)
2420+
}
2421+
24152422
if response.Result.NumRecords() != 1 ||
24162423
response.Result.AttributesListPtr == nil ||
24172424
response.Result.AttributesListPtr.VserverInfoPtr == nil {

‎storage_drivers/ontap/ontap_asa_nvme_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3013,7 +3013,7 @@ func TestGetBackendStateASANVMe(t *testing.T) {
30133013
physicalPools := map[string]storage.Pool{ONTAPTEST_VSERVER_AGGR_NAME: pool1}
30143014
driver.physicalPools = physicalPools
30153015

3016-
mockAPI.EXPECT().GetSVMState(ctx).Return(restAPIModels.SvmStateRunning, nil).AnyTimes().Times(1)
3016+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(restAPIModels.SvmStateRunning, nil).Times(1)
30173017
mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return(derivedPools, nil).AnyTimes()
30183018
mockAPI.EXPECT().IsDisaggregated().AnyTimes().Return(false)
30193019
mockAPI.EXPECT().IsSANOptimized().AnyTimes().Return(false)

‎storage_drivers/ontap/ontap_asa_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3013,7 +3013,7 @@ func TestGetBackendStateASA(t *testing.T) {
30133013
driver.physicalPools = physicalPools
30143014
},
30153015
setupMocks: func() {
3016-
mockAPI.EXPECT().GetSVMState(ctx).Return(restAPIModels.SvmStateRunning, nil).Times(1)
3016+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(restAPIModels.SvmStateRunning, nil).Times(1)
30173017
mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return(derivedPools, nil).Times(1)
30183018
mockAPI.EXPECT().NetInterfaceGetDataLIFs(ctx, sa.ISCSI).Return([]string{"1.2.3.4"}, nil).Times(1)
30193019
mockAPI.EXPECT().APIVersion(ctx, true).Return("9.14.1", nil).Times(1)
@@ -3034,7 +3034,7 @@ func TestGetBackendStateASA(t *testing.T) {
30343034
driver.physicalPools = physicalPools
30353035
},
30363036
setupMocks: func() {
3037-
mockAPI.EXPECT().GetSVMState(ctx).Return(restAPIModels.SvmStateRunning, nil).Times(1)
3037+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(restAPIModels.SvmStateRunning, nil).Times(1)
30383038
mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return(derivedPools, nil).Times(1)
30393039
mockAPI.EXPECT().NetFcpInterfaceGetDataLIFs(ctx, sa.FCP).Return([]string{"20:00:00:25:b5:11:a0:01", "20:00:00:25:b5:11:a0:02"}, nil).Times(1)
30403040
mockAPI.EXPECT().APIVersion(ctx, true).Return("9.14.1", nil).Times(1)
@@ -3055,7 +3055,7 @@ func TestGetBackendStateASA(t *testing.T) {
30553055
driver.physicalPools = physicalPools
30563056
},
30573057
setupMocks: func() {
3058-
mockAPI.EXPECT().GetSVMState(ctx).Return(restAPIModels.SvmStateRunning, nil).Times(1)
3058+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(restAPIModels.SvmStateRunning, nil).Times(1)
30593059
mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return(derivedPools, nil).Times(1)
30603060
mockAPI.EXPECT().NetFcpInterfaceGetDataLIFs(ctx, sa.FCP).Return([]string{}, nil).Times(1)
30613061
mockAPI.EXPECT().IsDisaggregated().Return(false).Times(1)
@@ -3074,7 +3074,7 @@ func TestGetBackendStateASA(t *testing.T) {
30743074
driver.physicalPools = physicalPools
30753075
},
30763076
setupMocks: func() {
3077-
mockAPI.EXPECT().GetSVMState(ctx).Return(restAPIModels.SvmStateRunning, nil).Times(1)
3077+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(restAPIModels.SvmStateRunning, nil).Times(1)
30783078
mockAPI.EXPECT().GetSVMAggregateNames(ctx).Return(derivedPools, nil).Times(1)
30793079
mockAPI.EXPECT().NetInterfaceGetDataLIFs(ctx, sa.ISCSI).Return([]string{"127.0.0.1"}, nil).Times(1)
30803080
mockAPI.EXPECT().APIVersion(ctx, true).Return("9.14.1", nil).Times(1)

‎storage_drivers/ontap/ontap_common.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ const (
119119
// maxSnapshotDeleteWait and maxSnapshotDeleteRetry should be used
120120
// together to balance snapshot deletion wait times and retries.
121121
maxSnapshotDeleteWait = 60 * time.Second
122+
getSVMStateTimeout = 20 * time.Second
122123

123124
VolTypeRW = "rw" // read-write
124125
VolTypeLS = "ls" // load-sharing
@@ -736,7 +737,11 @@ func getSVMState(
736737
ctx context.Context, client api.OntapAPI, protocol string, pools []string, configAggrs ...string,
737738
) (string, *roaring.Bitmap) {
738739
changeMap := roaring.New()
739-
svmState, err := client.GetSVMState(ctx)
740+
741+
stateCtx, cancel := context.WithTimeout(ctx, getSVMStateTimeout)
742+
defer cancel()
743+
744+
svmState, err := client.GetSVMState(stateCtx)
740745
if err != nil {
741746
// Could not get the SVM info or SVM is unreachable. Just log it.
742747
// Set state offline and reason as unreachable.

‎storage_drivers/ontap/ontap_common_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9455,7 +9455,7 @@ func TestGetSVMState(t *testing.T) {
94559455

94569456
// Test 1
94579457
// a: API.GetSVMState returns error
9458-
mockAPI.EXPECT().GetSVMState(ctx).Return("TestStateInvalid", errors.New("GetSVMState returned error"))
9458+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return("TestStateInvalid", errors.New("GetSVMState returned error"))
94599459

94609460
state, code := getSVMState(ctx, mockAPI, sa.ISCSI, derivedPoolsNil, aggrsNil...)
94619461
assert.Equal(t, StateReasonSVMUnreachable, state, "state returned should be TestStateUnknown")
@@ -9464,14 +9464,14 @@ func TestGetSVMState(t *testing.T) {
94649464

94659465
// Test 2
94669466
// a: SVM is stopped
9467-
mockAPI.EXPECT().GetSVMState(ctx).Return(models.SvmStateStopped, nil).Times(1)
9467+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(models.SvmStateStopped, nil).Times(1)
94689468

94699469
state, code = getSVMState(ctx, mockAPI, sa.ISCSI, derivedPoolsNil, aggrsNil...)
94709470
assert.Equal(t, StateReasonSVMStopped, state, "state should be SVM stopped")
94719471
assert.False(t, code.Contains(storage.BackendStatePoolsChange), "Should not be pool change")
94729472
assert.False(t, code.Contains(storage.BackendStateAPIVersionChange), "Should not be API version change")
94739473

9474-
mockAPI.EXPECT().GetSVMState(ctx).Return(models.SvmStateRunning, nil).AnyTimes()
9474+
mockAPI.EXPECT().GetSVMState(gomock.Any()).Return(models.SvmStateRunning, nil).AnyTimes()
94759475

94769476
// Test 3
94779477
// a: client.GetSVMAggregateNames returns error

‎storage_drivers/ontap/ontap_nas_flexgroup_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,7 @@ func TestNASFlexGroupStorageDriverGetBackendState(t *testing.T) {
11151115

11161116
// set fake values
11171117
mockDriver.physicalPool = storage.NewStoragePool(nil, "pool1")
1118-
mockApi.EXPECT().GetSVMState(ctx).Return("", errors.New("returning test error"))
1118+
mockApi.EXPECT().GetSVMState(gomock.Any()).Return("", errors.New("returning test error"))
11191119

11201120
reason, changeMap := mockDriver.GetBackendState(ctx)
11211121
assert.Equal(t, reason, StateReasonSVMUnreachable, "should be 'SVM is not reachable'")

‎storage_drivers/ontap/ontap_nas_qtree_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5073,7 +5073,7 @@ func TestCreate_WithErrorInApiOperation(t *testing.T) {
50735073
func TestNASQtreeStorageDriverGetBackendState(t *testing.T) {
50745074
mockApi, mockDriver := newMockOntapNasQtreeDriver(t)
50755075

5076-
mockApi.EXPECT().GetSVMState(ctx).Return("", errors.New("returning test error"))
5076+
mockApi.EXPECT().GetSVMState(gomock.Any()).Return("", errors.New("returning test error"))
50775077

50785078
reason, changeMap := mockDriver.GetBackendState(ctx)
50795079
assert.Equal(t, reason, StateReasonSVMUnreachable, "should be 'SVM is not reachable'")

‎storage_drivers/ontap/ontap_nas_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3594,7 +3594,7 @@ func TestOntapNasStorageDriverReconcileNodeAccess(t *testing.T) {
35943594
func TestNASStorageDriverGetBackendState(t *testing.T) {
35953595
mockApi, mockDriver := newMockOntapNASDriverWithSVM(t, "SVM1")
35963596

3597-
mockApi.EXPECT().GetSVMState(ctx).Return("", errors.New("returning test error"))
3597+
mockApi.EXPECT().GetSVMState(gomock.Any()).Return("", errors.New("returning test error"))
35983598
mockApi.EXPECT().IsDisaggregated().AnyTimes().Return(false)
35993599
mockApi.EXPECT().IsSANOptimized().AnyTimes().Return(true)
36003600

@@ -3606,7 +3606,7 @@ func TestNASStorageDriverGetBackendState(t *testing.T) {
36063606
func TestNASStorageDriverGetBackendState_AFX(t *testing.T) {
36073607
mockApi, mockDriver := newMockOntapNASDriverWithSVM(t, "SVM1")
36083608

3609-
mockApi.EXPECT().GetSVMState(ctx).Return("", errors.New("returning test error"))
3609+
mockApi.EXPECT().GetSVMState(gomock.Any()).Return("", errors.New("returning test error"))
36103610
mockApi.EXPECT().IsDisaggregated().AnyTimes().Return(true)
36113611
mockApi.EXPECT().IsSANOptimized().AnyTimes().Return(true)
36123612

‎storage_drivers/ontap/ontap_san_economy_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6195,7 +6195,7 @@ func TestOntapSanEconomyEnablePublishEnforcement_EnablesAccessControl(t *testing
61956195
func TestSANEconomyStorageDriverGetBackendState(t *testing.T) {
61966196
mockApi, mockDriver := newMockOntapSanEcoDriver(t)
61976197

6198-
mockApi.EXPECT().GetSVMState(ctx).Return("", errors.New("returning test error"))
6198+
mockApi.EXPECT().GetSVMState(gomock.Any()).Return("", errors.New("returning test error"))
61996199

62006200
reason, changeMap := mockDriver.GetBackendState(ctx)
62016201
assert.Equal(t, reason, StateReasonSVMUnreachable, "should be 'SVM is not reachable'")

0 commit comments

Comments
 (0)