Ecwid / consul-api

Java client for Consul HTTP API
Apache License 2.0
416 stars 177 forks source link

Cannot register script health check on service #171

Closed pparthenhs closed 5 years ago

pparthenhs commented 5 years ago

Hello to everyone,

I am using the current library (1.4.1) as a maven dependency, also I am using consul version 1.3. But I cannot register script Health check on service.

My code was working on consul version 1.0.7, and using the library version 1.4.0

        NewService.Check serviceCheck = new NewService.Check();
        serviceCheck.setScript("docker exec mysql  mysqladmin -uroot -pxUNvbFUbHvv6hnkrTg86g7fXe87W9fTg status");
        serviceCheck.setInterval("30s");

But now I am getting

Exception in thread "main" OperationException{statusCode=400, statusMessage='Bad Request', statusContent='Invalid check: TTL must be > 0 for TTL checks'}
    at com.ecwid.consul.v1.agent.AgentConsulClient.agentServiceRegister(AgentConsulClient.java:278)
    at com.ecwid.consul.v1.agent.AgentConsulClient.agentServiceRegister(AgentConsulClient.java:265)
    at com.ecwid.consul.v1.ConsulClient.agentServiceRegister(ConsulClient.java:304)
    at eu.orchestrator.agent.Agent.checkIfContainerServiceRunning(Agent.java:838)
    at eu.orchestrator.agent.Agent.phase6(Agent.java:296)
    at eu.orchestrator.agent.Agent.bootAgent(Agent.java:84)
    at eu.orchestrator.agent.Agent.main(Agent.java:69)
smith3v commented 5 years ago

@pparthenhs I can't help you with original issue. But it seems you just published the password for your database. I suggest you to rorate it with the new one.

pparthenhs commented 5 years ago

@smith3v thnx for the advice! I changed the password at the same time I published the issue!

aleksandrserbin commented 5 years ago

consul-api now uses Script field to create checks, from Consul doc:

Args (array) - Specifies command arguments to run to update the status of the check. Prior to Consul 1.0, checks used a single Script field to define the command to run, and would always run in a shell. In Consul 1.0, the Args array was added so that checks can be run without a shell. The Script field is deprecated, and you should include the shell in the Args to run under a shell, eg. "args": ["sh", "-c", "..."].

So for Consul > 1.0 consul-api should use Args parameters

@vgv, do we need compatibility with Consul versions below 1.0?

vgv commented 5 years ago

@pparthenhs

Hi,

You should use setTTL instead of setInterval. This is kinda strange, but this is consul requirement, please see the docs - https://www.consul.io/api/agent/check.html

vgv commented 5 years ago

@aleksandrserbin

Yes, I think I will add support for Args later

aleksandrserbin commented 5 years ago

Interval is also valid option

aleksandrserbin commented 5 years ago

Created pull-request