Skip to content

Commit

Permalink
Implementation of Weights Data structures (#4468)
Browse files Browse the repository at this point in the history 
* Implementation of Weights Data structures

Adding this datastructure will allow us to resolve the
issues #1088 and #4198

This new structure defaults to values:
```
   { Passing: 1, Warning: 0 }
```

Which means, use weight of 0 for a Service in Warning State
while use Weight 1 for a Healthy Service.
Thus it remains compatible with previous Consul versions.

* Implemented weights for DNS SRV Records

* DNS properly support agents with weight support while server does not (backwards compatibility)

* Use Warning value of Weights of 1 by default

When using DNS interface with only_passing = false, all nodes
with non-Critical healthcheck used to have a weight value of 1.
While having weight.Warning = 0 as default value, this is probably
a bad idea as it breaks ascending compatibility.

Thus, we put a default value of 1 to be consistent with existing behaviour.

* Added documentation for new weight field in service description

* Better documentation about weights as suggested by @banks

* Return weight = 1 for unknown Check states as suggested by @banks

* Fixed typo (of -> or) in error message as requested by @mkeeler

* Fixed unstable unit test TestRetryJoin

* Fixed unstable tests

* Fixed wrong Fatalf format in `testrpc/wait.go`

* Added notes regarding DNS SRV lookup limitations regarding number of instances

* Documentation fixes and clarification regarding SRV records with weights as requested by @banks

* Rephrase docs
  • Loading branch information
@pierresouchay @banks
pierresouchay authored and banks committed Sep 7, 2018
1 parent da93144 commit eddcf22
Show file tree
Hide file tree
Showing 20 changed files with 387 additions and 28 deletions.
15 changes: 15 additions & 0 deletions agent/agent_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ func (s *HTTPServer) AgentServices(resp http.ResponseWriter, req *http.Request)

// Use empty list instead of nil
for id, s := range services {
weights := api.AgentWeights{Passing: 1, Warning: 1}
if s.Weights != nil {
if s.Weights.Passing > 0 {
weights.Passing = s.Weights.Passing
}
weights.Warning = s.Weights.Warning
}
as := &api.AgentService{
Kind: api.ServiceKind(s.Kind),
ID: s.ID,
Expand All @@ -184,6 +191,7 @@ func (s *HTTPServer) AgentServices(resp http.ResponseWriter, req *http.Request)
CreateIndex: s.CreateIndex,
ModifyIndex: s.ModifyIndex,
ProxyDestination: s.ProxyDestination,
Weights: weights,
}
if as.Tags == nil {
as.Tags = []string{}
Expand Down Expand Up @@ -581,6 +589,13 @@ func (s *HTTPServer) AgentRegisterService(resp http.ResponseWriter, req *http.Re

// Get the node service.
ns := args.NodeService()
if ns.Weights != nil {
if err := structs.ValidateWeights(ns.Weights); err != nil {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, fmt.Errorf("Invalid Weights: %v", err))
return nil, nil
}
}
if err := structs.ValidateMetadata(ns.Meta, false); err != nil {
resp.WriteHeader(http.StatusBadRequest)
fmt.Fprint(resp, fmt.Errorf("Invalid Service Meta: %v", err))
Expand Down
13 changes: 12 additions & 1 deletion agent/agent_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,10 @@ func TestAgent_RegisterService(t *testing.T) {
TTL: 30 * time.Second,
},
},
Weights: &structs.Weights{
Passing: 100,
Warning: 3,
},
}
req, _ := http.NewRequest("PUT", "/v1/agent/service/register?token=abc123", jsonReader(args))

Expand All @@ -1347,6 +1351,12 @@ func TestAgent_RegisterService(t *testing.T) {
if val := a.State.Service("test").Meta["hello"]; val != "world" {
t.Fatalf("Missing meta: %v", a.State.Service("test").Meta)
}
if val := a.State.Service("test").Weights.Passing; val != 100 {
t.Fatalf("Expected 100 for Weights.Passing, got: %v", val)
}
if val := a.State.Service("test").Weights.Warning; val != 3 {
t.Fatalf("Expected 3 for Weights.Warning, got: %v", val)
}

// Ensure we have a check mapping
checks := a.State.Checks()
Expand All @@ -1370,7 +1380,7 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
defer a.Shutdown()
testrpc.WaitForLeader(t, a.RPC, "dc1")

json := `{"name":"test", "port":8000, "enable_tag_override": true, "meta": {"some": "meta"}}`
json := `{"name":"test", "port":8000, "enable_tag_override": true, "meta": {"some": "meta"}, "weights":{"passing": 16}}`
req, _ := http.NewRequest("PUT", "/v1/agent/service/register", strings.NewReader(json))

obj, err := a.srv.AgentRegisterService(nil, req)
Expand All @@ -1386,6 +1396,7 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
Meta: map[string]string{"some": "meta"},
Port: 8000,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 16, Warning: 0},
}

if got, want := a.State.Service("test"), svc; !verify.Values(t, "", got, want) {
Expand Down
5 changes: 5 additions & 0 deletions agent/agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,10 @@ func verifyIndexChurn(t *testing.T, tags []string) {
a := NewTestAgent(t.Name(), "")
defer a.Shutdown()

weights := &structs.Weights{
Passing: 1,
Warning: 1,
}
// Ensure we have a leader before we start adding the services
testrpc.WaitForLeader(t, a.RPC, "dc1")

Expand All @@ -649,6 +653,7 @@ func verifyIndexChurn(t *testing.T, tags []string) {
Service: "redis",
Port: 8000,
Tags: tags,
Weights: weights,
}
if err := a.AddService(svc, nil, true, ""); err != nil {
t.Fatalf("err: %v", err)
Expand Down
14 changes: 14 additions & 0 deletions agent/config/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,19 @@ func (b *Builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition {
} else {
meta = v.Meta
}
serviceWeights := &structs.Weights{Passing: 1, Warning: 1}
if v.Weights != nil {
if v.Weights.Passing != nil {
serviceWeights.Passing = *v.Weights.Passing
}
if v.Weights.Warning != nil {
serviceWeights.Warning = *v.Weights.Warning
}
}

if err := structs.ValidateWeights(serviceWeights); err != nil {
b.err = multierror.Append(fmt.Errorf("Invalid weight definition for service %s: %s", b.stringVal(v.Name), err))
}
return &structs.ServiceDefinition{
Kind: b.serviceKindVal(v.Kind),
ID: b.stringVal(v.ID),
Expand All @@ -1096,6 +1109,7 @@ func (b *Builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition {
Port: b.intVal(v.Port),
Token: b.stringVal(v.Token),
EnableTagOverride: b.boolVal(v.EnableTagOverride),
Weights: serviceWeights,
Checks: checks,
ProxyDestination: b.stringVal(v.ProxyDestination),
Connect: b.serviceConnectVal(v.Connect),
Expand Down
7 changes: 7 additions & 0 deletions agent/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,12 @@ type Autopilot struct {
UpgradeVersionTag *string `json:"upgrade_version_tag,omitempty" hcl:"upgrade_version_tag" mapstructure:"upgrade_version_tag"`
}

// ServiceWeights defines the registration of weights used in DNS for a Service
type ServiceWeights struct {
Passing *int `json:"passing,omitempty" hcl:"passing" mapstructure:"passing"`
Warning *int `json:"warning,omitempty" hcl:"warning" mapstructure:"warning"`
}

type ServiceDefinition struct {
Kind *string `json:"kind,omitempty" hcl:"kind" mapstructure:"kind"`
ID *string `json:"id,omitempty" hcl:"id" mapstructure:"id"`
Expand All @@ -333,6 +339,7 @@ type ServiceDefinition struct {
Check *CheckDefinition `json:"check,omitempty" hcl:"check" mapstructure:"check"`
Checks []CheckDefinition `json:"checks,omitempty" hcl:"checks" mapstructure:"checks"`
Token *string `json:"token,omitempty" hcl:"token" mapstructure:"token"`
Weights *ServiceWeights `json:"weights,omitempty" hcl:"weights" mapstructure:"weights"`
EnableTagOverride *bool `json:"enable_tag_override,omitempty" hcl:"enable_tag_override" mapstructure:"enable_tag_override"`
ProxyDestination *string `json:"proxy_destination,omitempty" hcl:"proxy_destination" mapstructure:"proxy_destination"`
Connect *ServiceConnect `json:"connect,omitempty" hcl:"connect" mapstructure:"connect"`
Expand Down
114 changes: 90 additions & 24 deletions agent/config/runtime_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2005,16 +2005,22 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
},
json: []string{
`{ "service": { "name": "a", "port": 80 } }`,
`{ "service": { "name": "b", "port": 90, "meta": {"my": "value"} } }`,
`{ "service": { "name": "b", "port": 90, "meta": {"my": "value"}, "weights": {"passing": 13} } }`,
},
hcl: []string{
`service = { name = "a" port = 80 }`,
`service = { name = "b" port = 90 meta={my="value"}}`,
`service = { name = "b" port = 90 meta={my="value"}, weights={passing=13}}`,
},
patch: func(rt *RuntimeConfig) {
rt.Services = []*structs.ServiceDefinition{
&structs.ServiceDefinition{Name: "a", Port: 80},
&structs.ServiceDefinition{Name: "b", Port: 90, Meta: map[string]string{"my": "value"}},
&structs.ServiceDefinition{Name: "a", Port: 80, Weights: &structs.Weights{
Passing: 1,
Warning: 1,
}},
&structs.ServiceDefinition{Name: "b", Port: 90, Meta: map[string]string{"my": "value"}, Weights: &structs.Weights{
Passing: 13,
Warning: 1,
}},
}
rt.DataDir = dataDir
},
Expand Down Expand Up @@ -2108,6 +2114,10 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
ScriptArgs: []string{"a", "b"},
},
},
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
}
rt.DataDir = dataDir
Expand Down Expand Up @@ -2167,6 +2177,10 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
},
},
},
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
}
},
Expand Down Expand Up @@ -2211,6 +2225,10 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
},
},
},
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
}
},
Expand Down Expand Up @@ -2266,10 +2284,18 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
},
},
},
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
&structs.ServiceDefinition{
Name: "service-A2",
Port: 81,
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
}
},
Expand Down Expand Up @@ -2747,6 +2773,10 @@ func TestFullConfig(t *testing.T) {
"address": "cOlSOhbp",
"token": "msy7iWER",
"port": 24237,
"weights": {
"passing": 100,
"warning": 1
},
"enable_tag_override": true,
"check": {
"id": "RMi85Dv8",
Expand Down Expand Up @@ -2855,6 +2885,10 @@ func TestFullConfig(t *testing.T) {
"address": "R6H6g8h0",
"token": "ZgY8gjMI",
"port": 38292,
"weights": {
"passing": 1979,
"warning": 6
},
"enable_tag_override": true,
"checks": [
{
Expand Down Expand Up @@ -3238,6 +3272,10 @@ func TestFullConfig(t *testing.T) {
address = "cOlSOhbp"
token = "msy7iWER"
port = 24237
weights = {
passing = 100,
warning = 1
}
enable_tag_override = true
check = {
id = "RMi85Dv8"
Expand Down Expand Up @@ -3346,6 +3384,10 @@ func TestFullConfig(t *testing.T) {
address = "R6H6g8h0"
token = "ZgY8gjMI"
port = 38292
weights = {
passing = 1979,
warning = 6
}
enable_tag_override = true
checks = [
{
Expand Down Expand Up @@ -3798,12 +3840,16 @@ func TestFullConfig(t *testing.T) {
ServerPort: 3757,
Services: []*structs.ServiceDefinition{
{
ID: "wI1dzxS4",
Name: "7IszXMQ1",
Tags: []string{"0Zwg8l6v", "zebELdN5"},
Address: "9RhqPSPB",
Token: "myjKJkWH",
Port: 72219,
ID: "wI1dzxS4",
Name: "7IszXMQ1",
Tags: []string{"0Zwg8l6v", "zebELdN5"},
Address: "9RhqPSPB",
Token: "myjKJkWH",
Port: 72219,
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
EnableTagOverride: true,
Checks: []*structs.CheckType{
&structs.CheckType{
Expand All @@ -3830,12 +3876,16 @@ func TestFullConfig(t *testing.T) {
},
},
{
ID: "MRHVMZuD",
Name: "6L6BVfgH",
Tags: []string{"7Ale4y6o", "PMBW08hy"},
Address: "R6H6g8h0",
Token: "ZgY8gjMI",
Port: 38292,
ID: "MRHVMZuD",
Name: "6L6BVfgH",
Tags: []string{"7Ale4y6o", "PMBW08hy"},
Address: "R6H6g8h0",
Token: "ZgY8gjMI",
Port: 38292,
Weights: &structs.Weights{
Passing: 1979,
Warning: 6,
},
EnableTagOverride: true,
Checks: structs.CheckTypes{
&structs.CheckType{
Expand Down Expand Up @@ -3897,15 +3947,23 @@ func TestFullConfig(t *testing.T) {
Port: 31471,
Kind: "connect-proxy",
ProxyDestination: "6L6BVfgH",
Weights: &structs.Weights{
Passing: 1,
Warning: 1,
},
},
{
ID: "dLOXpSCI",
Name: "o1ynPkp0",
Tags: []string{"nkwshvM5", "NTDWn3ek"},
Address: "cOlSOhbp",
Token: "msy7iWER",
Meta: map[string]string{"mymeta": "data"},
Port: 24237,
ID: "dLOXpSCI",
Name: "o1ynPkp0",
Tags: []string{"nkwshvM5", "NTDWn3ek"},
Address: "cOlSOhbp",
Token: "msy7iWER",
Meta: map[string]string{"mymeta": "data"},
Port: 24237,
Weights: &structs.Weights{
Passing: 100,
Warning: 1,
},
EnableTagOverride: true,
Connect: &structs.ServiceConnect{
Native: true,
Expand Down Expand Up @@ -4324,6 +4382,10 @@ func TestSanitize(t *testing.T) {
Check: structs.CheckType{
Name: "blurb",
},
Weights: &structs.Weights{
Passing: 67,
Warning: 3,
},
},
},
Checks: []*structs.CheckDefinition{
Expand Down Expand Up @@ -4552,7 +4614,11 @@ func TestSanitize(t *testing.T) {
"Port": 0,
"ProxyDestination": "",
"Tags": [],
"Token": "hidden"
"Token": "hidden",
"Weights": {
"Passing": 67,
"Warning": 3
}
}],
"SessionTTLMin": "0s",
"SkipLeaveOnInt": false,
Expand Down
Loading

0 comments on commit eddcf22

Please sign in to comment.