From 97a850e2fecff2c4920c53baebbd30ba8cac72ea Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 5 Aug 2024 18:27:15 +0200 Subject: [PATCH 1/3] 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 f58750892f1..f87a2440161 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 b4e81bf9519..ba7212483eb 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 56de60c58d0..b020bdef018 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 6f9120719e9..a69b0e63279 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 fc3fa242783..15f1db5cb97 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 388676cf566..fb2b6627546 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. From 3ea6ddc00b33ac658b7b75c560568ad55b84eff6 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Tue, 6 Aug 2024 18:02:34 +0200 Subject: [PATCH 2/3] journal: store CreationTime for VolumeGroups Signed-off-by: Niels de Vos --- internal/journal/volumegroupjournal.go | 29 +++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/internal/journal/volumegroupjournal.go b/internal/journal/volumegroupjournal.go index 6d36c9d9af2..8f81c0b4b07 100644 --- a/internal/journal/volumegroupjournal.go +++ b/internal/journal/volumegroupjournal.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "time" "github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util/log" @@ -75,6 +76,11 @@ type VolumeGroupJournal interface { // VolumeGroupJournalConfig contains the configuration. type VolumeGroupJournalConfig struct { Config + + // csiCreationTimeKey can hold the key for the time a group was + // created. At least RBD groups do not provide the creation time + // through API calls. + csiCreationTimeKey string } type volumeGroupJournalConnection struct { @@ -97,6 +103,7 @@ func NewCSIVolumeGroupJournal(suffix string) VolumeGroupJournalConfig { csiNameKey: "csi.volname", namespace: "", }, + csiCreationTimeKey: "csi.creationtime", } } @@ -229,6 +236,7 @@ func (vgjc *volumeGroupJournalConnection) CheckReservation(ctx context.Context, volGroupData.VolumeGroupAttributes = &VolumeGroupAttributes{} volGroupData.VolumeGroupAttributes.RequestName = savedVolumeGroupAttributes.RequestName volGroupData.VolumeGroupAttributes.VolumeMap = savedVolumeGroupAttributes.VolumeMap + volGroupData.VolumeGroupAttributes.CreationTime = savedVolumeGroupAttributes.CreationTime return volGroupData, nil } @@ -354,6 +362,12 @@ func (vgjc *volumeGroupJournalConnection) ReserveName(ctx context.Context, omapValues[cj.csiNameKey] = reqName omapValues[cj.csiImageKey] = groupName + t, err := time.Now().MarshalText() + if err != nil { + return "", "", err + } + omapValues[cj.csiCreationTimeKey] = string(t) + err = setOMapKeys(ctx, vgjc.connection, journalPool, cj.namespace, oid, omapValues) if err != nil { return "", "", err @@ -365,9 +379,10 @@ func (vgjc *volumeGroupJournalConnection) ReserveName(ctx context.Context, // VolumeGroupAttributes contains the request name and the volumeID's and // the corresponding snapshotID's. type VolumeGroupAttributes struct { - RequestName string // Contains the request name for the passed in UUID - GroupName string // Contains the group name - VolumeMap map[string]string // Contains the volumeID and the corresponding value mapping + RequestName string // Contains the request name for the passed in UUID + GroupName string // Contains the group name + CreationTime *time.Time // Contains the time of creation of the group + VolumeMap map[string]string // Contains the volumeID and the corresponding value mapping } func (vgjc *volumeGroupJournalConnection) GetVolumeGroupAttributes( @@ -390,13 +405,21 @@ func (vgjc *volumeGroupJournalConnection) GetVolumeGroupAttributes( log.WarningLog(ctx, "unable to read omap values: pool missing: %v", err) } + t := &time.Time{} + err = t.UnmarshalText([]byte(values[cj.csiCreationTimeKey])) + if err != nil { + t = nil + } + groupAttributes.RequestName = values[cj.csiNameKey] groupAttributes.GroupName = values[cj.csiImageKey] + groupAttributes.CreationTime = t // Remove request name key and group name key from the omap, as we are // looking for volumeID/snapshotID mapping delete(values, cj.csiNameKey) delete(values, cj.csiImageKey) + delete(values, cj.csiCreationTimeKey) groupAttributes.VolumeMap = map[string]string{} for k, v := range values { groupAttributes.VolumeMap[k] = v From a82b742717f3aba68bea7a35d1f908bb27a39902 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Thu, 8 Aug 2024 11:03:29 +0200 Subject: [PATCH 3/3] rbd: convert rbdVolume to rbdSnapshot After cloning the RBD snapshot, an rbdVolume is returned for the CSI.Snapshot object. In order to use the rbdSnapshot.ToCSI() function, the rbdVolume needs to be converted (back) to an rbdSnaphot. Signed-off-by: Niels de Vos --- internal/rbd/controllerserver.go | 27 ++++++++++++--------------- internal/rbd/snapshot.go | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index b020bdef018..f0ac0a47956 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -32,7 +32,6 @@ 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 ( @@ -1236,14 +1235,13 @@ func (cs *ControllerServer) CreateSnapshot( return nil, status.Error(codes.Internal, err.Error()) } + csiSnap, err := vol.toSnapshot().ToCSI(ctx) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + return &csi.CreateSnapshotResponse{ - Snapshot: &csi.Snapshot{ - SizeBytes: vol.VolSize, - SnapshotId: vol.VolID, - SourceVolumeId: req.GetSourceVolumeId(), - CreationTime: timestamppb.New(*vol.CreatedAt), - ReadyToUse: true, - }, + Snapshot: csiSnap, }, nil } @@ -1296,14 +1294,13 @@ func cloneFromSnapshot( } } + csiSnap, err := rbdSnap.ToCSI(ctx) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + return &csi.CreateSnapshotResponse{ - Snapshot: &csi.Snapshot{ - SizeBytes: rbdSnap.VolSize, - SnapshotId: rbdSnap.VolID, - SourceVolumeId: rbdSnap.SourceVolumeID, - CreationTime: timestamppb.New(*rbdSnap.CreatedAt), - ReadyToUse: true, - }, + Snapshot: csiSnap, }, nil } diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index 15f1db5cb97..fd7badb41e6 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -101,6 +101,27 @@ func cleanUpSnapshot( return nil } +func (rv *rbdVolume) toSnapshot() *rbdSnapshot { + return &rbdSnapshot{ + rbdImage: rbdImage{ + ClusterID: rv.ClusterID, + VolID: rv.VolID, + Monitors: rv.Monitors, + Pool: rv.Pool, + JournalPool: rv.JournalPool, + RadosNamespace: rv.RadosNamespace, + RbdImageName: rv.RbdImageName, + ImageID: rv.ImageID, + CreatedAt: rv.CreatedAt, + // copyEncryptionConfig cannot be used here because the volume and the + // snapshot will have the same volumeID which cases the panic in + // copyEncryptionConfig function. + blockEncryption: rv.blockEncryption, + fileEncryption: rv.fileEncryption, + }, + } +} + func (rbdSnap *rbdSnapshot) toVolume() *rbdVolume { return &rbdVolume{ rbdImage: rbdImage{ @@ -112,6 +133,7 @@ func (rbdSnap *rbdSnapshot) toVolume() *rbdVolume { RadosNamespace: rbdSnap.RadosNamespace, RbdImageName: rbdSnap.RbdSnapName, ImageID: rbdSnap.ImageID, + CreatedAt: rbdSnap.CreatedAt, // copyEncryptionConfig cannot be used here because the volume and the // snapshot will have the same volumeID which cases the panic in // copyEncryptionConfig function.