Use Elasticsearch cluster health for check instead of ping

[inkel]

As stated in #287, using ping will only allow returning healthy or unhealthy statuses. By using
Elasticsearch Cluster Health API we can also return degraded if the cluster is in the yellow state.

While implementing this I realized that for some use cases users won't need to check for degraded, so to keep backwards dependency I've added a new option method ElasticsearchOptions to enable using cluster health check:

var options = new ElasticsearchOptions()
    .UseClusterHealth();

By default, it will use the original ping check.

[inkel]

@unaizorrilla as discussed in #287 I've started a PR. It isn't ready yet for final review, but I wanted to start showing the direction I'm taking.

One thing I noticed is that the functional tests for this are ok, but not enough for this new kind of check, and then come to the realization that properly testing might involve some tweaking of docker-compose.yml. Just so get an idea, I'm using a custom docker-compose-es.yml file to test these different kind of results:

version: '3'
services:
  elasticsearch_red:
    image: docker.elastic.co/elasticsearch/elasticsearch:6.2.4
    ports:
      - 19200:9200
    environment:
      - "ES_JAVA_OPTS=-Xms512m -Xmx512m"
      - node.master=true
      - node.data=false
      - discovery.type=single-node
      - discovery.zen.minimum_master_nodes=2
      - xpack.security.enabled=false

  elasticsearch_green:
    image: docker.elastic.co/elasticsearch/elasticsearch:6.3.2
    ports:
      - 19201:9200
    environment:
      - "ES_JAVA_OPTS=-Xms512m -Xmx512m"
      - node.master=true
      - node.data=true
      - discovery.type=single-node
      - discovery.zen.minimum_master_nodes=1
      - xpack.security.enabled=false

  elasticsearch_yellow:
    image: docker.elastic.co/elasticsearch/elasticsearch:6.3.2
    ports:
      - 19202:9200
    environment:
      - "ES_JAVA_OPTS=-Xms512m -Xmx512m"
      - node.master=true
      - node.data=true
      - discovery.type=single-node
      - xpack.security.enabled=false

This is not enough, as for the yellow server I also need to run curl -XPOST http://localhost:19202/foo/bar -d '{"name":"foobar"}' to create an index and write some data that doesn't have any replicas and thus trigger the yellow cluster state.

One idea that I have for testing this is that instead of using an Elasticsearch Docker image we could add a mock HTTP server to the tests that return the different payloads Elasticsearch Cluster Health API sends. That will make it easier to setup and run the tests, though it might make the tests a bit more complex on their setup. What do you think?

[Xabaril]

Hi @inkel

Let me some days to review this PR

[inkel]

@unaizorrilla I've added the tests without needing to spawn Elasticsearch demons but rather mocking an HTTP server that returns the same payload as Cluster Health API. Please check commit 9220378 and let me know what you think.

Nevertheless, I think this is ready for review!

[inkel]

@Xabaril hi! I've just pushed some updates with tests for the new functionality. Yes, please, take as long as you need! And thank you for publishing all these packages, we are using it at my company and they are wonderful, they just work 😸

I do have more PRs in mind, though, I hope you don't mind 😬

[inkel]

🤔 AppVeyor tests are failing due to not finding a right .NET framework:

dotnet.exe : Testhost process exited with error: It was not possible to find any compatible framework version
At C:\projects\aspnetcore-diagnostics-healthchecks\build.ps1:46 char:16
+         exec { & dotnet test}
+                ~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (Testhost proces...amework version:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
The specified framework 'Microsoft.AspNetCore.App', version '3.0.0' was not found.
  - Check application dependencies and target a framework version installe
d at:
      C:\Program Files\dotnet
  - Installing .NET Core prerequisites might help resolve this problem:
      https://go.microsoft.com/fwlink/?LinkID=798306&clcid=0x409
  - The .NET Core framework and SDK can be installed from:
      https://aka.ms/dotnet-download
  - The following versions are installed:
      2.1.0 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.1 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.2 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.3 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.4 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.5 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.6 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.7 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.9 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.10 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.11 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.1.12 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.2.0 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.2.1 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.2.4 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.2.5 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      2.2.6 at [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
. Please check the diagnostic logs for more information.
Test Run Aborted.

[unaizorrilla]

Let me check appveyor because is now failing and we don't change anything, the same build that works some days ago is failing now! 👎

[unaizorrilla]

Hi @inkel

I think the issue is related with microsoft/vstest#2189 , I'm trying to solve it asap!

[unaizorrilla]

I think there is a bug on Microsoft.Net.Test.Sdk 16.3.0 , downgrade to 16.2.0 build is working again! I try to review PR asap

[inkel]

@unaizorrilla any comments on this one?

[unaizorrilla]

Hi @inkel

we try to review and merge it asap

[inkel]

Thanks @sungam3r for the suggestion!

[inkel]

Hi, are there any updates on this?

[inkel]

Hi! Sorry for being pushy, but is there any update on this one?

[sungam3r]

@inkel Hi. I help to maintain this repo. I found this PR looking through open issues (#287). If you want to get this merged then I can review. Reopen and rebase onto master if you interested.

[inkel]

Hi @sungam3r! I'm not working with # anymore at the moment, and I've even left the company I was working at the time we needed this merged. I'd love to see the changes integrated as I think it's the right way to have a health check on Elasticsearch, but I cannot make any changes to this branch nor help maintaining it.

[sungam3r]

Covered by #1577 .

[inkel]

Oh wow, what a blast from the past seeing action in this PR.

I'm glad this is now covered.