alphaheavy / consul-haskell Goto Github PK

Here I'm making a short list of key things to improve in consul-haskell.

• Document all functions.
• Provide some examples for common distributed-systems tasks, such as running a service with a health check from Haskell, or running a master-elected task across multiple Haskell servers.
• Sensibly group funciton docs into sections of tasks that you usually want to accomplish with consul.
• Use of Bool on errors.
  For example registerService :: ... -> m Bool returns False on any non-HTTP-200 response, discarding any useful error message, thus making it unclear how the caller should handle that case. Invalid request, or temporary loss of cluster leadership? The caller cannot tell.
  Some functions don't seem to check the status code at all; we need to fix that.
• Clean up comment this is here instead of the where to prevent typechecking nastiness
• Move the datacenter into ConsulClient (like the Python bindings do).
  Currently most functions take Maybe Datacenter as an argument, but it is usually not used, and if used, usually the same.
• Fix usage of plain async.
  That function isn't exception-safe (see first section of the main docs of the async package); use withAsync or other safe helper utilities that the docs instruct to use instead.
• Extract common code for functions that make requests.
  Most functions have very similar implementation:

  someFunctionality :: MonadIO m => ConsulClient -> FunctionalitySpecificRequestType -> m (Maybe FunctionalitySpecificResponseType)
  someFunctionality client@ConsulClient{..} request dc = do
    let hostnameWithScheme = hostWithScheme client
    initReq <- createRequest hostnameWithScheme
                             ccPort
                             "/v1/functionalitySpecificPath"
                             Nothing
                             (Just $ BL.toStrict $ encode request)
                             False
                             dc
    liftIO $ withResponse initReq ccManager $ \ response -> do
      case responseStatus response of
        x | x == status200 -> do
          bodyParts <- brConsume $ responseBody response
          return $ decode $ BL.fromStrict $ B.concat bodyParts
        _ -> return Nothing

  We should extract this common pattern into a helper function to make them easier to read.
• Implement ACL support.
• Add Github Actions based CI (Travis will discontinue OSS CI)
• Add tests for all functions, ideally also including coverage reports.

Hey Steve, do you plan to release this to hackage any time soon? I'd like to use it in a project (the key value functionality in particular).

If the function receives an index then expect that consul is going to block until there's an update to the thing you are watching. maybe consul-haskell should automatically modify the request to not timeout if this index value is there.

Finish implementing APIs from https://consul.io/docs/agent/http/catalog.html

https://consul.io/docs/agent/http/agent.html

Consul can do a lot. When writing tests, I want to look at a list and compare to confirm coverage.

The purpose of this issue is to scope out and document what should be tested against consul.

See here for example - https://github.com/alphaHeavy/consul-haskell/pull/49/checks?check_run_id=2287576964:

So the CI runner is unable to locate the consul executable.

All jobs are failing with this error.

https://consul.io/docs/agent/http/acl.html

This intends to pick up where #55 left off.

Ideas:

• Speed it up - caching? less to build?
• Drop the $PATH if we don't need it
• Save artifacts for a week?
• Separate job so we run only one haddock build, rather than one per version of consul
• Basic coverage in place (for docs), across modules/functions/types/etc.

See here: https://www.consul.io/api#authentication

We can include the Consul Token for authentication by either adding the X-Consul-Token header or Bearer scheme Authorization header (Authorization: Bearer).

We'll need functions for accepting and updating the token to use with the consul client. We'll also want this token to be optional (though highly encouraged, it's not required).

Swap master for something more meaningful (and appropriate/reasonable). develop seems to make a lot of sense for a few reasons:

• We have one, primary branch from which development happens and releases are cut from.
• The one branch is what we merge PRs into.
• A release is a tag of a commit on that branch.
• While main has a neutral tone, it doesn't seem much more meaningful/purposeful than master.

Hello,

Thank you for your work creating and maintaining this library. There are a pending tasks/tickets and PRs piling up, so I'd like to ask: what are the project's plans for maintaining the library?

Would it make sense to put out a call for maintainers?

When you watch something on Consul, it's a blocking HTTP request. Right now I'm passing in a managersettings to initializeConsulClient that disables the timeout when I want to watch things. This means I have to use two different consul clients & juggle them (because I don't want the no-timeout client used for non-watch requests.) Ideally I would manage my own manager & I could modify the request to never timeout if I wanted to watch data for changes on that one HTTP request.

Related to #2

Both #2 and #3 are blocking issue #1 IMHO

Implement Status https://consul.io/docs/agent/http/status.html

https://github.com/dysinger/consul-haskell/blob/master/src/Network/Consul.hs#L37

It would be nice to pass in a manager here instead of having it constructed inside this initializeConsulClient.

Hi, is this package known to work with Consul 1.0, which has some breaking changes?

At NixOS/nixpkgs#35602 we're planning to upgrade to Consul 1.0 so we're trying to gather a list of packaged software that might break.

Thanks!

• Require passing tests.
• Consider at least one approval before merging a PR.

cc @nh2

• Each release is a new section.
• Each logical "change" is described in a separate entry.
• Each entry includes a reference to the PR/commit/issue/etc.
• If possible, auto-generate for previous releases.

Upstream has a doc with a list of client libs: https://www.consul.io/api/libraries-and-sdks.html

consul-haskell is not on the list, and we should get it added.

In

I wait for Consul's port to be open before making requests to avoid race conditions between our tests and Consul not being ready.

Turns out this isn't enough, there is a time after which Consul will have opened the port but is still not ready. Evidenced by tests sometimes failing with http: Request PUT /v1/session/create, error: Missing node registration:

  Internal Session Tests
    testCreateSession:               2020/01/20 17:09:30 [ERR] http: Request PUT /v1/session/create, error: Missing node registration from=127.0.0.1:51866
FAIL
      tests/Main.hs:237:
      testCreateSession: No session was created
    testGetSessionInfo:              2020/01/20 17:09:30 [ERR] http: Request PUT /v1/session/create, error: Missing node registration from=127.0.0.1:51870
FAIL
      tests/Main.hs:250:
      testGetSessionInfo: No session was created
    testRenewSession:                2020/01/20 17:09:30 [ERR] http: Request PUT /v1/session/create, error: Missing node registration from=127.0.0.1:51874
FAIL
      tests/Main.hs:263:
      testRenewSession: No session was created
    testRenewNonexistentSession: OK
    testDestroySession:          OK

Very annoying; we'll have to have a more sophisticated check for Consul being ready.

See here: https://hackage.haskell.org/package/consul-haskell

Update that package with recent releases.

cc @nh2 can you help me with access to the package on hackage?

I was just messing around with this library, and I found that calling deregister service fails because it's making a GET request when the API calls for a PUT request.

Code example:

client <- initializeConsulClient "localhost" 8500 Nothing
print =<< getServices client Nothing Nothing
deregisterService client "test"

Results in output:

["test","consul"]
HttpExceptionRequest Request {
  host                 = "localhost"
  port                 = 8500
  secure               = False
  requestHeaders       = []
  path                 = "/v1/agent/service/deregister/test"
  queryString          = ""
  method               = "GET"
  proxy                = Nothing
  rawBody              = False
  redirectCount        = 10
  responseTimeout      = ResponseTimeoutDefault
  requestVersion       = HTTP/1.1
}
 (StatusCodeException (Response {responseStatus = Status {statusCode = 405, statusMessage = "Method Not Allowed"}, responseVersion = HTTP/1.1, responseHeaders = [("Allow","OPTIONS,PUT"),("Content-Type","text/plain; charset=utf-8"),("Vary","Accept-Encoding"),("X-Consul-Default-Acl-Policy","allow"),("Date","Sat, 03 Apr 2021 00:21:31 GMT"),("Content-Length","22")], responseBody = (), responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}) "method GET not allowed")

The following worked:

curl -XPUT localhost:8500/v1/agent/service/deregister/test

Consul version:

$ consul version
Consul v1.9.4
Revision 10bb6cb3b
Protocol 2 spoken by default, understands 2 to 3 (agent will automatically use protocol >2 when speaking to compatible agents)

This test code

connects to consul on port 8500 and inserts a TTL check.

When you build the package with tests enabled (which is what many build tools, in my case nix, do by default), this will mess with the consul on your machine.

It doesn't even delete the TTL check, so after the test executable runs, the TTL check will swiftly fail, possibly even paging you.