Skip to content

Commit

Permalink
pool: check for application already being set
Browse files Browse the repository at this point in the history
If the pool application is already set, skip setting it again
to avoid a warning message being logged that it is already
set.

Signed-off-by: Travis Nielsen <tnielsen@redhat.com>
  • Loading branch information
travisn committed Jan 25, 2022
1 parent 8fd1f1a commit e0cfb4f
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 3 deletions.
37 changes: 35 additions & 2 deletions pkg/daemon/ceph/client/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,30 @@ func GetPoolNamesByID(context *clusterd.Context, clusterInfo *ClusterInfo) (map[
return names, nil
}

func getPoolApplication(context *clusterd.Context, clusterInfo *ClusterInfo, poolName string) (string, error) {
args := []string{"osd", "pool", "application", "get", poolName}
appDetails, err := NewCephCommand(context, clusterInfo, args).Run()
if err != nil {
return "", errors.Wrapf(err, "failed to get current application for pool %s", poolName)
}

if len(appDetails) == 0 {
// no application name
return "", nil
}
var application map[string]interface{}
err = json.Unmarshal([]byte(appDetails), &application)
if err != nil {
return "", errors.Wrapf(err, "unmarshal failed raw buffer response %s", string(appDetails))
}
for name := range application {
// Return the first application name in the list since only one is expected
return name, nil
}
// No application name assigned
return "", nil
}

// GetPoolDetails gets all the details of a given pool
func GetPoolDetails(context *clusterd.Context, clusterInfo *ClusterInfo, name string) (CephStoragePoolDetails, error) {
args := []string{"osd", "pool", "get", name, "all"}
Expand Down Expand Up @@ -234,10 +258,19 @@ func DeletePool(context *clusterd.Context, clusterInfo *ClusterInfo, name string
}

func givePoolAppTag(context *clusterd.Context, clusterInfo *ClusterInfo, poolName, appName string) error {
currentAppName, err := getPoolApplication(context, clusterInfo, poolName)
if err != nil {
return errors.Wrapf(err, "failed to get application for pool %q", poolName)
}
if currentAppName == appName {
logger.Infof("application %q is already set on pool %q", appName, poolName)
return nil
}

args := []string{"osd", "pool", "application", "enable", poolName, appName, confirmFlag}
_, err := NewCephCommand(context, clusterInfo, args).Run()
_, err = NewCephCommand(context, clusterInfo, args).Run()
if err != nil {
return errors.Wrapf(err, "failed to enable application %s on pool %s", appName, poolName)
return errors.Wrapf(err, "failed to enable application %q on pool %q", appName, poolName)
}

return nil
Expand Down
61 changes: 60 additions & 1 deletion pkg/daemon/ceph/client/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/stretchr/testify/assert"
)

const emptyApplicationName = `{"":{}}`

func TestCreateECPoolWithOverwrites(t *testing.T) {
testCreateECPool(t, true, "")
}
Expand Down Expand Up @@ -79,6 +81,9 @@ func testCreateECPool(t *testing.T, overwrite bool, compressionMode string) {
}
}
if args[2] == "application" {
if args[3] == "get" {
return emptyApplicationName, nil
}
assert.Equal(t, "enable", args[3])
assert.Equal(t, "mypool", args[4])
assert.Equal(t, "myapp", args[5])
Expand All @@ -97,6 +102,53 @@ func testCreateECPool(t *testing.T, overwrite bool, compressionMode string) {
}
}

func TestSetPoolApplication(t *testing.T) {
poolName := "testpool"
appName := "testapp"
setAppName := false
blankAppName := false
clusterInfo := AdminTestClusterInfo("mycluster")
executor := &exectest.MockExecutor{}
context := &clusterd.Context{Executor: executor}
executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
logger.Infof("Command: %s %v", command, args)
if args[1] == "pool" && args[2] == "application" {
if args[3] == "get" {
assert.Equal(t, poolName, args[4])
if blankAppName {
return emptyApplicationName, nil
} else {
return fmt.Sprintf(`{"%s":{}}`, appName), nil
}
}
if args[3] == "enable" {
setAppName = true
assert.Equal(t, poolName, args[4])
assert.Equal(t, appName, args[5])
return "", nil
}
}
return "", errors.Errorf("unexpected ceph command %q", args)
}

t.Run("set pool application", func(t *testing.T) {
setAppName = false
blankAppName = true
err := givePoolAppTag(context, clusterInfo, poolName, appName)
assert.NoError(t, err)
assert.True(t, setAppName)
})

t.Run("pool application already set", func(t *testing.T) {
setAppName = false
blankAppName = false
err := givePoolAppTag(context, clusterInfo, poolName, appName)
assert.NoError(t, err)
assert.False(t, setAppName)
})

}

func TestCreateReplicaPoolWithFailureDomain(t *testing.T) {
testCreateReplicaPool(t, "osd", "mycrushroot", "", "")
}
Expand Down Expand Up @@ -137,6 +189,9 @@ func testCreateReplicaPool(t *testing.T, failureDomain, crushRoot, deviceClass,
return "", nil
}
if args[2] == "application" {
if args[3] == "get" {
return emptyApplicationName, nil
}
assert.Equal(t, "enable", args[3])
assert.Equal(t, "mypool", args[4])
assert.Equal(t, "myapp", args[5])
Expand Down Expand Up @@ -465,7 +520,11 @@ func testCreatePoolWithReplicasPerFailureDomain(t *testing.T, failureDomain, cru
poolRuleSet = true
return "", nil
}
if len(args) >= 4 && args[1] == "pool" && args[2] == "application" && args[3] == "enable" {
if len(args) >= 4 && args[1] == "pool" && args[2] == "application" {
if args[3] == "get" {
return emptyApplicationName, nil
}

crushRuleName := args[4]
assert.Equal(t, crushRuleName, poolSpec.Name)
poolAppEnable = true
Expand Down
3 changes: 3 additions & 0 deletions pkg/operator/ceph/pool/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func TestCreatePool(t *testing.T) {
return `{"k":"2","m":"1","plugin":"jerasure","technique":"reed_sol_van"}`, nil
}
if args[0] == "osd" && args[1] == "pool" && args[2] == "application" {
if args[3] == "get" {
return ``, nil
}
assert.Equal(t, "enable", args[3])
if args[5] != "rbd" {
enabledMetricsApp = true
Expand Down

0 comments on commit e0cfb4f

Please sign in to comment.