registrator
registrator copied to clipboard
gliderlabs
Update Consul API support to 1.2.0
Consul 1.0.7 was the last version to support the string Script API, which had been long deprecated. 1.1.0 and later use []string Args. This PR updates Consul support to this new property, bumps the Consul version to 1.2, and the Alpine version to 3.8.
Various docker/ dependencies are updated as well. The fsouza/go-dockerclient dependency is pinned to 1.2.2 for this change to work.
Hi @josegonzalez, can you please take a look at this PR and merge it if you accept the changes? It fixes a couple open issues.
Also a nudge @dsouzajude @mattatcha @progrium to see if we can get this merged. Thanks in advance!
We meet with the same problem.
Guys :) Would be nice to resolve this MR :) @josegonzalez @dsouzajude @mattatcha @progrium
@timiTao We'll get to it when we get to it. There isn't any need to ping anyone to do that. Thanks.
I think we are also experiencing the same issue with SERVICE_CHECK_SCRIPT. Hopefully this PR will help...
@timiTao We'll get to it when we get to it. There isn't any need to ping anyone to do that. Thanks.
We know there is no need to ping anyone but at this point, after almost six months, maybe it is time to ask for an update on the subject?
Are there any plans to support this?
If there aren't, we need to seriously consider moving away from using this project as we are currently blocked on our upgrade to consul in our infrastructure and we cannot have a mix setup forever.
Regardless, thanks for the opensource project that you created :-)
@josegonzalez We are blocked by the same issue. It would be great if we can get an update on the issue.
yeah I gave up and wrote my own version. sigh
https://github.com/hampsterx/ecs-consul-reg
you probably don't want to use it but works for us (using aws ECS). pretty simple..
@josegonzalez We're assuming you won't support this and have begun investigating an alternative solution that may take another 6-8 months to implement.
Some clarification from your side would help us plan such a long-term alternative. Thank you and appreciate everything you all have done.
I've come up with a similar fix in #669 before noticing this PR - would be good to get one of these merged, as the current tree simply fails to build for me otherwise.
Hi @josegonzalez
I have upgraded to consol 1.6.1 and started noticing errors in registrator to do with SERVICE_443_CHECK_SCRIPT like :
Unexpected response code: 400 (Invalid check: TTL must be > 0 for TTL checks)
My services are not being registered as expected.
I tried upgrading Registrator by deploying the latest docker image from master branch but am still seeing the above error.
after pulling down the branch for this PR and compiling the Registrator container and deploying it I can now see the services being successfully registered in consul and I'm able to browse the endpoints :
2019/11/19 04:34:51 added: xxxxxxxxxxxx xxxxxxxxxxx:xxxxxxxxx:443
Would you be able to review this PR and if you have no further objections approve and merge it into the master branch?
I think there are quite a few people experiencing this issue, so it would be nice to have this PR merged.
Registrator is a great application, let's keep the dream alive!
@nicofuccella this is your baby, have fun! :D
I just merged in another PR without conflicts that handles this :)
hi @mattatcha I pulled latest master Registrator container this morning and deployed, I'm seeing the following error in Consul in the Health Checks tab :
fork/exec curl --silent --fail http://xxxxxxxxx:xxxxxxxx/xxxxxxx/HealthCheckService: no such file or directory
I'm setting the healthcheck on our web server container via environment env :
SERVICE_4443_CHECK_SCRIPT=curl --silent --fail http://xxxxxxxxxx:xxxxxxx/xxxxxxxxx/HealthCheckService
If I revert the Registrator container back to the one compiled from this PR branch the health checks go green again and I'm able to browse the endpoint.
Did a quick google of the error and came across this :
https://github.com/hashicorp/consul/issues/3726#issuecomment-349111979
the Args array is executed directly without a shell, so it can't execute that command line as a single argument. You could do something like:
"Args": ["sh", "-c", "curl localhost --max-time 3 >/dev/null 2>&1"]
Is Consul expecting an array but getting the check via the environment variable as a string perhaps?
Just for laughs I tried to pass it through env as an array but got same error :
fork/exec ['sh', '-c', 'curl --silent --fail http://xxxxxxxx:xxxxxx/xxxxx/HealthCheckService']: no such file or directory
Is passsing through as an environment variable not supported anymore? Do I need to pass the check args through consul.json configuration?
@alaverty would you be able to create a PR with the correct changes to consul/consul.go?
@mattatcha I have branched from master and made the required changes to consul/consul.go
i have raised PR GH-670
I've compiled the PR branch and deployed it and can confirm the healthchecks are working as expected and healthy in consul.