Skip to content

Commit ea73fe3

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
fix calc state
1 parent ff41e18 commit ea73fe3

File tree

5 files changed

+57
-28
lines changed

5 files changed

+57
-28
lines changed

‎coderd/prebuilds/preset_snapshot.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (p PresetSnapshot) CalculateDesiredInstances(at time.Time) (int32, error) {
142142
// and calculates appropriate counts based on the current state of running prebuilds and
143143
// in-progress transitions. This state information is used to determine what reconciliation
144144
// actions are needed to reach the desired state.
145-
func (p PresetSnapshot) CalculateState() *ReconciliationState {
145+
func (p PresetSnapshot) CalculateState() (*ReconciliationState, error) {
146146
var (
147147
actual int32
148148
desired int32
@@ -161,9 +161,7 @@ func (p PresetSnapshot) CalculateState() *ReconciliationState {
161161
var err error
162162
desired, err = p.CalculateDesiredInstances(p.clock.Now())
163163
if err != nil {
164-
// In case of an error, fall back to the default desired instance count
165-
desired = p.Preset.DesiredInstances.Int32
166-
// TODO: log error
164+
return nil, xerrors.Errorf("failed to calculate number of desired instances: %w", err)
167165
}
168166
eligible = p.countEligible()
169167
extraneous = max(actual-expired-desired, 0)
@@ -181,7 +179,7 @@ func (p PresetSnapshot) CalculateState() *ReconciliationState {
181179
Starting: starting,
182180
Stopping: stopping,
183181
Deleting: deleting,
184-
}
182+
}, nil
185183
}
186184

187185
// CalculateActions determines what actions are needed to reconcile the current state with the desired state.
@@ -236,7 +234,10 @@ func (p PresetSnapshot) isActive() bool {
236234
//
237235
// The function returns a list of actions to be executed to achieve the desired state.
238236
func (p PresetSnapshot) handleActiveTemplateVersion() (actions []*ReconciliationActions, err error) {
239-
state := p.CalculateState()
237+
state, err := p.CalculateState()
238+
if err != nil {
239+
return nil, xerrors.Errorf("failed to calculate state: %w", err)
240+
}
240241

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

‎coderd/prebuilds/preset_snapshot_test.go

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ func TestNoPrebuilds(t *testing.T) {
8888
ps, err := snapshot.FilterByPreset(current.presetID)
8989
require.NoError(t, err)
9090

91-
state := ps.CalculateState()
91+
state, err := ps.CalculateState()
92+
require.NoError(t, err)
9293
actions, err := ps.CalculateActions(clock, backoffInterval)
9394
require.NoError(t, err)
9495

@@ -110,7 +111,8 @@ func TestNetNew(t *testing.T) {
110111
ps, err := snapshot.FilterByPreset(current.presetID)
111112
require.NoError(t, err)
112113

113-
state := ps.CalculateState()
114+
state, err := ps.CalculateState()
115+
require.NoError(t, err)
114116
actions, err := ps.CalculateActions(clock, backoffInterval)
115117
require.NoError(t, err)
116118

@@ -153,7 +155,8 @@ func TestOutdatedPrebuilds(t *testing.T) {
153155
require.NoError(t, err)
154156

155157
// THEN: we should identify that this prebuild is outdated and needs to be deleted.
156-
state := ps.CalculateState()
158+
state, err := ps.CalculateState()
159+
require.NoError(t, err)
157160
actions, err := ps.CalculateActions(clock, backoffInterval)
158161
require.NoError(t, err)
159162
validateState(t, prebuilds.ReconciliationState{
@@ -171,7 +174,8 @@ func TestOutdatedPrebuilds(t *testing.T) {
171174
require.NoError(t, err)
172175

173176
// THEN: we should not be blocked from creating a new prebuild while the outdate one deletes.
174-
state = ps.CalculateState()
177+
state, err = ps.CalculateState()
178+
require.NoError(t, err)
175179
actions, err = ps.CalculateActions(clock, backoffInterval)
176180
require.NoError(t, err)
177181
validateState(t, prebuilds.ReconciliationState{Desired: 1}, *state)
@@ -220,7 +224,8 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) {
220224

221225
// THEN: we should identify that this prebuild is outdated and needs to be deleted.
222226
// Despite the fact that deletion of another outdated prebuild is already in progress.
223-
state := ps.CalculateState()
227+
state, err := ps.CalculateState()
228+
require.NoError(t, err)
224229
actions, err := ps.CalculateActions(clock, backoffInterval)
225230
require.NoError(t, err)
226231
validateState(t, prebuilds.ReconciliationState{
@@ -464,7 +469,8 @@ func TestInProgressActions(t *testing.T) {
464469
require.NoError(t, err)
465470

466471
// THEN: we should identify that this prebuild is in progress.
467-
state := ps.CalculateState()
472+
state, err := ps.CalculateState()
473+
require.NoError(t, err)
468474
actions, err := ps.CalculateActions(clock, backoffInterval)
469475
require.NoError(t, err)
470476
tc.checkFn(*state, actions)
@@ -507,7 +513,8 @@ func TestExtraneous(t *testing.T) {
507513
require.NoError(t, err)
508514

509515
// THEN: an extraneous prebuild is detected and marked for deletion.
510-
state := ps.CalculateState()
516+
state, err := ps.CalculateState()
517+
require.NoError(t, err)
511518
actions, err := ps.CalculateActions(clock, backoffInterval)
512519
require.NoError(t, err)
513520
validateState(t, prebuilds.ReconciliationState{
@@ -688,7 +695,8 @@ func TestExpiredPrebuilds(t *testing.T) {
688695
require.NoError(t, err)
689696

690697
// THEN: we should identify that this prebuild is expired.
691-
state := ps.CalculateState()
698+
state, err := ps.CalculateState()
699+
require.NoError(t, err)
692700
actions, err := ps.CalculateActions(clock, backoffInterval)
693701
require.NoError(t, err)
694702
tc.checkFn(running, *state, actions)
@@ -724,7 +732,8 @@ func TestDeprecated(t *testing.T) {
724732
require.NoError(t, err)
725733

726734
// THEN: all running prebuilds should be deleted because the template is deprecated.
727-
state := ps.CalculateState()
735+
state, err := ps.CalculateState()
736+
require.NoError(t, err)
728737
actions, err := ps.CalculateActions(clock, backoffInterval)
729738
require.NoError(t, err)
730739
validateState(t, prebuilds.ReconciliationState{
@@ -777,7 +786,8 @@ func TestLatestBuildFailed(t *testing.T) {
777786
require.NoError(t, err)
778787

779788
// THEN: reconciliation should backoff.
780-
state := psCurrent.CalculateState()
789+
state, err := psCurrent.CalculateState()
790+
require.NoError(t, err)
781791
actions, err := psCurrent.CalculateActions(clock, backoffInterval)
782792
require.NoError(t, err)
783793
validateState(t, prebuilds.ReconciliationState{
@@ -795,7 +805,8 @@ func TestLatestBuildFailed(t *testing.T) {
795805
require.NoError(t, err)
796806

797807
// THEN: it should NOT be in backoff because all is OK.
798-
state = psOther.CalculateState()
808+
state, err = psOther.CalculateState()
809+
require.NoError(t, err)
799810
actions, err = psOther.CalculateActions(clock, backoffInterval)
800811
require.NoError(t, err)
801812
validateState(t, prebuilds.ReconciliationState{
@@ -809,7 +820,8 @@ func TestLatestBuildFailed(t *testing.T) {
809820
// THEN: a new prebuild should be created.
810821
psCurrent, err = snapshot.FilterByPreset(current.presetID)
811822
require.NoError(t, err)
812-
state = psCurrent.CalculateState()
823+
state, err = psCurrent.CalculateState()
824+
require.NoError(t, err)
813825
actions, err = psCurrent.CalculateActions(clock, backoffInterval)
814826
require.NoError(t, err)
815827
validateState(t, prebuilds.ReconciliationState{
@@ -872,7 +884,8 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
872884
ps, err := snapshot.FilterByPreset(presetOpts1.presetID)
873885
require.NoError(t, err)
874886

875-
state := ps.CalculateState()
887+
state, err := ps.CalculateState()
888+
require.NoError(t, err)
876889
actions, err := ps.CalculateActions(clock, backoffInterval)
877890
require.NoError(t, err)
878891

@@ -888,7 +901,8 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
888901
ps, err := snapshot.FilterByPreset(presetOpts2.presetID)
889902
require.NoError(t, err)
890903

891-
state := ps.CalculateState()
904+
state, err := ps.CalculateState()
905+
require.NoError(t, err)
892906
actions, err := ps.CalculateActions(clock, backoffInterval)
893907
require.NoError(t, err)
894908

@@ -992,7 +1006,8 @@ func TestPrebuildAutoscaling(t *testing.T) {
9921006
ps, err := snapshot.FilterByPreset(presetOpts1.presetID)
9931007
require.NoError(t, err)
9941008

995-
state := ps.CalculateState()
1009+
state, err := ps.CalculateState()
1010+
require.NoError(t, err)
9961011
actions, err := ps.CalculateActions(clock, backoffInterval)
9971012
require.NoError(t, err)
9981013

@@ -1013,7 +1028,8 @@ func TestPrebuildAutoscaling(t *testing.T) {
10131028
ps, err := snapshot.FilterByPreset(presetOpts2.presetID)
10141029
require.NoError(t, err)
10151030

1016-
state := ps.CalculateState()
1031+
state, err := ps.CalculateState()
1032+
require.NoError(t, err)
10171033
actions, err := ps.CalculateActions(clock, backoffInterval)
10181034
require.NoError(t, err)
10191035

‎enterprise/coderd/prebuilds/metricscollector.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,11 @@ 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 := presetSnapshot.CalculateState()
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+
}
183187

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

‎enterprise/coderd/prebuilds/reconcile.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,11 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
441441
}
442442
}
443443

444-
state := ps.CalculateState()
444+
state, err := ps.CalculateState()
445+
if err != nil {
446+
logger.Error(ctx, "failed to calculate state for preset", slog.Error(err))
447+
return err
448+
}
445449
actions, err := c.CalculateActions(ctx, ps)
446450
if err != nil {
447451
logger.Error(ctx, "failed to calculate actions for preset", slog.Error(err))

‎enterprise/coderd/prebuilds/reconcile_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,8 @@ 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 := presetState.CalculateState()
1371+
state, err := presetState.CalculateState()
1372+
require.NoError(t, err)
13721373
actions, err := reconciler.CalculateActions(ctx, *presetState)
13731374
require.NoError(t, err)
13741375
require.Equal(t, 1, len(actions))
@@ -1392,7 +1393,8 @@ func TestFailedBuildBackoff(t *testing.T) {
13921393
require.NoError(t, err)
13931394
presetState, err = snapshot.FilterByPreset(preset.ID)
13941395
require.NoError(t, err)
1395-
newState := presetState.CalculateState()
1396+
newState, err := presetState.CalculateState()
1397+
require.NoError(t, err)
13961398
newActions, err := reconciler.CalculateActions(ctx, *presetState)
13971399
require.NoError(t, err)
13981400
require.Equal(t, 1, len(newActions))
@@ -1410,7 +1412,8 @@ func TestFailedBuildBackoff(t *testing.T) {
14101412
require.NoError(t, err)
14111413
presetState, err = snapshot.FilterByPreset(preset.ID)
14121414
require.NoError(t, err)
1413-
state = presetState.CalculateState()
1415+
state, err = presetState.CalculateState()
1416+
require.NoError(t, err)
14141417
actions, err = reconciler.CalculateActions(ctx, *presetState)
14151418
require.NoError(t, err)
14161419
require.Equal(t, 1, len(actions))
@@ -1433,7 +1436,8 @@ func TestFailedBuildBackoff(t *testing.T) {
14331436
require.NoError(t, err)
14341437
presetState, err = snapshot.FilterByPreset(preset.ID)
14351438
require.NoError(t, err)
1436-
state = presetState.CalculateState()
1439+
state, err = presetState.CalculateState()
1440+
require.NoError(t, err)
14371441
actions, err = reconciler.CalculateActions(ctx, *presetState)
14381442
require.NoError(t, err)
14391443
require.Equal(t, 1, len(actions))

0 commit comments

Comments
 (0)