Improve resilience of diskTypes requests. #7923
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @Uladzislau97. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
k8s-ci-robot
added
area/cluster-autoscaler
area/provider/gce
size/L
|
/uncc @jayantjain93 |
|
/cc @pmendelski |
|
@Uladzislau97: GitHub didn't allow me to request PR reviews from the following users: pmendelski. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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-sigs/prow repository. |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/cc @pmendelski |
|
@Uladzislau97: GitHub didn't allow me to request PR reviews from the following users: pmendelski. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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-sigs/prow repository. |
|
/uncc @jayantjain93 |
|
/cc @krzysied |
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
|
/uncc @krzysied |
k8s-ci-robot
added
size/S
| // By using the provided region, the filter effectively selects all matching zones within that region. | ||
| // For example, if region is "us-central1", the filter will match all zones in that region (e.g., "us-central1-a", "us-central1-b"). | ||
| filter := fmt.Sprintf("zone eq .+%s.*", region) | ||
| req := client.gceService.DiskTypes.AggregatedList(client.projectId).Filter(filter).ReturnPartialSuccess(true) |
There was a problem hiding this comment.
How do we distinguish partial failure with information from region missing from a full success?
There was a problem hiding this comment.
Based on documentation we cannot get not full list of diskTypes for particular zone. It says "when partial success behavior is enabled, aggregatedList for a single zone scope either returns all resources in the zone or no resources, with an error code." So if we didn't get warning code for the zone it means we got full list of diskTypes.
There was a problem hiding this comment.
But you're not checking per-zone errors anywhere, so if a zonal API does go down, CA will think there are no disks in that zone.
There was a problem hiding this comment.
Currently CA will just get a mapping without requested zone and will throw error "no available disk types found for zone".
But we can send additional error from API to the CA to make it more clear why diskTypes request failed.
There was a problem hiding this comment.
Yes, let's be clear about the error returned. If one is interested in disks from a given region, the function fetching them should return an error upon failure to get a complete response for that region.
There was a problem hiding this comment.
Updated FetchAvailableDiskTypes to call diskTypes.list.
k8s-ci-robot
added
size/M
k8s-ci-robot
added
size/L
…AvailableDiskTypes.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Uladzislau97, x13n 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 |
k8s-ci-robot
added
the
approved
What type of PR is this?
/kind feature
/kind api-change
What this PR does / why we need it:
Replace diskTypes.aggregatedList request with diskTypes.list in FetchAvailableDiskTypes
Which issue(s) this PR fixes:
This PR fixes an issue where, during outages in some zones, information about other available zonesis not retrieved.
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: