Skip to content

Commit

Permalink
fix exponential backoff test handling
Browse files Browse the repository at this point in the history
Signed-off-by: James Munnelly <[email protected]>
  • Loading branch information
munnerz committed Nov 30, 2023
1 parent 60a2d87 commit 0ce8db0
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
13 changes: 13 additions & 0 deletions manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,23 @@ type Manager struct {
// requestNameGenerator generates a new random name for a certificaterequest object
// Defaults to uuid.NewUUID() from k8s.io/apimachinery/pkg/util/uuid.
requestNameGenerator func() string

// doNotUse_CallOnEachIssue is a field used SOLELY for testing, and cannot be configured by external package consumers.
// It is used to perform some action (e.g. counting) each time issue() is called.
// It will be removed as soon as we have actual metrics support in csi-lib, which will allow us to measure
// things like the number of times issue() is called.
// No thread safety is added around this field, and it MUST NOT be used for any implementation logic.
// It should not be used full-stop :).
doNotUse_CallOnEachIssue func()
}

// issue will step through the entire issuance flow for a volume.
func (m *Manager) issue(ctx context.Context, volumeID string) error {
// TODO: remove this code and replace with actual metrics support
if m.doNotUse_CallOnEachIssue != nil {
m.doNotUse_CallOnEachIssue()
}

log := m.log.WithValues("volume_id", volumeID)
log.Info("Processing issuance")

Expand Down
15 changes: 7 additions & 8 deletions manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ func TestManager_ManageVolume_exponentialBackOffRetryOnIssueErrors(t *testing.T)
expectGlobalTimeout := 2 * time.Second

var numOfRetries int32 = 0 // init

opts := newDefaultTestOptions(t)
opts.RenewalBackoffConfig = &wait.Backoff{
Duration: expBackOffDuration,
Expand All @@ -313,16 +312,16 @@ func TestManager_ManageVolume_exponentialBackOffRetryOnIssueErrors(t *testing.T)
Jitter: expBackOffJitter,
Steps: expBackOffSteps,
}
opts.ReadyToRequest = func(meta metadata.Metadata) (bool, string) {
// ReadyToRequest will be called by issue()
atomic.AddInt32(&numOfRetries, 1) // run in a goroutine, thus increment it atomically
return true, "" // AlwaysReadyToRequest
}
m, err := NewManager(opts)
m.issueRenewalTimeout = issueRenewalTimeout
if err != nil {
t.Fatal(err)
}
m.issueRenewalTimeout = issueRenewalTimeout
// Increment the 'numOfRetries' counter whenever issue() is called.
// TODO: replace usages of this function with reading from metrics.
m.doNotUse_CallOnEachIssue = func() {
atomic.AddInt32(&numOfRetries, 1) // run in a goroutine, thus increment it atomically
}

// Register a new volume with the metadata store
store := opts.MetadataReader.(storage.Interface)
Expand All @@ -347,7 +346,7 @@ func TestManager_ManageVolume_exponentialBackOffRetryOnIssueErrors(t *testing.T)

actualNumOfRetries := atomic.LoadInt32(&numOfRetries) // read atomically
if actualNumOfRetries != expectNumOfRetries {
t.Errorf("expect %d of retires, but got %d", expectNumOfRetries, actualNumOfRetries)
t.Errorf("expect %d retires, but got %d", expectNumOfRetries, actualNumOfRetries)
}
}

Expand Down

0 comments on commit 0ce8db0

Please sign in to comment.