Skip to content
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.

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

fix(jsonnet): add configurable timeout #798

Merged
merged 3 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 19 additions & 4 deletions jsonnetsecure/jsonnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,18 @@ import (
"path"
"runtime"
"testing"
"time"

"github.com/google/go-jsonnet"
)

type (
evaluationOptions struct {
evalTimeout time.Duration
}

EvaluationOptionModifier func(*evaluationOptions)

VM interface {
EvaluateAnonymousSnippet(filename string, snippet string) (json string, formattedErr error)
ExtCode(key string, val string)
Expand All @@ -31,10 +38,11 @@ type (
}

ProcessVM struct {
ctx context.Context
path string
args []string
params processParameters
ctx context.Context
path string
args []string
execTimeout time.Duration
params processParameters
}

vmOptions struct {
Expand All @@ -43,6 +51,7 @@ type (
args []string
ctx context.Context
pool *pool
execTimeout time.Duration
}

Option func(o *vmOptions)
Expand Down Expand Up @@ -70,6 +79,12 @@ func WithProcessIsolatedVM(ctx context.Context) Option {
}
}

func WithExecTimeout(timeout time.Duration) Option {
return func(o *vmOptions) {
o.execTimeout = timeout
}
}

func WithJsonnetBinary(jsonnetBinaryPath string) Option {
return func(o *vmOptions) {
o.jsonnetBinaryPath = jsonnetBinaryPath
Expand Down
23 changes: 13 additions & 10 deletions jsonnetsecure/jsonnet_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package jsonnetsecure

import (
"bufio"
"cmp"
"context"
"encoding/json"
"io"
Expand All @@ -23,11 +24,12 @@ import (

type (
processPoolVM struct {
path string
args []string
ctx context.Context
params processParameters
pool *pool
path string
args []string
ctx context.Context
params processParameters
execTimeout time.Duration
pool *pool
}
Pool interface {
Close()
Expand Down Expand Up @@ -183,7 +185,7 @@ func (vm *processPoolVM) EvaluateAnonymousSnippet(filename string, snippet strin
defer otelx.End(span, &err)

// TODO: maybe leave the timeout to the caller?
ctx, cancel := context.WithTimeout(ctx, 1*time.Second)
ctx, cancel := context.WithTimeout(ctx, cmp.Or(vm.execTimeout, 1*time.Second))
defer cancel()

params := vm.params
Expand Down Expand Up @@ -222,10 +224,11 @@ func NewProcessPoolVM(opts *vmOptions) VM {
ctx = context.Background()
}
return &processPoolVM{
path: opts.jsonnetBinaryPath,
args: opts.args,
ctx: ctx,
pool: opts.pool,
path: opts.jsonnetBinaryPath,
args: opts.args,
ctx: ctx,
pool: opts.pool,
execTimeout: opts.execTimeout,
}
}

Expand Down
11 changes: 6 additions & 5 deletions jsonnetsecure/jsonnet_processvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package jsonnetsecure

import (
"bytes"
"cmp"
"context"
"encoding/json"
"fmt"
Expand All @@ -23,9 +24,10 @@ import (

func NewProcessVM(opts *vmOptions) VM {
return &ProcessVM{
path: opts.jsonnetBinaryPath,
args: opts.args,
ctx: opts.ctx,
path: opts.jsonnetBinaryPath,
args: opts.args,
ctx: opts.ctx,
execTimeout: opts.execTimeout,
}
}

Expand All @@ -35,12 +37,11 @@ func (p *ProcessVM) EvaluateAnonymousSnippet(filename string, snippet string) (_
defer otelx.End(span, &err)

// We retry the process creation, because it sometimes times out.
const processVMTimeout = 1 * time.Second
return backoff.RetryWithData(func() (_ string, err error) {
ctx, span := tracer.Start(ctx, "jsonnetsecure.ProcessVM.EvaluateAnonymousSnippet.run")
defer otelx.End(span, &err)

ctx, cancel := context.WithTimeout(ctx, processVMTimeout)
ctx, cancel := context.WithTimeout(ctx, cmp.Or(p.execTimeout, 1*time.Second))
defer cancel()

var (
Expand Down
Loading