Commit 2bbe7b89 authored by Mahmood Ali's avatar Mahmood Ali
Browse files

Only initialize task.VolumeMounts when not-nil (#10990)

1.1.3 had a bug where task.VolumeMounts will be an empty slice instead of nil. Eventually, it gets canonicalized and is set to `nil`, but it seems to confuse dry-run planning.

The regression was introduced in https://github.com/hashicorp/nomad/pull/10855/files#diff-56b3c82fcbc857f8fb93a903f1610f6e6859b3610a4eddf92bad9ea27fdc85ecL1028-R1037 . Curiously, it's the only place where `len(apiTask.VolumeMounts)` check was dropped. I assume it was dropped accidentally.

Fixes #10981
parent d88526c5
Showing with 37 additions and 22 deletions
+37 -22
```release-note:bug
server: Fixed a bug where planning job update reports spurious in-place updates even if the update includes no changes
```
...@@ -1044,16 +1044,18 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, ...@@ -1044,16 +1044,18 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
} }
} }
structsTask.VolumeMounts = []*structs.VolumeMount{} if len(apiTask.VolumeMounts) > 0 {
for _, mount := range apiTask.VolumeMounts { structsTask.VolumeMounts = []*structs.VolumeMount{}
if mount != nil && mount.Volume != nil { for _, mount := range apiTask.VolumeMounts {
structsTask.VolumeMounts = append(structsTask.VolumeMounts, if mount != nil && mount.Volume != nil {
&structs.VolumeMount{ structsTask.VolumeMounts = append(structsTask.VolumeMounts,
Volume: *mount.Volume, &structs.VolumeMount{
Destination: *mount.Destination, Volume: *mount.Volume,
ReadOnly: *mount.ReadOnly, Destination: *mount.Destination,
PropagationMode: *mount.PropagationMode, ReadOnly: *mount.ReadOnly,
}) PropagationMode: *mount.PropagationMode,
})
}
} }
} }
......
...@@ -4,18 +4,17 @@ import ( ...@@ -4,18 +4,17 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"reflect" "reflect"
"strings"
"testing" "testing"
"time" "time"
"github.com/golang/snappy" "github.com/golang/snappy"
"github.com/hashicorp/nomad/api" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
api "github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs"
"github.com/kr/pretty"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
) )
func TestHTTP_JobsList(t *testing.T) { func TestHTTP_JobsList(t *testing.T) {
...@@ -2136,6 +2135,14 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { ...@@ -2136,6 +2135,14 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Weight: helper.Int8ToPtr(50), Weight: helper.Int8ToPtr(50),
}, },
}, },
VolumeMounts: []*api.VolumeMount{
{
Volume: helper.StringToPtr("vol"),
Destination: helper.StringToPtr("dest"),
ReadOnly: helper.BoolToPtr(false),
PropagationMode: helper.StringToPtr("a"),
},
},
RestartPolicy: &api.RestartPolicy{ RestartPolicy: &api.RestartPolicy{
Interval: helper.TimeToPtr(2 * time.Second), Interval: helper.TimeToPtr(2 * time.Second),
Attempts: helper.IntToPtr(10), Attempts: helper.IntToPtr(10),
...@@ -2510,6 +2517,14 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { ...@@ -2510,6 +2517,14 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Env: map[string]string{ Env: map[string]string{
"hello": "world", "hello": "world",
}, },
VolumeMounts: []*structs.VolumeMount{
{
Volume: "vol",
Destination: "dest",
ReadOnly: false,
PropagationMode: "a",
},
},
RestartPolicy: &structs.RestartPolicy{ RestartPolicy: &structs.RestartPolicy{
Interval: 2 * time.Second, Interval: 2 * time.Second,
Attempts: 10, Attempts: 10,
...@@ -2665,9 +2680,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { ...@@ -2665,9 +2680,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
structsJob := ApiJobToStructJob(apiJob) structsJob := ApiJobToStructJob(apiJob)
if diff := pretty.Diff(expected, structsJob); len(diff) > 0 { require.Equal(t, expected, structsJob)
t.Fatalf("bad:\n%s", strings.Join(diff, "\n"))
}
systemAPIJob := &api.Job{ systemAPIJob := &api.Job{
Stop: helper.BoolToPtr(true), Stop: helper.BoolToPtr(true),
...@@ -2903,10 +2916,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { ...@@ -2903,10 +2916,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
} }
systemStructsJob := ApiJobToStructJob(systemAPIJob) systemStructsJob := ApiJobToStructJob(systemAPIJob)
require.Equal(t, expectedSystemJob, systemStructsJob)
if diff := pretty.Diff(expectedSystemJob, systemStructsJob); len(diff) > 0 {
t.Fatalf("bad:\n%s", strings.Join(diff, "\n"))
}
} }
func TestJobs_ApiJobToStructsJobUpdate(t *testing.T) { func TestJobs_ApiJobToStructsJobUpdate(t *testing.T) {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment