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

Return typed errors for Azure client interfaces #86555

Merged
merged 4 commits into from Dec 25, 2019
Merged

Return typed errors for Azure client interfaces #86555

merged 4 commits into from Dec 25, 2019

Conversation

feiskyer
Copy link
Member

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

/kind feature
/sig cloud-provider
/area provider/azure
/priority important-soon

What this PR does / why we need it:

Continue of kubernetes-sigs/cloud-provider-azure#247, return typed errors for Azure client interfaces:

  • Add a new retry.Error
  • Return retry.Error for Azure client interfaces
  • Switch to new returned errors in other places

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. area/provider/azure Issues or PRs related to azure provider priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 23, 2019
@feiskyer
Copy link
Member Author

/test pull-kubernetes-e2e-aks-engine-azure
/test pull-kubernetes-e2e-azure-disk
/test pull-kubernetes-e2e-azure-disk-vmss

@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 23, 2019
@k8s-ci-robot k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 24, 2019
@feiskyer
Copy link
Member Author

/assign @andyzhangx @justaugustus

@feiskyer
Copy link
Member Author

/test pull-kubernetes-e2e-aks-engine-azure
/test pull-kubernetes-e2e-azure-disk
/test pull-kubernetes-e2e-azure-disk-vmss

@feiskyer
Copy link
Member Author

/test pull-kubernetes-integration

andyzhangx
andyzhangx reviewed Dec 24, 2019
View reviewed changes
staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error.go
HTTPStatusCode int
// RetryAfter indicates the time when the request should retry after throttling.
// A throttled request is retriable.
RetryAfter time.Time
Copy link
Member

Choose a reason for hiding this comment

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

RetryAfter is not used in this PR, right?

Copy link
Member Author

@feiskyer feiskyer Dec 24, 2019 •
edited

Choose a reason for hiding this comment

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

Right, it would be used in the following PRs.

@feiskyer
Copy link
Member Author

/retest

andyzhangx
andyzhangx approved these changes Dec 24, 2019
View reviewed changes
Copy link
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

/lgtm
/test pull-kubernetes-e2e-aks-engine-azure
/test pull-kubernetes-e2e-azure-disk
/test pull-kubernetes-e2e-azure-disk-vmss
/hold
hold until all tests pass

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Dec 24, 2019
@feiskyer
Copy link
Member Author

test passed.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 25, 2019
@feiskyer
Copy link
Member Author

@dims @liggitt could you help to approve the vendor updates?

/assign @dims @liggitt

@liggitt
Copy link
Member

liggitt commented Dec 25, 2019

/approve
for modules.txt change
all other packages already approved by existing owners

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feiskyer, liggitt

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 25, 2019
@feiskyer
Copy link
Member Author

/retest

@feiskyer
Copy link
Member Author

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit 6c1080b into kubernetes:master Dec 25, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Dec 25, 2019
@feiskyer feiskyer deleted the azure-retry-error branch December 25, 2019 12:17
@feiskyer feiskyer mentioned this pull request Dec 27, 2019
Merged
@phiphi282 phiphi282 mentioned this pull request Aug 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andyzhangx andyzhangx andyzhangx approved these changes

@justaugustus justaugustus Awaiting requested review from justaugustus

@karataliu karataliu Awaiting requested review from karataliu

Assignees

@dims dims

@justaugustus justaugustus

@liggitt liggitt

@andyzhangx andyzhangx

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cloudprovider area/provider/azure Issues or PRs related to azure provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.18
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@feiskyer @liggitt @k8s-ci-robot @andyzhangx @dims @justaugustus