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.

Sign up for GitHub

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

Jump to bottom

Disable response compression for k8s restAPI in client-go #3866

Merged
merged 9 commits into from
Nov 21, 2022
Merged

Disable response compression for k8s restAPI in client-go #3866

merged 9 commits into from
Nov 21, 2022

Conversation

ckuduvalli
Copy link
Contributor

@ckuduvalli ckuduvalli commented Nov 16, 2022 •
edited by zroubalik
Loading

Keda-operator timing out for large number of configmaps and secrets. Hence disabling compression in client-go.
#3863

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements

Fixes #3863

Chaitanya Kuduvalli Ramachandra added 2 commits November 16, 2022 14:56
@ckuduvalli ckuduvalli requested a review from a team as a code owner November 16, 2022 09:28
zroubalik
zroubalik reviewed Nov 16, 2022
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

  1. Shouldn't we add the same to Metrics Server config?

    keda/adapter/main.go

    Line 75 in dab87b5

    if cfg != nil {

  2. Could you please add this issue to Changelog?

  3. And also a document it, probably here: https://keda.sh/docs/2.9/operate/cluster/#kubernetes-client-parameters

@ckuduvalli
Copy link
Contributor Author

have added documentation here:
kedacore/keda-docs#978

Chaitanya Kuduvalli Ramachandra added 2 commits November 16, 2022 15:16
Updating metrics server with the same parameters
8bbdcba 
Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Adding the change to changelog
1b97043 
Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
@ckuduvalli
Copy link
Contributor Author

@zroubalik all feedback incorporated. Please check

@JorTurFer
Copy link
Member

JorTurFer commented Nov 16, 2022 •
edited by github-actions bot
Loading

/run-e2e aws*
Update: You can check the progress here

zroubalik
zroubalik approved these changes Nov 16, 2022
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks a lot!

zroubalik
zroubalik reviewed Nov 16, 2022
View reviewed changes
adapter/main.go Outdated Show resolved Hide resolved
Chaitanya Kuduvalli Ramachandra added 3 commits November 17, 2022 16:56
Set default value to true for disable compression
a3d9134 
Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Merge branch 'main' of https://github.com/kedacore/keda
3d0decd 
Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Changing default value to true in adapter
c6e7280 
Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
@JorTurFer
Copy link
Member

JorTurFer commented Nov 17, 2022 •
edited by github-actions bot
Loading

/run-e2e aws*
Update: You can check the progress here

@JorTurFer
Copy link
Member

JorTurFer commented Nov 17, 2022 •
edited by github-actions bot
Loading

/run-e2e
Update: You can check the progress here

@JorTurFer JorTurFer enabled auto-merge (squash) November 17, 2022 13:02
@JorTurFer JorTurFer disabled auto-merge November 17, 2022 13:03
@JorTurFer
Copy link
Member

JorTurFer commented Nov 21, 2022 •
edited by github-actions bot
Loading

/run-e2e
Update: You can check the progress here

@JorTurFer JorTurFer merged commit 7ffdf59 into kedacore:main Nov 21, 2022
josephangbc pushed a commit to josephangbc/keda that referenced this pull request Dec 6, 2022
@ckuduvalli @josephangbc
Disable response compression for k8s restAPI in client-go (kedacore#3866
9edc8f1 
)

* Disable response compression for k8s restAPI in client-go

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Updating metrics server with the same parameters

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Adding the change to changelog

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Set default value to true for disable compression

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Changing default value to true in adapter

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Co-authored-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
josephangbc pushed a commit to josephangbc/keda that referenced this pull request Dec 6, 2022
@ckuduvalli @josephangbc
Disable response compression for k8s restAPI in client-go (kedacore#3866
59dcdad 
)

* Disable response compression for k8s restAPI in client-go

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Updating metrics server with the same parameters

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Adding the change to changelog

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Set default value to true for disable compression

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

* Changing default value to true in adapter

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>

Signed-off-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Co-authored-by: Chaitanya Kuduvalli Ramachandra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik approved these changes

@JorTurFer JorTurFer JorTurFer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Keda-operator timing out for large number of configmaps and secrets
3 participants
@ckuduvalli @JorTurFer @zroubalik