Return verbose output for volume creation

Signed-off-by: Raamsri Kumar <[email protected]>
stratastor · Jan 29, 2025 · eb0765c · eb0765c
1 parent 0c49bfd
commit eb0765c
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 43 deletions.
diff --git a/pkg/zfs/api/dataset.go b/pkg/zfs/api/dataset.go
@@ -123,12 +123,13 @@ func (h *DatasetHandler) createVolume(c *gin.Context) {
 		return
 	}
 
-	if err := h.manager.CreateVolume(c.Request.Context(), cfg); err != nil {
+	result, err := h.manager.CreateVolume(c.Request.Context(), cfg)
+	if err != nil {
 		APIError(c, err)
 		return
 	}
 
-	c.Status(http.StatusCreated)
+	c.JSON(http.StatusCreated, gin.H{"result": result})
 }
 
 func (h *DatasetHandler) destroyDataset(c *gin.Context) {

diff --git a/pkg/zfs/common/properties.go b/pkg/zfs/common/properties.go
@@ -65,6 +65,7 @@ var nativeDatasetProps = map[string]struct{}{
 	"redundant_metadata":   {},
 	"refquota":             {},
 	"refreservation":       {},
+	"relatime":             {},
 	"reservation":          {},
 	"secondarycache":       {},
 	"setuid":               {},

diff --git a/pkg/zfs/dataset/dataset.go b/pkg/zfs/dataset/dataset.go
@@ -278,6 +278,40 @@ func (m *Manager) SetProperty(ctx context.Context, cfg SetPropertyConfig) error
 	return nil
 }
 
+func stitchCreateResult(out []byte) (CreateResult, error) {
+	result := CreateResult{
+		Properties: make(map[string]string),
+	}
+
+	// Parse output lines
+	lines := strings.Split(string(out), "\n")
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if line == "" {
+			continue
+		}
+
+		// Parse "create dataset" line
+		if strings.Contains(line, "create") {
+			parts := strings.Fields(line)
+			if len(parts) >= 2 {
+				result.Created = parts[len(parts)-1]
+			}
+			continue
+		}
+
+		// Parse "property name value" lines
+		if strings.Contains(line, "property") {
+			parts := strings.Fields(line)
+			if len(parts) >= 3 {
+				result.Properties[parts[1]] = parts[2]
+			}
+		}
+	}
+
+	return result, nil
+}
+
 // CreateFilesystem creates a new ZFS filesystem
 func (m *Manager) CreateFilesystem(
 	ctx context.Context,
@@ -317,42 +351,22 @@ func (m *Manager) CreateFilesystem(
 	}
 
 	// Parse output lines
-	lines := strings.Split(string(out), "\n")
-	for _, line := range lines {
-		line = strings.TrimSpace(line)
-		if line == "" {
-			continue
-		}
-
-		// Parse "create dataset" line
-		if strings.Contains(line, "create") {
-			parts := strings.Fields(line)
-			if len(parts) >= 2 {
-				result.Created = parts[len(parts)-1]
-			}
-			continue
-		}
-
-		// Parse "property name value" lines
-		if strings.Contains(line, "property") {
-			parts := strings.Fields(line)
-			if len(parts) >= 3 {
-				result.Properties[parts[1]] = parts[2]
-			}
-		}
+	result, err = stitchCreateResult(out)
+	if err != nil {
+		return result, errors.Wrap(err, errors.CommandOutputParse)
 	}
 
 	return result, nil
 }
 
 // CreateVolume creates a new ZFS volume
-func (m *Manager) CreateVolume(ctx context.Context, cfg VolumeConfig) error {
-	args := []string{"create"}
+func (m *Manager) CreateVolume(ctx context.Context, cfg VolumeConfig) (CreateResult, error) {
+	args := []string{"create", "-P", "-v"}
 
 	if cfg.Size != "" {
 		args = append(args, "-V", cfg.Size)
 	} else {
-		return errors.New(errors.ZFSInvalidSize, "volume size not specified")
+		return CreateResult{}, errors.New(errors.ZFSInvalidSize, "volume size not specified")
 	}
 	if cfg.Sparse {
 		args = append(args, "-s")
@@ -372,12 +386,6 @@ func (m *Manager) CreateVolume(ctx context.Context, cfg VolumeConfig) error {
 	if cfg.DryRun {
 		args = append(args, "-n")
 	}
-	if cfg.Parsable {
-		args = append(args, "-P")
-	}
-	if cfg.Verbose {
-		args = append(args, "-v")
-	}
 
 	for k, v := range cfg.Properties {
 		quotedValue := shellquote.Join(v)
@@ -389,12 +397,18 @@ func (m *Manager) CreateVolume(ctx context.Context, cfg VolumeConfig) error {
 	out, err := m.executor.Execute(ctx, command.CommandOptions{}, "zfs create", args...)
 	if err != nil {
 		if len(out) > 0 {
-			return errors.Wrap(err, errors.ZFSDatasetCreate).
+			return CreateResult{}, errors.Wrap(err, errors.ZFSDatasetCreate).
 				WithMetadata("output", string(out))
 		}
-		return errors.Wrap(err, errors.ZFSDatasetCreate)
+		return CreateResult{}, errors.Wrap(err, errors.ZFSDatasetCreate)
 	}
-	return nil
+
+	result, err := stitchCreateResult(out)
+	if err != nil {
+		return result, errors.Wrap(err, errors.CommandOutputParse)
+	}
+
+	return result, nil
 }
 
 // CreateSnapshot creates a new ZFS snapshot

diff --git a/pkg/zfs/dataset/dataset_test.go b/pkg/zfs/dataset/dataset_test.go
@@ -295,7 +295,7 @@ func TestDatasetOperations(t *testing.T) {
 		// Basic volume creation
 		t.Run("CreateVolume", func(t *testing.T) {
 			volName := poolName + "/vol1"
-			err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
+			_, err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
 				NameConfig: NameConfig{
 					Name: volName,
 				},
@@ -342,7 +342,7 @@ func TestDatasetOperations(t *testing.T) {
 		// Sparse volume creation
 		t.Run("CreateSparseVolume", func(t *testing.T) {
 			volName := poolName + "/vol2"
-			err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
+			_, err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
 				NameConfig: NameConfig{
 					Name: volName,
 				},
@@ -370,7 +370,7 @@ func TestDatasetOperations(t *testing.T) {
 		// Volume with parent creation
 		t.Run("CreateVolumeWithParent", func(t *testing.T) {
 			volName := poolName + "/datasets/volumes/vol3"
-			err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
+			_, err := datasetMgr.CreateVolume(context.Background(), VolumeConfig{
 				NameConfig: NameConfig{
 					Name: volName,
 				},

diff --git a/pkg/zfs/dataset/types.go b/pkg/zfs/dataset/types.go
@@ -123,12 +123,13 @@ type VolumeConfig struct {
 	Properties map[string]string `json:"properties,omitempty"`
 	Sparse     bool              `json:"sparse"` //  -s  Creates a sparse volume with no reservation
 	// -b blocksize
-	// Equivalent to -o volblocksize=blocksize.  If this option is specified in conjunction with -o volblocksize, the  resulting  be‐
-	// havior is undefined
+	// Equivalent to -o volblocksize=blocksize.
+	// If this option is specified in conjunction with
+	//  -o volblocksize, the  resulting  behavior is undefined
 	BlockSize string `json:"blocksize,omitempty"`
 	Parents   bool   `json:"parents"`
 	DryRun    bool   `json:"dry_run"`
-	Parsable  bool   `json:"parsable"` // -p  Print machine-parsable  verbose  information  about  the  created dataset
+	Parsable  bool   `json:"parsable"` // -P  Print machine-parsable  verbose  information  about  the  created dataset
 	Verbose   bool   `json:"verbose"`
 }