Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The work_queue in the controller should be replaced by a new construction method #2385

Closed
ShyunnY opened this issue Jun 23, 2023 · 2 comments · Fixed by #2411
Closed

The work_queue in the controller should be replaced by a new construction method #2385

ShyunnY opened this issue Jun 23, 2023 · 2 comments · Fixed by #2411

Comments

@ShyunnY
Copy link

ShyunnY commented Jun 23, 2023

I found that when creating a controller, the method of constructing workqueue has been enabled

return &controller.Controller{
		Do: options.Reconciler,
		MakeQueue: func() workqueue.RateLimitingInterface {
			return workqueue.NewNamedRateLimitingQueue(options.RateLimiter, name)
		},
		MaxConcurrentReconciles: options.MaxConcurrentReconciles,
		CacheSyncTimeout:        options.CacheSyncTimeout,
		Name:                    name,
		LogConstructor:          options.LogConstructor,
		RecoverPanic:            options.RecoverPanic,
		LeaderElected:           options.NeedLeaderElection,
	}, nil

Among them, it is recommended to use a new construction method for RateLimitingQueue
We can see from the workqueue in the client-go package that it would be better to use NewRateLimitingQueueWithConfig instead

// NewNamedRateLimitingQueue constructs a new named workqueue with rateLimited queuing ability.
// Deprecated: Use NewRateLimitingQueueWithConfig instead.
func NewNamedRateLimitingQueue(rateLimiter RateLimiter, name string) RateLimitingInterface {
	return NewRateLimitingQueueWithConfig(rateLimiter, RateLimitingQueueConfig{
		Name: name,
	})
}

I think it is possible to add new features to RateLimitingQueueConfig in the future

Tasks

No tasks being tracked yet.
@vincepri
Copy link
Member

MakeQueue is an internal constructor; users are expected to use Options.RateLimiter to customize rate limiting

@sbueringer
Copy link
Member

I think this issue is just about that we should move away from the deprecated func. Opened a PR for it

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants