Skip to content

Commit 985cb22

Browse files
authored
refactor: move deployment limiter into context (#1841)
#### What this PR does / Why we need it: Removes the global variable and puts deployment options into the context, to prevent prop-drilling #### Special notes for your reviewer: Coverage check may fail because of these two lines inside the `if` ```go if options.ConcurrentDeploymentsLimiter != nil { options.ConcurrentDeploymentsLimiter.Acquire() defer options.ConcurrentDeploymentsLimiter.Release() } ``` The question is: do we want to test that? Add a test that just checks if acquire/release was called? how: replace the limiter struct inside the options with an interface and put a stub into the context, so that the call can be checked. Should we write an integration test for it? how: stubbed clients that don't respond and we check if upserts were called {limitedConcurrentCount} times #### Does this PR introduce a user-facing change? No
1 parent 984985b commit 985cb22

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

pkg/deploy/deploy.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,29 @@ type DeployConfigsOptions struct {
6262
}
6363

6464
var (
65-
lock sync.Mutex
66-
concurrentDeploymentsLimiter *rest.ConcurrentRequestLimiter
67-
65+
lock sync.Mutex
6866
errSkip = errors.New("skip error")
6967
)
7068

69+
type ctxDeploymentLimiterKey struct{}
70+
71+
func newContextWithDeploymentLimiter(ctx context.Context, limiter *rest.ConcurrentRequestLimiter) context.Context {
72+
return context.WithValue(ctx, ctxDeploymentLimiterKey{}, limiter)
73+
}
74+
75+
func getDeploymentLimiterFromContext(ctx context.Context) *rest.ConcurrentRequestLimiter {
76+
if limiter, ok := ctx.Value(ctxDeploymentLimiterKey{}).(*rest.ConcurrentRequestLimiter); ok {
77+
return limiter
78+
}
79+
return nil
80+
}
81+
7182
func DeployForAllEnvironments(ctx context.Context, projects []project.Project, environmentClients dynatrace.EnvironmentClients, opts DeployConfigsOptions) error {
7283
maxConcurrentDeployments := environment.GetEnvValueIntLog(environment.ConcurrentDeploymentsEnvKey)
7384
if maxConcurrentDeployments > 0 {
7485
log.Info("%s set, limiting concurrent deployments to %d", environment.ConcurrentDeploymentsEnvKey, maxConcurrentDeployments)
75-
concurrentDeploymentsLimiter = rest.NewConcurrentRequestLimiter(maxConcurrentDeployments)
86+
limiter := rest.NewConcurrentRequestLimiter(maxConcurrentDeployments)
87+
ctx = newContextWithDeploymentLimiter(ctx, limiter)
7688
}
7789
deploymentErrs := make(deployErrors.EnvironmentDeploymentErrors)
7890

@@ -292,9 +304,9 @@ func (e ErrUnknownConfigType) Error() string {
292304
}
293305

294306
func deployConfig(ctx context.Context, c *config.Config, clientset *client.ClientSet, resolvedEntities config.EntityLookup) (entities.ResolvedEntity, error) {
295-
if concurrentDeploymentsLimiter != nil {
296-
concurrentDeploymentsLimiter.Acquire()
297-
defer concurrentDeploymentsLimiter.Release()
307+
if limiter := getDeploymentLimiterFromContext(ctx); limiter != nil {
308+
limiter.Acquire()
309+
defer limiter.Release()
298310
}
299311

300312
if c.Skip {

0 commit comments

Comments
 (0)