-
Notifications
You must be signed in to change notification settings - Fork 159
Reassign error returned from validateStoragePools so InvalidArgument is recorded #1710
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.
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
Reassign error returned from validateStoragePools so InvalidArgument is recorded #1710
Conversation
…error code is recorded instead of internal error code
5ce3f25
to
8aec1f5
Compare
/assign @mattcary |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amacaskill, mattcary 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 |
/retest |
/test pull-gcp-compute-persistent-disk-csi-driver-kubernetes-integration |
/easycla |
/cherry-pick release-1.13 |
@amacaskill: new pull request created: #1721 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. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Reassign error returned from validateStoragePools so InvalidArgument is recorded. Currently, since the error is not reassigned to have the InvalidArgumentCode, when
defer gceCS.Metrics.RecordOperationErrorMetrics("CreateVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
is executed, it passes an error that looks like fmt.Errorf("error") toRecordOperationErrorMetrics
, which is technically an Internal error.I'm fixing this first for Storage Pools, but it looks like we have this issue in a lot of places. In some cases, we don't reassign the error at all, meaning
defer gceCS.Metrics.RecordOperationErrorMetrics("CreateVolume", err, diskTypeForMetric, enableConfidentialCompute, enableStoragePools)
will actually pass a nil error, resulting in a OK status CreateVolume call being reported to the operation error metric. In some places (like in the Storage Pools case), we reassign the err, but we add an error code onto the error before returning. This won't get recorded to RecordOperationErrorMetrics unless we actually reassign this error, or implement RecordOperationErrorMetrics in a different way.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: