CSI: fix timestamp from volume snapshot responses (#12352)

Listing snapshots was incorrectly returning nanoseconds instead of
seconds, and formatting of timestamps both list and create snapshot
was treating the timestamp as though it were nanoseconds instead of
seconds. This resulted in create timestamps always being displayed as
zero values.

Fix the unit conversion error in the command line and the incorrect
extraction in the CSI plugin client code. Beef up the unit tests to
make sure this code is actually exercised.
This commit is contained in:
Tim Gross 2022-03-23 10:39:28 -04:00 committed by GitHub
parent b7075f04fd
commit 1743648901
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 26 additions and 10 deletions

3
.changelog/12352.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
csi: Fixed a bug where volume snapshot timestamps were always zero values
```

View File

@ -186,7 +186,7 @@ func csiFormatSnapshots(snapshots []*api.CSISnapshot, verbose bool) string {
v.ID,
limit(v.ExternalSourceVolumeID, length),
humanize.IBytes(uint64(v.SizeBytes)),
formatUnixNanoTime(v.CreateTime),
formatUnixNanoTime(v.CreateTime*1e9), // seconds to nanoseconds
v.IsReady,
))
}

2
go.mod
View File

@ -122,6 +122,7 @@ require (
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e
google.golang.org/grpc v1.44.0
google.golang.org/protobuf v1.27.1
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
gopkg.in/tomb.v2 v2.0.0-20140626144623-14b3d72120e8
oss.indeed.com/go/libtime v1.5.0
@ -265,7 +266,6 @@ require (
google.golang.org/api v0.60.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20211104193956-4c6863e31247 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/fsnotify.v1 v1.4.7 // indirect
gopkg.in/resty.v1 v1.12.0 // indirect

View File

@ -4,6 +4,7 @@ import (
"fmt"
"strings"
"testing"
"time"
msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
"github.com/hashicorp/nomad/acl"
@ -1322,12 +1323,14 @@ func TestCSIVolumeEndpoint_CreateSnapshot(t *testing.T) {
testutil.WaitForLeader(t, srv.RPC)
now := time.Now().Unix()
fake := newMockClientCSI()
fake.NextCreateSnapshotError = nil
fake.NextCreateSnapshotResponse = &cstructs.ClientCSIControllerCreateSnapshotResponse{
ID: "snap-12345",
ExternalSourceVolumeID: "vol-12345",
SizeBytes: 42,
CreateTime: now,
IsReady: true,
}

View File

@ -6,6 +6,7 @@ import (
"fmt"
"path/filepath"
"testing"
"time"
csipbv1 "github.com/container-storage-interface/spec/lib/go/csi"
"github.com/golang/protobuf/ptypes/wrappers"
@ -16,6 +17,7 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"
)
func newTestClient(t *testing.T) (*fake.IdentityClient, *fake.ControllerClient, *fake.NodeClient, CSIPlugin) {
@ -990,6 +992,8 @@ func TestClient_RPC_ControllerListVolume(t *testing.T) {
func TestClient_RPC_ControllerCreateSnapshot(t *testing.T) {
ci.Parallel(t)
now := time.Now()
cases := []struct {
Name string
Request *ControllerCreateSnapshotRequest
@ -1024,6 +1028,7 @@ func TestClient_RPC_ControllerCreateSnapshot(t *testing.T) {
SizeBytes: 100000,
SnapshotId: "snap-12345",
SourceVolumeId: "vol-12345",
CreationTime: timestamppb.New(now),
ReadyToUse: true,
},
},
@ -1039,11 +1044,13 @@ func TestClient_RPC_ControllerCreateSnapshot(t *testing.T) {
// note: there's nothing interesting to assert about the response
// here other than that we don't throw a NPE during transformation
// from protobuf to our struct
_, err := client.ControllerCreateSnapshot(context.TODO(), tc.Request)
resp, err := client.ControllerCreateSnapshot(context.TODO(), tc.Request)
if tc.ExpectedErr != nil {
require.EqualError(t, err, tc.ExpectedErr.Error())
} else {
require.NoError(t, err, tc.Name)
require.NotZero(t, resp.Snapshot.CreateTime)
require.Equal(t, now.Second(), time.Unix(resp.Snapshot.CreateTime, 0).Second())
}
})
}
@ -1120,16 +1127,15 @@ func TestClient_RPC_ControllerListSnapshots(t *testing.T) {
},
}
now := time.Now()
for _, tc := range cases {
t.Run(tc.Name, func(t *testing.T) {
_, cc, _, client := newTestClient(t)
defer client.Close()
cc.NextErr = tc.ResponseErr
if tc.ResponseErr != nil {
// note: there's nothing interesting to assert here other than
// that we don't throw a NPE during transformation from
// protobuf to our struct
if tc.ResponseErr == nil {
cc.NextListSnapshotsResponse = &csipbv1.ListSnapshotsResponse{
Entries: []*csipbv1.ListSnapshotsResponse_Entry{
{
@ -1138,6 +1144,7 @@ func TestClient_RPC_ControllerListSnapshots(t *testing.T) {
SnapshotId: "snap-12345",
SourceVolumeId: "vol-12345",
ReadyToUse: true,
CreationTime: timestamppb.New(now),
},
},
},
@ -1152,7 +1159,10 @@ func TestClient_RPC_ControllerListSnapshots(t *testing.T) {
}
require.NoError(t, err, tc.Name)
require.NotNil(t, resp)
require.Len(t, resp.Entries, 1)
require.NotZero(t, resp.Entries[0].Snapshot.CreateTime)
require.Equal(t, now.Second(),
time.Unix(resp.Entries[0].Snapshot.CreateTime, 0).Second())
})
}
}
@ -1298,7 +1308,7 @@ func TestClient_RPC_NodePublishVolume(t *testing.T) {
}
func TestClient_RPC_NodeUnpublishVolume(t *testing.T) {
ci.Parallel(t)
cases := []struct {
Name string
ExternalID string

View File

@ -788,7 +788,7 @@ func NewListSnapshotsResponse(resp *csipbv1.ListSnapshotsResponse) *ControllerLi
SizeBytes: snap.GetSizeBytes(),
ID: snap.GetSnapshotId(),
SourceVolumeID: snap.GetSourceVolumeId(),
CreateTime: int64(snap.GetCreationTime().GetNanos()),
CreateTime: snap.GetCreationTime().GetSeconds(),
IsReady: snap.GetReadyToUse(),
},
})