Skip to content

Commit

Permalink
Get sidecar weights from node service if not specified
Browse files Browse the repository at this point in the history
The sidecar service weight was set to default values if not
specified by use in connect block of service definition.
This patch aims of reusing the node service weights if no weights
is specified from connect definition block or if defined to default one.
  • Loading branch information
mougams committed Oct 25, 2022
1 parent d3aa2bd commit 74c3a81
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 3 deletions.
2 changes: 1 addition & 1 deletion agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2515,7 +2515,7 @@ func (a *Agent) validateService(service *structs.NodeService, chkTypes []*struct
// Set default weights if not specified. This is important as it ensures AE
// doesn't consider the service different since it has nil weights.
if service.Weights == nil {
service.Weights = &structs.Weights{Passing: 1, Warning: 1}
service.Weights = structs.DefaultServiceWeights()
}

// Warn if the service name is incompatible with DNS
Expand Down
2 changes: 1 addition & 1 deletion agent/agent_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3753,7 +3753,7 @@ func testAgent_RegisterService_TranslateKeys(t *testing.T, extraHCL string) {
TaggedAddresses: map[string]structs.ServiceAddress{},
Port: 8001,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 1, Warning: 1},
Weights: &structs.Weights{Passing: 16, Warning: 0},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "test",
Expand Down
2 changes: 1 addition & 1 deletion agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1630,7 +1630,7 @@ func (b *builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition {
} else {
meta = v.Meta
}
serviceWeights := &structs.Weights{Passing: 1, Warning: 1}
serviceWeights := structs.DefaultServiceWeights()
if v.Weights != nil {
if v.Weights.Passing != nil {
serviceWeights.Passing = *v.Weights.Passing
Expand Down
8 changes: 8 additions & 0 deletions agent/sidecar_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ func sidecarServiceFromNodeService(ns *structs.NodeService, token string) (*stru
sidecar.Tags = append(sidecar.Tags, ns.Tags...)
}

// Override sidecar weights with the ones the service it represents only if sidecar is using default ones
if ns.Weights != nil && (sidecar.Weights == nil || sidecar.Weights.IsEqual(structs.DefaultServiceWeights())) {
sidecar.Weights = &structs.Weights{
Passing: ns.Weights.Passing,
Warning: ns.Weights.Warning,
}
}

// Flag this as a sidecar - this is not persisted in catalog but only needed
// in local agent state to disambiguate lineage when deregistering the parent
// service later.
Expand Down
73 changes: 73 additions & 0 deletions agent/sidecar_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,79 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
token: "foo",
wantErr: "reserved for internal use",
},
{
name: "use sidecar service defined weights",
sd: &structs.ServiceDefinition{
ID: "web3",
Name: "web",
Port: 1313,
Weights: &structs.Weights{
Passing: 50,
Warning: 2,
},
Connect: &structs.ServiceConnect{
SidecarService: &structs.ServiceDefinition{
Weights: &structs.Weights{
Passing: 51,
Warning: 3,
},
},
},
},
wantNS: &structs.NodeService{
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
Kind: structs.ServiceKindConnectProxy,
ID: "web3-sidecar-proxy",
Service: "web-sidecar-proxy",
Port: 0,
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceID: "web3",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 1313,
},
Weights: &structs.Weights{
Passing: 51,
Warning: 3,
},
},
wantChecks: nil,
},
{
name: "inherit service weights when sidecar has default ones",
sd: &structs.ServiceDefinition{
ID: "web2",
Name: "web",
Port: 1212,
Weights: &structs.Weights{
Passing: 50,
Warning: 2,
},
Connect: &structs.ServiceConnect{
SidecarService: &structs.ServiceDefinition{},
},
},
wantNS: &structs.NodeService{
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
Kind: structs.ServiceKindConnectProxy,
ID: "web2-sidecar-proxy",
Service: "web-sidecar-proxy",
Port: 0,
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceID: "web2",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 1212,
},
Weights: &structs.Weights{
Passing: 50,
Warning: 2,
},
},
wantChecks: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
17 changes: 17 additions & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1154,12 +1154,29 @@ func (sn *ServiceNode) CompoundServiceName() ServiceName {
}
}

const (
DefaultWeightPassing = 1
DefaultWeightWarning = 1
)

// Weights represent the weight used by DNS for a given status
type Weights struct {
Passing int
Warning int
}

// IsEqual compares Weights values
func (w *Weights) IsEqual(other *Weights) bool {
return w.Passing == other.Passing && w.Warning == other.Warning
}

func DefaultServiceWeights() *Weights {
return &Weights{
Passing: DefaultWeightPassing,
Warning: DefaultWeightWarning,
}
}

type ServiceNodes []*ServiceNode

// ServiceKind is the kind of service being registered.
Expand Down
61 changes: 61 additions & 0 deletions agent/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2946,3 +2946,64 @@ func TestServiceList_Sort(t *testing.T) {
})
}
}

func TestDefaultServiceWeights(t *testing.T) {
w := DefaultServiceWeights()

require.Equal(t, DefaultWeightPassing, w.Passing)
require.Equal(t, DefaultWeightWarning, w.Warning)
}

func TestWeights_IsEqual(t *testing.T) {
t.Run("When passing and warning are equal", func(t *testing.T) {
var passing = 22
var warning = 11
w1 := &Weights{
Passing: passing,
Warning: warning,
}
w2 := &Weights{
Passing: passing,
Warning: warning,
}
require.True(t, w1.IsEqual(w2))
})

t.Run("When passing and warning are not equal", func(t *testing.T) {
w1 := &Weights{
Passing: 8,
Warning: 9,
}
w2 := &Weights{
Passing: 9,
Warning: 8,
}
require.False(t, w1.IsEqual(w2))
})

t.Run("When passing is equal but warning is not", func(t *testing.T) {
var passing = 33
w1 := &Weights{
Passing: passing,
Warning: 2,
}
w2 := &Weights{
Passing: passing,
Warning: 3,
}
require.False(t, w1.IsEqual(w2))
})

t.Run("When warning is equal but passing is not", func(t *testing.T) {
var warning = 44
w1 := &Weights{
Passing: 5,
Warning: warning,
}
w2 := &Weights{
Passing: 6,
Warning: warning,
}
require.False(t, w1.IsEqual(w2))
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ proxy.
- `port` - Defaults to being auto-assigned from a configurable
range specified by [`sidecar_min_port`](/docs/agent/config/config-files#sidecar_min_port)
and [`sidecar_max_port`](/docs/agent/config/config-files#sidecar_max_port).
- `weights` - Defaults to the weights of the parent service.
- `kind` - Defaults to `connect-proxy`. This can't be overridden currently.
- `check`, `checks` - By default we add a TCP check on the local address and
port for the proxy, and a [service alias
Expand Down

0 comments on commit 74c3a81

Please sign in to comment.