From 6d1ab1b8d92e400814bd6a822212a02a8feeabe9 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 5 Aug 2024 18:27:15 +0200 Subject: [PATCH] rbd: have GetCreationTime() return a time.Time struct Do not use protobuf types when there is no need. Just use the standard time.Time format instead. Signed-off-by: Niels de Vos --- internal/csi-addons/rbd/replication.go | 12 ++++++------ internal/csi-addons/rbd/replication_test.go | 10 +++++----- internal/rbd/controllerserver.go | 5 +++-- internal/rbd/rbd_util.go | 9 ++++----- internal/rbd/snapshot.go | 13 +++++++++++++ internal/rbd/types/volume.go | 5 +++-- 6 files changed, 34 insertions(+), 20 deletions(-) diff --git a/internal/csi-addons/rbd/replication.go b/internal/csi-addons/rbd/replication.go index f58750892..f87a24401 100644 --- a/internal/csi-addons/rbd/replication.go +++ b/internal/csi-addons/rbd/replication.go @@ -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()) @@ -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()) } @@ -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) @@ -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 diff --git a/internal/csi-addons/rbd/replication_test.go b/internal/csi-addons/rbd/replication_test.go index b4e81bf95..ba7212483 100644 --- a/internal/csi-addons/rbd/replication_test.go +++ b/internal/csi-addons/rbd/replication_test.go @@ -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 @@ -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, }, { @@ -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) } }) } diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 56de60c58..b020bdef0 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -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 ( @@ -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 @@ -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 diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 6f9120719..a69b0e632 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -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" @@ -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 @@ -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 } @@ -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 } diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index fc3fa2427..15f1db5cb 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -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" ) @@ -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, diff --git a/internal/rbd/types/volume.go b/internal/rbd/types/volume.go index 388676cf5..fb2b66275 100644 --- a/internal/rbd/types/volume.go +++ b/internal/rbd/types/volume.go @@ -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 @@ -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.