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.

Sign up for GitHub

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

Jump to bottom

[e2e] Increase the vCPU quota limit for EC2 instances #3002

Merged
merged 1 commit into from
Dec 13, 2021
Merged

[e2e] Increase the vCPU quota limit for EC2 instances #3002

merged 1 commit into from
Dec 13, 2021

Conversation

Ankitasw
Copy link
Member

@Ankitasw Ankitasw commented Dec 6, 2021 •
edited
Loading

What type of PR is this?
/kind failing-test

What this PR does / why we need it:
This PR fixes the vCPU quota limit issues while executing E2E upstream.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

  • squashed commits
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Release note:

None

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 6, 2021
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 6, 2021
@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 6, 2021

/priority critical-urgent
/triage accepted

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 6, 2021
sedefsavas
sedefsavas reviewed Dec 6, 2021
View reviewed changes
test/e2e/shared/defaults.go Outdated
ServiceCode: "ec2",
QuotaName: "Running On-Demand G and VT instances",
QuotaCode: "L-DB2E81BA",
DesiredMinimumValue: 32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this many vCPUs? For 1 GPU node --> 4 vCPU enough.

Your current vCPU limit is Running On-Demand All G instances = 8 vCPU.

A g4dn.xlarge instance has a footprint of 4 vCPU. Your account can already run two g4dn.xlarge instances, based on your G instances limit of 8.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reduced it to 8 for now

@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 6, 2021

/test pull-cluster-api-provider-aws-e2e

@sedefsavas
Copy link
Contributor

Test took 5 hours and timed out.

@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 7, 2021 •
edited
Loading

Test took 5 hours and timed out.

Yeah, that is weird, is it because we added some wrong value? Couldn't figure out the problem in local run as it works fine with my AWS account.

@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 7, 2021

/retest

@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 7, 2021 •
edited
Loading

@sedefsavas I see below log

Requesting service quota increase for ec2/Running On-Demand G and VT instances to 8

But again its getting stuck at same place while acquiring resources, not sure how should we proceed

@sedefsavas
Copy link
Contributor

sedefsavas commented Dec 7, 2021 •
edited
Loading

This is the problem:

classiclb: 20
ec2: 0
eip: 100
igw: 20
ngw: 20
vpc: 20

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-aws/3002/pull-cluster-api-provider-aws-e2e/1467902093854511104/artifacts/initial-resource-quotas.yaml

Until we figure out what the problem is, let's not run e2e test here. Takes 5 hours and blocks other PR's e2e tests.
I'd suggest to check if the change works locally by looking at your own AWS account to see if you can increase ec2 instances quota.

sedefsavas
sedefsavas reviewed Dec 7, 2021
View reviewed changes
test/e2e/shared/defaults.go Outdated
@@ -154,6 +154,13 @@ func getLimitedResources() map[string]*ServiceQuota {
QuotaCode: "L-E9E9831D",
DesiredMinimumValue: 20,
}

serviceQuotas["ec2"] = &ServiceQuota{
Copy link
Contributor

Choose a reason for hiding this comment

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

crit. serviceQuotas is a map, it is being overwritten here as we already set it for regular instances above. Need to have a different name for normal instances and GPU ones.

@sedefsavas
Copy link
Contributor

Also, we should update initial resources file as ec2-normal and ec2-GPU, since their quotas are different.

@sedefsavas
Copy link
Contributor

Disabled for now: #3007

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 8, 2021
@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 8, 2021 •
edited
Loading

I'd suggest to check if the change works locally by looking at your own AWS account to see if you can increase ec2 instances quota.

Current change works fine locally as the resource quota limit is sufficient in my AWS account, that's why trying to trigger and check in PR, looks like it should go through now

@pydctw pydctw mentioned this pull request Dec 8, 2021
Merged
4 tasks
@Ankitasw Ankitasw changed the title [e2e] Increase the vCPU quota limit for G instances [e2e] Increase the vCPU quota limit for EC2 instances Dec 9, 2021
@Ankitasw
Copy link
Member Author

Ankitasw commented Dec 9, 2021

/test pull-cluster-api-provider-aws-e2e

@Ankitasw Ankitasw force-pushed the e2e-gpu-test branch 2 times, most recently from de3e511 to d1c333b Compare December 10, 2021 14:03
@Ankitasw
Copy link
Member Author

/retest

@Ankitasw Ankitasw force-pushed the e2e-gpu-test branch 2 times, most recently from fb61838 to 50425d1 Compare December 13, 2021 09:48
@Ankitasw
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@Ankitasw
Copy link
Member Author

/assign @sedefsavas for approval

@k8s-ci-robot
Copy link
Contributor

@Ankitasw: GitHub didn't allow me to assign the following users: for, approval.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @sedefsavas for approval

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.

@sedefsavas
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sedefsavas

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 Dec 13, 2021
@k8s-ci-robot k8s-ci-robot merged commit 0672892 into kubernetes-sigs:main Dec 13, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.x milestone Dec 13, 2021
@Ankitasw Ankitasw deleted the e2e-gpu-test branch December 14, 2021 04:44
richardchen-db pushed a commit to databricks/cluster-api-provider-aws-1 that referenced this pull request Jan 14, 2023
@k8s-ci-robot
Merge pull request kubernetes-sigs#3002 from Ankitasw/e2e-gpu-test
dc4f58f 
[e2e] Increase the vCPU quota limit for EC2 instances
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlipovetsky dlipovetsky Awaiting requested review from dlipovetsky

@richardcase richardcase Awaiting requested review from richardcase

@sedefsavas sedefsavas Awaiting requested review from sedefsavas

Assignees

@sedefsavas sedefsavas

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ankitasw @sedefsavas @k8s-ci-robot