Skip to content

Commit 186b447

Browse files
gman0gman0
authored
consolidate log and error messages (kubernetes#1582)
This commit consolidates share/volume terms used in various log and error messages all over the codebase manila-csi. Where possible, it replaces the term "share" with CSI term "volume". Share names are used to identify volumes, since that is the CO-facing identifier. Some log messages were also removed, as they weren't adding any more information than what was already available.
1 parent 3431885 commit 186b447

File tree

4 files changed

+48
-56
lines changed

4 files changed

+48
-56
lines changed

‎pkg/csi/manila/controllerserver.go‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
9797

9898
// Check for pending CreateVolume for this volume name
9999
if _, isPending := pendingVolumes.LoadOrStore(req.GetName(), true); isPending {
100-
return nil, status.Errorf(codes.Aborted, "a volume named %s is already being created", req.GetName())
100+
return nil, status.Errorf(codes.Aborted, "volume %s is already being processed", req.GetName())
101101
}
102102
defer pendingVolumes.Delete(req.GetName())
103103

@@ -132,22 +132,20 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
132132
}
133133

134134
if err = verifyVolumeCompatibility(sizeInGiB, req, share, shareOpts, cs.d.compatOpts, shareTypeCaps); err != nil {
135-
return nil, status.Errorf(codes.AlreadyExists, "a share named %s already exists, but is incompatible with the request: %v", req.GetName(), err)
135+
return nil, status.Errorf(codes.AlreadyExists, "volume %s already exists, but is incompatible with the request: %v", req.GetName(), err)
136136
}
137137

138138
// Grant access to the share
139139

140140
ad := getShareAdapter(shareOpts.Protocol)
141141

142-
klog.V(4).Infof("creating an access rule for share %s", share.ID)
143-
144142
accessRight, err := ad.GetOrGrantAccess(&shareadapters.GrantAccessArgs{Share: share, ManilaClient: manilaClient, Options: shareOpts})
145143
if err != nil {
146144
if err == wait.ErrWaitTimeout {
147-
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for access rights %s for share %s to become available", accessRight.ID, share.ID)
145+
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for access rule %s for volume %s to become available", accessRight.ID, share.Name)
148146
}
149147

150-
return nil, status.Errorf(codes.Internal, "failed to grant access for share %s: %v", share.ID, err)
148+
return nil, status.Errorf(codes.Internal, "failed to grant access to volume %s: %v", share.Name, err)
151149
}
152150

153151
// Check if compatibility layer is needed and can be used
@@ -197,7 +195,7 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
197195
}
198196

199197
if err := deleteShare(req.GetVolumeId(), manilaClient); err != nil {
200-
return nil, status.Errorf(codes.Internal, "failed to delete share %s: %v", req.GetVolumeId(), err)
198+
return nil, status.Errorf(codes.Internal, "failed to delete volume %s: %v", req.GetVolumeId(), err)
201199
}
202200

203201
return &csi.DeleteVolumeResponse{}, nil
@@ -223,7 +221,7 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
223221

224222
// Check for pending CreateSnapshots for this snapshot name
225223
if _, isPending := pendingSnapshots.LoadOrStore(req.GetName(), true); isPending {
226-
return nil, status.Errorf(codes.Aborted, "a snapshot named %s is already being created", req.GetName())
224+
return nil, status.Errorf(codes.Aborted, "snapshot %s is already being processed", req.GetName())
227225
}
228226
defer pendingSnapshots.Delete(req.GetName())
229227

@@ -237,32 +235,30 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
237235
sourceShare, err := manilaClient.GetShareByID(req.GetSourceVolumeId())
238236
if err != nil {
239237
if clouderrors.IsNotFound(err) {
240-
return nil, status.Errorf(codes.NotFound, "failed to create a snapshot (%s) for share %s because the share doesn't exist: %v", req.GetName(), req.GetSourceVolumeId(), err)
238+
return nil, status.Errorf(codes.NotFound, "failed to create snapshot %s for volume %s because the volume doesn't exist: %v", req.GetName(), req.GetSourceVolumeId(), err)
241239
}
242240

243-
return nil, status.Errorf(codes.Internal, "failed to retrieve source share %s when creating a snapshot (%s): %v", req.GetSourceVolumeId(), req.GetName(), err)
241+
return nil, status.Errorf(codes.Internal, "failed to retrieve source volume %s when creating snapshot %s: %v", req.GetSourceVolumeId(), req.GetName(), err)
244242
}
245243

246244
if strings.ToUpper(sourceShare.ShareProto) != cs.d.shareProto {
247-
return nil, status.Errorf(codes.InvalidArgument, "share protocol mismatch: requested a snapshot of %s share %s, but share protocol selector is set to %s",
245+
return nil, status.Errorf(codes.InvalidArgument, "share protocol mismatch: requested snapshot of %s volume %s, but share protocol selector is set to %s",
248246
sourceShare.ShareProto, req.GetSourceVolumeId(), cs.d.shareProto)
249247
}
250248

251249
// Retrieve an existing snapshot or create a new one
252250

253-
klog.V(4).Infof("creating a snapshot (%s) of share %s", req.GetName(), req.GetSourceVolumeId())
254-
255251
snapshot, err := getOrCreateSnapshot(req.GetName(), sourceShare.ID, manilaClient)
256252
if err != nil {
257253
if err == wait.ErrWaitTimeout {
258-
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for snapshot %s of share %s to become available", snapshot.ID, req.GetSourceVolumeId())
254+
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for snapshot %s of volume %s to become available", snapshot.ID, req.GetSourceVolumeId())
259255
}
260256

261257
if clouderrors.IsNotFound(err) {
262-
return nil, status.Errorf(codes.NotFound, "failed to create a snapshot (%s) for share %s because the share doesn't exist: %v", req.GetName(), req.GetSourceVolumeId(), err)
258+
return nil, status.Errorf(codes.NotFound, "failed to create snapshot %s for volume %s because the volume doesn't exist: %v", req.GetName(), req.GetSourceVolumeId(), err)
263259
}
264260

265-
return nil, status.Errorf(codes.Internal, "failed to create a snapshot (%s) of share %s: %v", req.GetName(), req.GetSourceVolumeId(), err)
261+
return nil, status.Errorf(codes.Internal, "failed to create a snapshot %s of volume %s: %v", req.GetName(), req.GetSourceVolumeId(), err)
266262
}
267263

268264
if err = verifySnapshotCompatibility(snapshot, req); err != nil {
@@ -284,12 +280,12 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
284280

285281
manilaErrMsg, err := lastResourceError(snapshot.ID, manilaClient)
286282
if err != nil {
287-
return nil, status.Errorf(codes.Internal, "snapshot %s of share %s is in error state, error description could not be retrieved: %v", snapshot.ID, req.GetSourceVolumeId(), err.Error())
283+
return nil, status.Errorf(codes.Internal, "snapshot %s of volume %s is in error state, error description could not be retrieved: %v", snapshot.ID, req.GetSourceVolumeId(), err.Error())
288284
}
289285

290-
return nil, status.Errorf(manilaErrMsg.errCode.toRpcErrorCode(), "snapshot %s of share %s is in error state: %s", snapshot.ID, req.GetSourceVolumeId(), manilaErrMsg.message)
286+
return nil, status.Errorf(manilaErrMsg.errCode.toRpcErrorCode(), "snapshot %s of volume %s is in error state: %s", snapshot.ID, req.GetSourceVolumeId(), manilaErrMsg.message)
291287
default:
292-
return nil, status.Errorf(codes.Internal, "an error occurred while creating a snapshot (%s) of share %s: snapshot is in an unexpected state: wanted creating/available, got %s",
288+
return nil, status.Errorf(codes.Internal, "an error occurred while creating snapshot %s of volume %s: snapshot is in an unexpected state: wanted creating/available, got %s",
293289
req.GetName(), req.GetSourceVolumeId(), snapshot.Status)
294290
}
295291

@@ -376,18 +372,18 @@ func (cs *controllerServer) ValidateVolumeCapabilities(ctx context.Context, req
376372
share, err := manilaClient.GetShareByID(req.GetVolumeId())
377373
if err != nil {
378374
if clouderrors.IsNotFound(err) {
379-
return nil, status.Errorf(codes.NotFound, "share %s not found: %v", req.GetVolumeId(), err)
375+
return nil, status.Errorf(codes.NotFound, "volume %s not found: %v", req.GetVolumeId(), err)
380376
}
381377

382-
return nil, status.Errorf(codes.Internal, "failed to retrieve share %s: %v", req.GetVolumeId(), err)
378+
return nil, status.Errorf(codes.Internal, "failed to retrieve volume %s: %v", req.GetVolumeId(), err)
383379
}
384380

385381
if share.Status != shareAvailable {
386382
if share.Status == shareCreating {
387-
return nil, status.Errorf(codes.Unavailable, "share %s is in transient creating state", share.ID)
383+
return nil, status.Errorf(codes.Unavailable, "volume %s is in transient creating state", req.GetVolumeId())
388384
}
389385

390-
return nil, status.Errorf(codes.FailedPrecondition, "share %s is in an unexpected state: wanted %s, got %s", share.ID, shareAvailable, share.Status)
386+
return nil, status.Errorf(codes.FailedPrecondition, "volume %s is in an unexpected state: wanted %s, got %s", req.GetVolumeId(), shareAvailable, share.Status)
391387
}
392388

393389
if !compareProtocol(share.ShareProto, cs.d.shareProto) {
@@ -445,15 +441,15 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi
445441
share, err := manilaClient.GetShareByID(req.GetVolumeId())
446442
if err != nil {
447443
if clouderrors.IsNotFound(err) {
448-
return nil, status.Errorf(codes.NotFound, "share %s not found: %v", req.GetVolumeId(), err)
444+
return nil, status.Errorf(codes.NotFound, "volume %s not found: %v", req.GetVolumeId(), err)
449445
}
450446

451-
return nil, status.Errorf(codes.Internal, "failed to retrieve share %s: %v", req.GetVolumeId(), err)
447+
return nil, status.Errorf(codes.Internal, "failed to retrieve volume %s: %v", req.GetVolumeId(), err)
452448
}
453449

454450
// Check for pending operations on this volume
455-
if _, isPending := pendingVolumes.LoadOrStore(share.Name, true); isPending {
456-
return nil, status.Errorf(codes.Aborted, "volume named %s is already being processed", share.Name)
451+
if _, isPending := pendingVolumes.LoadOrStore(req.GetVolumeId(), true); isPending {
452+
return nil, status.Errorf(codes.Aborted, "volume %s is already being processed", share.Name)
457453
}
458454
defer pendingVolumes.Delete(share.Name)
459455

‎pkg/csi/manila/nodeserver.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,46 +62,47 @@ func (ns *nodeServer) buildVolumeContext(volID volumeID, shareOpts *options.Node
6262
if shareOpts.ShareID != "" {
6363
share, err = manilaClient.GetShareByID(shareOpts.ShareID)
6464
if err != nil {
65+
errCode := codes.Internal
6566
if clouderrors.IsNotFound(err) {
66-
return nil, nil, status.Errorf(codes.NotFound, "share %s not found: %v", shareOpts.ShareID, err)
67+
errCode = codes.NotFound
6768
}
6869

69-
return nil, nil, status.Errorf(codes.Internal, "failed to retrieve share %s: %v", shareOpts.ShareID, err)
70+
return nil, nil, status.Errorf(errCode, "failed to retrieve volume with share ID %s: %v", shareOpts.ShareID, err)
7071
}
7172
} else {
7273
share, err = manilaClient.GetShareByName(shareOpts.ShareName)
7374
if err != nil {
75+
errCode := codes.Internal
7476
if clouderrors.IsNotFound(err) {
75-
return nil, nil, status.Errorf(codes.NotFound, "no share named %s found: %v", shareOpts.ShareName, err)
77+
errCode = codes.NotFound
7678
}
7779

78-
return nil, nil, status.Errorf(codes.Internal, "failed to retrieve share named %s: %v", shareOpts.ShareName, err)
80+
return nil, nil, status.Errorf(errCode, "failed to retrieve volume with share name %s: %v", shareOpts.ShareName, err)
7981
}
8082
}
8183

8284
// Verify the plugin supports this share
8385

8486
if strings.ToLower(share.ShareProto) != strings.ToLower(ns.d.shareProto) {
8587
return nil, nil, status.Errorf(codes.InvalidArgument,
86-
"wrong share protocol %s for volume %s (share ID %s), the plugin is set to operate in %s",
87-
share.ShareProto, volID, share.ID, ns.d.shareProto)
88+
"wrong share protocol %s for volume %s, the plugin is set to operate in %s",
89+
share.ShareProto, volID, ns.d.shareProto)
8890
}
8991

9092
if share.Status != shareAvailable {
9193
if share.Status == shareCreating {
92-
return nil, nil, status.Errorf(codes.Unavailable, "share %s for volume %s is in transient creating state", share.ID, volID)
94+
return nil, nil, status.Errorf(codes.Unavailable, "volume %s is in transient creating state", volID)
9395
}
9496

95-
return nil, nil, status.Errorf(codes.FailedPrecondition, "invalid share status for volume %s (share ID %s): expected 'available', got '%s'",
96-
volID, share.ID, share.Status)
97+
return nil, nil, status.Errorf(codes.FailedPrecondition, "invalid share status for volume %s: expected 'available', got '%s'",
98+
volID, share.Status)
9799
}
98100

99101
// Get the access right for this share
100102

101103
accessRights, err := manilaClient.GetAccessRights(share.ID)
102104
if err != nil {
103-
return nil, nil, status.Errorf(codes.Internal, "failed to list access rights for volume %s (share ID %s): %v",
104-
volID, share.ID, err)
105+
return nil, nil, status.Errorf(codes.Internal, "failed to list access rights for volume %s: %v", volID, err)
105106
}
106107

107108
for i := range accessRights {
@@ -112,8 +113,8 @@ func (ns *nodeServer) buildVolumeContext(volID volumeID, shareOpts *options.Node
112113
}
113114

114115
if accessRight == nil {
115-
return nil, nil, status.Errorf(codes.InvalidArgument, "cannot find access right %s for volume %s (share ID %s)",
116-
shareOpts.ShareAccessID, volID, share.ID)
116+
return nil, nil, status.Errorf(codes.InvalidArgument, "cannot find access right %s for volume %s",
117+
shareOpts.ShareAccessID, volID)
117118
}
118119

119120
// Retrieve list of all export locations for this share.

‎pkg/csi/manila/share.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ func getOrCreateShare(shareName string, createOpts *shares.CreateOpts, manilaCli
6363
}
6464
} else {
6565
// Something else is wrong
66-
return nil, 0, fmt.Errorf("failed to probe for a share named %s: %v", shareName, err)
66+
return nil, 0, fmt.Errorf("failed to retrieve volume %s: %v", shareName, err)
6767
}
6868
} else {
69-
klog.V(4).Infof("a share named %s already exists", shareName)
69+
klog.V(4).Infof("volume %s already exists", shareName)
7070
}
7171

7272
// It exists, wait till it's Available
@@ -81,7 +81,7 @@ func getOrCreateShare(shareName string, createOpts *shares.CreateOpts, manilaCli
8181
func deleteShare(shareID string, manilaClient manilaclient.Interface) error {
8282
if err := manilaClient.DeleteShare(shareID); err != nil {
8383
if clouderrors.IsNotFound(err) {
84-
klog.V(4).Infof("share %s not found, assuming it to be already deleted", shareID)
84+
klog.V(4).Infof("volume with share ID %s not found, assuming it to be already deleted", shareID)
8585
} else {
8686
return err
8787
}
@@ -97,13 +97,13 @@ func tryDeleteShare(share *shares.Share, manilaClient manilaclient.Interface) {
9797

9898
if err := manilaClient.DeleteShare(share.ID); err != nil {
9999
// TODO failure to delete a share in an error state needs proper monitoring support
100-
klog.Errorf("couldn't delete share %s in a roll-back procedure: %v", share.ID, err)
100+
klog.Errorf("couldn't delete volume %s in a roll-back procedure: %v", share.Name, err)
101101
return
102102
}
103103

104104
_, _, err := waitForShareStatus(share.ID, shareDeleting, "", true, manilaClient)
105105
if err != nil && err != wait.ErrWaitTimeout {
106-
klog.Errorf("couldn't retrieve share %s in a roll-back procedure: %v", share.ID, err)
106+
klog.Errorf("couldn't retrieve volume %s in a roll-back procedure: %v", share.Name, err)
107107
}
108108
}
109109

@@ -119,10 +119,10 @@ func extendShare(share *shares.Share, newSizeInGiB int, manilaClient manilaclien
119119
share, manilaErrCode, err := waitForShareStatus(share.ID, shareExtending, shareAvailable, false, manilaClient)
120120
if err != nil {
121121
if err == wait.ErrWaitTimeout {
122-
return status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for share %s to become available", share.ID)
122+
return status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume ID %s to become available", share.Name)
123123
}
124124

125-
return status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to resize share %s: %v", share.ID, err)
125+
return status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to resize volume %s: %v", share.Name, err)
126126
}
127127

128128
return nil

‎pkg/csi/manila/volumesource.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient"
2626
"k8s.io/cloud-provider-openstack/pkg/csi/manila/options"
2727
clouderrors "k8s.io/cloud-provider-openstack/pkg/util/errors"
28-
"k8s.io/klog/v2"
2928
)
3029

3130
type volumeCreator interface {
@@ -35,8 +34,6 @@ type volumeCreator interface {
3534
type blankVolume struct{}
3635

3736
func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeInGiB int, manilaClient manilaclient.Interface, shareOpts *options.ControllerVolumeContext, shareMetadata map[string]string) (*shares.Share, error) {
38-
klog.V(4).Infof("creating a new share (%s) in AZ %s", shareName, coalesceValue(shareOpts.AvailabilityZone))
39-
4037
createOpts := &shares.CreateOpts{
4138
AvailabilityZone: shareOpts.AvailabilityZone,
4239
ShareProto: shareOpts.Protocol,
@@ -51,15 +48,15 @@ func (blankVolume) create(req *csi.CreateVolumeRequest, shareName string, sizeIn
5148
share, manilaErrCode, err := getOrCreateShare(shareName, createOpts, manilaClient)
5249
if err != nil {
5350
if err == wait.ErrWaitTimeout {
54-
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for share %s to become available", share.ID)
51+
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume %s to become available", shareName)
5552
}
5653

5754
if manilaErrCode != 0 {
5855
// An error has occurred, try to roll-back the share
5956
tryDeleteShare(share, manilaClient)
6057
}
6158

62-
return nil, status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to create a share (%s): %v", shareName, err)
59+
return nil, status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to create volume %s: %v", shareName, err)
6360
}
6461

6562
return share, err
@@ -79,8 +76,6 @@ func (volumeFromSnapshot) create(req *csi.CreateVolumeRequest, shareName string,
7976
return nil, status.Error(codes.InvalidArgument, "snapshot ID cannot be empty")
8077
}
8178

82-
klog.V(4).Infof("restoring snapshot %s into a share (%s) in AZ %s", snapshotSource.GetSnapshotId(), shareName, coalesceValue(shareOpts.AvailabilityZone))
83-
8479
snapshot, err := manilaClient.GetSnapshotByID(snapshotSource.GetSnapshotId())
8580
if err != nil {
8681
if clouderrors.IsNotFound(err) {
@@ -113,15 +108,15 @@ func (volumeFromSnapshot) create(req *csi.CreateVolumeRequest, shareName string,
113108
share, manilaErrCode, err := getOrCreateShare(shareName, createOpts, manilaClient)
114109
if err != nil {
115110
if err == wait.ErrWaitTimeout {
116-
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for share %s to become available", share.ID)
111+
return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while waiting for volume %s to become available", share.Name)
117112
}
118113

119114
if manilaErrCode != 0 {
120115
// An error has occurred, try to roll-back the share
121116
tryDeleteShare(share, manilaClient)
122117
}
123118

124-
return nil, status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to restore snapshot %s into a share (%s): %v", snapshotSource.GetSnapshotId(), shareName, err)
119+
return nil, status.Errorf(manilaErrCode.toRpcErrorCode(), "failed to restore snapshot %s into volume %s: %v", snapshotSource.GetSnapshotId(), shareName, err)
125120
}
126121

127122
return share, err

0 commit comments

Comments
 (0)