Skip to content

Commit

Permalink
Apply changes from code review; add changelog entry
Browse files Browse the repository at this point in the history
  • Loading branch information
DerekStrickland committed Sep 10, 2022
1 parent 7e8ab11 commit 5300ec5
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 48 deletions.
3 changes: 3 additions & 0 deletions .changelog/14519.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
rpc: check for spec changes in all regions when registering multiregion jobs
```
49 changes: 1 addition & 48 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
}

// Check if the job has changed at all
specChanged, err := j.specChanged(existingJob, args)
specChanged, err := j.multiregionSpecChanged(existingJob, args)
if err != nil {
return err
}
Expand Down Expand Up @@ -438,53 +438,6 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
return nil
}

// specChanged checks to see if the job spec has changed. If the job is multiregion,
// it checks all regions. This handles the case of a multiregion job that has been
// stopped in one or more regions being restarted via the run command.
func (j *Job) specChanged(existingJob *structs.Job, args *structs.JobRegisterRequest) (bool, error) {
if existingJob == nil {
return false, nil
}

if existingJob.SpecChanged(args.Job) {
return true, nil
}

if !existingJob.IsMultiregion() {
return false, nil
}

// If the spec hasn't changed, but this is a multiregion job, check other regions.
for _, region := range existingJob.Multiregion.Regions {
// Copy the job so we can mutate it by region and compare it with the response.
incomingJob := args.Job.Copy()
incomingJob.Region = region.Name
req := &structs.JobSpecificRequest{
JobID: incomingJob.ID,
QueryOptions: structs.QueryOptions{
Region: region.Name,
Namespace: incomingJob.Namespace,
AuthToken: args.AuthToken,
},
}
resp := structs.SingleJobResponse{}
j.logger.Debug("checking spec for multiregion job")
err := j.GetJob(req, &resp)
if err != nil {
j.logger.Error("job lookup failed", "error", err)
return false, err
}

// If the jobspec changed or is nil (purged) for that region, assume it's being re-registered
// and treat like a spec change.
if resp.Job.SpecChanged(incomingJob) || resp.Job == nil {
return true, nil
}
}

return false, nil
}

// propagateScalingPolicyIDs propagates scaling policy IDs from existing job
// to updated job, or generates random IDs in new job
func propagateScalingPolicyIDs(old, new *structs.Job) error {
Expand Down
9 changes: 9 additions & 0 deletions nomad/job_endpoint_oss.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,12 @@ func (j *Job) multiregionStop(job *structs.Job, args *structs.JobDeregisterReque
func (j *Job) interpolateMultiregionFields(args *structs.JobPlanRequest) error {
return nil
}

// multiregionSpecChanged checks to see if the job spec has changed. If the job is multiregion,
// it checks all regions to determine if any deployed jobs instances have been stopped or
// otherwise differ from the incoming jobspec. Since multiregion jobs require coordinated
// deployments and synchronized job versions across all regions, a change in one requires
// redeployment of all.
func (j *Job) multiregionSpecChanged(existingJob *structs.Job, args *structs.JobRegisterRequest) (bool, error) {
return existingJob.SpecChanged(args.Job), nil
}

0 comments on commit 5300ec5

Please sign in to comment.