Skip to content

Commit 396d080

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
refactor: change signature of calc-desired-instances
1 parent 158b92e commit 396d080

File tree

5 files changed

+33
-72
lines changed

5 files changed

+33
-72
lines changed

‎coderd/prebuilds/preset_snapshot.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,10 @@ func MatchesCron(cronExpression string, at time.Time) (bool, error) {
129129
// CalculateDesiredInstances returns the number of desired instances based on the provided time.
130130
// If the time matches any defined autoscaling schedule, the corresponding number of instances is returned.
131131
// Otherwise, it falls back to the default number of instances specified in the prebuild configuration.
132-
func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) (int32, error) {
132+
func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) int32 {
133133
if len(p.PrebuildSchedules) == 0 {
134134
// If no schedules are defined, fall back to the default desired instance count
135-
return p.Preset.DesiredInstances.Int32, nil
135+
return p.Preset.DesiredInstances.Int32
136136
}
137137

138138
// Validate that the provided timezone is valid
@@ -144,7 +144,7 @@ func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) (int32, error) {
144144
slog.Error(err))
145145

146146
// If timezone is invalid, fall back to the default desired instance count
147-
return p.Preset.DesiredInstances.Int32, nil
147+
return p.Preset.DesiredInstances.Int32
148148
}
149149

150150
// Look for a schedule whose cron expression matches the provided time
@@ -160,12 +160,12 @@ func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) (int32, error) {
160160
continue
161161
}
162162
if matches {
163-
return schedule.Instances, nil
163+
return schedule.Instances
164164
}
165165
}
166166

167167
// If no schedule matches, fall back to the default desired instance count
168-
return p.Preset.DesiredInstances.Int32, nil
168+
return p.Preset.DesiredInstances.Int32
169169
}
170170

171171
// CalculateState computes the current state of prebuilds for a preset, including:
@@ -180,7 +180,7 @@ func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) (int32, error) {
180180
// and calculates appropriate counts based on the current state of running prebuilds and
181181
// in-progress transitions. This state information is used to determine what reconciliation
182182
// actions are needed to reach the desired state.
183-
func (p PresetSnapshot) CalculateState() (*ReconciliationState, error) {
183+
func (p PresetSnapshot) CalculateState() *ReconciliationState {
184184
var (
185185
actual int32
186186
desired int32
@@ -196,11 +196,7 @@ func (p PresetSnapshot) CalculateState() (*ReconciliationState, error) {
196196
expired = int32(len(p.Expired))
197197

198198
if p.isActive() {
199-
var err error
200-
desired, err = p.CalculateDesiredInstances(p.clock.Now())
201-
if err != nil {
202-
return nil, xerrors.Errorf("failed to calculate number of desired instances: %w", err)
203-
}
199+
desired = p.CalculateDesiredInstances(p.clock.Now())
204200
eligible = p.countEligible()
205201
extraneous = max(actual-expired-desired, 0)
206202
}
@@ -217,7 +213,7 @@ func (p PresetSnapshot) CalculateState() (*ReconciliationState, error) {
217213
Starting: starting,
218214
Stopping: stopping,
219215
Deleting: deleting,
220-
}, nil
216+
}
221217
}
222218

223219
// CalculateActions determines what actions are needed to reconcile the current state with the desired state.
@@ -272,10 +268,7 @@ func (p PresetSnapshot) isActive() bool {
272268
//
273269
// The function returns a list of actions to be executed to achieve the desired state.
274270
func (p PresetSnapshot) handleActiveTemplateVersion() (actions []*ReconciliationActions, err error) {
275-
state, err := p.CalculateState()
276-
if err != nil {
277-
return nil, xerrors.Errorf("failed to calculate state: %w", err)
278-
}
271+
state := p.CalculateState()
279272

280273
// If we have expired prebuilds, delete them
281274
if state.Expired > 0 {

‎coderd/prebuilds/preset_snapshot_test.go

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ func TestNoPrebuilds(t *testing.T) {
9090
ps, err := snapshot.FilterByPreset(current.presetID)
9191
require.NoError(t, err)
9292

93-
state, err := ps.CalculateState()
94-
require.NoError(t, err)
93+
state := ps.CalculateState()
9594
actions, err := ps.CalculateActions(clock, backoffInterval)
9695
require.NoError(t, err)
9796

@@ -113,8 +112,7 @@ func TestNetNew(t *testing.T) {
113112
ps, err := snapshot.FilterByPreset(current.presetID)
114113
require.NoError(t, err)
115114

116-
state, err := ps.CalculateState()
117-
require.NoError(t, err)
115+
state := ps.CalculateState()
118116
actions, err := ps.CalculateActions(clock, backoffInterval)
119117
require.NoError(t, err)
120118

@@ -157,8 +155,7 @@ func TestOutdatedPrebuilds(t *testing.T) {
157155
require.NoError(t, err)
158156

159157
// THEN: we should identify that this prebuild is outdated and needs to be deleted.
160-
state, err := ps.CalculateState()
161-
require.NoError(t, err)
158+
state := ps.CalculateState()
162159
actions, err := ps.CalculateActions(clock, backoffInterval)
163160
require.NoError(t, err)
164161
validateState(t, prebuilds.ReconciliationState{
@@ -176,8 +173,7 @@ func TestOutdatedPrebuilds(t *testing.T) {
176173
require.NoError(t, err)
177174

178175
// THEN: we should not be blocked from creating a new prebuild while the outdate one deletes.
179-
state, err = ps.CalculateState()
180-
require.NoError(t, err)
176+
state = ps.CalculateState()
181177
actions, err = ps.CalculateActions(clock, backoffInterval)
182178
require.NoError(t, err)
183179
validateState(t, prebuilds.ReconciliationState{Desired: 1}, *state)
@@ -226,8 +222,7 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) {
226222

227223
// THEN: we should identify that this prebuild is outdated and needs to be deleted.
228224
// Despite the fact that deletion of another outdated prebuild is already in progress.
229-
state, err := ps.CalculateState()
230-
require.NoError(t, err)
225+
state := ps.CalculateState()
231226
actions, err := ps.CalculateActions(clock, backoffInterval)
232227
require.NoError(t, err)
233228
validateState(t, prebuilds.ReconciliationState{
@@ -471,8 +466,7 @@ func TestInProgressActions(t *testing.T) {
471466
require.NoError(t, err)
472467

473468
// THEN: we should identify that this prebuild is in progress.
474-
state, err := ps.CalculateState()
475-
require.NoError(t, err)
469+
state := ps.CalculateState()
476470
actions, err := ps.CalculateActions(clock, backoffInterval)
477471
require.NoError(t, err)
478472
tc.checkFn(*state, actions)
@@ -515,8 +509,7 @@ func TestExtraneous(t *testing.T) {
515509
require.NoError(t, err)
516510

517511
// THEN: an extraneous prebuild is detected and marked for deletion.
518-
state, err := ps.CalculateState()
519-
require.NoError(t, err)
512+
state := ps.CalculateState()
520513
actions, err := ps.CalculateActions(clock, backoffInterval)
521514
require.NoError(t, err)
522515
validateState(t, prebuilds.ReconciliationState{
@@ -697,8 +690,7 @@ func TestExpiredPrebuilds(t *testing.T) {
697690
require.NoError(t, err)
698691

699692
// THEN: we should identify that this prebuild is expired.
700-
state, err := ps.CalculateState()
701-
require.NoError(t, err)
693+
state := ps.CalculateState()
702694
actions, err := ps.CalculateActions(clock, backoffInterval)
703695
require.NoError(t, err)
704696
tc.checkFn(running, *state, actions)
@@ -734,8 +726,7 @@ func TestDeprecated(t *testing.T) {
734726
require.NoError(t, err)
735727

736728
// THEN: all running prebuilds should be deleted because the template is deprecated.
737-
state, err := ps.CalculateState()
738-
require.NoError(t, err)
729+
state := ps.CalculateState()
739730
actions, err := ps.CalculateActions(clock, backoffInterval)
740731
require.NoError(t, err)
741732
validateState(t, prebuilds.ReconciliationState{
@@ -788,8 +779,7 @@ func TestLatestBuildFailed(t *testing.T) {
788779
require.NoError(t, err)
789780

790781
// THEN: reconciliation should backoff.
791-
state, err := psCurrent.CalculateState()
792-
require.NoError(t, err)
782+
state := psCurrent.CalculateState()
793783
actions, err := psCurrent.CalculateActions(clock, backoffInterval)
794784
require.NoError(t, err)
795785
validateState(t, prebuilds.ReconciliationState{
@@ -807,8 +797,7 @@ func TestLatestBuildFailed(t *testing.T) {
807797
require.NoError(t, err)
808798

809799
// THEN: it should NOT be in backoff because all is OK.
810-
state, err = psOther.CalculateState()
811-
require.NoError(t, err)
800+
state = psOther.CalculateState()
812801
actions, err = psOther.CalculateActions(clock, backoffInterval)
813802
require.NoError(t, err)
814803
validateState(t, prebuilds.ReconciliationState{
@@ -822,8 +811,7 @@ func TestLatestBuildFailed(t *testing.T) {
822811
// THEN: a new prebuild should be created.
823812
psCurrent, err = snapshot.FilterByPreset(current.presetID)
824813
require.NoError(t, err)
825-
state, err = psCurrent.CalculateState()
826-
require.NoError(t, err)
814+
state = psCurrent.CalculateState()
827815
actions, err = psCurrent.CalculateActions(clock, backoffInterval)
828816
require.NoError(t, err)
829817
validateState(t, prebuilds.ReconciliationState{
@@ -886,8 +874,7 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
886874
ps, err := snapshot.FilterByPreset(presetOpts1.presetID)
887875
require.NoError(t, err)
888876

889-
state, err := ps.CalculateState()
890-
require.NoError(t, err)
877+
state := ps.CalculateState()
891878
actions, err := ps.CalculateActions(clock, backoffInterval)
892879
require.NoError(t, err)
893880

@@ -903,8 +890,7 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
903890
ps, err := snapshot.FilterByPreset(presetOpts2.presetID)
904891
require.NoError(t, err)
905892

906-
state, err := ps.CalculateState()
907-
require.NoError(t, err)
893+
state := ps.CalculateState()
908894
actions, err := ps.CalculateActions(clock, backoffInterval)
909895
require.NoError(t, err)
910896

@@ -1008,8 +994,7 @@ func TestPrebuildAutoscaling(t *testing.T) {
1008994
ps, err := snapshot.FilterByPreset(presetOpts1.presetID)
1009995
require.NoError(t, err)
1010996

1011-
state, err := ps.CalculateState()
1012-
require.NoError(t, err)
997+
state := ps.CalculateState()
1013998
actions, err := ps.CalculateActions(clock, backoffInterval)
1014999
require.NoError(t, err)
10151000

@@ -1030,8 +1015,7 @@ func TestPrebuildAutoscaling(t *testing.T) {
10301015
ps, err := snapshot.FilterByPreset(presetOpts2.presetID)
10311016
require.NoError(t, err)
10321017

1033-
state, err := ps.CalculateState()
1034-
require.NoError(t, err)
1018+
state := ps.CalculateState()
10351019
actions, err := ps.CalculateActions(clock, backoffInterval)
10361020
require.NoError(t, err)
10371021

@@ -1402,8 +1386,7 @@ func TestCalculateDesiredInstances(t *testing.T) {
14021386
tc := tc
14031387
t.Run(tc.name, func(t *testing.T) {
14041388
t.Parallel()
1405-
desiredInstances, err := tc.snapshot.CalculateDesiredInstances(tc.at)
1406-
require.NoError(t, err)
1389+
desiredInstances := tc.snapshot.CalculateDesiredInstances(tc.at)
14071390
require.Equal(t, tc.expectedCalculatedInstances, desiredInstances)
14081391
})
14091392
}

‎enterprise/coderd/prebuilds/metricscollector.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,7 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) {
179179
mc.logger.Error(context.Background(), "failed to filter by preset", slog.Error(err))
180180
continue
181181
}
182-
state, err := presetSnapshot.CalculateState()
183-
if err != nil {
184-
mc.logger.Error(context.Background(), "failed to calculate state for preset", slog.Error(err))
185-
continue
186-
}
182+
state := presetSnapshot.CalculateState()
187183

188184
metricsCh <- prometheus.MustNewConstMetric(desiredPrebuildsDesc, prometheus.GaugeValue, float64(state.Desired), preset.TemplateName, preset.Name, preset.OrganizationName)
189185
metricsCh <- prometheus.MustNewConstMetric(runningPrebuildsDesc, prometheus.GaugeValue, float64(state.Actual), preset.TemplateName, preset.Name, preset.OrganizationName)

‎enterprise/coderd/prebuilds/reconcile.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,7 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
442442
}
443443
}
444444

445-
state, err := ps.CalculateState()
446-
if err != nil {
447-
logger.Error(ctx, "failed to calculate state for preset", slog.Error(err))
448-
return err
449-
}
445+
state := ps.CalculateState()
450446
actions, err := c.CalculateActions(ctx, ps)
451447
if err != nil {
452448
logger.Error(ctx, "failed to calculate actions for preset", slog.Error(err))
@@ -620,10 +616,7 @@ func (c *StoreReconciler) executeReconciliationAction(ctx context.Context, logge
620616
// Unexpected things happen (i.e. bugs or bitflips); let's defend against disastrous outcomes.
621617
// See https://blog.robertelder.org/causes-of-bit-flips-in-computer-memory/.
622618
// This is obviously not comprehensive protection against this sort of problem, but this is one essential check.
623-
desired, err := ps.CalculateDesiredInstances(c.clock.Now())
624-
if err != nil {
625-
return xerrors.Errorf("failed to calculate desired instances: %w", err)
626-
}
619+
desired := ps.CalculateDesiredInstances(c.clock.Now())
627620

628621
if action.Create > desired {
629622
logger.Critical(ctx, "determined excessive count of prebuilds to create; clamping to desired count",

‎enterprise/coderd/prebuilds/reconcile_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,8 +1368,7 @@ func TestFailedBuildBackoff(t *testing.T) {
13681368
require.Len(t, snapshot.Presets, 1)
13691369
presetState, err := snapshot.FilterByPreset(preset.ID)
13701370
require.NoError(t, err)
1371-
state, err := presetState.CalculateState()
1372-
require.NoError(t, err)
1371+
state := presetState.CalculateState()
13731372
actions, err := reconciler.CalculateActions(ctx, *presetState)
13741373
require.NoError(t, err)
13751374
require.Equal(t, 1, len(actions))
@@ -1393,8 +1392,7 @@ func TestFailedBuildBackoff(t *testing.T) {
13931392
require.NoError(t, err)
13941393
presetState, err = snapshot.FilterByPreset(preset.ID)
13951394
require.NoError(t, err)
1396-
newState, err := presetState.CalculateState()
1397-
require.NoError(t, err)
1395+
newState := presetState.CalculateState()
13981396
newActions, err := reconciler.CalculateActions(ctx, *presetState)
13991397
require.NoError(t, err)
14001398
require.Equal(t, 1, len(newActions))
@@ -1412,8 +1410,7 @@ func TestFailedBuildBackoff(t *testing.T) {
14121410
require.NoError(t, err)
14131411
presetState, err = snapshot.FilterByPreset(preset.ID)
14141412
require.NoError(t, err)
1415-
state, err = presetState.CalculateState()
1416-
require.NoError(t, err)
1413+
state = presetState.CalculateState()
14171414
actions, err = reconciler.CalculateActions(ctx, *presetState)
14181415
require.NoError(t, err)
14191416
require.Equal(t, 1, len(actions))
@@ -1436,8 +1433,7 @@ func TestFailedBuildBackoff(t *testing.T) {
14361433
require.NoError(t, err)
14371434
presetState, err = snapshot.FilterByPreset(preset.ID)
14381435
require.NoError(t, err)
1439-
state, err = presetState.CalculateState()
1440-
require.NoError(t, err)
1436+
state = presetState.CalculateState()
14411437
actions, err = reconciler.CalculateActions(ctx, *presetState)
14421438
require.NoError(t, err)
14431439
require.Equal(t, 1, len(actions))

0 commit comments

Comments
 (0)