cleanup: do not pass EncodingVersion to GenerateVolID()

The only encoding version that exists is `1`. There is no need to have multiple constants for that version across different packages. Because there is only one version, `GenerateVolID()` does not really require it, and it can use a default version. If there is a need in the future to support an other encoding version, this can be revisited with a cleaner solution. Signed-off-by: Niels de Vos <[email protected]>
ceph · Mar 14, 2024 · 9d8728c · 9d8728c
1 parent 32de268
commit 9d8728c
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 28 deletions.
diff --git a/internal/cephfs/store/fsjournal.go b/internal/cephfs/store/fsjournal.go
@@ -192,7 +192,7 @@ func CheckVolExists(ctx context.Context,
 
 	// found a volume already available, process and return it!
 	vid.VolumeID, err = util.GenerateVolID(ctx, volOptions.Monitors, cr, volOptions.FscID,
-		"", volOptions.ClusterID, imageUUID, fsutil.VolIDVersion)
+		"", volOptions.ClusterID, imageUUID)
 	if err != nil {
 		return nil, err
 	}
@@ -299,7 +299,7 @@ func ReserveVol(ctx context.Context, volOptions *VolumeOptions, secret map[strin
 	volOptions.VolID = vid.FsSubvolName
 	// generate the volume ID to return to the CO system
 	vid.VolumeID, err = util.GenerateVolID(ctx, volOptions.Monitors, cr, volOptions.FscID,
-		"", volOptions.ClusterID, imageUUID, fsutil.VolIDVersion)
+		"", volOptions.ClusterID, imageUUID)
 	if err != nil {
 		return nil, err
 	}
@@ -345,7 +345,7 @@ func ReserveSnap(
 
 	// generate the snapshot ID to return to the CO system
 	vid.SnapshotID, err = util.GenerateVolID(ctx, volOptions.Monitors, cr, volOptions.FscID,
-		"", volOptions.ClusterID, imageUUID, fsutil.VolIDVersion)
+		"", volOptions.ClusterID, imageUUID)
 	if err != nil {
 		return nil, err
 	}
@@ -453,7 +453,7 @@ func CheckSnapExists(
 
 	// found a snapshot already available, process and return it!
 	sid.SnapshotID, err = util.GenerateVolID(ctx, volOptions.Monitors, cr, volOptions.FscID,
-		"", volOptions.ClusterID, snapUUID, fsutil.VolIDVersion)
+		"", volOptions.ClusterID, snapUUID)
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/cephfs/util/util.go b/internal/cephfs/util/util.go
@@ -20,9 +20,6 @@ package util
 type VolumeID string
 
 const (
-	// VolIDVersion is the version number of volume ID encoding scheme.
-	VolIDVersion uint16 = 1
-
 	// RadosNamespace to store CSI specific objects and keys.
 	RadosNamespace = "csi"
 )
diff --git a/internal/rbd/globals.go b/internal/rbd/globals.go
@@ -22,11 +22,6 @@ import (
 	"github.com/ceph/ceph-csi/internal/journal"
 )
 
-const (
-	// volIDVersion is the version number of volume ID encoding scheme.
-	volIDVersion uint16 = 1
-)
-
 var (
 	// CSIInstanceID is the instance ID that is unique to an instance of CSI, used when sharing
 	// ceph clusters across CSI instances, to differentiate omap names per CSI instance.

diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
@@ -192,7 +192,7 @@ func checkSnapCloneExists(
 	rbdSnap.VolSize = vol.VolSize
 	// found a snapshot already available, process and return its information
 	rbdSnap.VolID, err = util.GenerateVolID(ctx, rbdSnap.Monitors, cr, snapData.ImagePoolID, rbdSnap.Pool,
-		rbdSnap.ClusterID, snapUUID, volIDVersion)
+		rbdSnap.ClusterID, snapUUID)
 	if err != nil {
 		return false, err
 	}
@@ -328,7 +328,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 
 	// found a volume already available, process and return it!
 	rv.VolID, err = util.GenerateVolID(ctx, rv.Monitors, rv.conn.Creds, imageData.ImagePoolID, rv.Pool,
-		rv.ClusterID, rv.ReservedID, volIDVersion)
+		rv.ClusterID, rv.ReservedID)
 	if err != nil {
 		return false, err
 	}
@@ -405,7 +405,7 @@ func reserveSnap(ctx context.Context, rbdSnap *rbdSnapshot, rbdVol *rbdVolume, c
 	}
 
 	rbdSnap.VolID, err = util.GenerateVolID(ctx, rbdSnap.Monitors, cr, imagePoolID, rbdSnap.Pool,
-		rbdSnap.ClusterID, rbdSnap.ReservedID, volIDVersion)
+		rbdSnap.ClusterID, rbdSnap.ReservedID)
 	if err != nil {
 		return err
 	}
@@ -482,7 +482,7 @@ func reserveVol(ctx context.Context, rbdVol *rbdVolume, rbdSnap *rbdSnapshot, cr
 	}
 
 	rbdVol.VolID, err = util.GenerateVolID(ctx, rbdVol.Monitors, cr, imagePoolID, rbdVol.Pool,
-		rbdVol.ClusterID, rbdVol.ReservedID, volIDVersion)
+		rbdVol.ClusterID, rbdVol.ReservedID)
 	if err != nil {
 		return err
 	}
@@ -640,7 +640,7 @@ func RegenerateJournal(
 		}
 		// As the omap already exists for this image ID return nil.
 		rbdVol.VolID, err = util.GenerateVolID(ctx, rbdVol.Monitors, cr, imagePoolID, rbdVol.Pool,
-			rbdVol.ClusterID, rbdVol.ReservedID, volIDVersion)
+			rbdVol.ClusterID, rbdVol.ReservedID)
 		if err != nil {
 			return "", err
 		}
@@ -665,7 +665,7 @@ func RegenerateJournal(
 		}
 	}()
 	rbdVol.VolID, err = util.GenerateVolID(ctx, rbdVol.Monitors, cr, imagePoolID, rbdVol.Pool,
-		rbdVol.ClusterID, rbdVol.ReservedID, volIDVersion)
+		rbdVol.ClusterID, rbdVol.ReservedID)
 	if err != nil {
 		return "", err
 	}

diff --git a/internal/util/util.go b/internal/util/util.go
@@ -286,7 +286,6 @@ func GenerateVolID(
 	cr *Credentials,
 	locationID int64,
 	pool, clusterID, objUUID string,
-	volIDVersion uint16,
 ) (string, error) {
 	var err error
 
@@ -299,10 +298,9 @@ func GenerateVolID(
 
 	// generate the volume ID to return to the CO system
 	vi := CSIIdentifier{
-		LocationID:      locationID,
-		EncodingVersion: volIDVersion,
-		ClusterID:       clusterID,
-		ObjectUUID:      objUUID,
+		LocationID: locationID,
+		ClusterID:  clusterID,
+		ObjectUUID: objUUID,
 	}
 
 	volID, err := vi.ComposeCSIID()

diff --git a/internal/util/volid.go b/internal/util/volid.go
@@ -23,6 +23,11 @@ import (
 	"strings"
 )
 
+const (
+	// defaultEncodingVersion is only one version for the encoding at the moment.
+	defaultEncodingVersion = uint16(1)
+)
+
 /*
 CSIIdentifier contains the elements that form a CSI ID to be returned by the CSI plugin, and
 contains enough information to decompose and extract required cluster and pool information to locate
@@ -34,7 +39,7 @@ DecomposeCSIID is the inverse of the same function.
 The CSIIdentifier structure carries the following fields,
   - LocationID: 64 bit integer identifier determining the location of the volume on the Ceph cluster.
     It is the ID of the poolname or fsname, for RBD or CephFS backed volumes respectively.
-  - EncodingVersion: Carries the version number of the encoding scheme used to encode the CSI ID,
+  - encodingVersion: Carries the version number of the encoding scheme used to encode the CSI ID,
     and is preserved for any future proofing w.r.t changes in the encoding scheme, and to retain
     ability to parse backward compatible encodings.
   - ClusterID: Is a unique ID per cluster that the CSI instance is serving and is restricted to
@@ -44,7 +49,7 @@ The CSIIdentifier structure carries the following fields,
 */
 type CSIIdentifier struct {
 	LocationID      int64
-	EncodingVersion uint16
+	encodingVersion uint16
 	ClusterID       string
 	ObjectUUID      string
 }
@@ -82,7 +87,13 @@ func (ci CSIIdentifier) ComposeCSIID() (string, error) {
 		return "", errors.New("CSI ID invalid object uuid")
 	}
 
-	binary.BigEndian.PutUint16(buf16, ci.EncodingVersion)
+	// set the encoding version, as it is not set by default anywhere else
+	encodingVersion := ci.encodingVersion
+	if encodingVersion == 0 {
+		encodingVersion = defaultEncodingVersion
+	}
+
+	binary.BigEndian.PutUint16(buf16, encodingVersion)
 	versionEncodedHex := hex.EncodeToString(buf16)
 
 	binary.BigEndian.PutUint16(buf16, uint16(len(ci.ClusterID)))
@@ -112,7 +123,7 @@ func (ci *CSIIdentifier) DecomposeCSIID(composedCSIID string) error {
 	if err != nil {
 		return err
 	}
-	ci.EncodingVersion = binary.BigEndian.Uint16(buf16)
+	ci.encodingVersion = binary.BigEndian.Uint16(buf16)
 	// 4 for version encoding and 1 for '-' separator
 	bytesToProcess -= 5
 

diff --git a/internal/util/volid_test.go b/internal/util/volid_test.go
@@ -34,7 +34,7 @@ var testData = []testTuple{
 	{
 		vID: CSIIdentifier{
 			LocationID:      0xffff,
-			EncodingVersion: 0xffff,
+			encodingVersion: 0xffff,
 			ClusterID:       "01616094-9d93-4178-bf45-c7eac19e8b15",
 			ObjectUUID:      "00000000-1111-2222-bbbb-cacacacacaca",
 		},