search

interledgerjs / ilp-plugin-btp

This has been moved to the monorepo https://github.com/interledgerjs/interledgerjs
8 stars 7 forks source link

feat: optional http health check #53

Closed tangjeff0 closed 5 years ago

tangjeff0 commented 5 years ago

Kubernetes's ingress uses availability checks to make sure the endpoints that it routes traffic towards are indeed available. Because these checks use http(s), an http server must actually be responding with affirmatives (e.g. 200 OK). Fortunately, the websocket library used in this plugin can take in a server option, such that it may provide websocket connections as it always has, and additionally serve http responses as well.

jsf-clabot commented 5 years ago

CLA assistant check
All committers have signed the CLA.

tangjeff0 commented 5 years ago

I think it would better if the listener options included both port and server (the caller must provide exactly one of the two). The kubernetes ingress availability check would be handled by the caller of the plugin (or maybe a small wrapper plugin?), rather than the btp plugin itself. Not all consumers of ilp-plugin-btp/mini-accounts will be using kubernetes, and features specific to the container orchestration don't really belong in the plugin.

Availability checks by cloud providers are not too uncommon. Therefore I've added a boolean field in the _listener struct that will enable these checks to pass. By default, these checks will not occur and a plain websocket server will be used, as in the current version of master. Also, the logic of the httpServer and wsServer have been separated. Thoughts @sentientwaffle @sharafian ?

sentientwaffle commented 5 years ago

They aren't uncommon, but they aren't universal either. I still think the options should be able to provide either port or server. When its the latter, I don't think the plugin should add an endpoint.

To include this behavior you could create a wrapper plugin that extends the btp plugin.

sentientwaffle commented 5 years ago

LGTM once tests are passing