Skip to content

Commit

Permalink
connect: correctly deal with nil sidecar_service task stanza
Browse files Browse the repository at this point in the history
Before, if the sidecar_service stanza of a connect enabled service
was missing, the job submission would cause a panic in the nomad
agent. Since the panic was happening in the API handler the agent
itself continued running, but this change will the condition more
gracefully.

By fixing the `Copy` method, the API handler now returns the proper
error.

$ nomad job run foo.nomad
Error submitting job: Unexpected response code: 500 (1 error occurred:
	* Task group api validation failed: 2 errors occurred:
	* Missing tasks for task group
	* Task group service validation failed: 1 error occurred:
	* Service[0] count-api validation failed: 1 error occurred:
	* Consul Connect must be native or use a sidecar service
  • Loading branch information
shoenig committed Apr 10, 2020
1 parent 5b65b95 commit 79a5241
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
3 changes: 3 additions & 0 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,9 @@ func (s *ConsulSidecarService) HasUpstreams() bool {

// Copy the stanza recursively. Returns nil if nil.
func (s *ConsulSidecarService) Copy() *ConsulSidecarService {
if s == nil {
return nil
}
return &ConsulSidecarService{
Tags: helper.CopySliceString(s.Tags),
Port: s.Port,
Expand Down
30 changes: 30 additions & 0 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ func TestConsulConnect_CopyEquals(t *testing.T) {
}

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

task := MockJob().TaskGroups[0].Tasks[0]
sTask := &SidecarTask{
Name: "sidecar",
Expand Down Expand Up @@ -300,6 +302,8 @@ func TestConsulExposePath_exposePathsEqual(t *testing.T) {
}

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

require.Nil(t, (*ConsulExposeConfig)(nil).Copy())
require.Equal(t, &ConsulExposeConfig{
Paths: []ConsulExposePath{{
Expand All @@ -313,6 +317,8 @@ func TestConsulExposeConfig_Copy(t *testing.T) {
}

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

require.True(t, (*ConsulExposeConfig)(nil).Equals(nil))
require.True(t, (&ConsulExposeConfig{
Paths: []ConsulExposePath{{
Expand All @@ -324,3 +330,27 @@ func TestConsulExposeConfig_Equals(t *testing.T) {
}},
}))
}

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

t.Run("nil", func(t *testing.T) {
s := (*ConsulSidecarService)(nil)
result := s.Copy()
require.Nil(t, result)
})

t.Run("not nil", func(t *testing.T) {
s := &ConsulSidecarService{
Tags: []string{"foo", "bar"},
Port: "port1",
Proxy: &ConsulProxy{LocalServiceAddress: "10.0.0.1"},
}
result := s.Copy()
require.Equal(t, &ConsulSidecarService{
Tags: []string{"foo", "bar"},
Port: "port1",
Proxy: &ConsulProxy{LocalServiceAddress: "10.0.0.1"},
}, result)
})
}

0 comments on commit 79a5241

Please sign in to comment.