Repository: hashicorp/nomad
File: nomad/job_endpoint.go (Go)

Code before:

		args.Eval = &structs.Evaluation{
			ID:          uuid.Generate(),
			Namespace:   args.RequestNamespace(),
			Priority:    evalPriority,
			Type:        args.Job.Type,
			TriggeredBy: structs.EvalTriggerJobRegister,
			JobID:       args.Job.ID,
			Status:      structs.EvalStatusPending,
			CreateTime:  now,
			ModifyTime:  now,
		}
		reply.EvalID = args.Eval.ID
	}

	// Check if the job has changed at all
	specChanged, err := j.multiregionSpecChanged(existingJob, args)
	if err != nil {
		return err
	}
	// We know the job isn't nil, so if the spec hasn't changed,
	// there is an existing job.
	if !specChanged {
		reply.JobModifyIndex = existingJob.ModifyIndex

		// Force an eval for service/batch jobs
		if args.Eval != nil {
			update := &structs.EvalUpdateRequest{
				Evals:        []*structs.Evaluation{args.Eval},
				WriteRequest: structs.WriteRequest{Region: args.Region},
			}
			_, evalIndex, err := j.srv.raftApply(structs.EvalUpdateRequestType, update)
			if err != nil {
				j.logger.Error("eval create failed", "error", err, "method", "register")
				return err
			}
			reply.EvalCreateIndex = evalIndex
			reply.Index = evalIndex
		} else {
			reply.Index = existingJob.ModifyIndex
		}
		return nil
	}

	// Pre-register a deployment if necessary.
	args.Deployment = j.multiregionCreateDeployment(job, args.Eval)
	// Commit this update via Raft
	_, index, err := j.srv.raftApply(structs.JobRegisterRequestType, args)
	if err != nil {
		j.logger.Error("registering job failed", "error", err)
		return err

Review comment (suggestion):
This looks weird enough that we should probably leave a comment about this being left intentionally to allow triggering a new eval via job registration. Otherwise it looks like an unexpected side-effect.

Code after:

		args.Eval = &structs.Evaluation{
			ID:          uuid.Generate(),
			Namespace:   args.RequestNamespace(),
			Priority:    evalPriority,
			Type:        args.Job.Type,
			TriggeredBy: structs.EvalTriggerJobRegister,
			JobID:       args.Job.ID,
			Status:      structs.EvalStatusPending,
			CreateTime:  now,
			ModifyTime:  now,
		}
		reply.EvalID = args.Eval.ID
	}

	// Check if the job has changed at all
	specChanged, err := j.multiregionSpecChanged(existingJob, args)
	if err != nil {
		return err
	}
	// We know the job isn't nil, so if the spec hasn't changed,
	// there is an existing job.
	if !specChanged {
		reply.JobModifyIndex = existingJob.ModifyIndex

		// Force an eval for service/batch jobs. This is intentional
		// behavior to allow triggering of evaluations via re-registering
		// similar to `nomad job eval`.
		if args.Eval != nil {
			update := &structs.EvalUpdateRequest{
				Evals:        []*structs.Evaluation{args.Eval},
				WriteRequest: structs.WriteRequest{Region: args.Region},
			}
			_, evalIndex, err := j.srv.raftApply(structs.EvalUpdateRequestType, update)
			if err != nil {
				j.logger.Error("eval create failed", "error", err, "method", "register")
				return err
			}
			reply.EvalCreateIndex = evalIndex
			reply.Index = evalIndex
		} else {
			reply.Index = existingJob.ModifyIndex
		}
		return nil
	}

	// Pre-register a deployment if necessary.
	args.Deployment = j.multiregionCreateDeployment(job, args.Eval)
	// Commit this update via Raft
	_, index, err := j.srv.raftApply(structs.JobRegisterRequestType, args)
	if err != nil {