Repository: ceph/go-ceph (go)

Issue:
Add support for rados mgr command functions
librados has functions rados_mgr_command and rados_mgr_command_target . These functions are similar to the other rados_X_command functions that we already have wrapped.
Supporting these functions will allow callers to interact with features of the MGRs.

Pull request description:
rados: add MgrCommand & MgrCommandWithInputBuffer functions
First, the *Command functions and tests are moved into separate files to reduce the amount of stuff in conn.go & friends. The function signatures are simplified a bit.

Then new functions MgrCommand & MgrCommandWithInputBuffer are added that wrap rados_mgr_command are added along with tests.

Fixes #216 


## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code


Patch:
diff --git a/rados/command.go b/rados/command.go
new file mode 100644
index 0000000..ec41d1c
--- /dev/null
+++ b/rados/command.go
@@ -0,0 +1,194 @@
+package rados
+
+// #cgo LDFLAGS: -lrados
+// #include <stdlib.h>
+// #include <rados/librados.h>
+import "C"
+
+import (
+	"unsafe"
+)
+
+// MonCommand sends a command to one of the monitors
+func (c *Conn) MonCommand(args []byte) ([]byte, string, error) {
+	return c.monCommand(args, nil)
+}
+
+// MonCommandWithInputBuffer sends a command to one of the monitors, with an input buffer
+func (c *Conn) MonCommandWithInputBuffer(args, inputBuffer []byte) ([]byte, string, error) {
+	return c.monCommand(args, inputBuffer)
+}
+
+func (c *Conn) monCommand(args, inputBuffer []byte) ([]byte, string, error) {
+	argv := C.CString(string(args))
+	defer C.free(unsafe.Pointer(argv))
+
+	var (
+		info               string
+		buffer             []byte
+		outs, outbuf       *C.char
+		outslen, outbuflen C.size_t
+	)
+	inbuf := C.CString(string(inputBuffer))
+	inbufLen := len(inputBuffer)
+	defer C.free(unsafe.Pointer(inbuf))
+
+	ret := C.rados_mon_command(c.cluster,
+		&argv, 1,
+		inbuf,              // bulk input (e.g. crush map)
+		C.size_t(inbufLen), // length inbuf
+		&outbuf,            // buffer
+		&outbuflen,         // buffer length
+		&outs,              // status string
+		&outslen)
+
+	if outslen > 0 {
+		info = C.GoStringN(outs, C.int(outslen))
+		C.free(unsafe.Pointer(outs))
+	}
+	if outbuflen > 0 {
+		buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen))
+		C.free(unsafe.Pointer(outbuf))
+	}
+	if ret != 0 {
+		err := getError(ret)
+		return nil, info, err
+	}
+
+	return buffer, info, nil
+}
+
+// PGCommand sends a command to one of the PGs
+//
+// Implements:
+//  int rados_pg_command(rados_t cluster, const char *pgstr,
+//                       const char **cmd, size_t cmdlen,
+//                       const char *inbuf, size_t inbuflen,
+//                       char **outbuf, size_t *outbuflen,
+//                       char **outs, size_t *outslen);
+func (c *Conn) PGCommand(pgid []byte, args [][]byte) ([]byte, string, error) {
+	return c.pgCommand(pgid, args, nil)
+}
+
+// PGCommandWithInputBuffer sends a command to one of the PGs, with an input buffer
+//
+// Implements:
+//  int rados_pg_command(rados_t cluster, const char *pgstr,
+//                       const char **cmd, size_t cmdlen,
+//                       const char *inbuf, size_t inbuflen,
+//                       char **outbuf, size_t *outbuflen,
+//                       char **outs, size_t *outslen);
+func (c *Conn) PGCommandWithInputBuffer(pgid []byte, args [][]byte, inputBuffer []byte) ([]byte, string, error) {
+	return c.pgCommand(pgid, args, inputBuffer)
+}
+
+func (c *Conn) pgCommand(pgid []byte, args [][]byte, inputBuffer []byte) ([]byte, string, error) {
+	name := C.CString(string(pgid))
+	defer C.free(unsafe.Pointer(name))
+
+	argc := len(args)
+	argv := make([]*C.char, argc)
+
+	for i, arg := range args {
+		argv[i] = C.CString(string(arg))
+		defer C.free(unsafe.Pointer(argv[i]))
+	}
+
+	var (
+		info               string
+		buffer             []byte
+		outs, outbuf       *C.char
+		outslen, outbuflen C.size_t
+	)
+	inbuf := C.CString(string(inputBuffer))
+	inbufLen := len(inputBuffer)
+	defer C.free(unsafe.Pointer(inbuf))
+
+	ret := C.rados_pg_command(c.cluster,
+		name,
+		&argv[0],
+		C.size_t(argc),
+		inbuf,              // bulk input
+		C.size_t(inbufLen), // length inbuf
+		&outbuf,            // buffer
+		&outbuflen,         // buffer length
+		&outs,              // status string
+		&outslen)
+
+	if outslen > 0 {
+		info = C.GoStringN(outs, C.int(outslen))
+		C.free(unsafe.Pointer(outs))
+	}
+	if outbuflen > 0 {
+		buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen))
+		C.free(unsafe.Pointer(outbuf))
+	}
+	if ret != 0 {
+		err := getError(ret)
+		return nil, info, err
+	}
+
+	return buffer, info, nil
+}
+
+// MgrCommand sends a command to a ceph-mgr.
+func (c *Conn) MgrCommand(args [][]byte) ([]byte, string, error) {
+	return c.mgrCommand(args, nil)
+}
+
+// MgrCommandWithInputBuffer sends a command, with an input buffer, to a ceph-mgr.
+func (c *Conn) MgrCommandWithInputBuffer(args [][]byte, inputBuffer []byte) ([]byte, string, error) {
+	return c.mgrCommand(args, inputBuffer)
+}
+
+// Implements:
+//  int rados_mgr_command(rados_t cluster, const char **cmd,
+//                         size_t cmdlen, const char *inbuf,
+//                         size_t inbuflen, char **outbuf,
+//                         size_t *outbuflen, char **outs,
+//                          size_t *outslen);
+func (c *Conn) mgrCommand(args [][]byte, inputBuffer []byte) ([]byte, string, error) {
+	argc := len(args)
+	argv := make([]*C.char, argc)
+
+	for i, arg := range args {
+		argv[i] = C.CString(string(arg))
+		defer C.free(unsafe.Pointer(argv[i]))
+	}
+
+	var (
+		info               string
+		buffer             []byte
+		outs, outbuf       *C.char
+		outslen, outbuflen C.size_t
+	)
+	inbuf := C.CString(string(inputBuffer))
+	inbufLen := len(inputBuffer)
+	defer C.free(unsafe.Pointer(inbuf))
+
+	ret := C.rados_mgr_command(
+		c.cluster,
+		&argv[0],
+		C.size_t(argc),
+		inbuf,
+		C.size_t(inbufLen),
+		&outbuf,
+		&outbuflen,
+		&outs,
+		&outslen)
+
+	if outslen > 0 {
+		info = C.GoStringN(outs, C.int(outslen))
+		C.free(unsafe.Pointer(outs))
+	}
+	if outbuflen > 0 {
+		buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen))
+		C.free(unsafe.Pointer(outbuf))
+	}
+	if ret != 0 {
+		err := getError(ret)
+		return nil, info, err
+	}
+
+	return buffer, info, nil
+}
diff --git a/rados/conn.go b/rados/conn.go
index 16ca738..d1040f5 100644
--- a/rados/conn.go
+++ b/rados/conn.go
@@ -291,121 +291,3 @@ func (c *Conn) GetPoolByID(id int64) (string, error) {
 	}
 	return C.GoString((*C.char)(unsafe.Pointer(&buf[0]))), nil
 }
-
-// MonCommand sends a command to one of the monitors
-func (c *Conn) MonCommand(args []byte) (buffer []byte, info string, err error) {
-	return c.monCommand(args, nil)
-}
-
-// MonCommandWithInputBuffer sends a command to one of the monitors, with an input buffer
-func (c *Conn) MonCommandWithInputBuffer(args, inputBuffer []byte) (buffer []byte, info string, err error) {
-	return c.monCommand(args, inputBuffer)
-}
-
-func (c *Conn) monCommand(args, inputBuffer []byte) (buffer []byte, info string, err error) {
-	argv := C.CString(string(args))
-	defer C.free(unsafe.Pointer(argv))
-
-	var (
-		outs, outbuf       *C.char
-		outslen, outbuflen C.size_t
-	)
-	inbuf := C.CString(string(inputBuffer))
-	inbufLen := len(inputBuffer)
-	defer C.free(unsafe.Pointer(inbuf))
-
-	ret := C.rados_mon_command(c.cluster,
-		&argv, 1,
-		inbuf,              // bulk input (e.g. crush map)
-		C.size_t(inbufLen), // length inbuf
-		&outbuf,            // buffer
-		&outbuflen,         // buffer length
-		&outs,              // status string
-		&outslen)
-
-	if outslen > 0 {
-		info = C.GoStringN(outs, C.int(outslen))
-		C.free(unsafe.Pointer(outs))
-	}
-	if outbuflen > 0 {
-		buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen))
-		C.free(unsafe.Pointer(outbuf))
-	}
-	if ret != 0 {
-		err = getError(ret)
-		return nil, info, err
-	}
-
-	return
-}
-
-// PGCommand sends a command to one of the PGs
-//
-// Implements:
-//  int rados_pg_command(rados_t cluster, const char *pgstr,
-//                       const char **cmd, size_t cmdlen,
-//                       const char *inbuf, size_t inbuflen,
-//                       char **outbuf, size_t *outbuflen,
-//                       char **outs, size_t *outslen);
-func (c *Conn) PGCommand(pgid []byte, args [][]byte) (buffer []byte, info string, err error) {
-	return c.pgCommand(pgid, args, nil)
-}
-
-// PGCommandWithInputBuffer sends a command to one of the PGs, with an input buffer
-//
-// Implements:
-//  int rados_pg_command(rados_t cluster, const char *pgstr,
-//                       const char **cmd, size_t cmdlen,
-//                       const char *inbuf, size_t inbuflen,
-//                       char **outbuf, size_t *outbuflen,
-//                       char **outs, size_t *outslen);
-func (c *Conn) PGCommandWithInputBuffer(pgid []byte, args [][]byte, inputBuffer []byte) (buffer []byte, info string, err error) {
-	return c.pgCommand(pgid, args, inputBuffer)
-}
-
-func (c *Conn) pgCommand(pgid []byte, args [][]byte, inputBuffer []byte) (buffer []byte, info string, err error) {
-	name := C.CString(string(pgid))
-	defer C.free(unsafe.Pointer(name))
-
-	argc := len(args)
-	argv := make([]*C.char, argc)
-
-	for i, arg := range args {
-		argv[i] = C.CString(string(arg))
-		defer C.free(unsafe.Pointer(argv[i]))
-	}
-
-	var (
-		outs, outbuf       *C.char
-		outslen, outbuflen C.size_t
-	)
-	inbuf := C.CString(string(inputBuffer))
-	inbufLen := len(inputBuffer)
-	defer C.free(unsafe.Pointer(inbuf))
-
-	ret := C.rados_pg_command(c.cluster,
-		name,
-		&argv[0],
-		C.size_t(argc),
-		inbuf,              // bulk input
-		C.size_t(inbufLen), // length inbuf
-		&outbuf,            // buffer
-		&outbuflen,         // buffer length
-		&outs,              // status string
-		&outslen)
-
-	if outslen > 0 {
-		info = C.GoStringN(outs, C.int(outslen))
-		C.free(unsafe.Pointer(outs))
-	}
-	if outbuflen > 0 {
-		buffer = C.GoBytes(unsafe.Pointer(outbuf), C.int(outbuflen))
-		C.free(unsafe.Pointer(outbuf))
-	}
-	if ret != 0 {
-		err = getError(ret)
-		return nil, info, err
-	}
-
-	return
-}


Test patch:
diff --git a/rados/command_mimic_test.go b/rados/command_mimic_test.go
new file mode 100644
index 0000000..aaa93da
--- /dev/null
+++ b/rados/command_mimic_test.go
@@ -0,0 +1,37 @@
+// +build !luminous
+
+package rados
+
+import (
+	"encoding/json"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// A real test using input buffer is hard to find for mgr.
+// The simplest does not work on luminous, so we simply don't
+// provide the test for luminous.
+
+func (suite *RadosTestSuite) TestMgrCommandWithInputBuffer() {
+	suite.SetupConnection()
+
+	command, err := json.Marshal(
+		map[string]string{"prefix": "crash post", "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err := suite.conn.MgrCommandWithInputBuffer(
+		[][]byte{command}, []byte(`{"crash_id": "foobar", "timestamp": "2020-04-10 15:08:34.659679Z"}`))
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+	assert.Len(suite.T(), buf, 0)
+
+	command, err = json.Marshal(
+		map[string]string{"prefix": "crash rm", "id": "foobar", "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err = suite.conn.MgrCommandWithInputBuffer(
+		[][]byte{command}, nil)
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+	assert.Len(suite.T(), buf, 0)
+}
diff --git a/rados/command_test.go b/rados/command_test.go
new file mode 100644
index 0000000..6924741
--- /dev/null
+++ b/rados/command_test.go
@@ -0,0 +1,131 @@
+package rados
+
+import (
+	"encoding/json"
+	"fmt"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func (suite *RadosTestSuite) TestMonCommand() {
+	suite.SetupConnection()
+
+	command, err := json.Marshal(
+		map[string]string{"prefix": "df", "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err := suite.conn.MonCommand(command)
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+
+	var message map[string]interface{}
+	err = json.Unmarshal(buf, &message)
+	assert.NoError(suite.T(), err)
+}
+
+// NB: ceph octopus appears to be stricter about the formatting of the keyring
+// and now rejects whitespace that older versions did not have a problem with.
+const clientKeyFormat = `
+[%s]
+key = AQD4PGNXBZJNHhAA582iUgxe9DsN+MqFN4Z6Jw==
+`
+
+func (suite *RadosTestSuite) TestMonCommandWithInputBuffer() {
+	suite.SetupConnection()
+
+	entity := fmt.Sprintf("client.testMonCmdUser%d", time.Now().UnixNano())
+
+	// first add the new test user, specifying its key in the input buffer
+	command, err := json.Marshal(map[string]interface{}{
+		"prefix": "auth add",
+		"format": "json",
+		"entity": entity,
+	})
+	assert.NoError(suite.T(), err)
+
+	client_key := fmt.Sprintf(clientKeyFormat, entity)
+
+	inbuf := []byte(client_key)
+
+	buf, info, err := suite.conn.MonCommandWithInputBuffer(command, inbuf)
+	assert.NoError(suite.T(), err)
+	expected_info := fmt.Sprintf("added key for %s", entity)
+	assert.Equal(suite.T(), expected_info, info)
+	assert.Equal(suite.T(), "", string(buf[:]))
+
+	// get the key and verify that it's what we previously set
+	command, err = json.Marshal(map[string]interface{}{
+		"prefix": "auth get-key",
+		"format": "json",
+		"entity": entity,
+	})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err = suite.conn.MonCommand(command)
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), "", info)
+	assert.Equal(suite.T(),
+		`{"key":"AQD4PGNXBZJNHhAA582iUgxe9DsN+MqFN4Z6Jw=="}`,
+		string(buf[:]))
+}
+
+func (suite *RadosTestSuite) TestPGCommand() {
+	suite.SetupConnection()
+
+	pgid := "1.2"
+
+	command, err := json.Marshal(
+		map[string]string{"prefix": "query", "pgid": pgid, "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err := suite.conn.PGCommand([]byte(pgid), [][]byte{[]byte(command)})
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+
+	var message map[string]interface{}
+	err = json.Unmarshal(buf, &message)
+	assert.NoError(suite.T(), err)
+}
+
+func (suite *RadosTestSuite) TestMgrCommandDescrptions() {
+	suite.SetupConnection()
+
+	command, err := json.Marshal(
+		map[string]string{"prefix": "get_command_descriptions", "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err := suite.conn.MgrCommand([][]byte{command})
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+
+	var message map[string]interface{}
+	err = json.Unmarshal(buf, &message)
+	assert.NoError(suite.T(), err)
+}
+
+func (suite *RadosTestSuite) TestMgrCommand() {
+	suite.SetupConnection()
+
+	command, err := json.Marshal(
+		map[string]string{"prefix": "balancer status", "format": "json"})
+	assert.NoError(suite.T(), err)
+
+	buf, info, err := suite.conn.MgrCommand([][]byte{command})
+	assert.NoError(suite.T(), err)
+	assert.Equal(suite.T(), info, "")
+
+	var message map[string]interface{}
+	err = json.Unmarshal(buf, &message)
+	assert.NoError(suite.T(), err)
+}
+
+func (suite *RadosTestSuite) TestMgrCommandMalformedCommand() {
+	suite.SetupConnection()
+
+	command := []byte("JUNK!")
+	buf, info, err := suite.conn.MgrCommand([][]byte{command})
+	assert.Error(suite.T(), err)
+	assert.NotEqual(suite.T(), info, "")
+	assert.Len(suite.T(), buf, 0)
+}
diff --git a/rados/rados_test.go b/rados/rados_test.go
index fa95fc9..3499099 100644
--- a/rados/rados_test.go
+++ b/rados/rados_test.go
@@ -1,7 +1,6 @@
 package rados
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -595,86 +594,6 @@ func (suite *RadosTestSuite) TestGetPoolName() {
 	assert.Equal(suite.T(), name, suite.pool)
 }
 
-func (suite *RadosTestSuite) TestMonCommand() {
-	suite.SetupConnection()
-
-	command, err := json.Marshal(
-		map[string]string{"prefix": "df", "format": "json"})
-	assert.NoError(suite.T(), err)
-
-	buf, info, err := suite.conn.MonCommand(command)
-	assert.NoError(suite.T(), err)
-	assert.Equal(suite.T(), info, "")
-
-	var message map[string]interface{}
-	err = json.Unmarshal(buf, &message)
-	assert.NoError(suite.T(), err)
-}
-
-// NB: ceph octopus appears to be stricter about the formatting of the keyring
-// and now rejects whitespace that older versions did not have a problem with.
-const clientKeyFormat = `
-[%s]
-key = AQD4PGNXBZJNHhAA582iUgxe9DsN+MqFN4Z6Jw==
-`
-
-func (suite *RadosTestSuite) TestMonCommandWithInputBuffer() {
-	suite.SetupConnection()
-
-	entity := fmt.Sprintf("client.testMonCmdUser%d", time.Now().UnixNano())
-
-	// first add the new test user, specifying its key in the input buffer
-	command, err := json.Marshal(map[string]interface{}{
-		"prefix": "auth add",
-		"format": "json",
-		"entity": entity,
-	})
-	assert.NoError(suite.T(), err)
-
-	client_key := fmt.Sprintf(clientKeyFormat, entity)
-
-	inbuf := []byte(client_key)
-
-	buf, info, err := suite.conn.MonCommandWithInputBuffer(command, inbuf)
-	assert.NoError(suite.T(), err)
-	expected_info := fmt.Sprintf("added key for %s", entity)
-	assert.Equal(suite.T(), expected_info, info)
-	assert.Equal(suite.T(), "", string(buf[:]))
-
-	// get the key and verify that it's what we previously set
-	command, err = json.Marshal(map[string]interface{}{
-		"prefix": "auth get-key",
-		"format": "json",
-		"entity": entity,
-	})
-	assert.NoError(suite.T(), err)
-
-	buf, info, err = suite.conn.MonCommand(command)
-	assert.NoError(suite.T(), err)
-	assert.Equal(suite.T(), "", info)
-	assert.Equal(suite.T(),
-		`{"key":"AQD4PGNXBZJNHhAA582iUgxe9DsN+MqFN4Z6Jw=="}`,
-		string(buf[:]))
-}
-
-func (suite *RadosTestSuite) TestPGCommand() {
-	suite.SetupConnection()
-
-	pgid := "1.2"
-
-	command, err := json.Marshal(
-		map[string]string{"prefix": "query", "pgid": pgid, "format": "json"})
-	assert.NoError(suite.T(), err)
-
-	buf, info, err := suite.conn.PGCommand([]byte(pgid), [][]byte{[]byte(command)})
-	assert.NoError(suite.T(), err)
-	assert.Equal(suite.T(), info, "")
-
-	var message map[string]interface{}
-	err = json.Unmarshal(buf, &message)
-	assert.NoError(suite.T(), err)
-}
-
 func (suite *RadosTestSuite) TestObjectListObjects() {
 	suite.SetupConnection()