Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Utilize resource requests and limits #11608

Merged
merged 2 commits into from
Apr 15, 2019
Merged

Utilize resource requests and limits #11608

merged 2 commits into from
Apr 15, 2019

Conversation

naiduarvind
Copy link
Contributor

@naiduarvind naiduarvind commented Feb 21, 2019 •
edited
Loading

Signed-off-by: Arvind Naidu [email protected]

What this PR does / why we need it:

To be brief, this PR allow users of this Helm chart to take advantage of setting resource requests and limits which otherwise would not work as specified in the values.yaml / README.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 21, 2019
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 21, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @naiduarvind. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@naiduarvind naiduarvind changed the title Utilize resource requests and limit Utilize resource requests and limits Feb 21, 2019
@naiduarvind
Copy link
Contributor Author

/assign @unguiculus

cpanato
cpanato reviewed Mar 7, 2019
View reviewed changes
stable/efs-provisioner/templates/deployment.yaml Outdated
@@ -57,6 +57,8 @@ spec:
value: {{ .Values.efsProvisioner.awsRegion }}
- name: PROVISIONER_NAME
value: {{ .Values.efsProvisioner.provisionerName }}
resources:
{{ toYaml .Values.resources | indent 12 }}
Copy link
Member

Choose a reason for hiding this comment

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

you need to check if there are resources to add otherwise not add the resources entry

also rebase your branch to get the latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, I will update the PR by end of day.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cpanato Hi, been busy. As seen from the screenshot below, updated this chart to utilise the new defaults in a Chart template creation which will not create the resource limits if not defined.

I have also rebased my branch so we should be good to merge this changes.

cc: @paulczar @unguiculus

@stale
Copy link

stale bot commented Apr 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 11, 2019
@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 13, 2019
@paulczar
Copy link
Collaborator

/ok-to-test
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm Indicates that a PR is ready to be merged. ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: naiduarvind, paulczar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 15, 2019
@k8s-ci-robot k8s-ci-robot merged commit 1cad86d into helm:master Apr 15, 2019
TsuyoshiUshio pushed a commit to TsuyoshiUshio/charts that referenced this pull request Apr 18, 2019
@naiduarvind @TsuyoshiUshio
Utilize resource requests and limits (helm#11608)
8ba2187 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
hajnej pushed a commit to hajnej/charts that referenced this pull request Apr 24, 2019
@naiduarvind @hajnej
Utilize resource requests and limits (helm#11608)
623d3bc 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
devnulled pushed a commit to devnulled/charts that referenced this pull request Apr 25, 2019
@naiduarvind @devnulled
Utilize resource requests and limits (helm#11608)
bf28506 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
dermorz pushed a commit to dermorz/charts that referenced this pull request Apr 26, 2019
@naiduarvind
Utilize resource requests and limits (helm#11608)
b0b81ad 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
dpkirchner pushed a commit to dpkirchner/charts that referenced this pull request May 9, 2019
@naiduarvind @dpkirchner
Utilize resource requests and limits (helm#11608)
d20afa0 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
goshlanguage pushed a commit to goshlanguage/charts that referenced this pull request May 17, 2019
@naiduarvind @goshlanguage
Utilize resource requests and limits (helm#11608)
fede565 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
@naiduarvind @eyenx
Utilize resource requests and limits (helm#11608)
b29faf5 
* Utilize resource requests and limit

Signed-off-by: Arvind Naidu <[email protected]>

* Bump Chart version and update resources template

Signed-off-by: Arvind Naidu <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cpanato cpanato cpanato left review comments

@paulczar paulczar Awaiting requested review from paulczar

Assignees

@unguiculus unguiculus

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@naiduarvind @k8s-ci-robot @paulczar @cpanato @unguiculus @helm-bot @arvind-naidu