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.
hashicorp · Apr 19, 2022 · 49883fe · 49883fe
1 parent 2c40dba
commit 49883fe
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 9 deletions.
diff --git a/.changelog/12352.txt 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
@@ -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
@@ -134,6 +134,7 @@ require (
 	golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 // indirect
 	google.golang.org/api v0.13.0 // indirect
 	google.golang.org/grpc v1.29.1
+	google.golang.org/protobuf v1.27.1
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
 	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
 	gopkg.in/tomb.v2 v2.0.0-20140626144623-14b3d72120e8

diff --git a/go.sum b/go.sum
@@ -1002,8 +1002,9 @@ google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2
 google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU=
 google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
-google.golang.org/protobuf v1.26.0-rc.1 h1:7QnIQpGRHE5RnLKnESfDoxm2dTapTZua5a0kS0A+VXQ=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
+google.golang.org/protobuf v1.27.1 h1:SnqbnDw1V7RiZcXPx5MEeqPv2s79L9i7BJUlG/+RurQ=
+google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
 gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw=

diff --git 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
@@ -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
@@ -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(),
 				},
 			})