Set OSD pool size when creating `ceph` and `cephfs` storage pools #14044

masnax · 2024-09-04T21:49:14Z

Adds the keys ceph.osd.pool_size and cephfs.osd_pool_size (in keeping with the other naming schemes).

By default, if no value is supplied, the pool size will be pulled from the global default pool size. It can be set to any value larger than 0.

On update, we will try to re-apply the OSD pool size value, if it has changed.

github-actions · 2024-09-04T21:49:26Z

Heads up @mionaalex - the "Documentation" label was applied to this issue.

masnax · 2024-09-04T21:52:01Z

I can't seem to get the docs to pick up the new extensions, I just get

WARNING: Could not find target storage-ceph-pool-conf:ceph.osd.pool_size in api-extensions

Got it, it was make update-metadata.

masnax · 2024-09-04T22:07:20Z

@simondeziel Got any recommendations for setting up more than 1 OSD for microceph? I'd like to add a test case for the new key but it's tied to the number of available disks supplied to MicroCeph, which is 1 in the github workflow tests.

masnax · 2024-09-04T23:05:35Z

Looks like 1 thing I overlooked in my testing is that size=1 is disabled by default unless the global config is changed.

So while the impetus for this PR was to avoid running ceph config set global osd_pool_default_size 1 to set the default pool size, and adding these keys avoids that, a user will still need to run ceph config set global mon_allow_pool_size_one true to make the keys work if a user wants size=1

It's still added flexibility, but if you'd prefer to avoid managing these keys in LXD since it doesn't actually solve the underlying problem of needing to set global ceph configuration, I can close the PR @tomponline

simondeziel · 2024-09-05T16:57:44Z

@simondeziel Got any recommendations for setting up more than 1 OSD for microceph? I'd like to add a test case for the new key but it's tied to the number of available disks supplied to MicroCeph, which is 1 in the github workflow tests.

Could we maybe use the ephemeral disk, split it into 3 partitions and export each as loop dev backed by their respective disk part?

simondeziel · 2024-10-22T21:31:04Z

test/suites/storage_driver_ceph.sh

+    pool1="$("lxdtest-$(basename "${LXD_DIR}")-pool1")"
+    pool2="$("lxdtest-$(basename "${LXD_DIR}")-pool2")"
+    lxc storage create "${pool1}" ceph volume.size=25MiB ceph.osd.pg_num=16 ceph.osd.pool_size=1
+    ceph --cluster "${LXD_CEPH_CLUSTER}" osd pool get "${pool1}" size --format json | jq '.size' | grep -q 1


Using shell comparison should help with debugging as we will see what .size we got on the left hand side of the comparison:

Suggested change

ceph --cluster "${LXD_CEPH_CLUSTER}" osd pool get "${pool1}" size --format json | jq '.size' | grep -q 1

[[ "$(ceph --cluster "${LXD_CEPH_CLUSTER}" osd pool get "${pool1}" size --format json | jq '.size')" = "1" ]]

simondeziel · 2024-10-23T14:14:33Z

lxd/storage/drivers/driver_ceph.go

@@ -112,6 +114,29 @@ func (d *ceph) FillConfig() error {
 		d.config["ceph.osd.pg_num"] = "32"
 	}

+	if d.config["ceph.osd.pool_size"] == "" {
+		size, err := shared.TryRunCommand("ceph",
+			"--name", fmt.Sprintf("client.%s", d.config["ceph.user.name"]),


It's a tiny nit but simple string concat is more efficient and needs less keystrokes ;)

Suggested change

"--name", fmt.Sprintf("client.%s", d.config["ceph.user.name"]),

"--name", "client." + d.config["ceph.user.name"],

Someone was paying attention at the performance sessions in Madrid :)

Fixed

Signed-off-by: Max Asnaashari <[email protected]>

Seems this always returns an error because the pool was removed with the LXD storage pool. However, because it was the last line, the error was not propagating. To ensure there's no leftover state, we still run the command but do not expect it to pass. Signed-off-by: Max Asnaashari <[email protected]>

Signed-off-by: Max Asnaashari <[email protected]>

simondeziel

LGTM in general with the caveat that the setup-microceph action is being used elsewhere and should probably not default to using 3 partitions. Could you maybe make that an action parameter?

simondeziel · 2024-10-23T17:15:12Z

doc/api-extensions.md

@@ -2517,6 +2517,10 @@ Adds support for using a bridge network with a specified VLAN ID as an OVN uplin
 Adds `logical_cpus` field to `GET /1.0/cluster/members/{name}/state` which
 contains the total available logical CPUs available when LXD started.

+<<<<<<< HEAD


Merge/rebase leftover.

simondeziel · 2024-10-23T17:19:07Z

.github/actions/setup-microceph/action.yml

          sudo snap install microceph --channel "${{ inputs.microceph-channel }}"
          sudo microceph cluster bootstrap
          sudo microceph.ceph config set global osd_pool_default_size 1
+          sudo microceph.ceph config set global mon_allow_pool_size_one true


How about having the 2 mon_* settings grouped before the OSD ones? I'm thinking this would avoid mon complaining about the OSD pool size being 1 for a very brief moment?

simondeziel · 2024-10-23T17:20:10Z

.github/actions/setup-microceph/action.yml

+          sudo parted "${ephemeral_disk}" --script mklabel gpt
+          sudo parted "${ephemeral_disk}" --script mkpart primary 0% 33%
+          sudo parted "${ephemeral_disk}" --script mkpart primary 33% 66%
+          sudo parted "${ephemeral_disk}" --script mkpart primary 66% 100%


I like it and didn't know it could take %.

simondeziel · 2024-10-23T17:22:47Z

.github/actions/setup-microceph/action.yml

+          disk2="$(losetup -f)"
+          sudo losetup "${disk2}" "${ephemeral_disk}2"
+          disk3="$(losetup -f)"
+          sudo losetup "${disk3}" "${ephemeral_disk}3"


I take it that MicroCeph still cannot take partitions directly right? How about adding a link to canonical/microceph#251 in a comment?

simondeziel · 2024-10-23T17:27:11Z

lxd/storage/drivers/driver_ceph.go

+		//  type: string
+		//  defaultdesc: `3`
+		//  shortdesc: Number of RADOS object replicas. Set to 1 for no replication.
+		"ceph.osd.pool_size": validate.Optional(validate.IsInRange(1, math.MaxInt64)),


While technically correct, maybe we could put an upper bound that's a little less... gigantic :) Hardcoding 255 feels future-proof enough I'd say.

I just noticed that ceph.osd.pg_num has no such validation.

simondeziel · 2024-10-23T17:29:07Z

doc/metadata.txt

+:shortdesc: "Number of RADOS object replicas. Set to 1 for no replication."
+:type: "string"
+This option specifies the number of OSD pool replicas to use
+when creating an OSD pool.


Why have a long description for cephfs and not for ceph? Also, is this really something that needs to be done at creation time only?

masnax · 2024-10-23T17:35:44Z

LGTM in general with the caveat that the setup-microceph action is being used elsewhere and should probably not default to using 3 partitions. Could you maybe make that an action parameter?

Hmm, does it matter since we set the replication factor to 1 anyway?

simondeziel · 2024-10-23T17:38:53Z

LGTM in general with the caveat that the setup-microceph action is being used elsewhere and should probably not default to using 3 partitions. Could you maybe make that an action parameter?

Hmm, does it matter since we set the replication factor to 1 anyway?

That cuts the available space in 3 (so ~25GiB instead of ~75GiB IIRC) which might be a bit tight for some lxd-ci.git tests. Also, it comes with the losetup overhead.

github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Sep 4, 2024

masnax force-pushed the osd_pool_size branch from 44590c2 to 5430bf6 Compare September 4, 2024 22:04

masnax force-pushed the osd_pool_size branch 3 times, most recently from b44133e to 5a26cc4 Compare September 4, 2024 23:01

masnax force-pushed the osd_pool_size branch 2 times, most recently from 850c106 to 3d07d9c Compare September 5, 2024 15:59

masnax force-pushed the osd_pool_size branch from 3d07d9c to 861a503 Compare October 22, 2024 20:44

simondeziel reviewed Oct 22, 2024

View reviewed changes

masnax force-pushed the osd_pool_size branch from 861a503 to b8905a4 Compare October 22, 2024 23:08

masnax marked this pull request as ready for review October 22, 2024 23:20

masnax force-pushed the osd_pool_size branch from 0c39aed to e6f6866 Compare October 23, 2024 01:13

simondeziel reviewed Oct 23, 2024

View reviewed changes

masnax added 11 commits October 23, 2024 16:53

lxd/storage/drivers: Set default OSD pool size

99f1e58

Signed-off-by: Max Asnaashari <[email protected]>

lxd/storage/drivers: Set OSD pool size when creating

159de70

Signed-off-by: Max Asnaashari <[email protected]>

lxd/storage/drivers: Set OSD pool size when updating

86b599c

Signed-off-by: Max Asnaashari <[email protected]>

lxd/storage/drivers: Add config validators

28eeec4

Signed-off-by: Max Asnaashari <[email protected]>

shared/version: Add osd pool size API extension

2288205

Signed-off-by: Max Asnaashari <[email protected]>

lxd/metadata: Update config metadata

d805371

Signed-off-by: Max Asnaashari <[email protected]>

.github/workflows: Allow size 1 pools in tests

ae68415

Signed-off-by: Max Asnaashari <[email protected]>

.github/actions/setup-microceph: Setup microceph with 3 OSDs

ce95425

Signed-off-by: Max Asnaashari <[email protected]>

test/suites: Test OSD pool size flag

ec0d03b

Signed-off-by: Max Asnaashari <[email protected]>

lxd/storage/drivers: Remove unnecessary fmt.Sprintfs

6210423

Signed-off-by: Max Asnaashari <[email protected]>

masnax force-pushed the osd_pool_size branch from a552128 to 6210423 Compare October 23, 2024 16:53

simondeziel reviewed Oct 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set OSD pool size when creating `ceph` and `cephfs` storage pools #14044

Set OSD pool size when creating `ceph` and `cephfs` storage pools #14044

masnax commented Sep 4, 2024 •

edited

Loading

github-actions bot commented Sep 4, 2024

masnax commented Sep 4, 2024 •

edited

Loading

masnax commented Sep 4, 2024

masnax commented Sep 4, 2024 •

edited

Loading

simondeziel commented Sep 5, 2024 •

edited

Loading

simondeziel Oct 22, 2024

masnax Oct 22, 2024

simondeziel Oct 23, 2024

masnax Oct 23, 2024

tomponline Oct 23, 2024

simondeziel left a comment

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

simondeziel Oct 23, 2024

masnax commented Oct 23, 2024

simondeziel commented Oct 23, 2024

	ceph --cluster "${LXD_CEPH_CLUSTER}" osd pool get "${pool1}" size --format json \| jq '.size' \| grep -q 1
	[[ "$(ceph --cluster "${LXD_CEPH_CLUSTER}" osd pool get "${pool1}" size --format json \| jq '.size')" = "1" ]]

	"--name", fmt.Sprintf("client.%s", d.config["ceph.user.name"]),
	"--name", "client." + d.config["ceph.user.name"],

Set OSD pool size when creating ceph and cephfs storage pools #14044

Are you sure you want to change the base?

Set OSD pool size when creating ceph and cephfs storage pools #14044

Conversation

masnax commented Sep 4, 2024 • edited Loading

github-actions bot commented Sep 4, 2024

masnax commented Sep 4, 2024 • edited Loading

masnax commented Sep 4, 2024

masnax commented Sep 4, 2024 • edited Loading

simondeziel commented Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simondeziel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masnax commented Oct 23, 2024

simondeziel commented Oct 23, 2024

Set OSD pool size when creating `ceph` and `cephfs` storage pools #14044

Set OSD pool size when creating `ceph` and `cephfs` storage pools #14044

masnax commented Sep 4, 2024 •

edited

Loading

masnax commented Sep 4, 2024 •

edited

Loading

masnax commented Sep 4, 2024 •

edited

Loading

simondeziel commented Sep 5, 2024 •

edited

Loading