Skip to content

Fetch current OSD pool configuration over the API#409

Merged
UtkarshBhatthere merged 5 commits into
canonical:mainfrom
masnax:osd-pool-get
Sep 12, 2024
Merged

Fetch current OSD pool configuration over the API#409
UtkarshBhatthere merged 5 commits into
canonical:mainfrom
masnax:osd-pool-get

Conversation

@masnax
Copy link
Copy Markdown
Contributor

@masnax masnax commented Aug 29, 2024

PUT /1.0/pools-op lets you set the replication size for several OSD pools.

This PR adds the ability to fetch the current sizes (and some other relevant OSD pool information) via GET /1.0/pools. The goal of this is to obtain the correct OSD pool names to supply to PUT /1.0/pools-op, as well as to determine what the current size of each pool is.

As discussed on mattermost: https://chat.canonical.com/canonical/pl/g3heo1kmz7r6db39re9qyzfw4w

@masnax masnax requested a review from lmlg August 29, 2024 15:31
Comment thread microceph/ceph/osd.go Outdated
Copy link
Copy Markdown
Contributor

@UtkarshBhatthere UtkarshBhatthere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor nit.

@masnax
Copy link
Copy Markdown
Contributor Author

masnax commented Sep 9, 2024

@UtkarshBhatthere @sabaini @lmlg Sort of a side note/question here: I've been thinking about the PUT /1.0/pools-op implementation a bit and I'm wondering if it's a bit aggressive?

Right now it takes 1 size value, and applies it to all specified OSD pools, but also sets the default OSD pool size for all future pools. This means if you want to resize just 1 pool, you have to call this API twice, because the first call will update the default OSD pool size as well, so you'd have to restore that with another request but with a blank set of OSD pools.

I wonder if we can make the default OSD pool size update based on a query parameter? Like ?set-default=1 so that you're not locked into changing the default pool size. To accommodate the query param in the CLI, we can add a --set-default flag too, to set the default OSD pool size.

I noticed the CLI help is also wrong. It says microceph pool set-rf <SIZE> <POOLS>... but the only arguments it interprets are <POOLS>... with size being supplied by --size.

@UtkarshBhatthere
Copy link
Copy Markdown
Contributor

@masnax fair enough, can you please create a GH issue for this behaviour ? We will take it up soon.

Copy link
Copy Markdown
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @masnax , this lgtm. It would be great to have a test for this though

Comment thread microceph/api/pool.go
Copy link
Copy Markdown
Contributor

@UtkarshBhatthere UtkarshBhatthere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change requested

Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
@lmlg
Copy link
Copy Markdown
Contributor

lmlg commented Sep 12, 2024

@UtkarshBhatthere @sabaini @lmlg Sort of a side note/question here: I've been thinking about the PUT /1.0/pools-op implementation a bit and I'm wondering if it's a bit aggressive?

Right now it takes 1 size value, and applies it to all specified OSD pools, but also sets the default OSD pool size for all future pools. This means if you want to resize just 1 pool, you have to call this API twice, because the first call will update the default OSD pool size as well, so you'd have to restore that with another request but with a blank set of OSD pools.

I wonder if we can make the default OSD pool size update based on a query parameter? Like ?set-default=1 so that you're not locked into changing the default pool size. To accommodate the query param in the CLI, we can add a --set-default flag too, to set the default OSD pool size.

I noticed the CLI help is also wrong. It says microceph pool set-rf <SIZE> <POOLS>... but the only arguments it interprets are <POOLS>... with size being supplied by --size.

Yes, those look line sane changes to me. Not sure if there's any client that requires the current command format to be stable, though.

@UtkarshBhatthere
Copy link
Copy Markdown
Contributor

Fair enough @lmlg, I will share a post in Ceph Announcements room with the newly created ticket and the description of the change.

@UtkarshBhatthere UtkarshBhatthere merged commit ef42f09 into canonical:main Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants