Skip to content

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.

Comparing the ceph status output before and after writing the file shows that
the MicroCeph cluster has grown by 30MiB which is thrice the size of the file
we wrote (10MiB). This is because MicroCeph configures 3 way replication by default.
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