Skip to content

Conversation

mayooot
Copy link
Contributor

@mayooot mayooot commented May 21, 2025

What issue type does this pull request address? (keep at least one, remove the others)

/kind enhancement

What does this pull request do? Which issues does it resolve?

none

Please provide a short message that should be published in the vcluster release notes

Increased QPS and Burst for kubernetes config from 40 to 1000 for QPS and 80 to 2000 fo Burst.

@mayooot mayooot requested a review from a team as a code owner May 21, 2025 10:18
@matskiv
Copy link
Contributor

matskiv commented May 21, 2025

What does this pull request do? Which issues does it resolve?

none

Can you provide some reasoning why this change should be made? Ideally with some numbers...

matskiv
matskiv previously approved these changes May 26, 2025
Copy link
Contributor

@matskiv matskiv left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@mayooot
Copy link
Contributor Author

mayooot commented Jun 3, 2025

Looks good. Thank you!

@matskiv Please rerun the tests.

Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

Naming is hard. I've added my feedback, but will also seek additional maintainer feedback.

inClusterConfig.QPS = float32(qps)

// Get Burst from environment variable or default to 80
burstStr := os.Getenv("K8S_CLIENT_BURST")
Copy link
Contributor

Choose a reason for hiding this comment

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

similar comment as above

@mayooot
Copy link
Contributor Author

mayooot commented Jun 5, 2025

Naming is hard. I've added my feedback, but will also seek additional maintainer feedback.

Naming is hard. I've added my feedback, but will also seek additional maintainer feedback.

Ok, I'm done, and you can check it.

@cbron cbron requested a review from lizardruss July 11, 2025 19:15
Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

lgtm

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

Successfully merging this pull request may close these issues.

3 participants