Start etcd daemon upon starting of glusterd daemon #65
Conversation
Currently with starting of glusterd daemon etcd is not starting. For bootstrapping of etcd first basic requirement is to run etcd daemon on every node. With this patch it will start etcd daemon upon starting of glusterd daemon. Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
| @@ -41,4 +42,12 @@ func main() { | |||
| if err != nil { | |||
| log.Fatal("Could not start GlusterD Rest Server. Aborting.") | |||
| } | |||
|
|
|||
| // Starting etcd daemon upon starting of GlusterD | |||
| etcd_start := exec.Command("/bin/etcd") | |||
There was a problem hiding this comment.
Have you tested these changes? I don't think etcd instance will come up here. OTOH, we'd need to pass some additional parameters to start etcd. Refer to etcd documentation.
There was a problem hiding this comment.
I have tested this change only in sample programme. this is not my final change. will add some parameter in new PR.
Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
| @@ -0,0 +1,36 @@ | |||
| package context | |||
There was a problem hiding this comment.
Why this needs to be in context package? You could have a separate package called etcd-mgmt and then have bootstrap.go.
|
We'd also need to stop the etcd daemon on glusterd shutdown. |
Resolving previous commit comment Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
|
I'd also like to see unit test coverage for this. |
Resolving previous commit comment Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
Resolving previous commit comment Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
| return err | ||
| } | ||
|
|
||
| HostPort2379 := "http://" + hostname + ":2379" |
There was a problem hiding this comment.
We should not start names with caps unnecessarily until and unless they are exposed to other packages. Also I'd like to see a better name here. There is no harm of assigning same value to multiple variables. listenClientUrils, advClientUrls would be better for the sake of better code readability
|
You'd also need to merge the master and repush the changes as this branch indicates a conflict with the master |
|
@gaurav36 : Also add test for etcdmgmt package |
Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
Resolving previous commit comment Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
|
|
||
| advClientUrls := "http://" + hostname + ":2380" | ||
|
|
||
| etcdStart := exec.Command("/bin/etcd", |
There was a problem hiding this comment.
we are starting etcd it self. If we are executing any etcd command then we should have etcdCommand here.
There was a problem hiding this comment.
@gaurav36 : etcdCommand interprets what command string you have formed for the etcd instance to bring up. If you look at the signature of exec.Command it returns a command structure. Check other projects if you wish, mostly they use 'cmd' for the return value.
Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
| for { | ||
|
|
||
| // Waiting for 15 second. Within 15 second health of etcd should | ||
| // be true otherwise it should through an error |
Resolving previous commit comment Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
In previous commit adding log in better way Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
|
|
||
| initialAdvPeerUrls := "http://" + hostname + ":2380" | ||
|
|
||
| etcdStart := exec.Command("/bin/etcd", |
There was a problem hiding this comment.
This would fail in systems where etcd is not installed. This has to be taken care in install-req.sh
There was a problem hiding this comment.
Please do not use '/bin/etcd'. use etcd instead as the the executable should be picked up from $GOPATH
Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
Minor change in previous commit. Issue gluster#64 Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
Start etcd daemon upon starting of glusterd daemon
Currently with starting of glusterd daemon etcd is not starting.
For bootstrapping of etcd first basic requirement is to run etcd
daemon on every node.
With this patch it will start etcd daemon upon starting of
glusterd daemon.
Issue #64
Signed-off-by: Gaurav Kumar Garg garg.gaurav52@gmail.com