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

client/connect: ConsulProxy LocalServicePort/Address #6358

Merged
merged 1 commit into from
Sep 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,10 @@ type SidecarTask struct {

// ConsulProxy represents a Consul Connect sidecar proxy jobspec stanza.
type ConsulProxy struct {
Upstreams []*ConsulUpstream
Config map[string]interface{}
LocalServiceAddress string `mapstructure:"local_service_address"`
LocalServicePort int `mapstructure:"local_service_port"`
Upstreams []*ConsulUpstream
Config map[string]interface{}
}

// ConsulUpstream represents a Consul Connect upstream jobspec stanza.
Expand Down
29 changes: 29 additions & 0 deletions api/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,32 @@ func TestService_CheckRestart(t *testing.T) {
assert.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
assert.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
}

// TestService_Connect asserts Service.Connect settings are properly
// inherited by Checks.
func TestService_Connect(t *testing.T) {
job := &Job{Name: stringToPtr("job")}
tg := &TaskGroup{Name: stringToPtr("group")}
task := &Task{Name: "task"}
service := &Service{
Connect: &ConsulConnect{
SidecarService: &ConsulSidecarService{
Proxy: &ConsulProxy{
Upstreams: []*ConsulUpstream{
{
DestinationName: "upstream",
LocalBindPort: 80,
},
},
LocalServicePort: 8000,
},
},
},
}

service.Canonicalize(task, tg, job)
proxy := service.Connect.SidecarService.Proxy
assert.Equal(t, proxy.Upstreams[0].LocalBindPort, 80)
assert.Equal(t, proxy.Upstreams[0].DestinationName, "upstream")
assert.Equal(t, proxy.LocalServicePort, 8000)
}
14 changes: 11 additions & 3 deletions command/agent/consul/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1459,8 +1459,14 @@ func newConnect(serviceName string, nc *structs.ConsulConnect, networks structs.

// Bind to netns IP(s):port
proxyConfig := map[string]interface{}{}
if nc.SidecarService.Proxy != nil && nc.SidecarService.Proxy.Config != nil {
proxyConfig = nc.SidecarService.Proxy.Config
localServiceAddress := ""
localServicePort := 0
if nc.SidecarService.Proxy != nil {
localServiceAddress = nc.SidecarService.Proxy.LocalServiceAddress
localServicePort = nc.SidecarService.Proxy.LocalServicePort
if nc.SidecarService.Proxy.Config != nil {
proxyConfig = nc.SidecarService.Proxy.Config
}
}
proxyConfig["bind_address"] = "0.0.0.0"
proxyConfig["bind_port"] = port.To
Expand All @@ -1473,7 +1479,9 @@ func newConnect(serviceName string, nc *structs.ConsulConnect, networks structs.
// Automatically configure the proxy to bind to all addresses
// within the netns.
Proxy: &api.AgentServiceConnectProxyConfig{
Config: proxyConfig,
LocalServiceAddress: localServiceAddress,
LocalServicePort: localServicePort,
Config: proxyConfig,
},
}

Expand Down
8 changes: 6 additions & 2 deletions command/agent/consul/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ func TestConsul_Connect(t *testing.T) {
Name: "testconnect",
PortLabel: "9999",
Connect: &structs.ConsulConnect{
SidecarService: &structs.ConsulSidecarService{},
SidecarService: &structs.ConsulSidecarService{
Proxy: &structs.ConsulProxy{
LocalServicePort: 9000,
},
},
},
},
}
Expand Down Expand Up @@ -114,7 +118,7 @@ func TestConsul_Connect(t *testing.T) {
require.Equal(t, connectService.Proxy.DestinationServiceName, "testconnect")
require.Equal(t, connectService.Proxy.DestinationServiceID, serviceID)
require.Equal(t, connectService.Proxy.LocalServiceAddress, "127.0.0.1")
require.Equal(t, connectService.Proxy.LocalServicePort, 9999)
require.Equal(t, connectService.Proxy.LocalServicePort, 9000)
require.Equal(t, connectService.Proxy.Config, map[string]interface{}{
"bind_address": "0.0.0.0",
"bind_port": float64(9998),
Expand Down
4 changes: 3 additions & 1 deletion command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,9 @@ func ApiConsulConnectToStructs(in *api.ConsulConnect) *structs.ConsulConnect {
if in.SidecarService.Proxy != nil {

out.SidecarService.Proxy = &structs.ConsulProxy{
Config: in.SidecarService.Proxy.Config,
LocalServiceAddress: in.SidecarService.Proxy.LocalServiceAddress,
LocalServicePort: in.SidecarService.Proxy.LocalServicePort,
Config: in.SidecarService.Proxy.Config,
}

upstreams := make([]structs.ConsulUpstream, len(in.SidecarService.Proxy.Upstreams))
Expand Down
2 changes: 2 additions & 0 deletions jobspec/parse_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ func parseSidecarTask(item *ast.ObjectItem) (*api.SidecarTask, error) {

func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) {
valid := []string{
"local_service_address",
"local_service_port",
"upstreams",
"config",
}
Expand Down
18 changes: 13 additions & 5 deletions jobspec/test-fixtures/tg-network.hcl
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
job "foo" {
datacenters = ["dc1"]

Copy link
Member Author

Choose a reason for hiding this comment

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

I used hclfmt to clean up my own changes and it turns out this file was dirty. Apologies for the review noise.

group "bar" {
count = 3

network {
mode = "bridge"

port "http" {
static = 80
to = 8080
to = 8080
}
}

Expand All @@ -19,15 +22,18 @@ job "foo" {
connect {
sidecar_service {
proxy {
local_service_port = 8080

upstreams {
destination_name = "other-service"
local_bind_port = 4567
}
}
}

sidecar_task {
resources {
cpu = 500
cpu = 500
memory = 1024
}

Expand All @@ -42,13 +48,15 @@ job "foo" {

task "bar" {
driver = "raw_exec"

config {
command = "bash"
args = ["-c", "echo hi"]
command = "bash"
args = ["-c", "echo hi"]
}

resources {
network {
mbits = 10
mbits = 10
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2491,6 +2491,8 @@ func TestTaskGroupDiff(t *testing.T) {
SidecarService: &ConsulSidecarService{
Port: "http",
Proxy: &ConsulProxy{
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 8080,
Upstreams: []ConsulUpstream{
{
DestinationName: "foo",
Expand Down Expand Up @@ -2674,6 +2676,19 @@ func TestTaskGroupDiff(t *testing.T) {
{
Type: DiffTypeAdded,
Name: "ConsulProxy",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "LocalServiceAddress",
Old: "",
New: "127.0.0.1",
}, {
Type: DiffTypeAdded,
Name: "LocalServicePort",
Old: "",
New: "8080",
},
},
Objects: []*ObjectDiff{
{
Type: DiffTypeAdded,
Expand Down
19 changes: 19 additions & 0 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,17 @@ func (t *SidecarTask) MergeIntoTask(task *Task) {

// ConsulProxy represents a Consul Connect sidecar proxy jobspec stanza.
type ConsulProxy struct {

// LocalServiceAddress is the address the local service binds to.
// Usually 127.0.0.1 it is useful to customize in clusters with mixed
// Connect and non-Connect services.
LocalServiceAddress string

// LocalServicePort is the port the local service binds to. Usually
// the same as the parent service's port, it is useful to customize
// in clusters with mixed Connect and non-Connect services
LocalServicePort int

// Upstreams configures the upstream services this service intends to
// connect to.
Upstreams []ConsulUpstream
Expand All @@ -791,6 +802,8 @@ func (p *ConsulProxy) Copy() *ConsulProxy {
}

newP := ConsulProxy{}
newP.LocalServiceAddress = p.LocalServiceAddress
newP.LocalServicePort = p.LocalServicePort

if n := len(p.Upstreams); n > 0 {
newP.Upstreams = make([]ConsulUpstream, n)
Expand All @@ -817,6 +830,12 @@ func (p *ConsulProxy) Equals(o *ConsulProxy) bool {
return p == o
}

if p.LocalServiceAddress != o.LocalServiceAddress {
return false
}
if p.LocalServicePort != o.LocalServicePort {
return false
}
if len(p.Upstreams) != len(o.Upstreams) {
return false
}
Expand Down
2 changes: 2 additions & 0 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ func TestConsulConnect_CopyEquals(t *testing.T) {
SidecarService: &ConsulSidecarService{
Port: "9001",
Proxy: &ConsulProxy{
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 8080,
Upstreams: []ConsulUpstream{
{
DestinationName: "up1",
Expand Down
4 changes: 2 additions & 2 deletions website/source/docs/job-specification/network.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ job "docs" {
- `port` <code>([Port](#port-parameters): nil)</code> - Specifies a TCP/UDP port
allocation and can be used to specify both dynamic ports and reserved ports.

- `mode` `(string: "host")- Mode of the network. The following modes are available:
- `mode` `(string: "host")` - Mode of the network. The following modes are available:

- “none” - Task group will have an isolated network without any network interfaces.
- “bridge” - Task group will have an isolated network namespace with an interface
Expand Down Expand Up @@ -214,4 +214,4 @@ network {

### Limitations

Only one `network` stanza can be specified, when it is defined at the task group level.
Only one `network` stanza can be specified, when it is defined at the task group level.
10 changes: 8 additions & 2 deletions website/source/docs/job-specification/proxy.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ within the context of a `sidecar_service` stanza.

## `proxy` Parameters

- `upstreams` <code>([upstreams][]: nil)</code> Used to configure details of each upstream service that
- `local_service_address` `(string: "127.0.0.1")` - The address the local service binds to. Useful to
customize in clusters with mixed Connect and non-Connect services.
- `local_service_port` <code>(int:[port][])</code> - The port the local service binds to.
Usually the same as the parent service's port, it is useful to customize in clusters with mixed
Connect and non-Connect services
- `upstreams` <code>([upstreams][]: nil)</code> - Used to configure details of each upstream service that
this sidecar proxy communicates with.
- `config` - (map: nil)</code> - Proxy configuration that's opaque to Nomad and
- `config` <code>(map: nil)</code> - Proxy configuration that's opaque to Nomad and
passed directly to Consul. See [Consul Connect's
documentation](https://www.consul.io/docs/connect/proxies/envoy.html#dynamic-configuration)
for details.
Expand All @@ -81,3 +86,4 @@ The following example is a proxy specification that includes upstreams configura
[interpolation]: /docs/runtime/interpolation.html "Nomad interpolation"
[sidecar_service]: /docs/job-specification/sidecar_service.html "Nomad sidecar service Specification"
[upstreams]: /docs/job-specification/upstreams.html "Nomad upstream config Specification"
[port]: /docs/job-specification/network.html#port-parameters "Nomad network port configuration"