Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying a status field when registering checks. #859

Merged
merged 2 commits into from
May 11, 2015

Conversation

Heuriskein
Copy link
Contributor

Allow specifying a status field in the agent/service/register and agent/check/register endpoints.

This status must be one of the valid check statuses: 'passing', 'warning', 'critical', 'unknown'.
If the status field is not present or the empty string, the default of 'critical' is used.

This resolves issue #706

Feedback welcome on anything, of course, but I'm particularly interested in:

  • Where should the ValidStatus function (in consul/structs/structs.go) go? What should it be named?
  • Should the unknown status be acceptable when creating a new check?

…nt/check/register endpoints.

This status must be one of the valid check statuses: 'passing', 'warning', 'critical', 'unknown'.
If the status field is not present or the empty string, the default of 'critical' is used.
@salehe
Copy link

salehe commented Apr 12, 2015

I'm restating my opinion on #668 here :)
I think the best approach is to deprecate unknown state (less burden on client to reason about) and provide a default state when registering a service (delegate the reasoning to the registrar). This default indicates:

  • state of service when there is no check registered
  • state of a new check while it's status is unknown (no need to provide a default state while registering a check)

This approach prevents unnecessary race conditions and also generalizes service queries (e.g. DNS queries)

@ryanuber
Copy link
Member

@salehe we will still need to be able to pass check status during check registration, because we have service checks as well as node checks (not attached to any particular service). We should support passing the status in for both. This also keeps things simple. We would otherwise need to create a new field for storing the default status of the service, and persist that with the registration to be able to use it as checks were added later on.

@armon
Copy link
Member

armon commented Apr 13, 2015

@Heuriskein We should not allow "unknown" as it is being deprecated. It is already mostly on the way out, but the constants are left for backwards compatibility�.

@Heuriskein
Copy link
Contributor Author

Removed unknown.

On Mon, Apr 13, 2015 at 12:47 PM, Armon Dadgar notifications@github.com
wrote:

@Heuriskein https://github.com/Heuriskein We should not allow "unknown"
as it is being deprecated. It is already mostly on the way out, but the
constants are left for backwards compatibility.


Reply to this email directly or view it on GitHub
#859 (comment).

@armon
Copy link
Member

armon commented May 11, 2015

@Heuriskein Thanks! Sorry about the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants