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

[AWS] Set max results if its not set #106280

Merged
merged 1 commit into from Dec 28, 2021
Merged

[AWS] Set max results if its not set #106280

merged 1 commit into from Dec 28, 2021

Conversation

nckturner
Copy link
Contributor

What type of PR is this?

/kind bug
/sig cloud-provider
/cc @cheftako

What this PR does / why we need it:

For the legacy AWS cloud provider, if max results is not set and instance IDs are not provided for the DescribeInstances call, set max results to 1000. This prevents an expensive call against the EC2 API, which can result in timeouts or errors from the EC2 API.

Which issue(s) this PR fixes:

Ref: kubernetes/cloud-provider-aws#274
Ref: kubernetes/cloud-provider-aws#269

Special notes for your reviewer:

This PR is back porting a bug fix from the upstream AWS cloud provider, and is therefore a candidate to be merged to the legacy AWS cloud provider.

Does this PR introduce a user-facing change?

NONE

@nckturner
[AWS] Set max results if its not set
98f26a2 
* If max results is not set and instance IDs are not provided for the
  describe instances call in the aws cloud provider, set max results.
* This prevents an expensive call against the EC2 API, which can
  result in timeouts.
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 9, 2021
@k8s-ci-robot
Copy link
Contributor

@nckturner: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/cloudprovider labels Nov 9, 2021
@ialidzhikov
Copy link
Contributor

/retest

@ialidzhikov
Copy link
Contributor

/assign @cheftako @wongma7

@wongma7
Copy link
Contributor

wongma7 commented Dec 13, 2021

/lgtm

@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
@ialidzhikov
Copy link
Contributor

ping @cheftako
for review/approval

1 similar comment
@ialidzhikov
Copy link
Contributor

ping @cheftako
for review/approval

@cheftako
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, nckturner

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 28, 2021
@k8s-ci-robot k8s-ci-robot merged commit f5be505 into kubernetes:master Dec 28, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Dec 28, 2021
@ialidzhikov
Copy link
Contributor

@nckturner can you please backport this change to all maintained release branches?

@ialidzhikov
Copy link
Contributor

@nckturner can you please backport this change to all maintained release branches?

Any news @nckturner ?

@nckturner
Copy link
Contributor Author

Yes, I can do that.

@ialidzhikov
Copy link
Contributor

Yes, I can do that.

Thanks, looking forward to it!

This was referenced Jan 20, 2022
Merged
Merged
This was referenced Jan 20, 2022
Merged
Closed
k8s-ci-robot added a commit that referenced this pull request Jan 28, 2022
@k8s-ci-robot
Merge pull request #107652 from nckturner/automated-cherry-pick-of-#1…
559b798 
…06280-upstream-release-1.22

Automated cherry pick of #106280: Set max results if its not set
k8s-ci-robot added a commit that referenced this pull request Jan 28, 2022
@k8s-ci-robot
Merge pull request #107653 from nckturner/automated-cherry-pick-of-#1…
b37aecf 
…06280-upstream-release-1.21

Automated cherry pick of #106280: Set max results if its not set
k8s-ci-robot added a commit that referenced this pull request Jan 28, 2022
@k8s-ci-robot
Merge pull request #107651 from nckturner/automated-cherry-pick-of-#1…
a085e4c 
…06280-upstream-release-1.23

Automated cherry pick of #106280: Set max results if its not set
Swizzmaster pushed a commit to Swizzmaster/kubernetes that referenced this pull request Feb 29, 2024
@nckturner
Use a private tag in commit messages
b38a2d0 
* Use --EKS-PRIVATE-- in commit messages
  to determine which commits are intended
  to be private.
* When the prepare_patches.sh script tries
  to determine the number of private patches,
  it looks at the EKSDataPlaneKubernetes
  repository, so the number of private and public
  patches is based off the patches you have
  in EKSDataPlaneKubernetes.
* Use 0-public and 1-private for ordering sanity
* Remove individual files instead of entire directory.  Structure
  remove in a loop with a string concatenation so the target path
  is never empty.
* Add max results patch for kubernetes#106280
* See the upstream PR
  (kubernetes#106280)
* DROP in 1.24

cr: https://code.amazon.com/reviews/CR-64014162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cheftako cheftako Awaiting requested review from cheftako

Assignees

@cheftako cheftako

@wongma7 wongma7

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cloudprovider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.24
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nckturner @k8s-ci-robot @ialidzhikov @wongma7 @cheftako