Skip to content

Commit cc20317

Browse files
committed
make issueRenewalTimeout an internal parameter
Signed-off-by: Jing Liu <[email protected]>
1 parent 1e073df commit cc20317

File tree

3 files changed

+3
-27
lines changed

3 files changed

+3
-27
lines changed

manager/manager.go

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,6 @@ type Options struct {
8787

8888
// RenewalBackoffConfig configures the exponential backoff applied to certificate renewal failures.
8989
RenewalBackoffConfig *wait.Backoff
90-
// IssueRenewalTimeout defines timeout value for each issue() call in renewal process
91-
IssueRenewalTimeout time.Duration
92-
// IssuePollInterval defines an interval time for each subroutine call (check CSR status) within issue() function
93-
// Note, the IssuePollInterval should be less than IssueRenewalTimeout
94-
IssuePollInterval time.Duration
9590
}
9691

9792
// NewManager constructs a new manager used to manage volumes containing
@@ -153,16 +148,6 @@ func NewManager(opts Options) (*Manager, error) {
153148
if opts.MaxRequestsPerVolume < 0 {
154149
return nil, errors.New("MaxRequestsPerVolume cannot be less than zero")
155150
}
156-
if opts.IssueRenewalTimeout == 0 {
157-
opts.IssueRenewalTimeout = 10 * time.Second
158-
}
159-
if opts.IssuePollInterval == 0 {
160-
opts.IssuePollInterval = time.Second
161-
}
162-
if opts.IssuePollInterval >= opts.IssueRenewalTimeout {
163-
return nil, errors.New("IssuePollInterval should be less than IssueRenewalTimeout")
164-
}
165-
// Check IssuePollInterval is less than IssueRenewalTimeout
166151
if len(opts.NodeID) == 0 {
167152
return nil, errors.New("NodeID must be set")
168153
}
@@ -204,8 +189,7 @@ func NewManager(opts Options) (*Manager, error) {
204189
maxRequestsPerVolume: opts.MaxRequestsPerVolume,
205190
nodeNameHash: nodeNameHash,
206191
backoffConfig: *opts.RenewalBackoffConfig,
207-
issueRenewalTimeout: opts.IssueRenewalTimeout,
208-
issuePollInterval: opts.IssuePollInterval,
192+
issueRenewalTimeout: time.Second * 60, // issueRenewalTimeout set to align with NodePublishVolume timeout value
209193
requestNameGenerator: func() string {
210194
return string(uuid.NewUUID())
211195
},
@@ -310,9 +294,6 @@ type Manager struct {
310294

311295
// issueRenewalTimeout defines timeout value for each issue() call in renewal process
312296
issueRenewalTimeout time.Duration
313-
// issuePollInterval defines an interval time for each subroutine call (check CSR status) within issue() function
314-
// Note, the issuePollInterval should be less than issueRenewalTimeout
315-
issuePollInterval time.Duration
316297

317298
// requestNameGenerator generates a new random name for a certificaterequest object
318299
// Defaults to uuid.NewUUID() from k8s.io/apimachinery/pkg/util/uuid.
@@ -364,7 +345,7 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error {
364345

365346
// Poll every 1s for the CertificateRequest to be ready
366347
lastFailureReason := ""
367-
if err := wait.PollUntilWithContext(ctx, m.issuePollInterval, func(ctx context.Context) (done bool, err error) {
348+
if err := wait.PollUntilWithContext(ctx, time.Second, func(ctx context.Context) (done bool, err error) {
368349
updatedReq, err := m.lister.CertificateRequests(req.Namespace).Get(req.Name)
369350
if apierrors.IsNotFound(err) {
370351
// A NotFound error implies something deleted the resource - fail

manager/manager_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,6 @@ func TestManager_ManageVolume_exponentialBackOffRetryOnIssueErrors(t *testing.T)
293293
expBackOffJitter := 0.0 // No jitter to the 'duration', so we could calculate number of retries easily
294294
expBackOffSteps := 100 // The maximum number of backoff attempts
295295
issueRenewalTimeout := expBackOffDuration
296-
issuePollInterval := issueRenewalTimeout / 10
297296

298297
// Expected number of retries in each expBackOff cycle :=
299298
// ⌈log base expBackOffFactor of (expBackOffCap/expBackOffDuration)⌉
@@ -314,14 +313,13 @@ func TestManager_ManageVolume_exponentialBackOffRetryOnIssueErrors(t *testing.T)
314313
Jitter: expBackOffJitter,
315314
Steps: expBackOffSteps,
316315
}
317-
opts.IssueRenewalTimeout = issueRenewalTimeout
318-
opts.IssuePollInterval = issuePollInterval
319316
opts.ReadyToRequest = func(meta metadata.Metadata) (bool, string) {
320317
// ReadyToRequest will be called by issue()
321318
atomic.AddInt32(&numOfRetries, 1) // run in a goroutine, thus increment it atomically
322319
return true, "" // AlwaysReadyToRequest
323320
}
324321
m, err := NewManager(opts)
322+
m.issueRenewalTimeout = issueRenewalTimeout
325323
if err != nil {
326324
t.Fatal(err)
327325
}

test/driver/driver_testing.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"math"
2323
"net"
2424
"testing"
25-
"time"
2625

2726
cmclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned"
2827
fakeclient "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/fake"
@@ -117,8 +116,6 @@ func Run(t *testing.T, opts Options) (Options, csi.NodeClient, func()) {
117116
WriteKeypair: opts.WriteKeypair,
118117
ReadyToRequest: opts.ReadyToRequest,
119118
RenewalBackoffConfig: &wait.Backoff{Steps: math.MaxInt32}, // don't actually wait (i.e. set all backoff times to 0)
120-
IssueRenewalTimeout: time.Second, // timeout issue renewal call in a second
121-
IssuePollInterval: 100 * time.Millisecond, // check CSR status every 0.1s during issue call
122119
})
123120

124121
d := driver.NewWithListener(lis, *opts.Log, driver.Options{

0 commit comments

Comments
 (0)