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

add autoscaling #208

Merged
merged 1 commit into from
May 30, 2024
Merged

add autoscaling #208

merged 1 commit into from
May 30, 2024

Conversation

sja
Copy link
Contributor

@sja sja commented May 27, 2024

No description provided.

@pierluigilenoci
Copy link
Contributor

@sja, please add a helm chart bump and edit the artifacthub.io/changes section.

@sja
Copy link
Contributor Author

sja commented May 28, 2024

Thanks @pierluigilenoci for that info. I did a minor version bump, as it adds a feature. It is backwards compatible, old values do not change the result.

@sja
Copy link
Contributor Author

sja commented May 29, 2024

@pierluigilenoci I updated the MR again, inspired from the other MRs. Feel free to reach out to me, especially if you want me to modify the version numbers to not have a conflict. Usually, I react in 1 business day.

@pierluigilenoci
Copy link
Contributor

pierluigilenoci commented May 29, 2024

Can the test case with autoscaling enabled in the chart test suite be included?
Something like this https://github.com/oauth2-proxy/manifests/blob/main/helm/oauth2-proxy/ci/pdb-values.yaml

@sja
Copy link
Contributor Author

sja commented May 30, 2024

I added that, but I'm not familar with how to validate the result. Are the results compared to an expected value somewhere or is that more like a smoke test if the values can be rendered at all?

@pierluigilenoci pierluigilenoci merged commit 6116131 into oauth2-proxy:main May 30, 2024
1 check passed
@sja sja deleted the add-hpa branch May 31, 2024 05:45
@pierluigilenoci
Copy link
Contributor

@sja Usually, the test results are visible in the pipeline.
In this case, they are not, and I don't understand why.
However, from the point of view of the feature, this PR is OK like this.

@pierluigilenoci
Copy link
Contributor

@sja ok, fixed #211 🚀

@pierluigilenoci
Copy link
Contributor

@sja Thank you.

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.

2 participants