diff --git a/api/services.go b/api/services.go index 68e93ea99659..f66d80dd1ca0 100644 --- a/api/services.go +++ b/api/services.go @@ -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 { @@ -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 { @@ -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 { @@ -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"` @@ -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"` @@ -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 diff --git a/api/services_test.go b/api/services_test.go index bedfefd11c31..4bf5f92edebe 100644 --- a/api/services_test.go +++ b/api/services_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -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")} @@ -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) { @@ -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) + }) +} diff --git a/api/tasks.go b/api/tasks.go index 3ab4523a302a..b9b79af5470d 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -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)) @@ -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) }