Our larger RHTAP tenants need more memory quota

[amfred]

By making containers a little less CPU-hungry by default, we're now able to start up more containers. But now we're running into memory quotas for the larger tenants. Meanwhile, we still have plenty of actual capacity on our clusters.

I also removed the CPU limit quota. There's still a CPU request quota.

Paired with: codeready-toolchain/toolchain-e2e#861

[openshift-ci]

Hi @amfred. Thanks for your PR.

I'm waiting for a codeready-toolchain 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.

[alexeykazakov]

Please also create a paired PR (with the same branch name as this one) for our e2e tests. See an example of such paired PR here: codeready-toolchain/toolchain-e2e#853

[amfred]

/hold
Let's see if your bots recognize that :-)

[alexeykazakov]

/ok-to-test

[amfred]

Yay, passing tests.

[amfred]

I was able to manually test with these settings and my PipelineRuns were able to complete:
Default LimitRange settings:

spec:
  limits:
    - type: Container
      default:
        memory: 2Gi
      defaultRequest:
        cpu: 200m
        memory: 256Mi

compute-build ResourceQuota:

spec:
  hard:
    limits.memory: 512Gi
    requests.cpu: '24'
    requests.memory: 128Gi
  scopes:
    - Terminating

compute-deploy ResourceQuota:

spec:
  hard:
    limits.memory: 32Gi
    requests.cpu: 1750m
    requests.memory: 32Gi
  scopes:
    - NotTerminating

So I think we're OK to go with this change.

[amfred]

/retest

[amfred]

/retest

[amfred]

/remove-hold

[alexeykazakov]

Let's make sure the PR is clean and doesn't contain unrelated changes. Please take a look at my comment.

[alexeykazakov]

I'm not sure what's going on but there is a bunch of unrelated changes in the PR which has been already merged into the master branch. Not sure why GitHub still showing them here...
Like this CRD for example.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amfred
Once this PR has been reviewed and has the lgtm label, please assign michaelkleinhenz for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
18.5% Duplication on New Code

See analysis details on SonarCloud

[amfred]

@bamachrn What are the new PRs that replace this?

[amfred]

/hold

[codecov]

Codecov Report

Merging #944 (efc5ab2) into master (dc76273) will decrease coverage by 0.02%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #944      +/-   ##
==========================================
- Coverage   83.75%   83.74%   -0.02%     
==========================================
  Files          53       53              
  Lines        6187     6187              
==========================================
- Hits         5182     5181       -1     
- Misses        820      821       +1     
  Partials      185      185              

see 1 file with indirect coverage changes

[mfrancisc]

Should we close this ? As I see there is a new PR #969