Merge branch 'master' into edit-volume-metadata-API

e2e/config/1.toml

@@ -1,5 +1,5 @@
-workdir = "/tmp/gd2_func_test/w1"
-rundir = "/tmp/gd2_func_test/w1/run/gluster"
+workdir = "w1"
+rundir = "w1/run/gluster"
 logfile = "w1.log"
 peeraddress = "127.0.0.1:24008"
 clientaddress = "127.0.0.1:24007"

e2e/config/2.toml

@@ -1,5 +1,5 @@
-workdir = "/tmp/gd2_func_test/w2"
-rundir = "/tmp/gd2_func_test/w2/run/gluster"
+workdir = "w2"
+rundir = "w2/run/gluster"
 logfile = "w2.log"
 peeraddress = "127.0.0.1:23008"
 clientaddress = "127.0.0.1:23007"

e2e/config/3.toml

@@ -1,5 +1,5 @@
-workdir = "/tmp/gd2_func_test/w3"
-rundir = "/tmp/gd2_func_test/w3/run/gluster"
+workdir = "w3"
+rundir = "w3/run/gluster"
 logfile = "w3.log"
 peeraddress = "127.0.0.1:22008"
 clientaddress = "127.0.0.1:22007"

e2e/config/4.toml

@@ -1,5 +1,5 @@
-workdir = "/tmp/gd2_func_test/w4"
-rundir = "/tmp/gd2_func_test/w4/run/gluster"
+workdir = "w4"
+rundir = "w4/run/gluster"
 logfile = "w4.log"
 peeraddress = "127.0.0.1:21008"
 clientaddress = "127.0.0.1:21007"

e2e/main_test.go

@@ -9,15 +9,17 @@ import (
 )
 
 var (
-	binDir   string
-	functest bool
+	binDir      string
+	baseWorkdir = "/tmp/gd2_func_test"
+	functest    bool
 )
 
 func TestMain(m *testing.M) {
 	defBinDir, _ := filepath.Abs("../build")
 
 	flag.BoolVar(&functest, "functest", false, "Run or skip functional test")
 	flag.StringVar(&binDir, "bindir", defBinDir, "The directory containing glusterd2 binary")
+	flag.StringVar(&baseWorkdir, "workdir", baseWorkdir, "The base directory for test working directories")
 	flag.Parse()
 
 	if !functest {
@@ -32,7 +34,7 @@ func TestMain(m *testing.M) {
 	}
 
 	// Cleanup leftovers from previous test runs. But don't cleanup after.
-	os.RemoveAll("/tmp/gd2_func_test")
+	os.RemoveAll(baseWorkdir)
 
 	v := m.Run()
 	os.Exit(v)

e2e/utils_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"path/filepath"
 	"strings"
 	"syscall"
 	"time"
@@ -24,6 +25,7 @@ type gdProcess struct {
 	ClientAddress string `toml:"clientaddress"`
 	PeerAddress   string `toml:"peeraddress"`
 	Workdir       string `toml:"workdir"`
+	Rundir        string `toml:"rundir"`
 	uuid          string
 }
 
@@ -38,6 +40,17 @@ func (g *gdProcess) Stop() error {
 	return g.Cmd.Process.Kill()
 }
 
+func (g *gdProcess) UpdateDirs() {
+	g.Workdir = path.Clean(g.Workdir)
+	if !path.IsAbs(g.Workdir) {
+		g.Workdir = path.Join(baseWorkdir, g.Workdir)
+	}
+	g.Rundir = path.Clean(g.Rundir)
+	if !path.IsAbs(g.Rundir) {
+		g.Rundir = path.Join(baseWorkdir, g.Rundir)
+	}
+}
+
 func (g *gdProcess) EraseWorkdir() error {
 	return os.RemoveAll(g.Workdir)
 }
@@ -103,6 +116,7 @@ func spawnGlusterd(configFilePath string, cleanStart bool) (*gdProcess, error) {
 		return nil, err
 	}
 
+	g.UpdateDirs()
 	if cleanStart {
 		g.EraseWorkdir() // cleanup leftovers from previous test
 	}
@@ -111,8 +125,14 @@ func spawnGlusterd(configFilePath string, cleanStart bool) (*gdProcess, error) {
 		return nil, err
 	}
 
+	absConfigFilePath, err := filepath.Abs(configFilePath)
+	if err != nil {
+		return nil, err
+	}
 	g.Cmd = exec.Command(path.Join(binDir, "glusterd2"),
-		"--config", configFilePath,
+		"--config", absConfigFilePath,
+		"--workdir", g.Workdir,
+		"--rundir", g.Rundir,
 		"--logdir", path.Join(g.Workdir, "log"),
 		"--logfile", "glusterd2.log")
 

e2e/volume_ops_test.go

@@ -65,6 +65,9 @@ func TestVolume(t *testing.T) {
 
 	// Disperse volume test
 	t.Run("Disperse", testDisperse)
+
+	// Edit Volume Metadata test
+	t.Run("EditMetadata", testVolumeEditMetadata)
 }
 
 func testVolumeCreate(t *testing.T) {
@@ -409,3 +412,41 @@ func testDisperse(t *testing.T) {
 	r.Nil(client.VolumeStop(disperseVolName), "disperse volume stop failed")
 	r.Nil(client.VolumeDelete(disperseVolName), "disperse volume delete failed")
 }
+
+func testVolumeEditMetadata(t *testing.T) {
+	r := require.New(t)
+
+	var brickPaths []string
+
+	for i := 1; i <= 4; i++ {
+		brickPath, err := ioutil.TempDir(tmpDir, "brick")
+		r.Nil(err)
+		brickPaths = append(brickPaths, brickPath)
+	}
+
+	createReq := api.VolCreateReq{
+		Name: volname,
+		Subvols: []api.SubvolReq{
+			{
+				ReplicaCount: 2,
+				Type:         "replicate",
+				Bricks: []api.BrickReq{
+					{PeerID: gds[0].PeerID(), Path: brickPaths[0]},
+					{PeerID: gds[1].PeerID(), Path: brickPaths[1]},
+				},
+			},
+		},
+		Force: true,
+	}
+	_, err := client.VolumeCreate(createReq)
+
+	editMetadataReq := api.VolEditMetadataReq{
+		Metadata: map[string]string{
+			"owner": "gd2tests",
+		},
+	}
+	_, err = client.VolumeEditMetadata(volname, editMetadataReq)
+	r.Nil(err)
+	err = client.VolumeDelete(volname)
+	r.Nil(err)
+}

glusterd2/commands/volumes/bricks-status.go

@@ -71,7 +71,11 @@ func volumeBricksStatusHandler(w http.ResponseWriter, r *http.Request) {
 	volname := mux.Vars(r)["volname"]
 	vol, err := volume.GetVolume(volname)
 	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/commands.go

@@ -130,7 +130,8 @@ func (c *Command) Routes() route.Routes {
 		route.Route{
 			Name:        "VolumeEditMetadata",
 			Method:      "POST",
-			Pattern:     "/volumes/{volname}/edit-metadata",
+			Pattern:     "/volumes/{volname}/metadata",
+			Version:     1,
 			RequestType: utils.GetTypeString((*api.VolumeGetResp)(nil)),
 			HandlerFunc: volumeEditMetadataHandler},
 	}

glusterd2/commands/volumes/volume-create-txn.go

@@ -146,6 +146,12 @@ func newVolinfo(req *api.VolCreateReq) (*volume.Volinfo, error) {
 		return nil, err
 	}
 
+	if req.Metadata != nil {
+		volinfo.Metadata = req.Metadata
+	} else {
+		volinfo.Metadata = make(map[string]string)
+	}
+
 	return volinfo, nil
 }
 

glusterd2/commands/volumes/volume-create_test.go

@@ -30,6 +30,8 @@ func TestCreateVolinfo(t *testing.T) {
 		{PeerID: u.String(), Path: "/tmp/b1"},
 		{PeerID: u.String(), Path: "/tmp/b2"},
 	}}}
+	msg.Metadata = make(map[string]string)
+	msg.Metadata["owner"] = "gd2test"
 	vol, e := newVolinfo(msg)
 	assert.Nil(t, e)
 	assert.NotNil(t, vol)

glusterd2/commands/volumes/volume-delete.go

@@ -81,9 +81,11 @@ func volumeDeleteHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		// TODO: Distinguish between volume not present (404) and
-		// store access failure (503)
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-edit-metadata-txn.go

@@ -11,7 +11,9 @@ func editVolMetadata(c transaction.TxnCtx) error {
 	var req api.VolEditMetadataReq
 	var volname string
 
-	if err := c.Get("req", &req); err != nil {
+	if err := c.Get("metadata", &req); err != nil {
+		c.Logger().WithError(err).WithField(
+			"key", "metadata").Debug("Failed to get key from transaction context.")
 		return err
 	}
 
@@ -27,11 +29,12 @@ func editVolMetadata(c transaction.TxnCtx) error {
 		return err
 	}
 
-	for key, _ := range req.Metadata {
+	for key := range req.Metadata {
 		v.Metadata[key] = req.Metadata[key]
 	}
 
 	if err := c.Set("volinfo", v); err != nil {
+		c.Logger().WithError(err).Error("Failed to set volinfo into the store")
 		return err
 	}
 

glusterd2/commands/volumes/volume-edit-metadata.go

@@ -2,6 +2,7 @@ package volumecommands
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/gluster/glusterd2/glusterd2/gdctx"
 	restutils "github.com/gluster/glusterd2/glusterd2/servers/rest/utils"
@@ -39,6 +40,13 @@ func volumeEditMetadataHandler(w http.ResponseWriter, r *http.Request) {
 		restutils.SendHTTPError(ctx, w, http.StatusUnprocessableEntity, err)
 		return
 	}
+	for key := range req.Metadata {
+		if strings.HasPrefix(key, "_") {
+			logger.WithField("metadata-key", key).Error("Key names starting with '_' are restricted in metadata field")
+			restutils.SendHTTPError(ctx, w, http.StatusBadRequest, "Key names starting with '_' are restricted in metadata field")
+			return
+		}
+	}
 
 	//Lock on Volume Name
 	lock, unlock := transaction.CreateLockFuncs(volname)
@@ -104,6 +112,12 @@ func volumeEditMetadataHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	err = txn.Ctx.Get("volinfo", &v)
+	if err != nil {
+		logger.WithError(err).WithField("key", "volinfo").Error("Failed to get key from transaction context")
+		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		return
+	}
 	resp := createVolumeGetResp(v)
 	restutils.SendHTTPResponse(ctx, w, http.StatusOK, resp)
 

glusterd2/commands/volumes/volume-info.go

@@ -18,7 +18,11 @@ func volumeInfoHandler(w http.ResponseWriter, r *http.Request) {
 	volname := mux.Vars(r)["volname"]
 	v, err := volume.GetVolume(volname)
 	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-option.go

@@ -145,7 +145,11 @@ func volumeOptionsHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-start.go

@@ -84,9 +84,11 @@ func volumeStartHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		// TODO: Distinguish between volume not present (404) and
-		// store access failure (503)
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-statedump.go

@@ -116,9 +116,11 @@ func volumeStatedumpHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		// TODO: Distinguish between volume not present (404) and
-		// store access failure (503)
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, gderrors.ErrVolNotFound)
+		if err == gderrors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, gderrors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-status.go

@@ -20,9 +20,11 @@ func volumeStatusHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		// TODO: Distinguish between volume not present (404) and
-		// store access failure (503)
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/commands/volumes/volume-stop.go

@@ -91,9 +91,11 @@ func volumeStopHandler(w http.ResponseWriter, r *http.Request) {
 
 	volinfo, err := volume.GetVolume(volname)
 	if err != nil {
-		// TODO: Distinguish between volume not present (404) and
-		// store access failure (503)
-		restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		if err == errors.ErrVolNotFound {
+			restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound)
+		} else {
+			restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
+		}
 		return
 	}
 

glusterd2/volume/store-utils.go

@@ -3,9 +3,9 @@ package volume
 import (
 	"context"
 	"encoding/json"
-	"errors"
 
 	"github.com/gluster/glusterd2/glusterd2/store"
+	gderror "github.com/gluster/glusterd2/pkg/errors"
 
 	"github.com/coreos/etcd/clientv3"
 	"github.com/pborman/uuid"
@@ -49,7 +49,7 @@ func GetVolume(name string) (*Volinfo, error) {
 
 	if resp.Count != 1 {
 		log.WithField("volume", name).Error("volume not found")
-		return nil, errors.New("volume not found")
+		return nil, gderror.ErrVolNotFound
 	}
 
 	if e = json.Unmarshal(resp.Kvs[0].Value, &v); e != nil {