Skip to content

Commit

Permalink
return grpc errors instead of temporary from CreateVolume
Browse files Browse the repository at this point in the history
  • Loading branch information
riteshghorse committed Aug 19, 2024
1 parent 22f5f2e commit 60c6e08
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
7 changes: 7 additions & 0 deletions pkg/cloud_provider/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,13 @@ func existingErrorCode(err error) *codes.Code {
return nil
}

var te *common.TemporaryError
// explicitly check if the error type is a `common.TemporaryError`.
if errors.As(err, &te) {
if status, ok := status.FromError(err); ok {
return util.ErrCodePtr(status.Code())
}
}
// We want to make sure we catch other error types that are statusable.
// (eg. grpc-go/internal/status/status.go Error struct that wraps a status)
var googleErr *googleapi.Error
Expand Down
7 changes: 3 additions & 4 deletions pkg/csi_driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/klog/v2"
cloud "sigs.k8s.io/gcp-filestore-csi-driver/pkg/cloud_provider"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/cloud_provider/file"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/common"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/metrics"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/util"
)
Expand Down Expand Up @@ -183,7 +182,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu

if err != nil {
klog.Errorf("CreateVolume returned an error %v, for request %+v", err, req)
return nil, err
return nil, file.StatusError(err)
}
klog.Infof("CreateVolume response %v, for request %+v", response, req)
return response, nil
Expand Down Expand Up @@ -244,7 +243,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
if err != nil && !file.IsNotFoundErr(err) {
// Failed to GetInstance, however the Filestore instance may already be created.
// The error should be non-final.
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("CreateVolume, failed to get instance: %w", err))
return nil, file.StatusError(err)
}

if filer != nil {
Expand All @@ -262,7 +261,7 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
if filer.State != "READY" {
msg := fmt.Sprintf("Volume %v not ready, current state: %v", name, filer.State)
klog.V(4).Infof(msg)
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf(msg))
return nil, status.Errorf(codes.Unavailable, msg)
}
} else {
param := req.GetParameters()
Expand Down
11 changes: 7 additions & 4 deletions pkg/csi_driver/multishare_stateful_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package driver

import (
"fmt"

csi "github.com/container-storage-interface/spec/lib/go/csi"
"golang.org/x/net/context"
"google.golang.org/grpc/codes"
Expand All @@ -29,6 +31,7 @@ import (
listers "sigs.k8s.io/gcp-filestore-csi-driver/pkg/client/listers/multishare/v1"
cloud "sigs.k8s.io/gcp-filestore-csi-driver/pkg/cloud_provider"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/cloud_provider/file"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/common"
"sigs.k8s.io/gcp-filestore-csi-driver/pkg/util"
)

Expand Down Expand Up @@ -94,13 +97,13 @@ func (m *MultishareStatefulController) CreateVolume(ctx context.Context, req *cs
shareInfo, err := m.shareLister.ShareInfos(util.ManagedFilestoreCSINamespace).Get(pvName)
if err != nil {
if !errors.IsNotFound(err) {
return nil, status.Errorf(codes.Unavailable, "error getting shareInfo %q from informer: %s", pvName, err.Error())
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error getting shareInfo %q from informer: %s", pvName, err.Error()))
}
klog.Infof("querying ShareInfo %q from api server", pvName)
shareInfo, err = m.clientset.MultishareV1().ShareInfos(util.ManagedFilestoreCSINamespace).Get(context.TODO(), pvName, metav1.GetOptions{})
if err != nil {
if !errors.IsNotFound(err) {
return nil, status.Errorf(codes.Unavailable, "error getting shareInfo %q from api server: %s", pvName, err.Error())
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error getting shareInfo %q from api server: %s", pvName, err.Error()))
}
klog.V(6).Infof("shareInfo object for share %q not found in API server", pvName)
shareInfo = nil
Expand Down Expand Up @@ -131,7 +134,7 @@ func (m *MultishareStatefulController) CreateVolume(ctx context.Context, req *cs
klog.V(6).Infof("trying to create shareInfo object: %v", shareInfo)
shareInfo, err = m.createShareInfo(ctx, shareInfo)
if err != nil {
return nil, status.Errorf(codes.Unavailable, "error creating share object: %s", err.Error())
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error creating share object: %s", err.Error()))
}
}

Expand All @@ -141,7 +144,7 @@ func (m *MultishareStatefulController) CreateVolume(ctx context.Context, req *cs

if shareInfo.Status.ShareStatus != v1.READY {
if shareInfo.Status.Error != "" {
return nil, status.Errorf(codes.Unavailable, "error fetching share status: %s", shareInfo.Status.Error)
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error fetching share status: %s", shareInfo.Status.Error))
}
return nil, status.Errorf(codes.Aborted, "share %s is not ready yet", pvName)
}
Expand Down

0 comments on commit 60c6e08

Please sign in to comment.