Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optionally create entities for cephfs storage pool #12538

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Optionally create entities for cephfs storage pool #12538

merged 7 commits into from
Nov 28, 2023

Conversation

masnax
Copy link
Contributor

@masnax masnax commented Nov 20, 2023

Adds the config keys:

  • cephfs.create_missing
    • Creates the filesystem and the necessary data and metadata OSD pools if they don't exist
  • cephfs.osd_pg_num
    • Specifies the pg_num for the OSD pools that are created with create_missing. If unset, will default to 32.
  • cephfs.meta_pool
  • cephfs.data_pool
    • Specifies the respective names of the metadata and data OSD pools to expect if we need to create the filesystem with create_missing. If these OSD pools already exist, then they will be re-used, otherwise they will be created.

If any of the above keys are set but the fs already exists, then the creation will error out.

@masnax masnax requested a review from tomponline as a code owner November 20, 2023 23:55
@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Nov 20, 2023
@masnax masnax added API Changes to the REST API and removed API Changes to the REST API labels Nov 20, 2023
@masnax masnax force-pushed the cephfs-create branch 2 times, most recently from 548e384 to 5ad0de8 Compare November 21, 2023 00:51
tomponline
tomponline reviewed Nov 21, 2023
View reviewed changes
doc/api-extensions.md Outdated Show resolved Hide resolved
tomponline
tomponline requested changes Nov 21, 2023
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

A few minor code style changes and improvements to error handling.

@masnax
Copy link
Contributor Author

masnax commented Nov 21, 2023

@tomponline I'll put a pause on this until I've sorted out your comments which apply to the ceph driver as well. That way the two ceph drivers can be a bit more streamlined.

@masnax masnax marked this pull request as draft November 21, 2023 15:51
@tomponline
Copy link
Member

@masnax any news on progress for this one?

@masnax
Copy link
Contributor Author

masnax commented Nov 27, 2023

@masnax any news on progress for this one?

There's a lot of usages of TryRunCommand throughout the storage drivers that I'm trying to deduce the purpose of, so that's what's been halting progress here. I suppose in the mean time I can just update cephfs and make an issue for cleaning up the storage drivers.

@tomponline
Copy link
Member

@masnax any news on progress for this one?

There's a lot of usages of TryRunCommand throughout the storage drivers that I'm trying to deduce the purpose of, so that's what's been halting progress here. I suppose in the mean time I can just update cephfs and make an issue for cleaning up the storage drivers.

yes please

@masnax masnax force-pushed the cephfs-create branch 2 times, most recently from a193c00 to e6e3df6 Compare November 28, 2023 01:00
@masnax
lxd/storage/drivers: Update cephfs entity helpers
41ff925 
Adds a helper to find osd pools, and updates the fs lookup helper to
differentiate between exit codes from ceph.

Signed-off-by: Max Asnaashari <[email protected]>
@masnax masnax marked this pull request as ready for review November 28, 2023 01:20
@masnax
Copy link
Contributor Author

masnax commented Nov 28, 2023

@tomponline OK this has now been updated with the following:

  • The OSD Pool and FS creation will now be reverted if there's an error during creation.
  • The cephfs helpers for checking existence of entities now properly differentiate between exit codes.
  • TryRunCommand is not used for create/delete operations.
  • API extension has been renamed.

@masnax masnax force-pushed the cephfs-create branch 3 times, most recently from 0647572 to 58cf5c6 Compare November 28, 2023 05:32
tomponline
tomponline approved these changes Nov 28, 2023
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

tomponline
tomponline requested changes Nov 28, 2023
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Tests failing

 lxc storage delete cephfs2
+ set +x
+ timeout --foreground 120 /home/runner/go/bin/lxc storage delete cephfs2 --verbose
Storage pool cephfs2 deleted
+ ceph fs fail cephfs
Error ENOENT: Filesystem not found: 'cephfs'
+ cleanup
+ set +ex

@masnax masnax requested a review from tomponline November 28, 2023 15:00
@tomponline tomponline merged commit 80e12aa into canonical:main Nov 28, 2023
26 checks passed
@roosterfish roosterfish mentioned this pull request Dec 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomponline tomponline tomponline approved these changes

Assignees
No one assigned
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@masnax @tomponline