Zero server shutdown endpoint

[srfrog]

This PR adds a new HTTP endpoint at /shutdown to handle graceful service shutdowns. It also changes the process signal handling to work together with this command.

Example:

$ curl localhost:6080/shutdown
Server is shutting down...
# Done.

Closes #2285

---

This change is 

[manishrjain]

Are you willing to put in a bit more work for clarity? It would be great to switch this to y.Closer. Been on my mind for a while to do that.

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @golangcibot, @manishrjain, and @srfrog)

---

dgraph/cmd/zero/run.go, line 235 at r1 (raw file):

	// Initialize the servers.
	var st state
	st.serveGRPC(grpcListener, &wg, store)

can take in closer, but don't block on HasBeenClosed. Instead, just call defer closer.Done().

---

dgraph/cmd/zero/run.go, line 264 at r1 (raw file):

				}
				glog.Infof("--- Received %s signal", sig)
				signal.Stop(sdCh)

closer.Signal()

---

dgraph/cmd/zero/run.go, line 274 at r1 (raw file):

		<-st.zero.shutDownCh
		glog.Infoln("Shutting down...")
		close(st.zero.shutDownCh)

defer closer.Done()

---

dgraph/cmd/zero/run.go, line 283 at r1 (raw file):

	glog.Infoln("Running Dgraph Zero...")
	wg.Wait()

closer.Wait()

[codexnull]

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @golangcibot and @srfrog)

---

dgraph/cmd/zero/http.go, line 239 at r1 (raw file):

Previously, golangcibot (Bot from GolangCI) wrote…

Error return value of w.Write is not checked (from errcheck)

There's no authentication, right? So even if the dgraph server was started by root, it can still be shut down by any user?

[srfrog]

Sure, I'll change it

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @golangcibot and @srfrog)

[srfrog]

There's no authentication, right? So even if the dgraph server was started by root, it can still be shut down by any user?

That's correct. We need to add some access control to Zero.

[manishrjain]

Typically, devops folks tightly control who has ssh access to the servers. And you can only call these endpoints if you're calling it from localhost. So, there's certain security there.

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @golangcibot and @srfrog)

[codexnull]

Yes, and banks typically tightly control who has access to their vaults, yet they still occasionally get robbed. :-)

I understand that access to the server is most likely restricted, but two users who have access to the same Linux or Windows host still can't read each other's files or kill each other's processes, so they shouldn't be able to shut down each other's dgraph.

And servers get hacked all the time. Just last week there were news reports of 1 billion email address and passwords being uploaded to a hacking forum. The next leak may include a password used in a dgraph server.

I'm not trying to pick on this particular change or saying we have to fix it now, but I think if we're serious about being a production-ready database we need to try to prevent vectors for unauthorized access, denial-of-service attacks, etc. All other databases I've used do.

Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @golangcibot and @srfrog)

[srfrog]

I've created an issue specifically for zero access control - #2930
It could potentially ask an alpha for access or check the JWT itself. Or maybe even a simple HTTP digest.

[srfrog]

Reviewable status: 0 of 5 files reviewed, 5 unresolved discussions (waiting on @golangcibot and @manishrjain)

---

dgraph/cmd/zero/run.go, line 235 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

can take in closer, but don't block on HasBeenClosed. Instead, just call defer closer.Done().

Done.

---

dgraph/cmd/zero/run.go, line 264 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

closer.Signal()

Done.

---

dgraph/cmd/zero/run.go, line 274 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

defer closer.Done()

Done.

---

dgraph/cmd/zero/run.go, line 283 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

closer.Wait()

Done.

[manishrjain]

Valid points, @codexnull . Maybe we could just get rid of the /admin/shutdown and only trap Ctrl+C. I'm not sure why we had put the /admin/shutdown there in the first place. That'd then work with how linux treats the security of other users' processes.

Made a few changes, including removing shutdown endpoint. Can be merged.

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @golangcibot and @srfrog)

---

dgraph/cmd/zero/http.go, line 239 at r1 (raw file):

Previously, codexnull (Javier Alvarado) wrote…

There's no authentication, right? So even if the dgraph server was started by root, it can still be shut down by any user?

Let's not add this method then. Nobody asked for this anyway. In fact, we can try and remove a similar method from Alpha as well.

---

dgraph/cmd/zero/run.go, line 273 at r2 (raw file):

		<-st.zero.closer.HasBeenClosed()
		glog.Infoln("Shutting down...")
		close(sdCh)

Don't close sdCh, signal.Notify can still call it.

[srfrog]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @golangcibot and @manishrjain)

---

dgraph/cmd/zero/run.go, line 273 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Don't close sdCh, signal.Notify can still call it.

signal.Stop() shuts down this channel, it's safe to close it.