Use original error code when backing off

[mattcary]

/kind cleanup

This will make it easier to filter errors property for alerting.

-->

Use original error code when responding with a backoff error on publish or unpublish.

/assign @amacaskill

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

• OWNERS [mattcary]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[amacaskill]

code != nil will only happen when the error passed to CodeForError in next(backoffId, common.CodeForError(err)) is nil. In CodeForError, we default to Internal error codes when the error cannot be converted to a user error. See CodeForError, so I don't think your code is doing what you want.

Just a thought: CodeForError returns a pointer because it wasn't always used within an err != nil statement. Now that CodeForError is only called from LoggedError, and LoggedError clearly is logging an error, then we're basically guaranteed err != nil in CodeForError, so we can probably make CodeForError and the functions it calls to return a code.Code not a pointer to a code.Code. That would clear up some misunderstanding about when things are nil and what the default is, but that isn't necessary for this PR.

[amacaskill]

An optional suggestion: you could make the parameters, next(id csiErrorBackoffId, err error) and call CodeForError(err) within next(() which would allow you to effectively test the defaulting behavior of next()

[mattcary]

I like making CodeForError return a code and not a pointer, I think that would fix your objection?

I think the main thing is if the code is unset then returning Unavailble, which matches the existing backoff error code.

[amacaskill]

Changing CodeForErrror to return not a pointer helps, but I still have a comment about "I think the main thing is if the code is unset then returning Unavailble, which matches the existing backoff error code."

https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/pull/1298/files#r1270954464

[amacaskill]

/lgtm

[mattcary]

/cherry-pick release-1.10

[mattcary]

/cherry-pick release-1.9

[k8s-infra-cherrypick-robot]

@mattcary: new pull request created: #1311

In response to this:

/cherry-pick release-1.10

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-infra-cherrypick-robot]

@mattcary: new pull request created: #1312

In response to this:

/cherry-pick release-1.9

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.

[judemars]

/cherry-pick release-1.8

[k8s-infra-cherrypick-robot]

@judemars: #1298 failed to apply on top of branch "release-1.8":

Applying: Use original error code when backing off
Using index info to reconstruct a base tree...
M	pkg/common/utils.go
M	pkg/common/utils_test.go
M	pkg/gce-pd-csi-driver/controller.go
M	pkg/gce-pd-csi-driver/controller_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/gce-pd-csi-driver/controller_test.go
CONFLICT (content): Merge conflict in pkg/gce-pd-csi-driver/controller_test.go
Auto-merging pkg/gce-pd-csi-driver/controller.go
Auto-merging pkg/common/utils_test.go
Auto-merging pkg/common/utils.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use original error code when backing off
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.8

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.