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

Alias check - referenced check not found behavior #7374

Closed
oto313 opened this issue Mar 3, 2020 · 3 comments · Fixed by #7384
Closed

Alias check - referenced check not found behavior #7374

oto313 opened this issue Mar 3, 2020 · 3 comments · Fixed by #7384
Labels
theme/health-checks Health Check functionality type/bug Feature does not function as expected

Comments

@oto313
Copy link

oto313 commented Mar 3, 2020

Please search the existing issues for relevant feature requests, and use the reaction feature (https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to add upvotes to pre-existing requests.

Feature Description

Consul supports alias check which mirrors the state of a reference check. But in case the reference check does not exist the alias check is passing and the output message is "No checks found.". There could be an option for specifying behavior in such a situation. You could specify if it passes or fails.

There is already TODO in code: alias.go

Use Case(s)

In case you register services and checks (eg. one check and one alias check which points to the first check) and then deregister the referenced check which is used in alias check, the alias check is passing, even if the referenced check does not exist.

@oto313 oto313 changed the title Alias check - service not found set check to critical Alias check - service not found behavior Mar 3, 2020
@oto313 oto313 changed the title Alias check - service not found behavior Alias check - referenced check not found behavior Mar 3, 2020
@pierresouchay
Copy link
Contributor

@oto313 I looked a bit, it is a bit more complicated:

  • A service without checks is valid and supposed to be Healthy
  • The current system does not check about Service Existing

It means that:

  • Consul should probably checks whether the service does exists
  • if it does not, throw an error (such as "Service does not exists"), while keeping existing behavior (aka Passing if the service exists but has not health checks)

@rboyer Does it make sense (since you write the comment https://github.com/hashicorp/consul/blame/master/agent/checks/alias.go#L236 ) ?

@pierresouchay
Copy link
Contributor

@oto313 Do you think my fix #7384 make sense?
Would it fix your issue?

pierresouchay added a commit to pierresouchay/consul that referenced this issue Apr 3, 2020
…sider it failing

In current implementation of Consul, check alias cannot determine
if a service exists or not. Because a service without any check
is semantically considered as passing, so when no healthchecks
are found for an agent, the check was considered as passing.

But this make little sense as the current implementation does not
make any difference between:
 * a non-existing service (passing)
 * a service without any check (passing as well)

In order to make it work, we have to ensure that when a check did
not find any healthcheck, the service does indeed exists. If it
does not, lets consider the check as failing.
@danielehc danielehc added theme/health-checks Health Check functionality type/bug Feature does not function as expected waiting-reply Waiting on response from Original Poster or another individual in the thread labels Apr 17, 2020
@pierresouchay
Copy link
Contributor

Hello @danielehc,

I think this issue is not in status awaiting response since @oto313 has answered in my PR, saying it would probably fix the issue. Could a maintainer have a look at #7374?

This is a problem we also have on our side

@ghost ghost removed the waiting-reply Waiting on response from Original Poster or another individual in the thread label Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/health-checks Health Check functionality type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants