From 4a2513c13f3e7855daaed0a26659a054d937ba00 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 23 Mar 2022 10:39:28 -0400 Subject: [PATCH] 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. --- .changelog/12352.txt | 3 +++ command/volume_snapshot_list.go | 2 +- go.mod | 2 +- nomad/csi_endpoint_test.go | 3 +++ plugins/csi/client_test.go | 22 ++++++++++++++++------ plugins/csi/plugin.go | 2 +- 6 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 .changelog/12352.txt diff --git a/.changelog/12352.txt b/.changelog/12352.txt new file mode 100644 index 000000000000..688a1a9ba544 --- /dev/null +++ b/.changelog/12352.txt @@ -0,0 +1,3 @@ +```release-note:bug +csi: Fixed a bug where volume snapshot timestamps were always zero values +``` diff --git a/command/volume_snapshot_list.go b/command/volume_snapshot_list.go index ae0c0cd10eb5..aa32b6572a88 100644 --- a/command/volume_snapshot_list.go +++ b/command/volume_snapshot_list.go @@ -159,7 +159,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, )) } diff --git a/go.mod b/go.mod index 32372871e8fd..393fc4582246 100644 --- a/go.mod +++ b/go.mod @@ -120,6 +120,7 @@ require ( golang.org/x/sys v0.0.0-20211109065445-02f5c0300f6e golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e google.golang.org/grpc v1.42.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 ) @@ -248,7 +249,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 diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go index 26473ea413d3..50765cf5b1be 100644 --- a/nomad/csi_endpoint_test.go +++ b/nomad/csi_endpoint_test.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" "testing" + "time" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/acl" @@ -1119,12 +1120,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, } diff --git a/plugins/csi/client_test.go b/plugins/csi/client_test.go index 1c951b2bac96..73e7857b42a7 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -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" @@ -15,6 +16,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) { @@ -950,6 +952,8 @@ func TestClient_RPC_ControllerListVolume(t *testing.T) { func TestClient_RPC_ControllerCreateSnapshot(t *testing.T) { + now := time.Now() + cases := []struct { Name string Request *ControllerCreateSnapshotRequest @@ -984,6 +988,7 @@ func TestClient_RPC_ControllerCreateSnapshot(t *testing.T) { SizeBytes: 100000, SnapshotId: "snap-12345", SourceVolumeId: "vol-12345", + CreationTime: timestamppb.New(now), ReadyToUse: true, }, }, @@ -999,11 +1004,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()) } }) } @@ -1078,16 +1085,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{ { @@ -1096,6 +1102,7 @@ func TestClient_RPC_ControllerListSnapshots(t *testing.T) { SnapshotId: "snap-12345", SourceVolumeId: "vol-12345", ReadyToUse: true, + CreationTime: timestamppb.New(now), }, }, }, @@ -1110,7 +1117,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()) }) } } diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index 54c664b8fe63..55dcfb17e985 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -787,7 +787,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(), }, })