diff --git a/coderd/dynamicparameters/render.go b/coderd/dynamicparameters/render.go index 562517b6db284..5317116b45a00 100644 --- a/coderd/dynamicparameters/render.go +++ b/coderd/dynamicparameters/render.go @@ -30,11 +30,34 @@ import ( // Forgetting to do so will result in a memory leak. type Renderer interface { Render(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) + RenderWithoutCache(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) Close() } var ErrTemplateVersionNotReady = xerrors.New("template version job not finished") +// RenderCache is an interface for caching preview.Preview results. +type RenderCache interface { + get(templateVersionID, ownerID uuid.UUID, parameters map[string]string) (*preview.Output, bool) + put(templateVersionID, ownerID uuid.UUID, parameters map[string]string, output *preview.Output) + Close() +} + +// noopRenderCache is a no-op implementation of RenderCache that doesn't cache anything. +type noopRenderCache struct{} + +func (noopRenderCache) get(uuid.UUID, uuid.UUID, map[string]string) (*preview.Output, bool) { + return nil, false +} + +func (noopRenderCache) put(uuid.UUID, uuid.UUID, map[string]string, *preview.Output) { + // no-op +} + +func (noopRenderCache) Close() { + // no-op +} + // loader is used to load the necessary coder objects for rendering a template // version's parameters. The output is a Renderer, which is the object that uses // the cached objects to render the template version's parameters. @@ -46,6 +69,9 @@ type loader struct { job *database.ProvisionerJob terraformValues *database.TemplateVersionTerraformValue templateVariableValues *[]database.TemplateVersionVariable + + // renderCache caches preview.Preview results + renderCache RenderCache } // Prepare is the entrypoint for this package. It loads the necessary objects & @@ -54,6 +80,7 @@ type loader struct { func Prepare(ctx context.Context, db database.Store, cache files.FileAcquirer, versionID uuid.UUID, options ...func(r *loader)) (Renderer, error) { l := &loader{ templateVersionID: versionID, + renderCache: noopRenderCache{}, } for _, opt := range options { @@ -91,6 +118,12 @@ func WithTerraformValues(values database.TemplateVersionTerraformValue) func(r * } } +func WithRenderCache(cache RenderCache) func(r *loader) { + return func(r *loader) { + r.renderCache = cache + } +} + func (r *loader) loadData(ctx context.Context, db database.Store) error { if r.templateVersion == nil { tv, err := db.GetTemplateVersionByID(ctx, r.templateVersionID) @@ -227,6 +260,21 @@ type dynamicRenderer struct { } func (r *dynamicRenderer) Render(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + return r.render(ctx, ownerID, values, true) +} + +func (r *dynamicRenderer) RenderWithoutCache(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + return r.render(ctx, ownerID, values, false) +} + +func (r *dynamicRenderer) render(ctx context.Context, ownerID uuid.UUID, values map[string]string, useCache bool) (*preview.Output, hcl.Diagnostics) { + // Check cache first if enabled + if useCache { + if cached, ok := r.data.renderCache.get(r.data.templateVersionID, ownerID, values); ok { + return cached, nil + } + } + // Always start with the cached error, if we have one. ownerErr := r.ownerErrors[ownerID] if ownerErr == nil { @@ -258,7 +306,14 @@ func (r *dynamicRenderer) Render(ctx context.Context, ownerID uuid.UUID, values Logger: slog.New(slog.DiscardHandler), } - return preview.Preview(ctx, input, r.templateFS) + output, diags := preview.Preview(ctx, input, r.templateFS) + + // Store in cache if successful and caching is enabled + if useCache && !diags.HasErrors() { + r.data.renderCache.put(r.data.templateVersionID, ownerID, values, output) + } + + return output, diags } func (r *dynamicRenderer) getWorkspaceOwnerData(ctx context.Context, ownerID uuid.UUID) error { diff --git a/coderd/dynamicparameters/rendercache.go b/coderd/dynamicparameters/rendercache.go new file mode 100644 index 0000000000000..28f5da646774a --- /dev/null +++ b/coderd/dynamicparameters/rendercache.go @@ -0,0 +1,214 @@ +package dynamicparameters + +import ( + "context" + "fmt" + "sort" + "sync" + "time" + + "github.com/cespare/xxhash/v2" + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + + "github.com/coder/preview" + "github.com/coder/quartz" +) + +// RenderCacheImpl is a simple in-memory cache for preview.Preview results. +// It caches based on (templateVersionID, ownerID, parameterValues). +type RenderCacheImpl struct { + mu sync.RWMutex + entries map[cacheKey]*cacheEntry + + // Metrics (optional) + cacheHits prometheus.Counter + cacheMisses prometheus.Counter + cacheSize prometheus.Gauge + + // TTL cleanup + clock quartz.Clock + ttl time.Duration + stopOnce sync.Once + stopCh chan struct{} + doneCh chan struct{} +} + +type cacheEntry struct { + output *preview.Output + timestamp time.Time +} + +type cacheKey struct { + templateVersionID uuid.UUID + ownerID uuid.UUID + parameterHash uint64 +} + +// NewRenderCache creates a new render cache with a default TTL of 1 hour. +func NewRenderCache() *RenderCacheImpl { + return newCache(quartz.NewReal(), time.Hour, nil, nil, nil) +} + +// NewRenderCacheWithMetrics creates a new render cache with Prometheus metrics. +func NewRenderCacheWithMetrics(cacheHits, cacheMisses prometheus.Counter, cacheSize prometheus.Gauge) *RenderCacheImpl { + return newCache(quartz.NewReal(), time.Hour, cacheHits, cacheMisses, cacheSize) +} + +func newCache(clock quartz.Clock, ttl time.Duration, cacheHits, cacheMisses prometheus.Counter, cacheSize prometheus.Gauge) *RenderCacheImpl { + c := &RenderCacheImpl{ + entries: make(map[cacheKey]*cacheEntry), + clock: clock, + cacheHits: cacheHits, + cacheMisses: cacheMisses, + cacheSize: cacheSize, + ttl: ttl, + stopCh: make(chan struct{}), + doneCh: make(chan struct{}), + } + + // Start cleanup goroutine + go c.cleanupLoop(context.Background()) + + return c +} + +// NewRenderCacheForTest creates a new render cache for testing purposes. +func NewRenderCacheForTest() *RenderCacheImpl { + return NewRenderCache() +} + +// Close stops the cleanup goroutine and waits for it to finish. +func (c *RenderCacheImpl) Close() { + c.stopOnce.Do(func() { + close(c.stopCh) + <-c.doneCh + }) +} + +func (c *RenderCacheImpl) get(templateVersionID, ownerID uuid.UUID, parameters map[string]string) (*preview.Output, bool) { + key := makeKey(templateVersionID, ownerID, parameters) + c.mu.RLock() + entry, ok := c.entries[key] + c.mu.RUnlock() + + if !ok { + // Record miss + if c.cacheMisses != nil { + c.cacheMisses.Inc() + } + return nil, false + } + + // Check if entry has expired + if c.clock.Since(entry.timestamp) > c.ttl { + // Expired entry, treat as miss + if c.cacheMisses != nil { + c.cacheMisses.Inc() + } + return nil, false + } + + // Record hit and refresh timestamp + if c.cacheHits != nil { + c.cacheHits.Inc() + } + + // Refresh timestamp on hit to keep frequently accessed entries alive + c.mu.Lock() + entry.timestamp = c.clock.Now() + c.mu.Unlock() + + return entry.output, true +} + +func (c *RenderCacheImpl) put(templateVersionID, ownerID uuid.UUID, parameters map[string]string, output *preview.Output) { + key := makeKey(templateVersionID, ownerID, parameters) + c.mu.Lock() + defer c.mu.Unlock() + + c.entries[key] = &cacheEntry{ + output: output, + timestamp: c.clock.Now(), + } + + // Update cache size metric + if c.cacheSize != nil { + c.cacheSize.Set(float64(len(c.entries))) + } +} + +func makeKey(templateVersionID, ownerID uuid.UUID, parameters map[string]string) cacheKey { + return cacheKey{ + templateVersionID: templateVersionID, + ownerID: ownerID, + parameterHash: hashParameters(parameters), + } +} + +// hashParameters creates a deterministic hash of the parameter map. +func hashParameters(params map[string]string) uint64 { + if len(params) == 0 { + return 0 + } + + // Sort keys for deterministic hashing + keys := make([]string, 0, len(params)) + for k := range params { + keys = append(keys, k) + } + sort.Strings(keys) + + // Hash the sorted key-value pairs + var b string + for _, k := range keys { + b += fmt.Sprintf("%s:%s,", k, params[k]) + } + + return xxhash.Sum64String(b) +} + +// cleanupLoop runs periodically to remove expired cache entries. +func (c *RenderCacheImpl) cleanupLoop(ctx context.Context) { + defer close(c.doneCh) + + // Run cleanup every 15 minutes + cleanupFunc := func() error { + c.cleanup() + return nil + } + + // Run once immediately + _ = cleanupFunc() + + // Create a cancellable context for the ticker + tickerCtx, cancel := context.WithCancel(ctx) + defer cancel() + + // Create ticker for periodic cleanup + tkr := c.clock.TickerFunc(tickerCtx, 15*time.Minute, cleanupFunc, "render-cache-cleanup") + + // Wait for stop signal + <-c.stopCh + cancel() + + _ = tkr.Wait() +} + +// cleanup removes expired entries from the cache. +func (c *RenderCacheImpl) cleanup() { + c.mu.Lock() + defer c.mu.Unlock() + + now := c.clock.Now() + for key, entry := range c.entries { + if now.Sub(entry.timestamp) > c.ttl { + delete(c.entries, key) + } + } + + // Update cache size metric after cleanup + if c.cacheSize != nil { + c.cacheSize.Set(float64(len(c.entries))) + } +} diff --git a/coderd/dynamicparameters/rendercache_internal_test.go b/coderd/dynamicparameters/rendercache_internal_test.go new file mode 100644 index 0000000000000..3a6cf8489e9af --- /dev/null +++ b/coderd/dynamicparameters/rendercache_internal_test.go @@ -0,0 +1,354 @@ +package dynamicparameters + +import ( + "testing" + "time" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + promtestutil "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/testutil" + "github.com/coder/preview" + previewtypes "github.com/coder/preview/types" + "github.com/coder/quartz" +) + +func TestRenderCache_BasicOperations(t *testing.T) { + t.Parallel() + + cache := NewRenderCache() + defer cache.Close() + templateVersionID := uuid.New() + ownerID := uuid.New() + params := map[string]string{"region": "us-west-2"} + + // Cache should be empty initially + _, ok := cache.get(templateVersionID, ownerID, params) + require.False(t, ok, "cache should be empty initially") + + // Put an entry in the cache + expectedOutput := &preview.Output{ + Parameters: []previewtypes.Parameter{ + { + ParameterData: previewtypes.ParameterData{ + Name: "region", + Type: previewtypes.ParameterTypeString, + }, + }, + }, + } + cache.put(templateVersionID, ownerID, params, expectedOutput) + + // Get should now return the cached value + cachedOutput, ok := cache.get(templateVersionID, ownerID, params) + require.True(t, ok, "cache should contain the entry") + require.Same(t, expectedOutput, cachedOutput, "should return same pointer") +} + +func TestRenderCache_DifferentKeysAreSeparate(t *testing.T) { + t.Parallel() + + cache := NewRenderCache() + defer cache.Close() + + templateVersion1 := uuid.New() + templateVersion2 := uuid.New() + owner1 := uuid.New() + owner2 := uuid.New() + params := map[string]string{"region": "us-west-2"} + + output1 := &preview.Output{} + output2 := &preview.Output{} + output3 := &preview.Output{} + + // Put different entries for different keys + cache.put(templateVersion1, owner1, params, output1) + cache.put(templateVersion2, owner1, params, output2) + cache.put(templateVersion1, owner2, params, output3) + + // Verify each key returns its own entry + cached1, ok1 := cache.get(templateVersion1, owner1, params) + require.True(t, ok1) + require.Same(t, output1, cached1) + + cached2, ok2 := cache.get(templateVersion2, owner1, params) + require.True(t, ok2) + require.Same(t, output2, cached2) + + cached3, ok3 := cache.get(templateVersion1, owner2, params) + require.True(t, ok3) + require.Same(t, output3, cached3) +} + +func TestRenderCache_ParameterHashConsistency(t *testing.T) { + t.Parallel() + + cache := NewRenderCache() + defer cache.Close() + + templateVersionID := uuid.New() + ownerID := uuid.New() + + // Parameters in different order should produce same cache key + params1 := map[string]string{"a": "1", "b": "2", "c": "3"} + params2 := map[string]string{"c": "3", "a": "1", "b": "2"} + + output := &preview.Output{} + cache.put(templateVersionID, ownerID, params1, output) + + // Should hit cache even with different parameter order + cached, ok := cache.get(templateVersionID, ownerID, params2) + require.True(t, ok, "different parameter order should still hit cache") + require.Same(t, output, cached) +} + +func TestRenderCache_EmptyParameters(t *testing.T) { + t.Parallel() + + cache := NewRenderCache() + defer cache.Close() + + templateVersionID := uuid.New() + ownerID := uuid.New() + + // Test with empty parameters + emptyParams := map[string]string{} + output := &preview.Output{} + + cache.put(templateVersionID, ownerID, emptyParams, output) + + cached, ok := cache.get(templateVersionID, ownerID, emptyParams) + require.True(t, ok) + require.Same(t, output, cached) +} + +func TestRenderCache_PrebuildScenario(t *testing.T) { + t.Parallel() + + // This test simulates the prebuild scenario where multiple prebuilds + // are created from the same template version with the same preset parameters. + cache := NewRenderCache() + defer cache.Close() + + // In prebuilds, all instances use the same fixed ownerID + prebuildOwnerID := uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") // database.PrebuildsSystemUserID + templateVersionID := uuid.New() + presetParams := map[string]string{ + "instance_type": "t3.micro", + "region": "us-west-2", + } + + output := &preview.Output{} + + // First prebuild - cache miss + _, ok := cache.get(templateVersionID, prebuildOwnerID, presetParams) + require.False(t, ok, "first prebuild should miss cache") + + cache.put(templateVersionID, prebuildOwnerID, presetParams, output) + + // Second prebuild with same template version and preset - cache hit + cached2, ok2 := cache.get(templateVersionID, prebuildOwnerID, presetParams) + require.True(t, ok2, "second prebuild should hit cache") + require.Same(t, output, cached2, "should return cached output") + + // Third prebuild - also cache hit + cached3, ok3 := cache.get(templateVersionID, prebuildOwnerID, presetParams) + require.True(t, ok3, "third prebuild should hit cache") + require.Same(t, output, cached3, "should return cached output") + + // All three prebuilds shared the same cache entry +} + +func TestRenderCache_Metrics(t *testing.T) { + t.Parallel() + + // Create test metrics + cacheHits := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_cache_hits_total", + }) + cacheMisses := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_cache_misses_total", + }) + cacheSize := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "test_cache_entries", + }) + + cache := NewRenderCacheWithMetrics(cacheHits, cacheMisses, cacheSize) + defer cache.Close() + + templateVersionID := uuid.New() + ownerID := uuid.New() + params := map[string]string{"region": "us-west-2"} + + // Initially: 0 hits, 0 misses, 0 size + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheHits), "initial hits should be 0") + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheMisses), "initial misses should be 0") + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheSize), "initial size should be 0") + + // First get - should be a miss + _, ok := cache.get(templateVersionID, ownerID, params) + require.False(t, ok) + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheHits), "hits should still be 0") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should be 1") + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheSize), "size should still be 0") + + // Put an entry + output := &preview.Output{} + cache.put(templateVersionID, ownerID, params, output) + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheSize), "size should be 1 after put") + + // Second get - should be a hit + _, ok = cache.get(templateVersionID, ownerID, params) + require.True(t, ok) + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheHits), "hits should be 1") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should still be 1") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheSize), "size should still be 1") + + // Third get - another hit + _, ok = cache.get(templateVersionID, ownerID, params) + require.True(t, ok) + require.Equal(t, float64(2), promtestutil.ToFloat64(cacheHits), "hits should be 2") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should still be 1") + + // Put another entry with different params + params2 := map[string]string{"region": "us-east-1"} + cache.put(templateVersionID, ownerID, params2, output) + require.Equal(t, float64(2), promtestutil.ToFloat64(cacheSize), "size should be 2 after second put") + + // Get with different params - should be a hit + _, ok = cache.get(templateVersionID, ownerID, params2) + require.True(t, ok) + require.Equal(t, float64(3), promtestutil.ToFloat64(cacheHits), "hits should be 3") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should still be 1") +} + +func TestRenderCache_TTL(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + clock := quartz.NewMock(t) + + trapTickerFunc := clock.Trap().TickerFunc("render-cache-cleanup") + defer trapTickerFunc.Close() + + // Create cache with short TTL for testing + cache := newCache(clock, 100*time.Millisecond, nil, nil, nil) + defer cache.Close() + + // Wait for the initial cleanup and ticker to be created + trapTickerFunc.MustWait(ctx).Release(ctx) + + templateVersionID := uuid.New() + ownerID := uuid.New() + params := map[string]string{"region": "us-west-2"} + output := &preview.Output{} + + // Put an entry + cache.put(templateVersionID, ownerID, params, output) + + // Should be a hit immediately + cached, ok := cache.get(templateVersionID, ownerID, params) + require.True(t, ok, "should hit cache immediately") + require.Same(t, output, cached) + + // Advance time beyond TTL + clock.Advance(150 * time.Millisecond) + + // Should be a miss after TTL + _, ok = cache.get(templateVersionID, ownerID, params) + require.False(t, ok, "should miss cache after TTL expiration") +} + +func TestRenderCache_CleanupRemovesExpiredEntries(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + clock := quartz.NewMock(t) + + trapTickerFunc := clock.Trap().TickerFunc("render-cache-cleanup") + defer trapTickerFunc.Close() + + cacheSize := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "test_cache_entries", + }) + cache := newCache(clock, 100*time.Millisecond, nil, nil, cacheSize) + defer cache.Close() + + // Wait for the initial cleanup and ticker to be created + trapTickerFunc.MustWait(ctx).Release(ctx) + + // Initial size should be 0 after first cleanup + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheSize), "should have 0 entries initially") + + templateVersionID := uuid.New() + ownerID := uuid.New() + + // Add 3 entries + for i := 0; i < 3; i++ { + params := map[string]string{"index": string(rune(i))} + cache.put(templateVersionID, ownerID, params, &preview.Output{}) + } + + require.Equal(t, float64(3), promtestutil.ToFloat64(cacheSize), "should have 3 entries") + + // Advance time beyond TTL + clock.Advance(150 * time.Millisecond) + + // Trigger cleanup by advancing to the next ticker event (15 minutes from start minus what we already advanced) + clock.Advance(15*time.Minute - 150*time.Millisecond).MustWait(ctx) + + // All entries should be removed + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheSize), "all entries should be removed after cleanup") +} + +func TestRenderCache_TimestampRefreshOnHit(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + clock := quartz.NewMock(t) + + trapTickerFunc := clock.Trap().TickerFunc("render-cache-cleanup") + defer trapTickerFunc.Close() + + // Create cache with 1 second TTL for testing + cache := newCache(clock, time.Second, nil, nil, nil) + defer cache.Close() + + // Wait for the initial cleanup and ticker to be created + trapTickerFunc.MustWait(ctx).Release(ctx) + + templateVersionID := uuid.New() + ownerID := uuid.New() + params := map[string]string{"region": "us-west-2"} + output := &preview.Output{} + + // Put an entry at T=0 + cache.put(templateVersionID, ownerID, params, output) + + // Advance time to 600ms (still within TTL) + clock.Advance(600 * time.Millisecond) + + // Access the entry - should hit and refresh timestamp to T=600ms + cached, ok := cache.get(templateVersionID, ownerID, params) + require.True(t, ok, "should hit cache") + require.Same(t, output, cached) + + // Advance another 600ms (now at T=1200ms) + // Entry was created at T=0 but refreshed at T=600ms, so it should still be valid + clock.Advance(600 * time.Millisecond) + + // Should still hit because timestamp was refreshed at T=600ms + cached, ok = cache.get(templateVersionID, ownerID, params) + require.True(t, ok, "should still hit cache because timestamp was refreshed") + require.Same(t, output, cached) + + // Now advance another 1.1 seconds (to T=2300ms) + // Last refresh was at T=1200ms, so now it should be expired + clock.Advance(1100 * time.Millisecond) + + // Should miss because more than 1 second since last access + _, ok = cache.get(templateVersionID, ownerID, params) + require.False(t, ok, "should miss cache after TTL from last access") +} diff --git a/coderd/dynamicparameters/rendercache_prebuild_internal_test.go b/coderd/dynamicparameters/rendercache_prebuild_internal_test.go new file mode 100644 index 0000000000000..13fe99e7a322d --- /dev/null +++ b/coderd/dynamicparameters/rendercache_prebuild_internal_test.go @@ -0,0 +1,197 @@ +package dynamicparameters + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/hashicorp/hcl/v2" + "github.com/prometheus/client_golang/prometheus" + promtestutil "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" + "github.com/coder/preview" + previewtypes "github.com/coder/preview/types" + "github.com/coder/terraform-provider-coder/v2/provider" +) + +// TestRenderCache_PrebuildWithResolveParameters simulates the actual prebuild flow +// where ResolveParameters calls Render() twice - once with previous values and once +// with the final computed values. +func TestRenderCache_PrebuildWithResolveParameters(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + + // Create test metrics + cacheHits := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_prebuild_cache_hits_total", + }) + cacheMisses := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_prebuild_cache_misses_total", + }) + cacheSize := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "test_prebuild_cache_entries", + }) + + cache := NewRenderCacheWithMetrics(cacheHits, cacheMisses, cacheSize) + defer cache.Close() + + // Simulate prebuild scenario + prebuildOwnerID := uuid.MustParse("c42fdf75-3097-471c-8c33-fb52454d81c0") // database.PrebuildsSystemUserID + templateVersionID := uuid.New() + + // Preset parameters that all prebuilds share + presetParams := []database.TemplateVersionPresetParameter{ + {Name: "instance_type", Value: "t3.micro"}, + {Name: "region", Value: "us-west-2"}, + } + + // Create a mock renderer that returns consistent parameter definitions + mockRenderer := &mockRenderer{ + cache: cache, + templateVersionID: templateVersionID, + output: &preview.Output{ + Parameters: []previewtypes.Parameter{ + { + ParameterData: previewtypes.ParameterData{ + Name: "instance_type", + Type: previewtypes.ParameterTypeString, + FormType: provider.ParameterFormTypeInput, + Mutable: true, + DefaultValue: previewtypes.StringLiteral("t3.micro"), + Required: true, + }, + Value: previewtypes.StringLiteral("t3.micro"), + Diagnostics: nil, + }, + { + ParameterData: previewtypes.ParameterData{ + Name: "region", + Type: previewtypes.ParameterTypeString, + FormType: provider.ParameterFormTypeInput, + Mutable: true, + DefaultValue: previewtypes.StringLiteral("us-west-2"), + Required: true, + }, + Value: previewtypes.StringLiteral("us-west-2"), + Diagnostics: nil, + }, + }, + }, + } + + // Initial metrics should be 0 + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheHits), "initial hits should be 0") + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheMisses), "initial misses should be 0") + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheSize), "initial size should be 0") + + // === FIRST PREBUILD === + // First build: no previous values, preset values provided + values1, err := ResolveParameters(ctx, prebuildOwnerID, mockRenderer, true, + []database.WorkspaceBuildParameter{}, // No previous values (first build) + []codersdk.WorkspaceBuildParameter{}, // No build-specific values + presetParams, // Preset values from template + ) + require.NoError(t, err) + require.NotNil(t, values1) + + // After first prebuild: + // - ResolveParameters calls Render() twice: + // 1. RenderWithoutCache with previousValuesMap (empty {}) → no cache operation + // 2. Render with values.ValuesMap() ({preset}) → miss, creates cache entry + // Expected: 0 hits, 1 miss, 1 cache entry + t.Logf("After first prebuild: hits=%v, misses=%v, size=%v", + promtestutil.ToFloat64(cacheHits), + promtestutil.ToFloat64(cacheMisses), + promtestutil.ToFloat64(cacheSize)) + + require.Equal(t, float64(0), promtestutil.ToFloat64(cacheHits), "first prebuild should have 0 hits") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "first prebuild should have 1 miss") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheSize), "should have 1 cache entry after first prebuild") + + // === SECOND PREBUILD === + // Second build: previous values now set to preset values + previousValues := []database.WorkspaceBuildParameter{ + {Name: "instance_type", Value: "t3.micro"}, + {Name: "region", Value: "us-west-2"}, + } + + values2, err := ResolveParameters(ctx, prebuildOwnerID, mockRenderer, false, + previousValues, // Previous values from first build + []codersdk.WorkspaceBuildParameter{}, + presetParams, + ) + require.NoError(t, err) + require.NotNil(t, values2) + + // After second prebuild: + // - ResolveParameters calls Render() twice: + // 1. RenderWithoutCache with previousValuesMap ({preset}) → no cache operation + // 2. Render with values.ValuesMap() ({preset}) → HIT (cache entry from first prebuild's 2nd render) + // Expected: 1 hit, 1 miss (still), 1 cache entry (still) + t.Logf("After second prebuild: hits=%v, misses=%v, size=%v", + promtestutil.ToFloat64(cacheHits), + promtestutil.ToFloat64(cacheMisses), + promtestutil.ToFloat64(cacheSize)) + + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheHits), "second prebuild should have 1 hit") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should still be 1") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheSize), "should still have 1 cache entry") + + // === THIRD PREBUILD === + values3, err := ResolveParameters(ctx, prebuildOwnerID, mockRenderer, false, + previousValues, + []codersdk.WorkspaceBuildParameter{}, + presetParams, + ) + require.NoError(t, err) + require.NotNil(t, values3) + + // After third prebuild: + // - ResolveParameters calls Render() twice: + // 1. RenderWithoutCache with previousValuesMap ({preset}) → no cache operation + // 2. Render with values.ValuesMap() ({preset}) → HIT + // Expected: 2 hits, 1 miss (still), 1 cache entry (still) + t.Logf("After third prebuild: hits=%v, misses=%v, size=%v", + promtestutil.ToFloat64(cacheHits), + promtestutil.ToFloat64(cacheMisses), + promtestutil.ToFloat64(cacheSize)) + + require.Equal(t, float64(2), promtestutil.ToFloat64(cacheHits), "third prebuild should have 2 total hits") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheMisses), "misses should still be 1") + require.Equal(t, float64(1), promtestutil.ToFloat64(cacheSize), "should still have 1 cache entry") + + // Summary: With 3 prebuilds, we should have: + // - 2 cache hits (1 from 2nd prebuild, 1 from 3rd prebuild) + // - 1 cache miss (1 from 1st prebuild) + // - 1 cache entry (for preset params only - introspection renders are not cached) +} + +// mockRenderer is a simple renderer that uses the cache for testing +type mockRenderer struct { + cache RenderCache + templateVersionID uuid.UUID + output *preview.Output +} + +func (m *mockRenderer) Render(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + // This simulates what dynamicRenderer does - check cache first + if cached, ok := m.cache.get(m.templateVersionID, ownerID, values); ok { + return cached, nil + } + + // Not in cache, "render" (just return our mock output) and cache it + m.cache.put(m.templateVersionID, ownerID, values, m.output) + return m.output, nil +} + +func (m *mockRenderer) RenderWithoutCache(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + // For test purposes, just return output without caching + return m.output, nil +} + +func (m *mockRenderer) Close() {} diff --git a/coderd/dynamicparameters/rendermock/rendermock.go b/coderd/dynamicparameters/rendermock/rendermock.go index 996b02a555b08..731db67d12023 100644 --- a/coderd/dynamicparameters/rendermock/rendermock.go +++ b/coderd/dynamicparameters/rendermock/rendermock.go @@ -69,3 +69,18 @@ func (mr *MockRendererMockRecorder) Render(ctx, ownerID, values any) *gomock.Cal mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Render", reflect.TypeOf((*MockRenderer)(nil).Render), ctx, ownerID, values) } + +// RenderWithoutCache mocks base method. +func (m *MockRenderer) RenderWithoutCache(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RenderWithoutCache", ctx, ownerID, values) + ret0, _ := ret[0].(*preview.Output) + ret1, _ := ret[1].(hcl.Diagnostics) + return ret0, ret1 +} + +// RenderWithoutCache indicates an expected call of RenderWithoutCache. +func (mr *MockRendererMockRecorder) RenderWithoutCache(ctx, ownerID, values any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RenderWithoutCache", reflect.TypeOf((*MockRenderer)(nil).RenderWithoutCache), ctx, ownerID, values) +} diff --git a/coderd/dynamicparameters/resolver.go b/coderd/dynamicparameters/resolver.go index 7fc67d29a0d55..b2e647f339976 100644 --- a/coderd/dynamicparameters/resolver.go +++ b/coderd/dynamicparameters/resolver.go @@ -69,7 +69,10 @@ func ResolveParameters( // // This is how the form should look to the user on their workspace settings page. // This is the original form truth that our validations should initially be based on. - output, diags := renderer.Render(ctx, ownerID, previousValuesMap) + // + // Skip caching this render - it's only used for introspection to identify ephemeral + // parameters. The actual values used for the build will be rendered and cached below. + output, diags := renderer.RenderWithoutCache(ctx, ownerID, previousValuesMap) if diags.HasErrors() { // Top level diagnostics should break the build. Previous values (and new) should // always be valid. If there is a case where this is not true, then this has to diff --git a/coderd/dynamicparameters/resolver_test.go b/coderd/dynamicparameters/resolver_test.go index e6675e6f4c7dc..65ccd6cafb643 100644 --- a/coderd/dynamicparameters/resolver_test.go +++ b/coderd/dynamicparameters/resolver_test.go @@ -28,6 +28,25 @@ func TestResolveParameters(t *testing.T) { render := rendermock.NewMockRenderer(ctrl) // A single immutable parameter with no previous value. + render.EXPECT(). + RenderWithoutCache(gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + Return(&preview.Output{ + Parameters: []previewtypes.Parameter{ + { + ParameterData: previewtypes.ParameterData{ + Name: "immutable", + Type: previewtypes.ParameterTypeString, + FormType: provider.ParameterFormTypeInput, + Mutable: false, + DefaultValue: previewtypes.StringLiteral("foo"), + Required: true, + }, + Value: previewtypes.StringLiteral("foo"), + Diagnostics: nil, + }, + }, + }, nil) render.EXPECT(). Render(gomock.Any(), gomock.Any(), gomock.Any()). AnyTimes(). @@ -78,7 +97,7 @@ func TestResolveParameters(t *testing.T) { // A single immutable parameter with no previous value. render.EXPECT(). - Render(gomock.Any(), gomock.Any(), gomock.Any()). + RenderWithoutCache(gomock.Any(), gomock.Any(), gomock.Any()). // Return the mutable param first Return(&preview.Output{ Parameters: []previewtypes.Parameter{ diff --git a/coderd/dynamicparameters/static.go b/coderd/dynamicparameters/static.go index fec5de2581aef..68268c67fa646 100644 --- a/coderd/dynamicparameters/static.go +++ b/coderd/dynamicparameters/static.go @@ -40,6 +40,15 @@ func (r *loader) staticRender(ctx context.Context, db database.Store) (*staticRe } func (r *staticRender) Render(_ context.Context, _ uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + return r.render(values) +} + +func (r *staticRender) RenderWithoutCache(_ context.Context, _ uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + // Static renderer doesn't use cache, so this is the same as Render + return r.render(values) +} + +func (r *staticRender) render(values map[string]string) (*preview.Output, hcl.Diagnostics) { params := r.staticParams for i := range params { param := ¶ms[i] diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 7d388966c96d0..ee62145f8dfa3 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -88,6 +88,9 @@ type Builder struct { parameterRender dynamicparameters.Renderer workspaceTags *map[string]string + // renderCache caches template rendering results + renderCache dynamicparameters.RenderCache + prebuiltWorkspaceBuildStage sdkproto.PrebuiltWorkspaceBuildStage verifyNoLegacyParametersOnce bool } @@ -253,6 +256,14 @@ func (b Builder) TemplateVersionPresetID(id uuid.UUID) Builder { return b } +// RenderCache sets the render cache to use for template rendering. +// This allows multiple workspace builds to share cached render results. +func (b Builder) RenderCache(cache dynamicparameters.RenderCache) Builder { + // nolint: revive + b.renderCache = cache + return b +} + type BuildError struct { // Status is a suitable HTTP status code Status int @@ -686,6 +697,22 @@ func (b *Builder) getDynamicParameterRenderer() (dynamicparameters.Renderer, err return nil, xerrors.Errorf("get template version variables: %w", err) } + // Pass render cache if available + if b.renderCache != nil { + renderer, err := dynamicparameters.Prepare(b.ctx, b.store, b.fileCache, tv.ID, + dynamicparameters.WithTemplateVersion(*tv), + dynamicparameters.WithProvisionerJob(*job), + dynamicparameters.WithTerraformValues(*tfVals), + dynamicparameters.WithTemplateVariableValues(variableValues), + dynamicparameters.WithRenderCache(b.renderCache), + ) + if err != nil { + return nil, xerrors.Errorf("get template version renderer: %w", err) + } + b.parameterRender = renderer + return renderer, nil + } + renderer, err := dynamicparameters.Prepare(b.ctx, b.store, b.fileCache, tv.ID, dynamicparameters.WithTemplateVersion(*tv), dynamicparameters.WithProvisionerJob(*job), diff --git a/enterprise/cli/create_test.go b/enterprise/cli/create_test.go index 44218abb5a58d..1a4623cfa8a6c 100644 --- a/enterprise/cli/create_test.go +++ b/enterprise/cli/create_test.go @@ -371,6 +371,9 @@ func TestEnterpriseCreateWithPreset(t *testing.T) { notifications.NewNoopEnqueuer(), newNoopUsageCheckerPtr(), ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -482,6 +485,9 @@ func TestEnterpriseCreateWithPreset(t *testing.T) { notifications.NewNoopEnqueuer(), newNoopUsageCheckerPtr(), ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 217a9ff09614a..a9563888c83c1 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -168,6 +168,9 @@ func TestClaimPrebuild(t *testing.T) { cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(spy, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) api.AGPL.PrebuildsClaimer.Store(&claimer) diff --git a/enterprise/coderd/prebuilds/metricscollector_test.go b/enterprise/coderd/prebuilds/metricscollector_test.go index aa9886fb7ad1b..91cb95f825aca 100644 --- a/enterprise/coderd/prebuilds/metricscollector_test.go +++ b/enterprise/coderd/prebuilds/metricscollector_test.go @@ -1,6 +1,7 @@ package prebuilds_test import ( + "context" "fmt" "slices" "testing" @@ -198,6 +199,9 @@ func TestMetricsCollector(t *testing.T) { db, pubsub := dbtestutil.NewDB(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ctx := testutil.Context(t, testutil.WaitLong) createdUsers := []uuid.UUID{database.PrebuildsSystemUserID} @@ -330,6 +334,9 @@ func TestMetricsCollector_DuplicateTemplateNames(t *testing.T) { db, pubsub := dbtestutil.NewDB(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ctx := testutil.Context(t, testutil.WaitLong) collector := prebuilds.NewMetricsCollector(db, logger, reconciler) @@ -478,6 +485,9 @@ func TestMetricsCollector_ReconciliationPausedMetric(t *testing.T) { cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) registry := prometheus.NewPedanticRegistry() reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), registry, newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ctx := testutil.Context(t, testutil.WaitLong) // Ensure no pause setting is set (default state) @@ -507,6 +517,9 @@ func TestMetricsCollector_ReconciliationPausedMetric(t *testing.T) { cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) registry := prometheus.NewPedanticRegistry() reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), registry, newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ctx := testutil.Context(t, testutil.WaitLong) // Set reconciliation to paused @@ -536,6 +549,9 @@ func TestMetricsCollector_ReconciliationPausedMetric(t *testing.T) { cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) registry := prometheus.NewPedanticRegistry() reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), registry, newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ctx := testutil.Context(t, testutil.WaitLong) // Set reconciliation back to not paused diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go index 17a56d484c9f6..681645c4bbe7e 100644 --- a/enterprise/coderd/prebuilds/reconcile.go +++ b/enterprise/coderd/prebuilds/reconcile.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/provisionerjobs" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/dynamicparameters" "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/prebuilds" @@ -58,6 +59,9 @@ type StoreReconciler struct { metrics *MetricsCollector // Operational metrics reconciliationDuration prometheus.Histogram + + // renderCache caches template rendering results to avoid expensive re-parsing + renderCache dynamicparameters.RenderCache } var _ prebuilds.ReconciliationOrchestrator = &StoreReconciler{} @@ -119,6 +123,30 @@ func NewStoreReconciler(store database.Store, Help: "Duration of each prebuilds reconciliation cycle.", Buckets: prometheus.DefBuckets, }) + + // Create metrics for the render cache + renderCacheHits := factory.NewCounter(prometheus.CounterOpts{ + Namespace: "coderd", + Subsystem: "prebuilds", + Name: "render_cache_hits_total", + Help: "Total number of render cache hits.", + }) + renderCacheMisses := factory.NewCounter(prometheus.CounterOpts{ + Namespace: "coderd", + Subsystem: "prebuilds", + Name: "render_cache_misses_total", + Help: "Total number of render cache misses.", + }) + renderCacheSize := factory.NewGauge(prometheus.GaugeOpts{ + Namespace: "coderd", + Subsystem: "prebuilds", + Name: "render_cache_size_entries", + Help: "Current number of entries in the render cache.", + }) + + reconciler.renderCache = dynamicparameters.NewRenderCacheWithMetrics(renderCacheHits, renderCacheMisses, renderCacheSize) + } else { + reconciler.renderCache = dynamicparameters.NewRenderCache() } return reconciler @@ -240,6 +268,10 @@ func (c *StoreReconciler) Stop(ctx context.Context, cause error) { } } + // Close the render cache to stop its cleanup goroutine + // This must be done regardless of whether the reconciler is running + c.renderCache.Close() + // If the reconciler is not running, there's nothing else to do. if !c.running.Load() { return @@ -900,7 +932,8 @@ func (c *StoreReconciler) provision( builder := wsbuilder.New(workspace, transition, *c.buildUsageChecker.Load()). Reason(database.BuildReasonInitiator). Initiator(database.PrebuildsSystemUserID). - MarkPrebuild() + MarkPrebuild(). + RenderCache(c.renderCache) if transition != database.WorkspaceTransitionDelete { // We don't specify the version for a delete transition, diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 7548faebd7dab..bb871fe5682b5 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -53,6 +53,9 @@ func TestNoReconciliationActionsIfNoPresets(t *testing.T) { logger := testutil.Logger(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) controller := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + controller.Stop(context.Background(), nil) + }) // given a template version with no presets org := dbgen.Organization(t, db, database.Organization{}) @@ -96,6 +99,9 @@ func TestNoReconciliationActionsIfNoPrebuilds(t *testing.T) { logger := testutil.Logger(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) controller := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + controller.Stop(context.Background(), nil) + }) // given there are presets, but no prebuilds org := dbgen.Organization(t, db, database.Organization{}) @@ -426,6 +432,9 @@ func (tc testCase) run(t *testing.T) { } cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + controller.Stop(context.Background(), nil) + }) // Run the reconciliation multiple times to ensure idempotency // 8 was arbitrary, but large enough to reasonably trust the result @@ -1212,6 +1221,9 @@ func TestRunLoop(t *testing.T) { db, pubSub := dbtestutil.NewDB(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -1340,6 +1352,9 @@ func TestFailedBuildBackoff(t *testing.T) { db, ps := dbtestutil.NewDB(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) // Given: an active template version with presets and prebuilds configured. const desiredInstances = 2 @@ -1462,6 +1477,7 @@ func TestReconciliationLock(t *testing.T) { prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + defer reconciler.Stop(context.Background(), nil) reconciler.WithReconciliationLock(ctx, logger, func(_ context.Context, _ database.Store) error { lockObtained := mutex.TryLock() // As long as the postgres lock is held, this mutex should always be unlocked when we get here. @@ -1492,6 +1508,9 @@ func TestTrackResourceReplacement(t *testing.T) { registry := prometheus.NewRegistry() cache := files.New(registry, &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) // Given: a template admin to receive a notification. templateAdmin := dbgen.User(t, db, database.User{ @@ -2099,6 +2118,9 @@ func TestCancelPendingPrebuilds(t *testing.T) { cache := files.New(registry, &coderdtest.FakeAuthorizer{}) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}).Leveled(slog.LevelDebug) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) owner := coderdtest.CreateFirstUser(t, client) // Given: a template with a version containing a preset with 1 prebuild instance @@ -2336,6 +2358,9 @@ func TestCancelPendingPrebuilds(t *testing.T) { cache := files.New(registry, &coderdtest.FakeAuthorizer{}) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}).Leveled(slog.LevelDebug) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) owner := coderdtest.CreateFirstUser(t, client) // Given: template A with 2 versions @@ -2401,6 +2426,9 @@ func TestReconciliationStats(t *testing.T) { cache := files.New(registry, &coderdtest.FakeAuthorizer{}) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}).Leveled(slog.LevelDebug) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) owner := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) @@ -2912,6 +2940,9 @@ func TestReconciliationRespectsPauseSetting(t *testing.T) { logger := testutil.Logger(t) cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer(), newNoopUsageCheckerPtr()) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) // Setup a template with a preset that should create prebuilds org := dbgen.Organization(t, db, database.Organization{}) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 7cf9cd890b6df..c9a40f539afa5 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1978,6 +1978,9 @@ func TestPrebuildsAutobuild(t *testing.T) { notificationsNoop, api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -2100,6 +2103,9 @@ func TestPrebuildsAutobuild(t *testing.T) { notificationsNoop, api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -2222,6 +2228,9 @@ func TestPrebuildsAutobuild(t *testing.T) { notificationsNoop, api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -2366,6 +2375,9 @@ func TestPrebuildsAutobuild(t *testing.T) { notificationsNoop, api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -2511,6 +2523,9 @@ func TestPrebuildsAutobuild(t *testing.T) { notificationsNoop, api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer) @@ -2957,6 +2972,9 @@ func TestWorkspaceProvisionerdServerMetrics(t *testing.T) { notifications.NewNoopEnqueuer(), api.AGPL.BuildUsageChecker, ) + t.Cleanup(func() { + reconciler.Stop(context.Background(), nil) + }) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) api.AGPL.PrebuildsClaimer.Store(&claimer)