canonical / microceph

MicroCeph is snap-deployed Ceph with built-in clustering
https://snapcraft.io/microceph
GNU Affero General Public License v3.0
225 stars 35 forks source link

`microceph pool set-rf` aggressively sets the default OSD pool size. #420

Open masnax opened 2 months ago

masnax commented 2 months ago

This command takes a list of pool names as arguments, and a --size flag which accepts an integer for the pool size.

However, behind the scenes, this command also updates the default pool size along with the specified pools. This means if you want to just change 1 pool's size, you have to call the command twice, once with the pool name in question, and then once again with no pool name and the old default pool size to reset it.

Instead, we can add a flag --set-default which acts as a true/false query parameter on the API itself, determining whether the user wants to set the default OSD pool size or not.

Assuming the user wants to set a pool size of 1, we should still ensure that mon_allow_pool_size_one is set, even when not changing the default pool size.

This will also help with this case in the test suite:

 sudo microceph pool set-rf --size 1 ""

where an explicit empty string needs to be passed to set the default pool size without changing any pools. Instead, we can validate that if --set-default is given, the minimum argument length is actually 0.

masnax commented 2 months ago

The empty string case is accommodated in the API handlers as well with this block, which feels hacky. With the query parameter, this would not be necessary:

https://github.com/canonical/microceph/blob/54cf5221106faa9c73d8aa888c8057413dfeb48d/microceph/ceph/osd.go#L1108-L1111