fix: wait instruction hanging forever when service_name field is no…

…t passed (#197)

## Description:
Were are moving the validation to the interpretation layer to detect the
missing field before executing the instructions.

fixes #174

Now if the  argument is not specified this error will be thrown:
<img width="1387" alt="image"
src="https://user-images.githubusercontent.com/29951623/225121657-469398f8-d44c-4787-b28c-08866105643c.png">

core/server/api_container/server/startosis_engine/kurtosis_instruction/exec/exec.go

@@ -4,6 +4,7 @@ import (
  "context"
  "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network"
+ "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/builtin_argument"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction"
@@ -12,6 +13,7 @@ import (
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_validator"
  "github.com/kurtosis-tech/stacktrace"
+ "github.com/sirupsen/logrus"
  "go.starlark.net/starlark"
 )
 
@@ -71,19 +73,23 @@ type ExecCapabilities struct {
 }
 
 func (builtin *ExecCapabilities) Interpret(arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) {
- var serviceName service.ServiceName
+ execRecipe, err := builtin_argument.ExtractArgumentValue[*recipe.ExecRecipe](arguments, RecipeArgName)
+ if err != nil {
+ return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", RecipeArgName)
+ }
 
+ var serviceName service.ServiceName
  if arguments.IsSet(ServiceNameArgName) {
  serviceNameArgumentValue, err := builtin_argument.ExtractArgumentValue[starlark.String](arguments, ServiceNameArgName)
  if err != nil {
  return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", ServiceNameArgName)
  }
  serviceName = service.ServiceName(serviceNameArgumentValue.GoString())
- }
-
- execRecipe, err := builtin_argument.ExtractArgumentValue[*recipe.ExecRecipe](arguments, RecipeArgName)
- if err != nil {
- return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", RecipeArgName)
+ } else if execRecipe.GetServiceName() != shared_helpers.EmptyServiceName {
+ serviceName = execRecipe.GetServiceName()
+  logrus.Warnf("The recipe.service_name field will be deprecated soon, users will have to pass the service name value direclty to the 'exec', 'request' and 'wait' instructions")
+ } else {
+ return nil, startosis_errors.NewInterpretationError("Service name is not set, either as an exec instruction's argument or as a recipe field. You can fix it passing the 'service_name' argument in the 'exec' call")
  }
 
  resultUuid, err := builtin.runtimeValueStore.CreateValue()

core/server/api_container/server/startosis_engine/kurtosis_instruction/request/request.go

@@ -4,6 +4,7 @@ import (
  "context"
  "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network"
+ "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/builtin_argument"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction"
@@ -12,6 +13,7 @@ import (
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_validator"
  "github.com/kurtosis-tech/stacktrace"
+ "github.com/sirupsen/logrus"
  "go.starlark.net/starlark"
 )
 
@@ -72,19 +74,23 @@ type RequestCapabilities struct {
 }
 
 func (builtin *RequestCapabilities) Interpret(arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) {
- var serviceName service.ServiceName
+ httpRequestRecipe, err := builtin_argument.ExtractArgumentValue[*recipe.HttpRequestRecipe](arguments, RecipeArgName)
+ if err != nil {
+ return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", RecipeArgName)
+ }
 
+ var serviceName service.ServiceName
  if arguments.IsSet(ServiceNameArgName) {
  serviceNameArgumentValue, err := builtin_argument.ExtractArgumentValue[starlark.String](arguments, ServiceNameArgName)
  if err != nil {
  return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", ServiceNameArgName)
  }
  serviceName = service.ServiceName(serviceNameArgumentValue.GoString())
- }
-
- httpRequestRecipe, err := builtin_argument.ExtractArgumentValue[*recipe.HttpRequestRecipe](arguments, RecipeArgName)
- if err != nil {
- return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", RecipeArgName)
+ } else if httpRequestRecipe.GetServiceName() != shared_helpers.EmptyServiceName {
+ serviceName = httpRequestRecipe.GetServiceName()
+  logrus.Warnf("The recipe.service_name field will be deprecated soon, users will have to pass the service name value direclty to the 'exec', 'request' and 'wait' instructions")
+ } else {
+ return nil, startosis_errors.NewInterpretationError("Service name is not set, either as a request instruction's argument or as a recipe field. You can fix it passing the 'service_name' argument in the 'request' call")
  }
 
  resultUuid, err := builtin.runtimeValueStore.CreateValue()

core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/consts.go

@@ -0,0 +1,7 @@
+package shared_helpers
+
+import "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
+
+const (
+ EmptyServiceName = service.ServiceName("")
+)

core/server/api_container/server/startosis_engine/kurtosis_instruction/wait/wait.go

@@ -7,6 +7,7 @@ import (
  "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/assert"
+ "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/builtin_argument"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction"
@@ -15,6 +16,7 @@ import (
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_errors"
  "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_validator"
  "github.com/kurtosis-tech/stacktrace"
+ "github.com/sirupsen/logrus"
  "go.starlark.net/starlark"
  "time"
 )
@@ -130,16 +132,6 @@ type WaitCapabilities struct {
 }
 
 func (builtin *WaitCapabilities) Interpret(arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) {
- var serviceName service.ServiceName
-
- if arguments.IsSet(ServiceNameArgName) {
- serviceNameArgumentValue, err := builtin_argument.ExtractArgumentValue[starlark.String](arguments, ServiceNameArgName)
- if err != nil {
- return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", ServiceNameArgName)
- }
- serviceName = service.ServiceName(serviceNameArgumentValue.GoString())
- }
-
  var genericRecipe recipe.Recipe
  httpRecipe, err := builtin_argument.ExtractArgumentValue[*recipe.HttpRequestRecipe](arguments, RecipeArgName)
  if err != nil {
@@ -152,6 +144,21 @@ func (builtin *WaitCapabilities) Interpret(arguments *builtin_argument.ArgumentV
  genericRecipe = httpRecipe
  }
 
+ var serviceName service.ServiceName
+
+ if arguments.IsSet(ServiceNameArgName) {
+ serviceNameArgumentValue, err := builtin_argument.ExtractArgumentValue[starlark.String](arguments, ServiceNameArgName)
+ if err != nil {
+ return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", ServiceNameArgName)
+ }
+ serviceName = service.ServiceName(serviceNameArgumentValue.GoString())
+ } else if genericRecipe.GetServiceName() != shared_helpers.EmptyServiceName {
+ serviceName = genericRecipe.GetServiceName()
+ logrus.Warnf("The recipe.service_name field will be deprecated soon, users will have to pass the service name value direclty to the 'exec', 'request' and 'wait' instructions")
+ } else {
+ return nil, startosis_errors.NewInterpretationError("Service name is not set, either as a wait instruction's argument or as a recipe field. You can fix it passing the 'service_name' argument in the 'wait' call")
+ }
+
  valueField, err := builtin_argument.ExtractArgumentValue[starlark.String](arguments, ValueFieldArgName)
  if err != nil {
  return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", ValueFieldArgName)

core/server/api_container/server/startosis_engine/recipe/exec_recipe.go

@@ -113,15 +113,10 @@ func (recipe *ExecRecipe) Execute(
  commandWithRuntimeValue = append(commandWithRuntimeValue, maybeSubCommandWithRuntimeValues)
  }
 
- var serviceNameStr string
- if serviceName != emptyServiceName {
- serviceNameStr = string(serviceName)
- } else if recipe.serviceName != emptyServiceName { //TODO this will be removed when we deprecate the service_name field, more here: https://app.zenhub.com/workspaces/engineering-636cff9fc978ceb2aac05a1d/issues/gh/kurtosis-tech/kurtosis-private/1128
- serviceNameStr = string(recipe.serviceName)
- logrus.Warnf("The exec.service_name field will be deprecated soon, users will have to pass the service name value direclty to the 'exec', 'request' and 'wait' instructions")
- } else {
+ if serviceName == emptyServiceName {
  return nil, stacktrace.NewError("The service name parameter can't be an empty string")
  }
+ serviceNameStr := string(serviceName)
 
  exitCode, commandOutput, err := serviceNetwork.ExecCommand(ctx, serviceNameStr, commandWithRuntimeValue)
  if err != nil {
@@ -168,6 +163,11 @@ func (recipe *ExecRecipe) CreateStarlarkReturnValue(resultUuid string) (*starlar
  return dict, nil
 }
 
+//TODO this will be removed when we deprecate the service_name field, more here: https://app.zenhub.com/workspaces/engineering-636cff9fc978ceb2aac05a1d/issues/gh/kurtosis-tech/kurtosis-private/1128
+func (recipe *ExecRecipe) GetServiceName() service.ServiceName {
+ return recipe.serviceName
+}
+
 func MakeExecRequestRecipe(_ *starlark.Thread, builtin *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
  var serviceNameStr string
  var unpackedCommandList *starlark.List

core/server/api_container/server/startosis_engine/recipe/http_request_recipe.go

@@ -252,15 +252,10 @@ func (recipe *HttpRequestRecipe) Execute(
  return nil, stacktrace.Propagate(err, "An error occurred while replacing runtime values in the body of the http recipe")
  }
 
- var serviceNameStr string
- if serviceName != emptyServiceName {
- serviceNameStr = string(serviceName)
- } else if recipe.serviceName != emptyServiceName { //TODO this will be removed when we deprecate the service_name field, more here: https://app.zenhub.com/workspaces/engineering-636cff9fc978ceb2aac05a1d/issues/gh/kurtosis-tech/kurtosis-private/1128
- serviceNameStr = string(recipe.serviceName)
- logrus.Warnf("The http_request_recipe.service_name field will be deprecated soon, users will have to pass the service name value direclty to the 'exec', 'request' and 'wait' instructions")
- } else {
+ if serviceName == emptyServiceName {
  return nil, stacktrace.NewError("The service name parameter can't be an empty string")
  }
+ serviceNameStr := string(serviceName)
 
  response, err = serviceNetwork.HttpRequestService(
  ctx,
@@ -392,6 +387,11 @@ func (recipe *HttpRequestRecipe) CreateStarlarkReturnValue(resultUuid string) (*
  return dict, nil
 }
 
+//TODO this will be removed when we deprecate the service_name field, more here: https://app.zenhub.com/workspaces/engineering-636cff9fc978ceb2aac05a1d/issues/gh/kurtosis-tech/kurtosis-private/1128
+func (recipe *HttpRequestRecipe) GetServiceName() service.ServiceName {
+ return recipe.serviceName
+}
+
 func convertMapToStarlarkDict(inputMap map[string]string) (*starlark.Dict, *startosis_errors.InterpretationError) {
  sizeOfExtractors := len(inputMap)
  dict := starlark.NewDict(sizeOfExtractors)

core/server/api_container/server/startosis_engine/recipe/recipe.go

@@ -18,4 +18,5 @@ type Recipe interface {
  ) (map[string]starlark.Comparable, error)
  CreateStarlarkReturnValue(resultUuid string) (*starlark.Dict, *startosis_errors.InterpretationError)
  ResultMapToString(resultMap map[string]starlark.Comparable) string
+ GetServiceName() service.ServiceName //TODO this will be removed when we deprecate the service_name field, more here: https://app.zenhub.com/workspaces/engineering-636cff9fc978ceb2aac05a1d/issues/gh/kurtosis-tech/kurtosis-private/1128
 }

core/server/api_container/server/startosis_engine/startosis_interpreter_test.go

@@ -770,7 +770,6 @@ def run(plan):
  require.Len(t, instructions, 2)
 }
 
-// TODO remove this when we deprecate the service_name field in
 func TestStartosisInterpreter_ValidExecRecipeWithoutServiceName(t *testing.T) {
  packageContentProvider := mock_package_content_provider.NewMockPackageContentProvider()
  defer packageContentProvider.RemoveAll()
@@ -782,7 +781,7 @@ def run(plan):
  recipe = ExecRecipe(
  command = ["mkdir", "/tmp/foo"]
  )
- plan.exec(recipe = recipe)
+ plan.exec(recipe = recipe, service_name="my-service")
 `
 
  _, _, interpretationError := interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, script, startosis_constants.EmptyInputArgs)