fix(boot.go,src/healthsrv): add readiness and liveness probes by arschles · Pull Request #57 · deis/minio

arschles · 2016-02-04T23:11:44Z

Fixes #54

After this is merged and a new container is built, deis/charts#86 needs to be done

bacongobbler · 2016-02-05T17:14:30Z

  version: bb44bb2e4817fe71ba7082d351fd582e7d40e3ea
 - name: github.com/fsouza/go-dockerclient
-  version: 76fd6c68cf24c48ee6a2b25def997182a29f940e
+  version: ""


I'm not sure why glide did this. Investigating...

@bacongobbler after running glide up again (more precisely, make glideup), this version number didn't come back. In arschles@08099c9, however, another version number did come back for a launchpad.net dependency...

@technosophos suggested that I file an issue on glide for this, so I'm working on getting a solid set of repro steps down before I do.

krancour · 2016-02-05T17:26:00Z

I'm torn on whether it might be better to create the minio client used by the health check in / closer to the healthcheck code instead of doing so in boot.go and passing it in. On the one hand, the complexity of creating that client seems like it should be handled a little closer to where it's used rather than the boot script which really handles overall orchestration. On the other hand, secrets are used in constructing the minio client and you probably don't want to moves dependencies on kubernetes down into the healthcheck code. Given that I can see both sides of this, I wouldn't let this stand in the way of my LGTM, but I just figured I'd mention it in case it inspires you in some way.

and a single health check endpoint ‘/healthz’

so that the health check server can use it to check the health of the minio server

…lth checks

…rrors

and add a test for it

also specify the health check server port

…tch the default …while still keeping it explicitly defined

…ace for listing buckets The Go minio client adheres to it, and it will be used for more easily testing the healthz endpoint handler. The alternative was to use this interface: https://godoc.org/github.com/minio/minio-go#CloudStorageClient …

…handler

arschles · 2016-02-05T17:57:39Z

@krancour good point, and thanks for that. I think the other option for minio client creation location is in the Serve function in the ./src/healthsrv package. I don't think it would harm anything, but it seemed like it was more prudent to create it inside boot.go, since it may be used elsewhere in the future. Also, it may be beneficial for tests, in the case where I may want to test the Serve function in the future (I don't right now though since it's very basic)

gets a version back for launchpad

mboersma · 2016-02-05T18:20:22Z

This code looks good to me once the glide issue with go-dockerclient is sorted out.

also re-run ‘glide up’, which adds versions back in for github repositories. finally, set the minio version to latest (as of this writing)

arschles · 2016-02-05T21:16:37Z

@mboersma @bacongobbler after running a glide up, it looks like there are no more empty versions for github projects left

bacongobbler · 2016-02-05T21:20:01Z

-  version: 68415e7123da32b07eab49c96d2c4d6158360e9b
-  repo: https://github.com/golang/protobuf
+  version: ""
+  repo: https://code.google.com/p/goprotobuf


code.google.com no longer exists. Any idea how this would occur?

no idea. glide seems to toggle that just about every time I run glide up

fix(boot.go,src/healthsrv): add readiness and liveness probes

arschles added bug in progress labels Feb 4, 2016

arschles self-assigned this Feb 4, 2016

arschles added this to the v2.0-beta1 milestone Feb 4, 2016

arschles mentioned this pull request Feb 4, 2016

move minio config templates out, make more modular #60

Closed

arschles added awaiting review and removed in progress labels Feb 5, 2016

arschles changed the title ~~fix(boot.go,src/healthsrv): add readiness and liveness probes (WIP)~~ fix(boot.go,src/healthsrv): add readiness and liveness probes Feb 5, 2016

bacongobbler reviewed Feb 5, 2016
View reviewed changes

arschles force-pushed the health-check branch from 57fe51e to 74015a7 Compare February 5, 2016 17:16

krancour added the LGTM1 label Feb 5, 2016

Aaron Schlesinger added 11 commits February 5, 2016 09:41

fix(boot.go,src/healthsrv): add health check server

f46d64b

and a single health check endpoint ‘/healthz’

fix(boot.go): add log line to indicate starting the minio server

040206b

fix(glide.lock,glide.yaml): add minio-go dependency

abdbcc1

so that the health check server can use it to check the health of the minio server

fix(boot.go,src/healthsrv): use the minio client to test minio on hea…

a0c231a

…lth checks

fix(src/healthsrv/healthz_handler.go): add logging for health check e…

7597cb8

…rrors

fix(Makefile): add ability to run go unit tests

e4376bd

fix(boot.go,boot_test.go): move minio client creation logic out

ed3bc61

and add a test for it

fix(manifests/deis-minio-rc.yaml): add liveness and readiness probes

566a81e

also specify the health check server port

fix(manifests/deis-minio-rc.yaml): change the health check port to ma…

acdbcf7

…tch the default …while still keeping it explicitly defined

fix(src/healthsrv/healthz_handler_test.go): add test for the healthz …

4e774e9

…handler

arschles force-pushed the health-check branch from 611c4c5 to 4e774e9 Compare February 5, 2016 17:41

arschles mentioned this pull request Feb 5, 2016

add readiness and liveness probes to the deis-dev minio RC manifest deis/charts#86

Closed

fix(glide.lock): re-run glide up

08099c9

gets a version back for launchpad

mboersma added the LGTM2 label Feb 5, 2016

fix(glide.yaml): add version to minio-go dependency

9b7d68b

krancour removed the awaiting review label Feb 5, 2016

fix(glide.yaml,glide.lock): add utils and aboutme sub-packages

c2353e4

also re-run ‘glide up’, which adds versions back in for github repositories. finally, set the minio version to latest (as of this writing)

bacongobbler reviewed Feb 5, 2016
View reviewed changes

arschles added a commit that referenced this pull request Feb 5, 2016

Merge pull request #57 from arschles/health-check

8a743ba

fix(boot.go,src/healthsrv): add readiness and liveness probes

arschles merged commit 8a743ba into deis:master Feb 5, 2016

arschles deleted the health-check branch February 5, 2016 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(boot.go,src/healthsrv): add readiness and liveness probes#57

fix(boot.go,src/healthsrv): add readiness and liveness probes#57
arschles merged 14 commits into
deis:masterfrom
arschles:health-check

arschles commented Feb 4, 2016

Uh oh!

bacongobbler Feb 5, 2016

Uh oh!

arschles Feb 5, 2016

Uh oh!

arschles Feb 5, 2016

Uh oh!

krancour commented Feb 5, 2016

Uh oh!

arschles commented Feb 5, 2016

Uh oh!

mboersma commented Feb 5, 2016

Uh oh!

arschles commented Feb 5, 2016

Uh oh!

bacongobbler Feb 5, 2016

Uh oh!

arschles Feb 5, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

arschles commented Feb 4, 2016

Uh oh!

bacongobbler Feb 5, 2016

Choose a reason for hiding this comment

Uh oh!

arschles Feb 5, 2016

Choose a reason for hiding this comment

Uh oh!

arschles Feb 5, 2016

Choose a reason for hiding this comment

Uh oh!

krancour commented Feb 5, 2016

Uh oh!

arschles commented Feb 5, 2016

Uh oh!

mboersma commented Feb 5, 2016

Uh oh!

arschles commented Feb 5, 2016

Uh oh!

bacongobbler Feb 5, 2016

Choose a reason for hiding this comment

Uh oh!

arschles Feb 5, 2016

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants