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

Get sidecar weights from node service if not specified #15112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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