Skip to content

Commit

Permalink
feat: Return error on SDK if Starlark run on any step (#634)
Browse files Browse the repository at this point in the history
## Description:
Currently Starlark run remote package, run package and run script
blocking calls on the SDK have the following behavior:
1. If an error happens on the APIC side, we return `(*StarlarkRunResult
== nil, error != nil)`
2. If an error happens on the Starlark side, we return
`(*StarlarkRunResult != nil, error == nil)`, with either
`StarlarkRunResult.[InterpretationError, ValidationErrors,
ExecutionError] != nil`
3. If no errors happen on the Starlark side, we return
`(*StarlarkRunResult != nil, error == nil)` with all
`StarlarkRunResult.[InterpretationError, ValidationErrors,
ExecutionError] == nil`

This behavior is not very ergonomic, given that most people are only
interested if the Starlark succeeded or not, they should be able to do
this with a simple `err != nil` check, which is the Go idiomatic way of
dong it.

If the user wants to investigate further the interpreted instructions,
the breakdown of which phase failed, etc, this will still be accessible
via the `StarlarkRunResult`.

## Is this change user facing?
YES
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
ava-labs/subnet-evm#649
  • Loading branch information
victorcolombo committed May 23, 2023
1 parent 46bbee5 commit 8a01cff
Show file tree
Hide file tree
Showing 16 changed files with 49 additions and 44 deletions.
29 changes: 25 additions & 4 deletions api/golang/core/lib/enclaves/enclave_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"google.golang.org/protobuf/types/known/emptypb"
"io"
"path"
"strings"
)

type EnclaveUUID string
Expand Down Expand Up @@ -96,9 +97,10 @@ func (enclaveCtx *EnclaveContext) RunStarlarkScript(ctx context.Context, seriali
func (enclaveCtx *EnclaveContext) RunStarlarkScriptBlocking(ctx context.Context, serializedScript string, serializedParams string, dryRun bool, parallelism int32) (*StarlarkRunResult, error) {
starlarkRunResponseLineChan, _, err := enclaveCtx.RunStarlarkScript(ctx, serializedScript, serializedParams, dryRun, parallelism)
if err != nil {
return nil, stacktrace.Propagate(err, "Error running Starlark Script")
return nil, stacktrace.Propagate(err, "Error requesting Starlark Script run")
}
return ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan), nil
starlarkResponse := ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan)
return starlarkResponse, getErrFromStarlarkRunResult(starlarkResponse)
}

// Docs available at https://docs.kurtosis.com/sdk/#runstarlarkpackagestring-packagerootpath-string-serializedparams-boolean-dryrun---streamstarlarkrunresponseline-responselines-error-error
Expand Down Expand Up @@ -138,7 +140,8 @@ func (enclaveCtx *EnclaveContext) RunStarlarkPackageBlocking(ctx context.Context
if err != nil {
return nil, stacktrace.Propagate(err, "Error running Starlark package")
}
return ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan), nil
starlarkResponse := ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan)
return starlarkResponse, getErrFromStarlarkRunResult(starlarkResponse)
}

// Docs available at https://docs.kurtosis.com/sdk/#runstarlarkremotepackagestring-packageid-string-serializedparams-boolean-dryrun---streamstarlarkrunresponseline-responselines-error-error
Expand Down Expand Up @@ -170,7 +173,8 @@ func (enclaveCtx *EnclaveContext) RunStarlarkRemotePackageBlocking(ctx context.C
if err != nil {
return nil, stacktrace.Propagate(err, "Error running remote Starlark package")
}
return ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan), nil
starlarkResponse := ReadStarlarkRunResponseLineBlocking(starlarkRunResponseLineChan)
return starlarkResponse, getErrFromStarlarkRunResult(starlarkResponse)
}

// Docs available at https://docs.kurtosis.com/sdk#getservicecontextstring-serviceidentifier---servicecontext-servicecontext
Expand Down Expand Up @@ -373,6 +377,23 @@ func runReceiveStarlarkResponseLineRoutine(cancelCtxFunc context.CancelFunc, str
}
}

func getErrFromStarlarkRunResult(result *StarlarkRunResult) error {
if result.InterpretationError != nil {
return stacktrace.NewError(result.InterpretationError.GetErrorMessage())
}
if len(result.ValidationErrors) > 0 {
errorMessages := []string{}
for _, validationErr := range result.ValidationErrors {
errorMessages = append(errorMessages, validationErr.GetErrorMessage())
}
return stacktrace.NewError("Found %v validation errors: %v", len(result.ValidationErrors), strings.Join(errorMessages, "\n"))
}
if result.ExecutionError != nil {
return stacktrace.NewError(result.ExecutionError.GetErrorMessage())
}
return nil
}

func (enclaveCtx *EnclaveContext) assembleRunStartosisPackageArg(packageRootPath string, serializedParams string, dryRun bool, parallelism int32) (*kurtosis_core_rpc_api_bindings.RunStarlarkPackageArgs, error) {
kurtosisYamlFilepath := path.Join(packageRootPath, kurtosisYamlFilename)

Expand Down
7 changes: 2 additions & 5 deletions internal_testsuites/golang/test_helpers/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func RunScriptWithDefaultConfig(ctx context.Context, enclaveCtx *enclaves.Enclav
return enclaveCtx.RunStarlarkScriptBlocking(ctx, script, emptyParams, defaultDryRun, defaultParallelism)
}

func SetupSimpleEnclaveAndRunScript(t *testing.T, ctx context.Context, testName string, script string) *enclaves.StarlarkRunResult {
func SetupSimpleEnclaveAndRunScript(t *testing.T, ctx context.Context, testName string, script string) (*enclaves.StarlarkRunResult, error) {

// ------------------------------------- ENGINE SETUP ----------------------------------------------
enclaveCtx, _, destroyEnclaveFunc, err := CreateEnclave(t, ctx, testName, partitioningDisabled)
Expand All @@ -328,10 +328,7 @@ func SetupSimpleEnclaveAndRunScript(t *testing.T, ctx context.Context, testName
logrus.Infof("Executing Startosis script...")
logrus.Debugf("Startosis script content: \n%v", script)

runResult, err := RunScriptWithDefaultConfig(ctx, enclaveCtx, script)
require.NoError(t, err, "Unexpected error executing startosis script")

return runResult
return RunScriptWithDefaultConfig(ctx, enclaveCtx, script)
}

func WaitForHealthy(ctx context.Context, client GrpcAvailabilityChecker, retries uint32, retriesDelayMilliseconds uint32) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ func TestAddServiceWithInvalidServiceNameFailsValidation(t *testing.T) {
logrus.Infof("Executing Starlark script...")
logrus.Debugf("Starlark script contents: \n%v", fmt.Sprintf(addServiceInvalidServiceNameTestScript, invalidServiceName))

runResult, err := test_helpers.RunScriptWithDefaultConfig(ctx, enclaveCtx, fmt.Sprintf(addServiceInvalidServiceNameTestScript, invalidServiceName))
require.NoError(t, err, "Unexpected error executing Starlark script")
runResult, _ := test_helpers.RunScriptWithDefaultConfig(ctx, enclaveCtx, fmt.Sprintf(addServiceInvalidServiceNameTestScript, invalidServiceName))

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error.")
require.NotEmpty(t, runResult.ValidationErrors, "Expected some validation errors")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ func TestStartosis_AddServiceWithReadyConditionsCheck(t *testing.T) {

script := fmt.Sprintf(addServiceWithReadyConditionsScript, okStatusCode)

runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServiceWithReadyConditionsTestName, script)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
require.Empty(t, runResult.ExecutionError, "Unexpected execution error")
_, err := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServiceWithReadyConditionsTestName, script)
require.Nil(t, err)
}

func TestStartosis_AddServiceWithReadyConditionsCheckFail(t *testing.T) {
Expand All @@ -61,7 +58,7 @@ func TestStartosis_AddServiceWithReadyConditionsCheckFail(t *testing.T) {

script := fmt.Sprintf(addServiceWithReadyConditionsScript, serverErrorStatusCode)

runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServiceWithReadyConditionsTestName, script)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServiceWithReadyConditionsTestName, script)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,9 @@ func TestStartosis_AddServicesWithReadyConditionsCheck(t *testing.T) {

script := fmt.Sprintf(addServicesWithReadyConditionsScript, okStatusCode)

runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServicesWithReadyConditionsTestName, script)
_, err := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServicesWithReadyConditionsTestName, script)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
require.Empty(t, runResult.ExecutionError, "Unexpected execution error")
require.Nil(t, err)
}

func TestStartosis_AddServicesWithReadyConditionsCheckFail(t *testing.T) {
Expand All @@ -68,7 +66,7 @@ func TestStartosis_AddServicesWithReadyConditionsCheckFail(t *testing.T) {

script := fmt.Sprintf(addServicesWithReadyConditionsScript, serverErrorStatusCode)

runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServicesWithReadyConditionsTestName, script)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, addServicesWithReadyConditionsTestName, script)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ func TestStartosisPackage_NoMainFile(t *testing.T) {

expectedErrorContents := `An error occurred while verifying that 'main.star' exists in the package 'github.com/sample/sample-kurtosis-package' at '/kurtosis-data/startosis-packages/sample/sample-kurtosis-package/main.star'
Caused by: stat /kurtosis-data/startosis-packages/sample/sample-kurtosis-package/main.star: no such file or directory`
runResult, err := enclaveCtx.RunStarlarkPackageBlocking(ctx, packageDirpath, emptyRunParams, defaultDryRun, defaultParallelism)
require.Nil(t, err, "Unexpected error executing package")
runResult, _ := enclaveCtx.RunStarlarkPackageBlocking(ctx, packageDirpath, emptyRunParams, defaultDryRun, defaultParallelism)
require.NotNil(t, runResult.InterpretationError)
require.Equal(t, runResult.InterpretationError.GetErrorMessage(), expectedErrorContents)
require.Empty(t, runResult.ValidationErrors)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func TestStartosisPackage_NoMainInMainStar(t *testing.T) {
logrus.Infof("Starlark package path: \n%v", packageDirpath)

expectedInterpretationErr := "No 'run' function found in file 'github.com/sample/sample-kurtosis-package/main.star'; a 'run' entrypoint function with the signature `run(args)` or `run()` is required in the main.star file of any Kurtosis package"
runResult, err := enclaveCtx.RunStarlarkPackageBlocking(ctx, packageDirpath, emptyRunParams, defaultDryRun, defaultParallelism)
require.Nil(t, err, "Unexpected error executing Starlark package")
runResult, _ := enclaveCtx.RunStarlarkPackageBlocking(ctx, packageDirpath, emptyRunParams, defaultDryRun, defaultParallelism)
require.NotNil(t, runResult.InterpretationError)
require.Contains(t, runResult.InterpretationError.GetErrorMessage(), expectedInterpretationErr)
require.Empty(t, runResult.ValidationErrors)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ func TestStartosisPackage_ValidPackageWithInput_MissingKeyInParams(t *testing.T)
logrus.Infof("Startosis module path: \n%v", moduleDirpath)

params := `{"hello": "world"}` // expecting key 'greetings' here
runResult, err := enclaveCtx.RunStarlarkPackageBlocking(ctx, moduleDirpath, params, defaultDryRun, defaultParallelism)
require.NoError(t, err, "Unexpected error executing startosis module")
runResult, _ := enclaveCtx.RunStarlarkPackageBlocking(ctx, moduleDirpath, params, defaultDryRun, defaultParallelism)

require.NotNil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Contains(t, runResult.InterpretationError.GetErrorMessage(), "Evaluation error: key \"greetings\" not in dict")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,14 @@ def run(plan):

func TestStartosis_AssertSuccessPortChecks(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertSuccessTestName, assertSuccessScript)
_, err := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertSuccessTestName, assertSuccessScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
require.Empty(t, runResult.ExecutionError, "Unexpected execution error")
require.Nil(t, err)
}

func TestStartosis_AssertFailBecausePortIsNotOpen(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName, assertFailScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName, assertFailScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand All @@ -69,7 +67,7 @@ func TestStartosis_AssertFailBecausePortIsNotOpen(t *testing.T) {

func TestStartosis_AssertFailBecauseEmptyStringIsNotValid(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName2, assertFailScript2)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName2, assertFailScript2)

require.NotNil(t, runResult.InterpretationError, "Expected interpretation error coming from wait validation")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(plan):

func TestStarlark_InvalidPortIdRequest(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, requestInvalidPortIDTest, requestInvalidPortIDFailScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, requestInvalidPortIDTest, requestInvalidPortIDFailScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.NotEmpty(t, runResult.ValidationErrors, "Expected validation errors")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(plan):

func TestStarlark_InvalidPortIdWait(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, waitInvalidPortIDTest, waitInvalidPortIDFailScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, waitInvalidPortIDTest, waitInvalidPortIDFailScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.NotEmpty(t, runResult.ValidationErrors, "Expected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(plan):

func TestStarlark_InvalidServiceRequest(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, requestInvalidServiceName, requestInvalidServiceNameScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, requestInvalidServiceName, requestInvalidServiceNameScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.NotEmpty(t, runResult.ValidationErrors, "Expected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(plan):

func TestStarlark_InvalidServiceWait(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, waitInvalidServiceTest, waitInvalidServiceTestScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, waitInvalidServiceTest, waitInvalidServiceTestScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.NotEmpty(t, runResult.ValidationErrors, "Expected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def run(plan):

func TestStartosis_AssertFail(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName, assertFailScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, assertFailTestName, assertFailScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,8 @@ def run(plan):

func TestStartosis_ComplexRequestWaitAssert(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, complexRequestWaitAssertTestName, complexRequestWaitAssertStartosisScript)
_, err := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, complexRequestWaitAssertTestName, complexRequestWaitAssertStartosisScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
require.Nil(t, runResult.ExecutionError, "Unexpected execution error")
require.Nil(t, err)
logrus.Infof("Successfully ran Startosis script")
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(plan):

func TestStartosis_TimeoutWait(t *testing.T) {
ctx := context.Background()
runResult := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, timeoutWaitTestName, timeoutWaitStartosisScript)
runResult, _ := test_helpers.SetupSimpleEnclaveAndRunScript(t, ctx, timeoutWaitTestName, timeoutWaitStartosisScript)

require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error")
require.Empty(t, runResult.ValidationErrors, "Unexpected validation error")
Expand Down

0 comments on commit 8a01cff

Please sign in to comment.