Deprecate deregisterCriticalServiceAfter option (#674)

The deregisterCriticalServiceAfter option applied to the HTTP health
check for services that are registered in Consul.

Commit ea3a365 introduced additional Consul TTL checks. These TTL
checks have DeregisterCriticalServiceAfter set to the minimum of one
minute and will therefore always beat or tie the HTTP check's
DeregisterCriticalServiceAfter option, rendering it obsolete.

Document this fact in the usage message and HTML docs.

Signed-off-by: Peter Schultz <peter.schultz@classmarkets.com>

config/config.go

@@ -136,25 +136,24 @@ type File struct {
 }
 
 type Consul struct {
-	Addr                                string
-	Scheme                              string
-	Token                               string
-	KVPath                              string
-	NoRouteHTMLPath                     string
-	TagPrefix                           string
-	Register                            bool
-	ServiceAddr                         string
-	ServiceName                         string
-	ServiceTags                         []string
-	ServiceStatus                       []string
-	CheckInterval                       time.Duration
-	CheckTimeout                        time.Duration
-	CheckScheme                         string
-	CheckTLSSkipVerify                  bool
-	CheckDeregisterCriticalServiceAfter string
-	ChecksRequired                      string
-	ServiceMonitors                     int
-	TLS                                 ConsulTlS
+	Addr               string
+	Scheme             string
+	Token              string
+	KVPath             string
+	NoRouteHTMLPath    string
+	TagPrefix          string
+	Register           bool
+	ServiceAddr        string
+	ServiceName        string
+	ServiceTags        []string
+	ServiceStatus      []string
+	CheckInterval      time.Duration
+	CheckTimeout       time.Duration
+	CheckScheme        string
+	CheckTLSSkipVerify bool
+	ChecksRequired     string
+	ServiceMonitors    int
+	TLS                ConsulTlS
 }
 
 type Custom struct {

config/default.go

@@ -50,21 +50,20 @@ var defaultConfig = &Config{
 	Registry: Registry{
 		Backend: "consul",
 		Consul: Consul{
-			Addr:                                "localhost:8500",
-			Scheme:                              "http",
-			KVPath:                              "/fabio/config",
-			NoRouteHTMLPath:                     "/fabio/noroute.html",
-			TagPrefix:                           "urlprefix-",
-			Register:                            true,
-			ServiceAddr:                         ":9998",
-			ServiceName:                         "fabio",
-			ServiceStatus:                       []string{"passing"},
-			ServiceMonitors:                     1,
-			CheckInterval:                       time.Second,
-			CheckTimeout:                        3 * time.Second,
-			CheckScheme:                         "http",
-			CheckDeregisterCriticalServiceAfter: "90m",
-			ChecksRequired:                      "one",
+			Addr:            "localhost:8500",
+			Scheme:          "http",
+			KVPath:          "/fabio/config",
+			NoRouteHTMLPath: "/fabio/noroute.html",
+			TagPrefix:       "urlprefix-",
+			Register:        true,
+			ServiceAddr:     ":9998",
+			ServiceName:     "fabio",
+			ServiceStatus:   []string{"passing"},
+			ServiceMonitors: 1,
+			CheckInterval:   time.Second,
+			CheckTimeout:    3 * time.Second,
+			CheckScheme:     "http",
+			ChecksRequired:  "one",
 		},
 		Custom: Custom{
 			Host:               "",

config/load.go

@@ -118,6 +118,8 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c
 	var readTimeout, writeTimeout time.Duration
 	var gzipContentTypesValue string
 
+	var obsoleteStr string
+
 	f.BoolVar(&cfg.Insecure, "insecure", defaultConfig.Insecure, "allow fabio to run as root when set to true")
 	f.IntVar(&cfg.Proxy.MaxConn, "proxy.maxconn", defaultConfig.Proxy.MaxConn, "maximum number of cached connections")
 	f.StringVar(&cfg.Proxy.Strategy, "proxy.strategy", defaultConfig.Proxy.Strategy, "load balancing strategy")
@@ -186,7 +188,7 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c
 	f.DurationVar(&cfg.Registry.Consul.CheckInterval, "registry.consul.register.checkInterval", defaultConfig.Registry.Consul.CheckInterval, "service check interval")
 	f.DurationVar(&cfg.Registry.Consul.CheckTimeout, "registry.consul.register.checkTimeout", defaultConfig.Registry.Consul.CheckTimeout, "service check timeout")
 	f.BoolVar(&cfg.Registry.Consul.CheckTLSSkipVerify, "registry.consul.register.checkTLSSkipVerify", defaultConfig.Registry.Consul.CheckTLSSkipVerify, "service check TLS verification")
-	f.StringVar(&cfg.Registry.Consul.CheckDeregisterCriticalServiceAfter, "registry.consul.register.checkDeregisterCriticalServiceAfter", defaultConfig.Registry.Consul.CheckDeregisterCriticalServiceAfter, "critical service deregistration timeout")
+	f.StringVar(&obsoleteStr, "registry.consul.register.checkDeregisterCriticalServiceAfter", "", "This option is deprecated and has no effect.")
 	f.StringVar(&cfg.Registry.Consul.ChecksRequired, "registry.consul.checksRequired", defaultConfig.Registry.Consul.ChecksRequired, "number of checks which must pass: one or all")
 	f.IntVar(&cfg.Registry.Consul.ServiceMonitors, "registry.consul.serviceMonitors", defaultConfig.Registry.Consul.ServiceMonitors, "concurrency for route updates")
 	f.IntVar(&cfg.Runtime.GOGC, "runtime.gogc", defaultConfig.Runtime.GOGC, "sets runtime.GOGC")

docs/content/ref/registry.consul.register.deregisterCriticalServiceAfter.md

@@ -2,15 +2,18 @@
 title: "registry.consul.register.deregisterCriticalServiceAfter"
 ---
 
-`registry.consul.register.deregisterCriticalServiceAfter` configures the time the
-service health check is allowed to be in state `critical` until Consul automatically
-deregisters it.
-If fabio is still running, the service will be re-registered almost immediately after
-being deleted by Consul.
+This option is deprecated and has no effect in versions after 1.5.11. Services
+are now always deregistered shortly after fabio exits for any reason.
+
+In versions up to and including 1.5.11
+`registry.consul.register.deregisterCriticalServiceAfter` configures the
+duration after which registered services are removed from Consul after fabio
+exits abruptly (services are always deregistered immediately when fabio exits
+normally).
 
 At the time of this writing, Consul enforces a minimum value of one minute and runs
 its reaper process every thirty seconds. 
 
-The default is
+The default for fabio <= 1.5.11 is
 
-	registry.consul.register.deregisterCriticalServiceAfter = 90m
+	registry.consul.register.deregisterCriticalServiceAfter = 90m

registry/consul/register.go

@@ -137,28 +137,31 @@ func serviceRegistration(cfg *config.Consul, serviceName string) (*api.AgentServ
 		//
 		// Both checks must pass for Consul to consider the service healthy and therefore serve the fabio instance to clients.
 		Checks: []*api.AgentServiceCheck{
-			// If fabio doesn't exit cleanly, it doesn't auto-deregister itself from Consul.
-			// In order to address this, we introduce a TTL check to prove the fabio instance is alive and able to route this service.
-			// The TTL check must be refreshed before its timeout is crossed.
-			// If the timeout is crossed, the check fails.
-			// If the check fails, Consul considers this service to have become unhealthy.
-			// If the check is failing (critical) after DeregisterCriticalServiceAfter is elapsed, the Consul reaper will remove it from Consul.
+			// If fabio doesn't exit cleanly, it doesn't auto-deregister itself
+			// from Consul. In order to address this, we introduce a TTL check
+			// to confirm that the fabio instance is alive and able to route
+			// this service.
+			//
+			// The TTL check must be refreshed before its timeout is crossed,
+			// otherwise the check fails. If the check fails, Consul considers
+			// this service to have become unhealthy. If the check is failing
+			// (critical) for the DeregisterCriticalServiceAfter duration, the
+			// Consul reaper will remove it from Consul.
+			//
 			// For more info, read https://www.consul.io/api/agent/check.html#deregistercriticalserviceafter.
 			{
 				CheckID:                        computeServiceTTLCheckId(serviceID),
 				TTL:                            TTLInterval.String(),
 				DeregisterCriticalServiceAfter: TTLDeregisterCriticalServiceAfter.String(),
 			},
-			// HTTP check is meant to confirm fabio health endpoint is reachable from the Consul agent.
-			// If the check fails, Consul considers this service to have become unhealthy.
-			// If the check fails and registry.consul.register.deregisterCriticalServiceAfter is set, the service will be deregistered from Consul.
-			// For more info, read https://www.consul.io/api/agent/check.html#deregistercriticalserviceafter.
+			// HTTP check is meant to confirm fabio health endpoint is
+			// reachable from the Consul agent. If the check fails, Consul
+			// considers this service to have become unhealthy.
 			{
-				HTTP:                           checkURL,
-				Interval:                       cfg.CheckInterval.String(),
-				Timeout:                        cfg.CheckTimeout.String(),
-				TLSSkipVerify:                  cfg.CheckTLSSkipVerify,
-				DeregisterCriticalServiceAfter: cfg.CheckDeregisterCriticalServiceAfter,
+				HTTP:          checkURL,
+				Interval:      cfg.CheckInterval.String(),
+				Timeout:       cfg.CheckTimeout.String(),
+				TLSSkipVerify: cfg.CheckTLSSkipVerify,
 			},
 		},
 	}
@@ -167,5 +170,5 @@ func serviceRegistration(cfg *config.Consul, serviceName string) (*api.AgentServ
 }
 
 func computeServiceTTLCheckId(serviceID string) string {
-	return strings.Join([]string{serviceID, "ttl"}, "-")
+	return serviceID + "-ttl"
 }