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

Setting kube-api-qps and kube-api-burst are not working #3952

Closed
kevinteng525 opened this issue Dec 4, 2022 · 5 comments · Fixed by #3955
Closed

Setting kube-api-qps and kube-api-burst are not working #3952

kevinteng525 opened this issue Dec 4, 2022 · 5 comments · Fixed by #3955
Assignees
Labels
bug Something isn't working

Comments

@kevinteng525
Copy link
Contributor

Report

Sets kube-api-qps and kube-api-burst for KEDA adapter will not take effective.

It was set here

	cfg, err := config.GetConfig()
	if cfg != nil {
		cfg.QPS = adapterClientRequestQPS
		cfg.Burst = adapterClientRequestBurst
		cfg.DisableCompression = disableCompression
	}

However never got used, when new Manager, it will use another config

         mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
		MetricsBindAddress: metricsBindAddress,
		Scheme:             scheme,
		Namespace:          namespace,
		LeaseDuration:      leaseDuration,
		RenewDeadline:      renewDeadline,
		RetryPeriod:        retryPeriod,
	})

Expected Behavior

Setting kube-api-qps and kube-api-burst should be working

Actual Behavior

Setting kube-api-qps and kube-api-burst are not working

Steps to Reproduce the Problem

  1. Pass kube-api-qps and kube-api-burst as args, as following
- /usr/local/bin/keda-adapter
        - --secure-port=6443
        - --logtostderr=false
        - --log_file=/tmp/apiserver.log
        - --log_dir=/tmp/log
        - --kube-api-qps=70
        - --kube-api-burst=100

Logs from KEDA operator

N/A

KEDA Version

2.8.1

Kubernetes Version

1.23

Platform

Any

Scaler Details

No response

Anything else?

No response

@kevinteng525 kevinteng525 added the bug Something isn't working label Dec 4, 2022
@tomkerkhove tomkerkhove moved this to Proposed in Roadmap - KEDA Core Dec 4, 2022
@v-shenoy
Copy link
Contributor

v-shenoy commented Dec 4, 2022

The main branch code is using the correct config.

keda/main.go

Lines 117 to 122 in af948a5

cfg := ctrl.GetConfigOrDie()
cfg.QPS = adapterClientRequestQPS
cfg.Burst = adapterClientRequestBurst
cfg.DisableCompression = disableCompression
mgr, err := ctrl.NewManager(cfg, ctrl.Options{

This feature was added recently, and will be released as part of KEDA v2.9.

@v-shenoy
Copy link
Contributor

v-shenoy commented Dec 4, 2022

The main branch code is using the correct config.

keda/main.go

Lines 117 to 122 in af948a5

cfg := ctrl.GetConfigOrDie()
cfg.QPS = adapterClientRequestQPS
cfg.Burst = adapterClientRequestBurst
cfg.DisableCompression = disableCompression
mgr, err := ctrl.NewManager(cfg, ctrl.Options{

This feature was added recently, and will be released as part of KEDA v2.9.

Ah, I am dumb. You're talking about the metrics server, where I can see it's not being used.

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{

@zroubalik
Copy link
Member

Good catch, thanks for reporting.

@zroubalik zroubalik self-assigned this Dec 5, 2022
@zroubalik zroubalik moved this from Proposed to In Progress in Roadmap - KEDA Core Dec 5, 2022
@zroubalik
Copy link
Member

Relates to #3730

@kevinteng525
Copy link
Contributor Author

Relates to #3730

Thanks @zroubalik for the fixing! I was also planning to submit a PR for this:)

Repository owner moved this from In Progress to Ready To Ship in Roadmap - KEDA Core Dec 5, 2022
@tomkerkhove tomkerkhove moved this from Ready To Ship to Done in Roadmap - KEDA Core Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants