-
Notifications
You must be signed in to change notification settings - Fork 82
Updating quick user guide #1076
base: master
Are you sure you want to change the base?
Conversation
doc/quick-start-user-guide.md
Outdated
```sh | ||
# yum install rpcbind | ||
``` | ||
For other distors please install rpcbind and then follow the steps given below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpcbind is no longer needed for glusterd2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prashanthpai I had a discussion with @kshlm regarding same, as he suggested I needed to check with others before removing it. Will remove it. and update the PR
binaries of the latest release. Like all Go programs, glusterd2 is a single | ||
binary (statically linked) without external dependencies. You can download the | ||
[latest release](https://github.com/gluster/glusterd2/releases) from Github. | ||
For other distros, you can download binaries of the [latest release](https://github.com/gluster/glusterd2/releases) from Github. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really do not see how this change fixes or improves anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fedora does not come under "non-rpm distros", but to install glusterd2 in Fedora we need to use binaries, so it is better to mention just distros to make it more generalized.
doc/quick-start-user-guide.md
Outdated
@@ -71,9 +77,41 @@ etcdcurls = "http://192.168.56.101:2379" | |||
etcdpurls = "http://192.168.56.101:2380" | |||
``` | |||
|
|||
**Authentication:** In glusterd2 REST API authentication is enabled by default. To disable rest authentication add `restauth=false` in Glusterd2 config file(Default path is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except when you have links, documentation in markdown shouldn't exceed 80 character limit.
doc/quick-start-user-guide.md
Outdated
@@ -71,9 +77,41 @@ etcdcurls = "http://192.168.56.101:2379" | |||
etcdpurls = "http://192.168.56.101:2380" | |||
``` | |||
|
|||
**Authentication:** In glusterd2 REST API authentication is enabled by default. To disable rest authentication add `restauth=false` in Glusterd2 config file(Default path is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default path of config file should be a separate subsection that comes before Authentication
section
doc/quick-start-user-guide.md
Outdated
**Start glusterd2 process:** Glusterd2 is not a daemon and currently can run only in the foreground. | ||
### Running glusterd2 for RPM installation | ||
|
||
**Enable glusterd2 service:** To enable glusterd2 service run: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling can mean anything. Please mention that this command is to ensure glusterd2 service starts automatically when the system starts
doc/quick-start-user-guide.md
Outdated
``` | ||
Please ensure that glusterd2 service status is "active (runnning)" before proceeding. | ||
|
||
**Authentication:** In glusterd2, REST API authentication is enabled by default. To disable rest authentication add `restauth=false` in Glusterd2 config file(`/var/lib/glusterd2/glusterd2.toml`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is present twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made the changes.
@prashanthpai @aravindavk @Madhu-1 Please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a few questions. have raised them as a comment. If they arent valid, do let me know.
And from line 97 in the existing code, can we have that line above the commands like add peer and stuff. It has instructions about rest and cli which to all commands. This will make it clearer.
doc/quick-start-user-guide.md
Outdated
etcdendpoints = "http://[ip_address]:[port]" | ||
noembed = true | ||
``` | ||
Provide ip-address and port of node where etcd is running. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we point them to the link where they can find how to set up external etcd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now do we have any documentation related to external etcd?
doc/quick-start-user-guide.md
Outdated
|
||
```sh | ||
# systemctl restart glusterd2 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have to mention it for binaries as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, will do that. For binaries I think we will have to stop the glusterd2 if already running, and then start it with same command.
doc/development-guide.md
Outdated
@@ -71,7 +71,7 @@ Once your PR has has been submitted for review the following critieria will need | |||
* Each PR needs reviews accepting the change from at least two developers for merging | |||
* It is common to request reviews from those reviewers automatically suggested by github | |||
* Each PR needs to have been open for at least 24 working hours to allow for community feedback | |||
* The 24 working hours counts hours occuring Mon-Fri in the local timezone of the submitter | |||
* The 24 working hours counts hours occurring Mon-Fri in the local timezone of the submitter | |||
* Each PR must be fully updated to master and tests must have passed | |||
|
|||
When the criteria are met a project maintainer can merge your changes into the project's master branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: When the criteria are met, a project maintainer ...
doc/quick-start-user-guide.md
Outdated
``` | ||
|
||
### Output | ||
|
||
You will see an output similar to the following: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we dont get an output like this if we use RPMs or if we mention the log file. Can you verify that and change this accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I will check.
doc/quick-start-user-guide.md
Outdated
|
||
```sh | ||
# ./glusterd2 --config conf.toml | ||
``` | ||
|
||
### Authentication | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any step to be performed while using the default authentication? I remember having messed up something here so that i had to turn it off. If yes, can you mention that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for using the default authentication they will have to go through the authentication document #1021, IMO in quick user guide we should help users in using GD2 with minimal setup efforts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small set of changes requested.
doc/development-guide.md
Outdated
@@ -71,7 +71,7 @@ Once your PR has has been submitted for review the following critieria will need | |||
* Each PR needs reviews accepting the change from at least two developers for merging | |||
* It is common to request reviews from those reviewers automatically suggested by github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub (in place of github)
doc/quick-start-user-guide.md
Outdated
|
||
### Running glusterd2 | ||
### Custom configuration for glusterd2 [Optional Step]. | ||
|
||
**Create a working directory:** This is where glusterd2 will store all data which includes logs, pid files, etcd information etc. For this example, we will be using a temporary path. If a working directory is not specified, it defaults to current directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the purposes of this example,
doc/quick-start-user-guide.md
Outdated
|
||
```sh | ||
# ./glusterd2 --config conf.toml | ||
``` | ||
|
||
### Authentication | ||
|
||
In glusterd2, REST API authentication is enabled by default. To disable rest authentication add `restauth=false` in Glusterd2 config file(`/etc/glusterd2/glusterd2.toml`) or the custom config file provided by you (conf.toml as per above example) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something that is recommended? If not, perhaps it is prudent to add a disclaimer like "the developers would recommend that the ReST API authentication is not disabled"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO sure will add the disclaimer, but IMO for the pupose of speeding up the setup, its better to let user disable the authentication for trying out GD2.
…er_guide Conflicts: doc/quick-start-user-guide.md
5212264
to
b5392ce
Compare
doc/quick-start-user-guide.md
Outdated
**Create a working directory:** This is where glusterd2 will store all data which includes logs, pid files, etcd information etc. For this example, we will be using a temporary path. If a working directory is not specified, it defaults to current directory. | ||
### Custom configuration for glusterd2 [Optional]. | ||
|
||
**Create a working directory:** This is where glusterd2 will store all data which includes logs, pid files, etcd information etc. For the purposes of this example, we will be using a temporary path. If a working directory is not specified, it defaults to current directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no such thing as working directory anymore. The location of logs and pid files can be different.
@@ -69,12 +69,12 @@ If you are planning on making a large set of changes or a major architectural ch | |||
**Review Process:** | |||
Once your PR has has been submitted for review the following critieria will need to be met before it will be merged: | |||
* Each PR needs reviews accepting the change from at least two developers for merging | |||
* It is common to request reviews from those reviewers automatically suggested by github | |||
* It is common to request reviews from those reviewers automatically suggested by GitHub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title of PR needs to be more accurate and concise. Titles such as Updating quick user guide
don't give a any useful information to reviewers. This applies to commit messages as well.
doc/quick-start-user-guide.md
Outdated
**Start glusterd2 process:** Glusterd2 is not a daemon and currently can run only in the foreground. | ||
### Using external etcd instead of embedded etcd. | ||
|
||
By default glusterd2 uses embedded etcd, but glusterd2 also supports usage of external etcd. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tone of this section must reflect that external etcd is the default and is the recommended way and supported way. It currently says otherwise.
doc/quick-start-user-guide.md
Outdated
|
||
By default glusterd2 uses embedded etcd, but glusterd2 also supports usage of external etcd. | ||
|
||
To provide external etcd details. Edit glusterd2 config file by adding `noembed` option and specifying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A sentence cannot begin with To provide external etcd details
and just end there ;)
doc/quick-start-user-guide.md
Outdated
To provide external etcd details. Edit glusterd2 config file by adding `noembed` option and specifying | ||
the etcd endpoint: | ||
|
||
```toml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also useful to tell what file this is.
doc/quick-start-user-guide.md
Outdated
etcdendpoints = "http://[ip_address]:[port]" | ||
noembed = true | ||
``` | ||
Provide ip-address and port of node where etcd is running. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is vague. What needs to be provided here is the etcd client URL.
``` | ||
|
||
Check the status of glusterd2 service: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra line
doc/quick-start-user-guide.md
Outdated
``` | ||
|
||
**Start glusterd2 service:** To start glusterd2 process run: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra line
doc/quick-start-user-guide.md
Outdated
|
||
### Running glusterd2 for RPM installation | ||
|
||
**Enable glusterd2 service:** Enable glusterd2 service to ensure that glusterd2 service starts automatically when system starts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no meaning to enable glusterd2 service
, it's just that systemctl operation is named so.
To ensure..., run:
b5392ce
to
095df04
Compare
Closes : #1058