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 routes to a service in warning status #117

Merged
merged 1 commit into from
Jun 21, 2016

Conversation

erikvanoosten
Copy link
Contributor

Currently Fabio only creates routes to services in the passing state. We have a use case for also wanting to use services in warning status. In consul the warning status indicates that the service is still good to go, but requires attention to keep it that way.

In this PR the acceptable statuses can be configured in fabio.properties where the default is just "passing" for backward compatibility.

As a bonus a few minor typos were fixed.

Unlike my previous attempt ( #116 ) this PR has a single squashed commit.

@erikvanoosten
Copy link
Contributor Author

@magiconair in #116 you commented:

s/AcceptableStates/States/

However, the field was renamed to AcceptableStatus to reflect Consul terminology. I have no idea what the plural is of status is. Please advice.

@magiconair
Copy link
Contributor

@erikvanoosten Just use Status

@CLAassistant
Copy link

CLAassistant commented Jun 21, 2016

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@@ -10,7 +10,7 @@ import (
// passingServices filters out health checks for services which have
// passing health checks and where the neither the service instance itself
// nor the node is in maintenance mode.
func passingServices(checks []*api.HealthCheck) []*api.HealthCheck {
func passingServices(checks []*api.HealthCheck, acceptableStatus []string) []*api.HealthCheck {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svdberg please rename here also

@magiconair
Copy link
Contributor

Could you please rename the field in the config struct to ServiceStatus? Then it is in line with the config parameter and the rest of theServiceXXX fields in the Config struct. Leave the function parameters at status. Then we're good. Sorry for bothering again. Should have seen this yesterday.

BTW, is there a way for me to amend this change like in gerrit so that I don't have to bother you with this?

@magiconair magiconair added this to the 1.1.5 milestone Jun 21, 2016
@magiconair
Copy link
Contributor

Also, no need to squash the commits beforehand. I can do that in github now

@erikvanoosten
Copy link
Contributor Author

@magiconair you can just checkout the branch and ask @svdberg for push rights on his repo.

@magiconair magiconair force-pushed the support-warning-state branch from c8f3821 to 8912c6f Compare June 21, 2016 12:34
magiconair added a commit to svdberg/fabio that referenced this pull request Jun 21, 2016
Consul allows services to be in several states. This change
allows fabio to consider more states as OK for routing than
just 'passing' by configuring the set of allowed states.
Consul allows services to be in several states. This change
allows fabio to consider more states as OK for routing than
just 'passing' by configuring the set of allowed states.
@magiconair magiconair force-pushed the support-warning-state branch from 8912c6f to 11a666f Compare June 21, 2016 12:36
@magiconair magiconair merged commit 5e8aaf4 into fabiolb:master Jun 21, 2016
magiconair pushed a commit that referenced this pull request Jun 23, 2016
Consul allows services to be in several states. This change
allows fabio to consider more states as OK for routing than
just 'passing' by configuring the set of allowed states.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants