Skip to content

Commit

Permalink
rbd: have GetCreationTime() return a time.Time struct
Browse files Browse the repository at this point in the history
Do not use protobuf types when there is no need. Just use the standard
time.Time format instead.

Signed-off-by: Niels de Vos <[email protected]>
  • Loading branch information
nixpanic committed Aug 9, 2024
1 parent 2fd9257 commit 97a850e
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 20 deletions.
12 changes: 6 additions & 6 deletions internal/csi-addons/rbd/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context,
return nil, status.Error(codes.Internal, err.Error())
}

creationTime, err := rbdVol.GetCreationTime()
creationTime, err := rbdVol.GetCreationTime(ctx)
if err != nil {
log.ErrorLog(ctx, err.Error())

Expand Down Expand Up @@ -693,7 +693,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context,
ready = checkRemoteSiteStatus(ctx, sts.GetAllSitesStatus())
}

creationTime, err := rbdVol.GetCreationTime()
creationTime, err := rbdVol.GetCreationTime(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get image info for %s: %s", rbdVol, err.Error())
}
Expand All @@ -717,8 +717,8 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context,
if sErr != nil {
return nil, status.Errorf(codes.Internal, "failed to parse image creation time: %s", sErr.Error())
}
log.DebugLog(ctx, "image %s, savedImageTime=%v, currentImageTime=%v", rbdVol, st, creationTime.AsTime())
if req.GetForce() && st.Equal(creationTime.AsTime()) {
log.DebugLog(ctx, "image %s, savedImageTime=%v, currentImageTime=%v", rbdVol, st, creationTime)
if req.GetForce() && st.Equal(*creationTime) {
err = mirror.Resync(ctx)
if err != nil {
return nil, getGRPCError(err)
Expand Down Expand Up @@ -746,8 +746,8 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context,
}

// timestampToString converts the time.Time object to string.
func timestampToString(st *timestamppb.Timestamp) string {
return fmt.Sprintf("seconds:%d nanos:%d", st.GetSeconds(), st.GetNanos())
func timestampToString(st *time.Time) string {
return fmt.Sprintf("seconds:%d nanos:%d", st.Unix(), st.Nanosecond())
}

// timestampFromString parses the timestamp string and returns the time.Time
Expand Down
10 changes: 5 additions & 5 deletions internal/csi-addons/rbd/replication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func TestGetGRPCError(t *testing.T) {
}

func Test_timestampFromString(t *testing.T) {
tm := timestamppb.Now()
tm := time.Now()
t.Parallel()
tests := []struct {
name string
Expand All @@ -627,8 +627,8 @@ func Test_timestampFromString(t *testing.T) {
}{
{
name: "valid timestamp",
timestamp: timestampToString(tm),
want: tm.AsTime().Local(),
timestamp: timestampToString(&tm),
want: tm,
wantErr: false,
},
{
Expand Down Expand Up @@ -669,8 +669,8 @@ func Test_timestampFromString(t *testing.T) {
if (err != nil) != tt.wantErr {
t.Errorf("timestampFromString() error = %v, wantErr %v", err, tt.wantErr)
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("timestampFromString() = %v, want %v", got, tt.want)
if !tt.want.Equal(got) {
t.Errorf("timestampFromString() = %q, want %q", got, tt.want)
}
})
}
Expand Down
5 changes: 3 additions & 2 deletions internal/rbd/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/kubernetes-csi/csi-lib-utils/protosanitizer"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"
)

const (
Expand Down Expand Up @@ -1240,7 +1241,7 @@ func (cs *ControllerServer) CreateSnapshot(
SizeBytes: vol.VolSize,
SnapshotId: vol.VolID,
SourceVolumeId: req.GetSourceVolumeId(),
CreationTime: vol.CreatedAt,
CreationTime: timestamppb.New(*vol.CreatedAt),
ReadyToUse: true,
},
}, nil
Expand Down Expand Up @@ -1300,7 +1301,7 @@ func cloneFromSnapshot(
SizeBytes: rbdSnap.VolSize,
SnapshotId: rbdSnap.VolID,
SourceVolumeId: rbdSnap.SourceVolumeID,
CreationTime: rbdSnap.CreatedAt,
CreationTime: timestamppb.New(*rbdSnap.CreatedAt),
ReadyToUse: true,
},
}, nil
Expand Down
9 changes: 4 additions & 5 deletions internal/rbd/rbd_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ import (
librbd "github.com/ceph/go-ceph/rbd"
"github.com/ceph/go-ceph/rbd/admin"
"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/golang/protobuf/ptypes/timestamp"
"google.golang.org/protobuf/types/known/timestamppb"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cloud-provider/volume/helpers"
mount "k8s.io/mount-utils"
Expand Down Expand Up @@ -138,7 +136,7 @@ type rbdImage struct {
// fileEncryption provides access to optional VolumeEncryption functions (e.g fscrypt)
fileEncryption *util.VolumeEncryption

CreatedAt *timestamp.Timestamp
CreatedAt *time.Time

// conn is a connection to the Ceph cluster obtained from a ConnPool
conn *util.ClusterConnection
Expand Down Expand Up @@ -1595,7 +1593,7 @@ func (rv *rbdVolume) setImageOptions(ctx context.Context, options *librbd.ImageO

// GetCreationTime returns the creation time of the image. if the image
// creation time is not set, it queries the image info and returns the creation time.
func (ri *rbdImage) GetCreationTime() (*timestamppb.Timestamp, error) {
func (ri *rbdImage) GetCreationTime(ctx context.Context) (*time.Time, error) {
if ri.CreatedAt != nil {
return ri.CreatedAt, nil
}
Expand Down Expand Up @@ -1648,8 +1646,9 @@ func (ri *rbdImage) getImageInfo() error {
if err != nil {
return err
}

t := time.Unix(tm.Sec, tm.Nsec)
ri.CreatedAt = timestamppb.New(t)
ri.CreatedAt = &t

return nil
}
Expand Down
13 changes: 13 additions & 0 deletions internal/rbd/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import (
"errors"
"fmt"

"github.com/container-storage-interface/spec/lib/go/csi"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/ceph/ceph-csi/internal/util"
"github.com/ceph/ceph-csi/internal/util/log"
)
Expand Down Expand Up @@ -118,6 +121,16 @@ func (rbdSnap *rbdSnapshot) toVolume() *rbdVolume {
}
}

func (rbdSnap *rbdSnapshot) ToCSI(ctx context.Context) (*csi.Snapshot, error) {
return &csi.Snapshot{
SizeBytes: rbdSnap.VolSize,
SnapshotId: rbdSnap.VolID,
SourceVolumeId: rbdSnap.SourceVolumeID,
CreationTime: timestamppb.New(*rbdSnap.CreatedAt),
ReadyToUse: true,
}, nil
}

func undoSnapshotCloning(
ctx context.Context,
parentVol *rbdVolume,
Expand Down
5 changes: 3 additions & 2 deletions internal/rbd/types/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ package types

import (
"context"
"time"

"github.com/container-storage-interface/spec/lib/go/csi"
"google.golang.org/protobuf/types/known/timestamppb"
)

//nolint:interfacebloat // more than 10 methods are needed for the interface
Expand All @@ -42,7 +42,8 @@ type Volume interface {
RemoveFromGroup(ctx context.Context, vg VolumeGroup) error

// GetCreationTime returns the creation time of the volume.
GetCreationTime() (*timestamppb.Timestamp, error)
GetCreationTime(ctx context.Context) (*time.Time, error)

// GetMetadata returns the value of the metadata key from the volume.
GetMetadata(key string) (string, error)
// SetMetadata sets the value of the metadata key on the volume.
Expand Down

0 comments on commit 97a850e

Please sign in to comment.