Skip to content

system deployments: remove old rolling eval creation #27007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 29, 2025
Merged

system deployments: remove old rolling eval creation #27007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b999e8
system deployments: remove old rolling eval creation
tgross Oct 28, 2025
7a53ae4
remove nextEval field
tgross Oct 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions scheduler/scheduler_system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ type SystemScheduler struct {
deployment *structs.Deployment

limitReached bool
nextEval *structs.Evaluation

failedTGAllocs map[string]*structs.AllocMetric
queuedAllocs map[string]int
Expand Down Expand Up @@ -100,7 +99,7 @@ func (s *SystemScheduler) Process(eval *structs.Evaluation) (err error) {
// Verify the evaluation trigger reason is understood
if !s.canHandle(eval.TriggeredBy) {
desc := fmt.Sprintf("scheduler cannot handle '%s' evaluation reason", eval.TriggeredBy)
return setStatus(s.logger, s.planner, s.eval, s.nextEval, nil,
return setStatus(s.logger, s.planner, s.eval, nil, nil,
s.failedTGAllocs, s.planAnnotations, structs.EvalStatusFailed, desc,
s.queuedAllocs, s.deployment.GetID())
}
Expand All @@ -114,15 +113,15 @@ func (s *SystemScheduler) Process(eval *structs.Evaluation) (err error) {
progress := func() bool { return progressMade(s.planResult) }
if err := retryMax(limit, s.process, progress); err != nil {
if statusErr, ok := err.(*SetStatusError); ok {
return setStatus(s.logger, s.planner, s.eval, s.nextEval, nil,
return setStatus(s.logger, s.planner, s.eval, nil, nil,
s.failedTGAllocs, s.planAnnotations, statusErr.EvalStatus, err.Error(),
s.queuedAllocs, s.deployment.GetID())
}
return err
}

// Update the status to complete
return setStatus(s.logger, s.planner, s.eval, s.nextEval, nil,
return setStatus(s.logger, s.planner, s.eval, nil, nil,
s.failedTGAllocs, s.planAnnotations, structs.EvalStatusComplete, "",
s.queuedAllocs, s.deployment.GetID())
}
Expand Down Expand Up @@ -190,17 +189,6 @@ func (s *SystemScheduler) process() (bool, error) {
return true, nil
}

// If the limit of placements was reached we need to create an evaluation
// to pickup from here after the stagger period.
if s.limitReached && s.nextEval == nil {
Copy link
Member

@schmichael schmichael Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT nextEval is now always nil ... can we just remove it? I removed it on main and everything compiled, but I hit a test failure.

Copy link
Member

@schmichael schmichael Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise it appears limitReached is no longer used which may allow for some removal of code from evictAndPlace

Copy link
Member Author

@tgross tgross Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed nextEval from this scheduler but not from the setStatus function params where it's consumed, because the generic scheduler writes evals there still. I want to leave limitReached in place because I know @pkazmierczak is busy working on that area of code; we'll clean it up afterwards.

s.nextEval = s.eval.NextRollingEval(s.job.Update.MinHealthyTime)
if err := s.planner.CreateEval(s.nextEval); err != nil {
s.logger.Error("failed to make next eval for rolling update", "error", err)
return false, err
}
s.logger.Debug("rolling update limit reached, next eval created", "next_eval_id", s.nextEval.ID)
}

// Submit the plan
if s.eval.AnnotatePlan {
s.plan.Annotations = s.planAnnotations
Expand Down
22 changes: 0 additions & 22 deletions scheduler/scheduler_system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,28 +702,6 @@ func TestSystemSched_JobModify_Rolling(t *testing.T) {
}

h.AssertEvalStatus(t, structs.EvalStatusComplete)

// Ensure a follow up eval was created
eval = h.Evals[0]
if eval.NextEval == "" {
t.Fatalf("missing next eval")
}

// Check for create
if len(h.CreateEvals) == 0 {
t.Fatalf("missing created eval")
}
create := h.CreateEvals[0]
if eval.NextEval != create.ID {
t.Fatalf("ID mismatch")
}
if create.PreviousEval != eval.ID {
t.Fatalf("missing previous eval")
}

if create.TriggeredBy != structs.EvalTriggerRollingUpdate {
t.Fatalf("bad: %#v", create)
}
}

func TestSystemSched_JobModify_InPlace(t *testing.T) {
Expand Down