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

Allow for resource limits and requests for pre-deployment jobs #23077

Merged
merged 7 commits into from
Mar 15, 2023

Conversation

yjperez
Copy link
Contributor

@yjperez yjperez commented Mar 14, 2023

Adding the ability to allow for requests and limits to be added to the predeploy_job like already done for deployments.

Adding the ability to allow for requests and limits to be added to the predeploy_job like already done for deployments.
Add the ability for the proxy predeploy_job to have limits and requests like the deployment. Using same values from Values file.
@yjperez yjperez marked this pull request as ready for review March 14, 2023 20:03
@yjperez yjperez requested a review from hugoShaka March 14, 2023 20:03
Copy link
Contributor

@hugoShaka hugoShaka left a comment

Choose a reason for hiding this comment

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

The template change looks good :)

Can you add tests covering the feature?

You can find test guidelines here, and might want to copy the main deployment resource tests.

Adding tests for resource limits on predeploy job for auth and proxy.
@yjperez
Copy link
Contributor Author

yjperez commented Mar 15, 2023

@hugoShaka I have added the tests. It was my first time building those, please let me know if I missed something.

@yjperez yjperez changed the title Update predeploy_job.yaml Allow for resource limits and requests for pre-deployment jobs Mar 15, 2023
Copy link
Contributor

@hugoShaka hugoShaka left a comment

Choose a reason for hiding this comment

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

LGTM once the suggestions are addressed

examples/chart/teleport-cluster/tests/predeploy_test.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-cluster/tests/predeploy_test.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-cluster/tests/predeploy_test.yaml Outdated Show resolved Hide resolved
examples/chart/teleport-cluster/tests/predeploy_test.yaml Outdated Show resolved Hide resolved
yjperez and others added 4 commits March 15, 2023 11:00
Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
@hugoShaka hugoShaka requested a review from r0mant March 15, 2023 16:02
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@yjperez Do you want to backport this? If so, can you add respective backport/branch/xxx labels to the PR.

@yjperez
Copy link
Contributor Author

yjperez commented Mar 15, 2023

@r0mant no, this is only applicable starting Teleport V12.

@hugoShaka hugoShaka added this pull request to the merge queue Mar 15, 2023
@r0mant
Copy link
Collaborator

r0mant commented Mar 15, 2023

no, this is only applicable starting Teleport V12.

@yjperez Then you need backport/branch/v12 label?

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 15, 2023
@hugoShaka hugoShaka added this pull request to the merge queue Mar 15, 2023
Merged via the queue into master with commit 54951cf Mar 15, 2023
@yjperez yjperez deleted the yjperez-helm-auth-predeploy-limits branch March 15, 2023 17:24
@public-teleport-github-review-bot

@yjperez See the table below for backport results.

Branch Result
branch/v12 Create PR

@hugoShaka hugoShaka restored the yjperez-helm-auth-predeploy-limits branch March 15, 2023 17:35
@hugoShaka hugoShaka deleted the yjperez-helm-auth-predeploy-limits branch March 15, 2023 17:35
@yjperez yjperez added the cs-tse Internal Customer Success Label label Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cs-tse Internal Customer Success Label helm size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants