Adding unit tests and configurable parameter for clone split delay.

storage_drivers/ontap/ontap_common.go

@@ -108,8 +108,6 @@ const (
 	StateReasonSVMUnreachable = "SVM is not reachable"
 )
 
-var CloneSplitDelay = 10 * time.Second
-
 // CleanBackendName removes brackets and replaces colons with periods to avoid regex parsing errors.
 func CleanBackendName(backendName string) string {
 	backendName = strings.ReplaceAll(backendName, "[", "")
@@ -1226,6 +1224,7 @@ const (
 	DefaultNfsMountOptionsKubernetes = ""
 	DefaultSplitOnClone              = "false"
 	DefaultCloneSplitDelay           = "10"
+	DefaultCloneSplitDelayValue      = 10
 	DefaultLuksEncryption            = "false"
 	DefaultMirroring                 = "false"
 	DefaultLimitAggregateUsage       = ""
@@ -1319,9 +1318,9 @@ func PopulateConfigurationDefaults(ctx context.Context, config *drivers.OntapSto
 	if config.CloneSplitDelay == "" {
 		config.CloneSplitDelay = DefaultCloneSplitDelay
 	} else {
-		_, err := strconv.ParseInt(config.CloneSplitDelay, 10, 64)
-		if err != nil {
-			return fmt.Errorf("invalid integer value for cloneSplitDelay: %v", err)
+		v, err := strconv.ParseInt(config.CloneSplitDelay, 10, 0)
+		if err != nil || v <= 0 {
+			return fmt.Errorf("invalid value for cloneSplitDelay: %v", config.CloneSplitDelay)
 		}
 	}
 
@@ -1654,13 +1653,14 @@ func SplitVolumeFromBusySnapshotWithDelay(
 	cloneSplitTimers map[string]time.Time,
 ) {
 	snapshotID := snapConfig.ID()
-	if config.CloneSplitDelay != "" {
-		cloneSplitDelayDuration, err := strconv.ParseInt(config.CloneSplitDelay, 10, 0)
-		if err == nil {
-			// Valid value provided, use it
-			CloneSplitDelay = time.Duration(cloneSplitDelayDuration) * time.Second
-		}
+
+	// Find the configured value for cloneSplitDelay
+	cfgVal, err := strconv.ParseInt(config.CloneSplitDelay, 10, 0)
+	if err != nil || cfgVal < 0 {
+		// Should not come here, but just in case.
+		cfgVal = DefaultCloneSplitDelayValue
 	}
+	cloneSplitDelay := time.Duration(cfgVal) * time.Second
 
 	// If this is the first delete, just log the time and return.
 	firstDeleteTime, ok := cloneSplitTimers[snapshotID]
@@ -1669,7 +1669,7 @@ func SplitVolumeFromBusySnapshotWithDelay(
 
 		Logc(ctx).WithFields(LogFields{
 			"snapshot":           snapshotID,
-			"secondsBeforeSplit": fmt.Sprintf("%3.2f", CloneSplitDelay.Seconds()),
+			"secondsBeforeSplit": fmt.Sprintf("%3.2f", cloneSplitDelay.Seconds()),
 		}).Warning("Initial locked snapshot delete, starting clone split timer.")
 
 		return
@@ -1686,7 +1686,7 @@ func SplitVolumeFromBusySnapshotWithDelay(
 	}
 
 	// This isn't the first delete, and the delay has not expired, so there is nothing to do.
-	if time.Now().Sub(firstDeleteTime) < CloneSplitDelay {
+	if time.Now().Sub(firstDeleteTime) < cloneSplitDelay {
 
 		Logc(ctx).WithFields(LogFields{
 			"snapshot":           snapshotID,
@@ -1705,7 +1705,7 @@ func SplitVolumeFromBusySnapshotWithDelay(
 
 		Logc(ctx).WithFields(LogFields{
 			"snapshot":           snapshotID,
-			"secondsBeforeSplit": fmt.Sprintf("%3.2f", CloneSplitDelay.Seconds()),
+			"secondsBeforeSplit": fmt.Sprintf("%3.2f", cloneSplitDelay.Seconds()),
 		}).Warning("Retried locked snapshot delete, clone split failed, restarted timer.")
 
 	} else {

storage_drivers/ontap/ontap_common_test.go

@@ -4734,6 +4734,42 @@ func TestSplitVolumeFromBusySnapshot(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestSplitVolumeFromBusySnapshotWithDelay(t *testing.T) {
+	ctx := context.Background()
+	mockCtrl := gomock.NewController(t)
+	mockAPI := mockapi.NewMockOntapAPI(mockCtrl)
+	commonConfig := &drivers.CommonStorageDriverConfig{
+		DebugTraceFlags: map[string]bool{"method": true},
+	}
+	config := &drivers.OntapStorageDriverConfig{
+		CommonStorageDriverConfig: commonConfig,
+	}
+	snapConfig := &storage.SnapshotConfig{
+		VolumeInternalName: "fakeVolInternalName",
+		InternalName:       "fakeInternalName",
+	}
+
+	// Test 1: No snap found
+	cloneSplitTimers := make(map[string]time.Time)
+	SplitVolumeFromBusySnapshotWithDelay(ctx, snapConfig, config, mockAPI, mockCloneSplitStart, cloneSplitTimers)
+
+	// Test 2: time difference is < 0
+	cloneSplitTimers[snapConfig.ID()] = time.Now().Add(10 * time.Millisecond)
+	SplitVolumeFromBusySnapshotWithDelay(ctx, snapConfig, config, mockAPI, mockCloneSplitStart, cloneSplitTimers)
+
+	time.Sleep(50 * time.Millisecond)
+	// Test 3: time difference < config.CloneSplitDelay, which is defaulted to 10 seconds.
+	SplitVolumeFromBusySnapshotWithDelay(ctx, snapConfig, config, mockAPI, mockCloneSplitStart, cloneSplitTimers)
+
+	// Test 4: SplitVolumeFromBusySnapshot returning error
+	// Add the time for first delete in past so that SplitVolume is called.
+	cloneSplitTimers[snapConfig.ID()] = time.Now().Add(-10 * time.Second)
+	mockAPI.EXPECT().VolumeListBySnapshotParent(ctx, snapConfig.InternalName,
+		snapConfig.VolumeInternalName).Return(api.VolumeNameList{},
+		fmt.Errorf("error returned by VolumeListBySnapshotParent"))
+	SplitVolumeFromBusySnapshotWithDelay(ctx, snapConfig, config, mockAPI, mockCloneSplitStart, cloneSplitTimers)
+}
+
 func TestGetVolumeExternalCommon(t *testing.T) {
 	volume := api.Volume{
 		Name:            "fakeVolume",
@@ -5122,6 +5158,12 @@ func TestPopulateConfigurationDefaults(t *testing.T) {
 	err = PopulateConfigurationDefaults(ctx, config)
 
 	assert.Error(t, err)
+
+	// Test 8 - Invalid value for cloneSplitDelay
+	config.SplitOnClone = "true"
+	config.CloneSplitDelay = "-123"
+	err = PopulateConfigurationDefaults(ctx, config)
+	assert.Error(t, err)
 }
 
 func TestNewOntapTelemetry(t *testing.T) {

storage_drivers/types.go

@@ -119,7 +119,7 @@ type OntapStorageDriverConfig struct {
 	QtreesPerFlexvol                 string     `json:"qtreesPerFlexvol"`                 // default to 200
 	LUNsPerFlexvol                   string     `json:"lunsPerFlexvol"`                   // default to 100
 	EmptyFlexvolDeferredDeletePeriod string     `json:"emptyFlexvolDeferredDeletePeriod"` // in seconds, default to 28800
-	CloneSplitDelay                  string     `json:"cloneSplitDelay"`                  // in seconds, default to 10
+	CloneSplitDelay                  string     `json:"cloneSplitDelay,omitEmpty"`        // in seconds, default to 10
 	NfsMountOptions                  string     `json:"nfsMountOptions"`
 	LimitAggregateUsage              string     `json:"limitAggregateUsage"`
 	AutoExportPolicy                 bool       `json:"autoExportPolicy"`