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.

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

Change error reporting to temporary in CreateVolume workflow #924

Merged
merged 3 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
leiyiz marked this conversation as resolved.
Show resolved Hide resolved
}
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