Fix dispatched optional meta correctly #4403
Conversation
New -bash: Dispatch: command not found field is used to denote if the Job is a child dispatched job of a parameterized job.
…rams as empty strings"" This reverts commit c17e0fc.
nickethier
force-pushed
the
b-fix-dispatched-optional-meta
branch
from
a06f5c5
to
e325e85
Compare
nomad/job_endpoint.go
Outdated
| @@ -1327,6 +1325,13 @@ func validateJob(job *structs.Job) (invalid, warnings error) { | |||
|
|
|||
| // validateJobUpdate ensures updates to a job are valid. | |||
| func validateJobUpdate(old, new *structs.Job) error { | |||
| if old == nil { | |||
There was a problem hiding this comment.
Add a comment above saying we are validating a new job
| @@ -4343,6 +4381,9 @@ func TestJobEndpoint_Dispatch(t *testing.T) { | |||
| if out.ParentID != tc.parameterizedJob.ID { | |||
| t.Fatalf("bad parent ID") | |||
| } | |||
| if !out.Dispatched { | |||
| t.Fatal("expected dispatched job") | |||
There was a problem hiding this comment.
Add a test that the parameterized stanza is on the job
There was a problem hiding this comment.
| @@ -1403,6 +1412,7 @@ func (j *Job) Dispatch(args *structs.JobDispatchRequest, reply *structs.JobDispa | |||
| dispatchJob.ParentID = parameterizedJob.ID | |||
| dispatchJob.Name = dispatchJob.ID | |||
| dispatchJob.SetSubmitTime() | |||
| dispatchJob.Dispatched = true | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
client/driver/env/env.go
Outdated
| // if job is parameterized initialize optional meta to empty strings | ||
| if alloc.Job.Dispatched { | ||
| b.taskMeta = make(map[string]string, | ||
| taskMetaSize+(len(alloc.Job.ParameterizedJob.MetaOptional)*2)) |
There was a problem hiding this comment.
Extract the size into a named variable so reading this is clearer.
| @@ -458,6 +458,33 @@ func TestJobEndpoint_Register_ParameterizedJob(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestJobEndpoint_Register_Dispatched(t *testing.T) { | |||
There was a problem hiding this comment.
Use require for new tests
client/driver/env/env_test.go
Outdated
| task := a.Job.TaskGroups[0].Tasks[0] | ||
| task.Meta = map[string]string{"metaopt1": "metaopt1val"} | ||
| env := NewBuilder(mock.Node(), a, task, "global").Build() | ||
| require.Equal(t, "metaopt1val", env.ReplaceEnv("${NOMAD_META_metaopt1}")) |
There was a problem hiding this comment.
For consistency with other tests, initialize require := require.New(t)
|
|
||
| new = mock.Job() | ||
| new.Dispatched = true | ||
| require.Error(validateJobUpdate(old, new), |
There was a problem hiding this comment.
Generally you want to assert the error you are expecting. require.Contains(err.Error(), "foo bar")
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This fixes a bug in PR #4262 due to the fact that dispatched jobs never return true for
job.IsParameterizedthus missing the logic implemented in the PR. The fix was to add a newDispatchedfield to theJobstruct which is set when a new job is dispatched and not allowed to be mutated from the api.fixes #3720