New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug 1372952: add liveness/readiness endpoints #214

Merged
merged 2 commits into from Jun 20, 2017

Conversation

Projects
None yet
2 participants
@escattone
Member

escattone commented Jun 15, 2017

This PR adds "liveness" and "readiness" endpoints to KumaScript for use by Kubernetes or any similar control system.

  • add "liveness"/"readiness" endpoints and their tests
  • throw error if FileLoader is unable to find any macros during initialization, and add test for that case

I wrote the "readiness" endpoint under the idea that KumaScript is not ready unless it satisfies three things:

  • it is up and running (the Express app can accept and respond to requests)
  • it can load one or more macros
  • it knows the document service (Kuma) is ready

Just for reference, Kuma's "readiness" only depends on two things:

  • it is up and running (the Django app can accept and respond to requests)
  • it can query the database

@escattone escattone requested a review from jwhitlock Jun 15, 2017

bug 1372952: add liveness/readiness endpoints
* add liveness/readiness endpoints to server.js
* throw error if FileLoader is unable to find
  any macros during initialization
* add tests for endpoints
* add test for FileLoader

@jwhitlock jwhitlock self-assigned this Jun 19, 2017

@jwhitlock

👍 Looks good. One optional nit in test setup around API /healthz endpoint.

it('The FileLoader should detect duplicate macros', function () {
assert.throws(
function() {
new ks_loaders.FileLoader({
root_dir: 'tests/fixtures'
});
}
},
/duplicate macros:[\s\S]+/

This comment has been minimized.

@jwhitlock

jwhitlock Jun 19, 2017

Member

Regex assertion of error messages is interesting. Python doesn't have it, I think. This is the first thing I've been jealous of.

@jwhitlock

jwhitlock Jun 19, 2017

Member

Regex assertion of error messages is interesting. Python doesn't have it, I think. This is the first thing I've been jealous of.

*/
readiness_GET: function (req, res) {
var msg = 'service unavailable ',
parts = url.parse(this.options.document_url_template),

This comment has been minimized.

@jwhitlock

jwhitlock Jun 19, 2017

Member

I wonder if we should add this as a config item, or do similar work to parse document_url_template, to fix bug 1368134. No changes for this PR, just thinking of the next PR.

@jwhitlock

jwhitlock Jun 19, 2017

Member

I wonder if we should add this as a config item, or do similar work to parse document_url_template, to fix bug 1368134. No changes for this PR, just thinking of the next PR.

This comment has been minimized.

@escattone

escattone Jun 19, 2017

Member

Good question. I think I like the idea of a separate config item, as that might be clearer and more controllable/flexible?

@escattone

escattone Jun 19, 2017

Member

Good question. I think I like the idea of a separate config item, as that might be clearer and more controllable/flexible?

try {
// If there are no macros or duplicate macros, an error
// will be thrown.
this.macro_processor.makeLoader();

This comment has been minimized.

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works if I rename the macro folder

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works if I rename the macro folder

// Finally, check that the document service is ready.
request.get(kumaReadiness, function (err, resp, body) {
if (!err && ((resp.statusCode >= 200) && (resp.statusCode < 400))) {

This comment has been minimized.

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works if I shut down the API server (docker-compose stop api)

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works if I shut down the API server (docker-compose stop api)

* from this service.
*/
liveness_GET: function (req, res) {
res.sendStatus(204);

This comment has been minimized.

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works

@jwhitlock

jwhitlock Jun 19, 2017

Member

👍 works

Show outdated Hide outdated tests/test-server.js
@escattone

This comment has been minimized.

Show comment
Hide comment
@escattone

escattone Jun 19, 2017

Member

Thanks for the thorough review (as always) @jwhitlock!

Member

escattone commented Jun 19, 2017

Thanks for the thorough review (as always) @jwhitlock!

@jwhitlock jwhitlock merged commit 4236fe0 into mdn:master Jun 20, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jwhitlock added a commit to jwhitlock/kuma that referenced this pull request Jun 20, 2017

Update Kumascript (PRs 165, 204, 205, 208, 5 more)
* mdn/kumascript#165 - csssyntax: Only link to named colors
* mdn/kumascript#204 - Macro testing framework
* mdn/kumascript#205 - CSSRef, CSS_Ref: Prep for title change
* mdn/kumascript#208 - CSS_Ref, csssyntax: Prep for syntax change
* mdn/kumascript#210 - GroupData, InterfaceData, etc: add Long Tasks API
* mdn/kumascript#211 - spec2, etc: Add * WEBGL_compressed_texture_s2tc_srgb
* mdn/kumascript#212 - Firefox_for_developers: Pass max version
* mdn/kumascript#214 - Add liveness, readiness endpoints
* mdn/kumascript#216 - SpecName, spec2: Update performance specs

@escattone escattone deleted the escattone:add-endpoints-for-k8s-1372952 branch Jun 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment