Skip to content

Commit

Permalink
api: canonicalize connect components
Browse files Browse the repository at this point in the history 
Add `Canonicalize` methods to the connect components of a service
definition in the `api` package. Without these, we have been relying
on good input for the connect stanza.

Fixes #7993
  • Loading branch information
@shoenig
shoenig committed May 19, 2020
1 parent 9400ac6 commit 9774f4f
Show file tree
Hide file tree
Showing 3 changed files with 224 additions and 16 deletions.
87 changes: 87 additions & 0 deletions api/services.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) {
s.AddressMode = "auto"
}

s.Connect.Canonicalize()

// Canonicalize CheckRestart on Checks and merge Service.CheckRestart
// into each check.
for i, check := range s.Checks {
Expand All @@ -143,6 +145,15 @@ type ConsulConnect struct {
SidecarTask *SidecarTask `mapstructure:"sidecar_task"`
}

func (cc *ConsulConnect) Canonicalize() {
if cc == nil {
return
}

cc.SidecarService.Canonicalize()
cc.SidecarTask.Canonicalize()
}

// ConsulSidecarService represents a Consul Connect SidecarService jobspec
// stanza.
type ConsulSidecarService struct {
Expand All @@ -151,6 +162,18 @@ type ConsulSidecarService struct {
Proxy *ConsulProxy
}

func (css *ConsulSidecarService) Canonicalize() {
if css == nil {
return
}

if len(css.Tags) == 0 {
css.Tags = nil
}

css.Proxy.Canonicalize()
}

// SidecarTask represents a subset of Task fields that can be set to override
// the fields of the Task generated for the sidecar
type SidecarTask struct {
Expand All @@ -167,6 +190,44 @@ type SidecarTask struct {
KillSignal string `mapstructure:"kill_signal"`
}

func (st *SidecarTask) Canonicalize() {
if st == nil {
return
}

if len(st.Config) == 0 {
st.Config = nil
}

if len(st.Env) == 0 {
st.Env = nil
}

if st.Resources == nil {
st.Resources = DefaultResources()
} else {
st.Resources.Canonicalize()
}

if st.LogConfig == nil {
st.LogConfig = DefaultLogConfig()
} else {
st.LogConfig.Canonicalize()
}

if len(st.Meta) == 0 {
st.Meta = nil
}

if st.KillTimeout == nil {
st.KillTimeout = timeToPtr(5 * time.Second)
}

if st.ShutdownDelay == nil {
st.ShutdownDelay = timeToPtr(0)
}
}

// ConsulProxy represents a Consul Connect sidecar proxy jobspec stanza.
type ConsulProxy struct {
LocalServiceAddress string `mapstructure:"local_service_address"`
Expand All @@ -176,6 +237,22 @@ type ConsulProxy struct {
Config map[string]interface{}
}

func (cp *ConsulProxy) Canonicalize() {
if cp == nil {
return
}

cp.ExposeConfig.Canonicalize()

if len(cp.Upstreams) == 0 {
cp.Upstreams = nil
}

if len(cp.Config) == 0 {
cp.Config = nil
}
}

// ConsulUpstream represents a Consul Connect upstream jobspec stanza.
type ConsulUpstream struct {
DestinationName string `mapstructure:"destination_name"`
Expand All @@ -186,6 +263,16 @@ type ConsulExposeConfig struct {
Path []*ConsulExposePath `mapstructure:"path"`
}

func (cec *ConsulExposeConfig) Canonicalize() {
if cec == nil {
return
}

if len(cec.Path) == 0 {
cec.Path = nil
}
}

type ConsulExposePath struct {
Path string
Protocol string
Expand Down
151 changes: 135 additions & 16 deletions api/services_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -45,22 +44,110 @@ func TestService_CheckRestart(t *testing.T) {
}

service.Canonicalize(task, tg, job)
assert.Equal(t, service.Checks[0].CheckRestart.Limit, 22)
assert.Equal(t, *service.Checks[0].CheckRestart.Grace, 22*time.Second)
assert.True(t, service.Checks[0].CheckRestart.IgnoreWarnings)
require.Equal(t, service.Checks[0].CheckRestart.Limit, 22)
require.Equal(t, *service.Checks[0].CheckRestart.Grace, 22*time.Second)
require.True(t, service.Checks[0].CheckRestart.IgnoreWarnings)

assert.Equal(t, service.Checks[1].CheckRestart.Limit, 33)
assert.Equal(t, *service.Checks[1].CheckRestart.Grace, 33*time.Second)
assert.True(t, service.Checks[1].CheckRestart.IgnoreWarnings)
require.Equal(t, service.Checks[1].CheckRestart.Limit, 33)
require.Equal(t, *service.Checks[1].CheckRestart.Grace, 33*time.Second)
require.True(t, service.Checks[1].CheckRestart.IgnoreWarnings)

assert.Equal(t, service.Checks[2].CheckRestart.Limit, 11)
assert.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
assert.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
require.Equal(t, service.Checks[2].CheckRestart.Limit, 11)
require.Equal(t, *service.Checks[2].CheckRestart.Grace, 11*time.Second)
require.True(t, service.Checks[2].CheckRestart.IgnoreWarnings)
}

// TestService_Connect asserts Service.Connect settings are properly
// inherited by Checks.
func TestService_Connect(t *testing.T) {
func TestService_Connect_Canonicalize(t *testing.T) {
t.Parallel()

t.Run("nil connect", func(t *testing.T) {
cc := (*ConsulConnect)(nil)
cc.Canonicalize()
require.Nil(t, cc)
})

t.Run("empty connect", func(t *testing.T) {
cc := new(ConsulConnect)
cc.Canonicalize()
require.False(t, cc.Native)
require.Nil(t, cc.SidecarService)
require.Nil(t, cc.SidecarTask)
})
}

func TestService_Connect_ConsulSidecarService_Canonicalize(t *testing.T) {
t.Parallel()

t.Run("nil sidecar_service", func(t *testing.T) {
css := (*ConsulSidecarService)(nil)
css.Canonicalize()
require.Nil(t, css)
})

t.Run("empty sidecar_service", func(t *testing.T) {
css := new(ConsulSidecarService)
css.Canonicalize()
require.Empty(t, css.Tags)
require.Nil(t, css.Proxy)
})

t.Run("non-empty sidecar_service", func(t *testing.T) {
css := &ConsulSidecarService{
Tags: make([]string, 0),
Port: "port",
Proxy: &ConsulProxy{
LocalServiceAddress: "lsa",
LocalServicePort: 80,
},
}
css.Canonicalize()
require.Equal(t, &ConsulSidecarService{
Tags: nil,
Port: "port",
Proxy: &ConsulProxy{
LocalServiceAddress: "lsa",
LocalServicePort: 80},
}, css)
})
}

func TestService_Connect_ConsulProxy_Canonicalize(t *testing.T) {
t.Parallel()

t.Run("nil proxy", func(t *testing.T) {
cp := (*ConsulProxy)(nil)
cp.Canonicalize()
require.Nil(t, cp)
})

t.Run("empty proxy", func(t *testing.T) {
cp := new(ConsulProxy)
cp.Canonicalize()
require.Empty(t, cp.LocalServiceAddress)
require.Zero(t, cp.LocalServicePort)
require.Nil(t, cp.ExposeConfig)
require.Nil(t, cp.Upstreams)
require.Empty(t, cp.Config)
})

t.Run("non empty proxy", func(t *testing.T) {
cp := &ConsulProxy{
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 80,
ExposeConfig: new(ConsulExposeConfig),
Upstreams: make([]*ConsulUpstream, 0),
Config: make(map[string]interface{}),
}
cp.Canonicalize()
require.Equal(t, "127.0.0.1", cp.LocalServiceAddress)
require.Equal(t, 80, cp.LocalServicePort)
require.Equal(t, &ConsulExposeConfig{}, cp.ExposeConfig)
require.Nil(t, cp.Upstreams)
require.Nil(t, cp.Config)
})
}

func TestService_Connect_proxy_settings(t *testing.T) {
t.Parallel()

job := &Job{Name: stringToPtr("job")}
Expand All @@ -84,9 +171,9 @@ func TestService_Connect(t *testing.T) {

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)
require.Equal(t, proxy.Upstreams[0].LocalBindPort, 80)
require.Equal(t, proxy.Upstreams[0].DestinationName, "upstream")
require.Equal(t, proxy.LocalServicePort, 8000)
}

func TestService_Tags(t *testing.T) {
Expand All @@ -108,3 +195,35 @@ func TestService_Tags(t *testing.T) {
r.Equal([]string{"a", "b"}, service.Tags)
r.Equal([]string{"c", "d"}, service.CanaryTags)
}

func TestService_Connect_SidecarTask_Canonicalize(t *testing.T) {
t.Parallel()

t.Run("nil sidecar_task", func(t *testing.T) {
st := (*SidecarTask)(nil)
st.Canonicalize()
require.Nil(t, st)
})

t.Run("empty sidecar_task", func(t *testing.T) {
st := new(SidecarTask)
st.Canonicalize()
require.Nil(t, st.Config)
require.Nil(t, st.Env)
require.Equal(t, DefaultResources(), st.Resources)
require.Equal(t, DefaultLogConfig(), st.LogConfig)
require.Nil(t, st.Meta)
require.Equal(t, 5*time.Second, *st.KillTimeout)
require.Equal(t, 0*time.Second, *st.ShutdownDelay)
})

t.Run("non empty sidecar_task resources", func(t *testing.T) {
exp := DefaultResources()
exp.MemoryMB = intToPtr(333)
st := &SidecarTask{
Resources: &Resources{MemoryMB: intToPtr(333)},
}
st.Canonicalize()
require.Equal(t, exp, st.Resources)
})
}
2 changes: 2 additions & 0 deletions api/tasks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ func (g *TaskGroup) Canonicalize(job *Job) {
if g.Name == nil {
g.Name = stringToPtr("")
}

if g.Count == nil {
if g.Scaling != nil && g.Scaling.Min != nil {
g.Count = intToPtr(int(*g.Scaling.Min))
Expand Down Expand Up @@ -676,6 +677,7 @@ func (t *Task) Canonicalize(tg *TaskGroup, job *Job) {
t.Resources = &Resources{}
}
t.Resources.Canonicalize()

if t.KillTimeout == nil {
t.KillTimeout = timeToPtr(5 * time.Second)
}
Expand Down

0 comments on commit 9774f4f

Please sign in to comment.