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.

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

Update mount-block-device.rst #408

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ismailkayi
Copy link

@ismailkayi ismailkayi commented Aug 29, 2024

1- Missing sudo command added. (#404)
2 - Steps have been added for the issue : #406

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Documentation update (Doc only change)

How Has This Been Tested?

NOTE: All functional changes should accompany corresponding tests (unit tests, functional tests etc).

Please describe the addition/modification of tests done to verify this change. Please also list any relevant details for your test configuration.

Contributor's Checklist

Please check that you have:

  • updated the user documentation with corresponding changes.

1- Missing sudo command added. (canonical#404)
2 - Steps have been added for the issue : canonical#406
Copy link
Contributor

@lmlg lmlg left a comment

Choose a reason for hiding this comment

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

lgtm, although I'm wondering if it wouldn't also be possible to specify the ceph.conf path to the specific commands instead of using a symbolic link

Comment on lines +110 to +111
$ sudo ln -s /var/snap/microceph/current/conf/ceph.conf /etc/ceph/
$ sudo ln -s /var/snap/microceph/current/conf/ceph.keyring /etc/ceph/
Copy link
Contributor

@UtkarshBhatthere UtkarshBhatthere Sep 3, 2024

Choose a reason for hiding this comment

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

These 2 could be combined to:
$ sudo ln -s /var/snap/microceph/current/conf/ /etc/ceph

Copy link
Contributor

Choose a reason for hiding this comment

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

Also look at line 126, 127. These would have to be removed as we are explicitly providing the conf paths.

we wrote (10MiB). This is because MicroCeph configures 3 way replication by default.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a change here ?

@UtkarshBhatthere
Copy link
Contributor

lgtm, although I'm wondering if it wouldn't also be possible to specify the ceph.conf path to the specific commands instead of using a symbolic link

Yes we can, infact that is what the document does at the moment. I still feel that having a symlink there could be good. (we have plans to do the symlinking automatically from inside microceph soon).

@UtkarshBhatthere
Copy link
Contributor

@ismailkayi please sign your commits using a GPG key. ref. This is required for all contributions to be accepted for MicroCeph (check failing CI test)

@UtkarshBhatthere UtkarshBhatthere added the documentation Improvements or additions to documentation label Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants