From f2cd554a8775944c2668d97be127d37e16754554 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Wed, 10 Apr 2024 23:06:02 -0400 Subject: [PATCH 01/23] impl update service --- .../objects/service/service_config.go | 8 + .../interpretation_time_value_store.go | 36 +++- .../startosis_engine/kurtosis_builtins.go | 2 + .../add_service/add_service.go | 18 +- .../update_service/update_service.go | 180 ++++++++++++++++++ .../startosis_interpreter_test.go | 31 +++ 6 files changed, 268 insertions(+), 7 deletions(-) create mode 100644 core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go diff --git a/container-engine-lib/lib/backend_interface/objects/service/service_config.go b/container-engine-lib/lib/backend_interface/objects/service/service_config.go index f3cb33d100..f5b50e3def 100644 --- a/container-engine-lib/lib/backend_interface/objects/service/service_config.go +++ b/container-engine-lib/lib/backend_interface/objects/service/service_config.go @@ -133,10 +133,18 @@ func (serviceConfig *ServiceConfig) GetContainerImageName() string { return serviceConfig.privateServiceConfig.ContainerImageName } +func (serviceConfig *ServiceConfig) SetContainerImageName(containerImage string) { + serviceConfig.privateServiceConfig.ContainerImageName = containerImage +} + func (serviceConfig *ServiceConfig) GetImageBuildSpec() *image_build_spec.ImageBuildSpec { return serviceConfig.privateServiceConfig.ImageBuildSpec } +func (serviceConfig *ServiceConfig) SetImageBuildSpec(imageBuildSpec *image_build_spec.ImageBuildSpec) { + serviceConfig.privateServiceConfig.ImageBuildSpec = imageBuildSpec +} + func (serviceConfig *ServiceConfig) GetImageRegistrySpec() *image_registry_spec.ImageRegistrySpec { return serviceConfig.privateServiceConfig.ImagerRegistrySpec } diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go index af89cd79cc..a8a178b572 100644 --- a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go @@ -5,11 +5,13 @@ import ( "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types" "github.com/kurtosis-tech/stacktrace" + "github.com/sirupsen/logrus" ) type InterpretationTimeValueStore struct { - serviceValues *serviceInterpretationValueRepository - serde *kurtosis_types.StarlarkValueSerde + serviceConfigValues map[service.ServiceName]*service.ServiceConfig + serviceValues *serviceInterpretationValueRepository + serde *kurtosis_types.StarlarkValueSerde } func CreateInterpretationTimeValueStore(enclaveDb *enclave_db.EnclaveDB, serde *kurtosis_types.StarlarkValueSerde) (*InterpretationTimeValueStore, error) { @@ -17,7 +19,10 @@ func CreateInterpretationTimeValueStore(enclaveDb *enclave_db.EnclaveDB, serde * if err != nil { return nil, stacktrace.Propagate(err, "An error occurred creating interpretation time value store") } - return &InterpretationTimeValueStore{serviceValues: serviceValuesRepository, serde: serde}, nil + return &InterpretationTimeValueStore{ + serviceConfigValues: map[service.ServiceName]*service.ServiceConfig{}, + serviceValues: serviceValuesRepository, + serde: serde}, nil } func (itvs *InterpretationTimeValueStore) PutService(name service.ServiceName, service *kurtosis_types.Service) error { @@ -34,3 +39,28 @@ func (itvs *InterpretationTimeValueStore) GetService(name service.ServiceName) ( } return serviceStarlark, nil } + +func (itvs *InterpretationTimeValueStore) PutServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) error { + if currServiceConfig, ok := itvs.serviceConfigValues[name]; !ok { + itvs.serviceConfigValues[name] = serviceConfig + } else { + if serviceConfig.GetImageBuildSpec() != nil { + currServiceConfig.SetImageBuildSpec(serviceConfig.GetImageBuildSpec()) + logrus.Infof("Updating image build spec of service config for '%v'", name) + } + if serviceConfig.GetContainerImageName() != "" { + currServiceConfig.SetContainerImageName(serviceConfig.GetContainerImageName()) + logrus.Infof("Updating container image to use for service config for '%v'", name) + } + itvs.serviceConfigValues[name] = currServiceConfig + } + return nil +} + +func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { + serviceConfig, ok := itvs.serviceConfigValues[name] + if !ok { + return nil, stacktrace.NewError("Did not find service config for '%v' in interpretation time value store.", name) + } + return serviceConfig, nil +} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go index 4224f5e5bc..ba547ea219 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go @@ -20,6 +20,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/stop_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/store_service_files" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/tasks" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/upload_files" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/verify" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/wait" @@ -71,6 +72,7 @@ func KurtosisPlanInstructions( add_service.NewAddService(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), add_service.NewAddServices(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), get_service.NewGetService(interpretationTimeValueStore), + update_service.NewUpdateService(serviceNetwork, interpretationTimeValueStore, packageId, packageContentProvider, packageReplaceOptions, imageDownloadMode), get_files_artifact.NewGetFilesArtifact(), verify.NewVerify(runtimeValueStore), exec.NewExec(serviceNetwork, runtimeValueStore), diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 7db6f39962..bcca118546 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -140,7 +140,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfig, readyCondition, interpretationErr := validateAndConvertConfigAndReadyCondition( + apiServiceConfig, readyCondition, interpretationErr := ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfig, locatorOfModuleInWhichThisBuiltInIsBeingCalled, @@ -154,7 +154,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt } builtin.serviceName = service.ServiceName(serviceName.GoString()) - builtin.serviceConfig = apiServiceConfig + builtin.serviceConfig = apiServiceConfig // store this in the interpretation time value store builtin.readyCondition = readyCondition builtin.resultUuid, err = builtin.runtimeValueStore.GetOrCreateValueAssociatedWithService(builtin.serviceName) if err != nil { @@ -172,6 +172,11 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt if err != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while persisting return value for service '%v'", serviceName) } + err = builtin.interpretationTimeValueStore.PutServiceConfig(builtin.serviceName, builtin.serviceConfig) + if err != nil { + return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while persisting return value for service '%v'", serviceName) + } + return builtin.returnValue, nil } @@ -183,7 +188,12 @@ func (builtin *AddServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu } func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { - replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) + // pull service config from interpretation time value store + serviceConfig, err := builtin.interpretationTimeValueStore.GetServiceConfig(builtin.serviceName) + if err != nil { + return "", stacktrace.Propagate(err, "An error occurred getting service config value from interpretation time value store for service '%v'", builtin.serviceName) + } + replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, serviceConfig) if err != nil { return "", stacktrace.Propagate(err, "An error occurred replace a magic string in '%s' instruction arguments for service '%s'. Execution cannot proceed", AddServiceBuiltinName, builtin.serviceName) } @@ -308,7 +318,7 @@ func (builtin *AddServiceCapabilities) Description() string { return builtin.description } -func validateAndConvertConfigAndReadyCondition( +func ValidateAndConvertConfigAndReadyCondition( serviceNetwork service_network.ServiceNetwork, rawConfig starlark.Value, locatorOfModuleInWhichThisBuiltInIsBeingCalled string, diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go new file mode 100644 index 0000000000..a85f1119e5 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go @@ -0,0 +1,180 @@ +package update_service + +import ( + "context" + "fmt" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" + "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/enclave_plan_persistence" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service" + "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" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_type_constructor" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types/service_config" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/plan_yaml" + "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_packages" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_validator" + "go.starlark.net/starlark" + "reflect" +) + +const ( + UpdateServiceBuiltinName = "update_service" + ServiceNameArgName = "name" + UpdateServiceConfigArgName = "config" + + descriptionFormatStr = "Updating config of service '%v'" +) + +func NewUpdateService( + serviceNetwork service_network.ServiceNetwork, + interpretationTimeStore *interpretation_time_value_store.InterpretationTimeValueStore, + packageId string, + packageContentProvider startosis_packages.PackageContentProvider, + packageReplaceOptions map[string]string, + imageDownloadMode image_download_mode.ImageDownloadMode, +) *kurtosis_plan_instruction.KurtosisPlanInstruction { + return &kurtosis_plan_instruction.KurtosisPlanInstruction{ + KurtosisBaseBuiltin: &kurtosis_starlark_framework.KurtosisBaseBuiltin{ + Name: UpdateServiceBuiltinName, + Arguments: []*builtin_argument.BuiltinArgument{ + { + Name: ServiceNameArgName, + IsOptional: false, + ZeroValueProvider: builtin_argument.ZeroValueProvider[starlark.String], + Validator: func(value starlark.Value) *startosis_errors.InterpretationError { + return builtin_argument.NonEmptyString(value, ServiceNameArgName) + }, + }, + { + Name: UpdateServiceConfigArgName, + IsOptional: false, + ZeroValueProvider: builtin_argument.ZeroValueProvider[*service_config.ServiceConfig], + Validator: func(value starlark.Value) *startosis_errors.InterpretationError { + // we just try to convert the configs here to validate their shape, to avoid code duplication with Interpret + _, ok := value.(*service_config.ServiceConfig) + if !ok { + return startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", UpdateServiceConfigArgName, reflect.TypeOf(value)) + } + return nil + }, + }, + }, + Deprecation: nil, + }, + Capabilities: func() kurtosis_plan_instruction.KurtosisPlanInstructionCapabilities { + return &UpdateServiceCapabilities{ + interpretationTimeStore: interpretationTimeStore, + serviceNetwork: serviceNetwork, + serviceName: "", // populated at interpretation time + serviceConfig: nil, // populated at interpretation time + packageId: packageId, + packageContentProvider: packageContentProvider, + packageReplaceOptions: packageReplaceOptions, + description: "", // populated at interpretation time + imageDownloadMode: imageDownloadMode, + } + }, + DefaultDisplayArguments: map[string]bool{ + ServiceNameArgName: true, + }, + } +} + +type UpdateServiceCapabilities struct { + interpretationTimeStore *interpretation_time_value_store.InterpretationTimeValueStore + serviceName service.ServiceName + serviceConfig *service.ServiceConfig + + serviceNetwork service_network.ServiceNetwork + + // These params are needed to successfully convert service config if an ImageBuildSpec was provided + packageId string + packageContentProvider startosis_packages.PackageContentProvider + packageReplaceOptions map[string]string + imageVal starlark.Value + + imageDownloadMode image_download_mode.ImageDownloadMode + description string +} + +func (builtin *UpdateServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled string, arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) { + 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()) + + builtin.serviceName = serviceName + + updatedServiceConfig, err := builtin_argument.ExtractArgumentValue[*service_config.ServiceConfig](arguments, UpdateServiceConfigArgName) + if err != nil { + return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", UpdateServiceConfigArgName) + } + rawImageVal, found, interpretationErr := kurtosis_type_constructor.ExtractAttrValue[starlark.Value](updatedServiceConfig.KurtosisValueTypeDefault, service_config.ImageAttr) + if interpretationErr != nil { + return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract raw image attribute.") + } + if !found { + return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") + } + builtin.imageVal = rawImageVal + updatedApiServiceConfig, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( + builtin.serviceNetwork, + updatedServiceConfig, + locatorOfModuleInWhichThisBuiltInIsBeingCalled, + builtin.packageId, + builtin.packageContentProvider, + builtin.packageReplaceOptions, + builtin.imageDownloadMode, + ) + if interpretationErr != nil { + return nil, interpretationErr + } + + // implement a Put Service Config that overwrites whatever service config existed for that service + err = builtin.interpretationTimeStore.PutServiceConfig(serviceName, updatedApiServiceConfig) + if err != nil { + return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while fetching service '%v' from the store", serviceName) + } + + builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(descriptionFormatStr, builtin.serviceName)) + return starlark.None, nil +} + +func (builtin *UpdateServiceCapabilities) Validate(_ *builtin_argument.ArgumentValuesSet, validatorEnvironment *startosis_validator.ValidatorEnvironment) *startosis_errors.ValidationError { + if exists := validatorEnvironment.DoesServiceNameExist(builtin.serviceName); exists == startosis_validator.ComponentNotFound { + return startosis_errors.NewValidationError("Service '%v' required by '%v' instruction doesn't exist", builtin.serviceName, UpdateServiceBuiltinName) + } + return nil +} + +func (builtin *UpdateServiceCapabilities) Execute(_ context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { + // Note this is a no-op. + return fmt.Sprintf("Fetched service '%v'", builtin.serviceName), nil +} + +func (builtin *UpdateServiceCapabilities) TryResolveWith(instructionsAreEqual bool, _ *enclave_plan_persistence.EnclavePlanInstruction, enclaveComponents *enclave_structure.EnclaveComponents) enclave_structure.InstructionResolutionStatus { + if instructionsAreEqual { + return enclave_structure.InstructionIsEqual + } + return enclave_structure.InstructionIsUnknown +} + +func (builtin *UpdateServiceCapabilities) FillPersistableAttributes(builder *enclave_plan_persistence.EnclavePlanInstructionBuilder) { + builder.SetType(UpdateServiceBuiltinName).AddServiceName(builtin.serviceName) +} + +func (builtin *UpdateServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) error { + // update service does not affect the plan + return nil +} + +func (builtin *UpdateServiceCapabilities) Description() string { + return builtin.description +} diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go index 7e2d1de37e..aabcbb2fc2 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go @@ -396,6 +396,37 @@ def run(plan): require.Nil(suite.T(), instructionsPlan) } +func (suite *StartosisInterpreterTestSuite) TestInterpreter() { + script := ` +def run(plan): + config = ServiceConfig( + image = "someContainer", + ports = { + "grpc": PortSpec(number=1234, transport_protocol = "TCP") # port number should be an int + } + ) + plan.add_service(name = "tedi", config = config) + updatedConfig = ServiceConfig( + image = "someNewContainer", + ) + plan.update_service(name="tedi", config=updatedConfig) +` + + _, instructionsPlan, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) + require.Nil(suite.T(), interpretationError) + require.Equal(suite.T(), 2, instructionsPlan.Size()) + //expectedError := startosis_errors.NewInterpretationErrorWithCauseAndCustomMsg( + // startosis_errors.NewInterpretationError(`The following argument(s) could not be parsed or did not pass validation: {"number":"Value for 'number' was expected to be an integer between 1 and 65535, but it was 'starlark.String'"}`), + // []startosis_errors.CallFrame{ + // *startosis_errors.NewCallFrame("run", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 11, 20)), + // *startosis_errors.NewCallFrame("PortSpec", startosis_errors.NewScriptPosition("", 0, 0)), + // }, + // "Evaluation error: Cannot construct 'PortSpec' from the provided arguments.", + //).ToAPIType() + //require.Equal(suite.T(), expectedError, interpretationError) + //require.Nil(suite.T(), instructionsPlan) +} + func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_ValidSimpleScriptWithInstructionPortNumberAsString() { script := ` def run(plan): From d3601d71a9502394fad992f75677f776301e5874 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 11 Apr 2024 09:49:06 -0400 Subject: [PATCH 02/23] lint --- .../kurtosis_instruction/update_service/update_service.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go index a85f1119e5..d005e6d2e1 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go @@ -73,6 +73,7 @@ func NewUpdateService( serviceNetwork: serviceNetwork, serviceName: "", // populated at interpretation time serviceConfig: nil, // populated at interpretation time + imageVal: nil, // populated at interpretation time packageId: packageId, packageContentProvider: packageContentProvider, packageReplaceOptions: packageReplaceOptions, From 7abfcf98f70e7383e90165e17775b11d2795eefa Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Mon, 22 Apr 2024 13:52:52 -0400 Subject: [PATCH 03/23] rename to set service --- .../startosis_engine/kurtosis_builtins.go | 4 +- .../set_service.go} | 30 +++++----- .../startosis_interpreter_test.go | 60 +++++++++---------- 3 files changed, 47 insertions(+), 47 deletions(-) rename core/server/api_container/server/startosis_engine/kurtosis_instruction/{update_service/update_service.go => set_service/set_service.go} (84%) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go index ba547ea219..2e0308b5c6 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go @@ -16,11 +16,11 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/remove_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/render_templates" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/request" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/start_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/stop_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/store_service_files" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/tasks" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/upload_files" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/verify" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/wait" @@ -72,7 +72,7 @@ func KurtosisPlanInstructions( add_service.NewAddService(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), add_service.NewAddServices(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), get_service.NewGetService(interpretationTimeValueStore), - update_service.NewUpdateService(serviceNetwork, interpretationTimeValueStore, packageId, packageContentProvider, packageReplaceOptions, imageDownloadMode), + set_service.NewSetService(serviceNetwork, interpretationTimeValueStore, packageId, packageContentProvider, packageReplaceOptions, imageDownloadMode), get_files_artifact.NewGetFilesArtifact(), verify.NewVerify(runtimeValueStore), exec.NewExec(serviceNetwork, runtimeValueStore), diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go similarity index 84% rename from core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go rename to core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index d005e6d2e1..793b387a4b 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/update_service/update_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -1,4 +1,4 @@ -package update_service +package set_service import ( "context" @@ -24,14 +24,14 @@ import ( ) const ( - UpdateServiceBuiltinName = "update_service" + SetServiceBuiltinName = "set_service" ServiceNameArgName = "name" UpdateServiceConfigArgName = "config" descriptionFormatStr = "Updating config of service '%v'" ) -func NewUpdateService( +func NewSetService( serviceNetwork service_network.ServiceNetwork, interpretationTimeStore *interpretation_time_value_store.InterpretationTimeValueStore, packageId string, @@ -41,7 +41,7 @@ func NewUpdateService( ) *kurtosis_plan_instruction.KurtosisPlanInstruction { return &kurtosis_plan_instruction.KurtosisPlanInstruction{ KurtosisBaseBuiltin: &kurtosis_starlark_framework.KurtosisBaseBuiltin{ - Name: UpdateServiceBuiltinName, + Name: SetServiceBuiltinName, Arguments: []*builtin_argument.BuiltinArgument{ { Name: ServiceNameArgName, @@ -68,7 +68,7 @@ func NewUpdateService( Deprecation: nil, }, Capabilities: func() kurtosis_plan_instruction.KurtosisPlanInstructionCapabilities { - return &UpdateServiceCapabilities{ + return &SetServiceCapabilities{ interpretationTimeStore: interpretationTimeStore, serviceNetwork: serviceNetwork, serviceName: "", // populated at interpretation time @@ -87,7 +87,7 @@ func NewUpdateService( } } -type UpdateServiceCapabilities struct { +type SetServiceCapabilities struct { interpretationTimeStore *interpretation_time_value_store.InterpretationTimeValueStore serviceName service.ServiceName serviceConfig *service.ServiceConfig @@ -104,7 +104,7 @@ type UpdateServiceCapabilities struct { description string } -func (builtin *UpdateServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled string, arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) { +func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuiltInIsBeingCalled string, arguments *builtin_argument.ArgumentValuesSet) (starlark.Value, *startosis_errors.InterpretationError) { 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) @@ -148,34 +148,34 @@ func (builtin *UpdateServiceCapabilities) Interpret(locatorOfModuleInWhichThisBu return starlark.None, nil } -func (builtin *UpdateServiceCapabilities) Validate(_ *builtin_argument.ArgumentValuesSet, validatorEnvironment *startosis_validator.ValidatorEnvironment) *startosis_errors.ValidationError { +func (builtin *SetServiceCapabilities) Validate(_ *builtin_argument.ArgumentValuesSet, validatorEnvironment *startosis_validator.ValidatorEnvironment) *startosis_errors.ValidationError { if exists := validatorEnvironment.DoesServiceNameExist(builtin.serviceName); exists == startosis_validator.ComponentNotFound { - return startosis_errors.NewValidationError("Service '%v' required by '%v' instruction doesn't exist", builtin.serviceName, UpdateServiceBuiltinName) + return startosis_errors.NewValidationError("Service '%v' required by '%v' instruction doesn't exist", builtin.serviceName, SetServiceBuiltinName) } return nil } -func (builtin *UpdateServiceCapabilities) Execute(_ context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { +func (builtin *SetServiceCapabilities) Execute(_ context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { // Note this is a no-op. return fmt.Sprintf("Fetched service '%v'", builtin.serviceName), nil } -func (builtin *UpdateServiceCapabilities) TryResolveWith(instructionsAreEqual bool, _ *enclave_plan_persistence.EnclavePlanInstruction, enclaveComponents *enclave_structure.EnclaveComponents) enclave_structure.InstructionResolutionStatus { +func (builtin *SetServiceCapabilities) TryResolveWith(instructionsAreEqual bool, _ *enclave_plan_persistence.EnclavePlanInstruction, enclaveComponents *enclave_structure.EnclaveComponents) enclave_structure.InstructionResolutionStatus { if instructionsAreEqual { return enclave_structure.InstructionIsEqual } return enclave_structure.InstructionIsUnknown } -func (builtin *UpdateServiceCapabilities) FillPersistableAttributes(builder *enclave_plan_persistence.EnclavePlanInstructionBuilder) { - builder.SetType(UpdateServiceBuiltinName).AddServiceName(builtin.serviceName) +func (builtin *SetServiceCapabilities) FillPersistableAttributes(builder *enclave_plan_persistence.EnclavePlanInstructionBuilder) { + builder.SetType(SetServiceBuiltinName).AddServiceName(builtin.serviceName) } -func (builtin *UpdateServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) error { +func (builtin *SetServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) error { // update service does not affect the plan return nil } -func (builtin *UpdateServiceCapabilities) Description() string { +func (builtin *SetServiceCapabilities) Description() string { return builtin.description } diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go index aabcbb2fc2..a2eceac26f 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go @@ -396,36 +396,36 @@ def run(plan): require.Nil(suite.T(), instructionsPlan) } -func (suite *StartosisInterpreterTestSuite) TestInterpreter() { - script := ` -def run(plan): - config = ServiceConfig( - image = "someContainer", - ports = { - "grpc": PortSpec(number=1234, transport_protocol = "TCP") # port number should be an int - } - ) - plan.add_service(name = "tedi", config = config) - updatedConfig = ServiceConfig( - image = "someNewContainer", - ) - plan.update_service(name="tedi", config=updatedConfig) -` - - _, instructionsPlan, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) - require.Nil(suite.T(), interpretationError) - require.Equal(suite.T(), 2, instructionsPlan.Size()) - //expectedError := startosis_errors.NewInterpretationErrorWithCauseAndCustomMsg( - // startosis_errors.NewInterpretationError(`The following argument(s) could not be parsed or did not pass validation: {"number":"Value for 'number' was expected to be an integer between 1 and 65535, but it was 'starlark.String'"}`), - // []startosis_errors.CallFrame{ - // *startosis_errors.NewCallFrame("run", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 11, 20)), - // *startosis_errors.NewCallFrame("PortSpec", startosis_errors.NewScriptPosition("", 0, 0)), - // }, - // "Evaluation error: Cannot construct 'PortSpec' from the provided arguments.", - //).ToAPIType() - //require.Equal(suite.T(), expectedError, interpretationError) - //require.Nil(suite.T(), instructionsPlan) -} +//func (suite *StartosisInterpreterTestSuite) TestInterpreter() { +// script := ` +//def run(plan): +// config = ServiceConfig( +// image = "someContainer", +// ports = { +// "grpc": PortSpec(number=1234, transport_protocol = "TCP") # port number should be an int +// } +// ) +// plan.add_service(name = "tedi", config = config) +// updatedConfig = ServiceConfig( +// image = "someNewContainer", +// ) +// plan.update_service(name="tedi", config=updatedConfig) +//` +// +// _, instructionsPlan, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) +// require.Nil(suite.T(), interpretationError) +// require.Equal(suite.T(), 2, instructionsPlan.Size()) +// //expectedError := startosis_errors.NewInterpretationErrorWithCauseAndCustomMsg( +// // startosis_errors.NewInterpretationError(`The following argument(s) could not be parsed or did not pass validation: {"number":"Value for 'number' was expected to be an integer between 1 and 65535, but it was 'starlark.String'"}`), +// // []startosis_errors.CallFrame{ +// // *startosis_errors.NewCallFrame("run", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 11, 20)), +// // *startosis_errors.NewCallFrame("PortSpec", startosis_errors.NewScriptPosition("", 0, 0)), +// // }, +// // "Evaluation error: Cannot construct 'PortSpec' from the provided arguments.", +// //).ToAPIType() +// //require.Equal(suite.T(), expectedError, interpretationError) +// //require.Nil(suite.T(), instructionsPlan) +//} func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_ValidSimpleScriptWithInstructionPortNumberAsString() { script := ` From 426987fbceca6b1117fe443b61bd856268fbbac3 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Mon, 22 Apr 2024 16:49:26 -0400 Subject: [PATCH 04/23] override image values --- .../objects/service/service_config.go | 19 +++++-- .../interpretation_time_value_store.go | 18 +----- .../add_service/add_service.go | 8 +-- .../set_service/set_service.go | 40 +++++++++---- .../test_engine/set_service_framework_test.go | 56 +++++++++++++++++++ 5 files changed, 103 insertions(+), 38 deletions(-) create mode 100644 core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go diff --git a/container-engine-lib/lib/backend_interface/objects/service/service_config.go b/container-engine-lib/lib/backend_interface/objects/service/service_config.go index f5b50e3def..0aef1da1ca 100644 --- a/container-engine-lib/lib/backend_interface/objects/service/service_config.go +++ b/container-engine-lib/lib/backend_interface/objects/service/service_config.go @@ -29,9 +29,12 @@ type privateServiceConfig struct { // Configuration for container engine to pull an in a private registry behind authentication // If nil, we will use the ContainerImageName and not use any auth - // Mutually exclusive from ImageBuildSpec, ContainerImageName - ImagerRegistrySpec *image_registry_spec.ImageRegistrySpec + // Mutually exclusive from ImageBuildSpec, ContainerImageName, NixBuildSpec + ImageRegistrySpec *image_registry_spec.ImageRegistrySpec + // Configuration for container engine to using Nix + // If nil, we will use the ContainerImageName and not use any Nix + // Mutually exclusive from ImageBuildSpec, ContainerImageName, ImageRegistrySpec NixBuildSpec *nix_build_spec.NixBuildSpec PrivatePorts map[string]*port_spec.PortSpec @@ -104,7 +107,7 @@ func CreateServiceConfig( internalServiceConfig := &privateServiceConfig{ ContainerImageName: containerImageName, ImageBuildSpec: imageBuildSpec, - ImagerRegistrySpec: imageRegistrySpec, + ImageRegistrySpec: imageRegistrySpec, NixBuildSpec: nixBuildSpec, PrivatePorts: privatePorts, PublicPorts: publicPorts, @@ -146,13 +149,21 @@ func (serviceConfig *ServiceConfig) SetImageBuildSpec(imageBuildSpec *image_buil } func (serviceConfig *ServiceConfig) GetImageRegistrySpec() *image_registry_spec.ImageRegistrySpec { - return serviceConfig.privateServiceConfig.ImagerRegistrySpec + return serviceConfig.privateServiceConfig.ImageRegistrySpec +} + +func (serviceConfig *ServiceConfig) SetImageRegistrySpec(imageRegistrySpec *image_registry_spec.ImageRegistrySpec) { + serviceConfig.privateServiceConfig.ImageRegistrySpec = imageRegistrySpec } func (serviceConfig *ServiceConfig) GetNixBuildSpec() *nix_build_spec.NixBuildSpec { return serviceConfig.privateServiceConfig.NixBuildSpec } +func (serviceConfig *ServiceConfig) SetNixBuildSpec(nixBuildSpec *nix_build_spec.NixBuildSpec) { + serviceConfig.privateServiceConfig.NixBuildSpec = nixBuildSpec +} + func (serviceConfig *ServiceConfig) GetPrivatePorts() map[string]*port_spec.PortSpec { return serviceConfig.privateServiceConfig.PrivatePorts } diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go index a8a178b572..c8ce3313c4 100644 --- a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go @@ -5,7 +5,6 @@ import ( "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types" "github.com/kurtosis-tech/stacktrace" - "github.com/sirupsen/logrus" ) type InterpretationTimeValueStore struct { @@ -40,21 +39,8 @@ func (itvs *InterpretationTimeValueStore) GetService(name service.ServiceName) ( return serviceStarlark, nil } -func (itvs *InterpretationTimeValueStore) PutServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) error { - if currServiceConfig, ok := itvs.serviceConfigValues[name]; !ok { - itvs.serviceConfigValues[name] = serviceConfig - } else { - if serviceConfig.GetImageBuildSpec() != nil { - currServiceConfig.SetImageBuildSpec(serviceConfig.GetImageBuildSpec()) - logrus.Infof("Updating image build spec of service config for '%v'", name) - } - if serviceConfig.GetContainerImageName() != "" { - currServiceConfig.SetContainerImageName(serviceConfig.GetContainerImageName()) - logrus.Infof("Updating container image to use for service config for '%v'", name) - } - itvs.serviceConfigValues[name] = currServiceConfig - } - return nil +func (itvs *InterpretationTimeValueStore) PutServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { + itvs.serviceConfigValues[name] = serviceConfig } func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index bcca118546..bf19373e94 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -154,7 +154,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt } builtin.serviceName = service.ServiceName(serviceName.GoString()) - builtin.serviceConfig = apiServiceConfig // store this in the interpretation time value store + builtin.serviceConfig = apiServiceConfig builtin.readyCondition = readyCondition builtin.resultUuid, err = builtin.runtimeValueStore.GetOrCreateValueAssociatedWithService(builtin.serviceName) if err != nil { @@ -172,10 +172,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt if err != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while persisting return value for service '%v'", serviceName) } - err = builtin.interpretationTimeValueStore.PutServiceConfig(builtin.serviceName, builtin.serviceConfig) - if err != nil { - return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while persisting return value for service '%v'", serviceName) - } + builtin.interpretationTimeValueStore.PutServiceConfig(builtin.serviceName, builtin.serviceConfig) return builtin.returnValue, nil } @@ -188,7 +185,6 @@ func (builtin *AddServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu } func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { - // pull service config from interpretation time value store serviceConfig, err := builtin.interpretationTimeValueStore.GetServiceConfig(builtin.serviceName) if err != nil { return "", stacktrace.Propagate(err, "An error occurred getting service config value from interpretation time value store for service '%v'", builtin.serviceName) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 793b387a4b..be33954906 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -24,9 +24,9 @@ import ( ) const ( - SetServiceBuiltinName = "set_service" - ServiceNameArgName = "name" - UpdateServiceConfigArgName = "config" + SetServiceBuiltinName = "set_service" + ServiceNameArgName = "name" + SetServiceConfigArgName = "config" descriptionFormatStr = "Updating config of service '%v'" ) @@ -52,14 +52,14 @@ func NewSetService( }, }, { - Name: UpdateServiceConfigArgName, + Name: SetServiceConfigArgName, IsOptional: false, ZeroValueProvider: builtin_argument.ZeroValueProvider[*service_config.ServiceConfig], Validator: func(value starlark.Value) *startosis_errors.InterpretationError { // we just try to convert the configs here to validate their shape, to avoid code duplication with Interpret _, ok := value.(*service_config.ServiceConfig) if !ok { - return startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", UpdateServiceConfigArgName, reflect.TypeOf(value)) + return startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", SetServiceConfigArgName, reflect.TypeOf(value)) } return nil }, @@ -113,9 +113,9 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt builtin.serviceName = serviceName - updatedServiceConfig, err := builtin_argument.ExtractArgumentValue[*service_config.ServiceConfig](arguments, UpdateServiceConfigArgName) + updatedServiceConfig, err := builtin_argument.ExtractArgumentValue[*service_config.ServiceConfig](arguments, SetServiceConfigArgName) if err != nil { - return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", UpdateServiceConfigArgName) + return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", SetServiceConfigArgName) } rawImageVal, found, interpretationErr := kurtosis_type_constructor.ExtractAttrValue[starlark.Value](updatedServiceConfig.KurtosisValueTypeDefault, service_config.ImageAttr) if interpretationErr != nil { @@ -125,7 +125,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - updatedApiServiceConfig, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( + apiServiceConfigOverride, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, updatedServiceConfig, locatorOfModuleInWhichThisBuiltInIsBeingCalled, @@ -138,12 +138,14 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, interpretationErr } - // implement a Put Service Config that overwrites whatever service config existed for that service - err = builtin.interpretationTimeStore.PutServiceConfig(serviceName, updatedApiServiceConfig) + // get existing service config for service and merge with apiServiceConfigOverride + currApiServiceConfig, err := builtin.interpretationTimeStore.GetServiceConfig(builtin.serviceName) if err != nil { - return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while fetching service '%v' from the store", serviceName) + return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred retrieving service config for service: %v'", builtin.serviceName) } + builtin.interpretationTimeStore.PutServiceConfig(serviceName, upsertServiceConfigs(currApiServiceConfig, apiServiceConfigOverride)) + builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(descriptionFormatStr, builtin.serviceName)) return starlark.None, nil } @@ -157,7 +159,7 @@ func (builtin *SetServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu func (builtin *SetServiceCapabilities) Execute(_ context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { // Note this is a no-op. - return fmt.Sprintf("Fetched service '%v'", builtin.serviceName), nil + return fmt.Sprintf("Set service config on service '%v'.", builtin.serviceName), nil } func (builtin *SetServiceCapabilities) TryResolveWith(instructionsAreEqual bool, _ *enclave_plan_persistence.EnclavePlanInstruction, enclaveComponents *enclave_structure.EnclaveComponents) enclave_structure.InstructionResolutionStatus { @@ -179,3 +181,17 @@ func (builtin *SetServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) func (builtin *SetServiceCapabilities) Description() string { return builtin.description } + +// Takes values set in [serviceConfigOverride] and sets them on [currServiceConfig], leaving other values of [currServiceConfig] untouched +func upsertServiceConfigs(currServiceConfig, serviceConfigOverride *service.ServiceConfig) *service.ServiceConfig { + // only one of these image values will be set, the others will be nil or empty string + // as Starlark service config gurantees that the both service config objects has one set + currServiceConfig.SetContainerImageName(serviceConfigOverride.GetContainerImageName()) + currServiceConfig.SetImageBuildSpec(serviceConfigOverride.GetImageBuildSpec()) + currServiceConfig.SetImageRegistrySpec(serviceConfigOverride.GetImageRegistrySpec()) + currServiceConfig.SetNixBuildSpec(serviceConfigOverride.GetNixBuildSpec()) + + // TODO: impl logic for updating other fields + // TODO: note: entrypoint, cmd, env vars, and ports will require special behavior to handle future references that could be overriden + return currServiceConfig +} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go new file mode 100644 index 0000000000..08a66aa314 --- /dev/null +++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go @@ -0,0 +1,56 @@ +package test_engine + +import ( + "fmt" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider" + "github.com/stretchr/testify/require" + "go.starlark.net/starlark" + "testing" +) + +type setServiceTestCase struct { + *testing.T + serviceNetwork *service_network.MockServiceNetwork + packageContentProvider *mock_package_content_provider.MockPackageContentProvider + interpretationTimeValueStore *interpretation_time_value_store.InterpretationTimeValueStore +} + +func (suite *KurtosisPlanInstructionTestSuite) TestSetService() { + suite.run(&setServiceTestCase{ + T: suite.T(), + serviceNetwork: suite.serviceNetwork, + packageContentProvider: suite.packageContentProvider, + interpretationTimeValueStore: suite.interpretationTimeValueStore, + }) +} + +func (t *setServiceTestCase) GetInstruction() *kurtosis_plan_instruction.KurtosisPlanInstruction { + return set_service.NewSetService( + t.serviceNetwork, + t.interpretationTimeValueStore, + testModulePackageId, + t.packageContentProvider, + testNoPackageReplaceOptions, + image_download_mode.ImageDownloadMode_Missing) +} + +func (t *setServiceTestCase) GetStarlarkCode() string { + serviceConfig := fmt.Sprintf("ServiceConfig(image=%q)", testContainerImageName) + return fmt.Sprintf(`%s(%s=%q, %s=%s)`, set_service.SetServiceBuiltinName, set_service.ServiceNameArgName, testServiceName, set_service.SetServiceConfigArgName, serviceConfig) +} + +func (t *setServiceTestCase) GetStarlarkCodeForAssertion() string { + return "" +} + +func (t *setServiceTestCase) Assert(interpretationResult starlark.Value, executionResult *string) { + require.Equal(t, starlark.None, interpretationResult) + + expectedExecutionResult := fmt.Sprintf("Set service config on service'%s'.", testServiceName) + require.Regexp(t, expectedExecutionResult, *executionResult) +} From 3d88950364bc1d7a12195e1cd715864096fef7dc Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 09:43:32 -0400 Subject: [PATCH 05/23] add tests --- .../objects/service/service_config.go | 36 ++++++++- .../set_service/set_service.go | 62 +++++++++++++-- .../test_engine/set_service_framework_test.go | 35 ++++++++- .../startosis_interpreter_test.go | 75 +++++++++++-------- 4 files changed, 168 insertions(+), 40 deletions(-) diff --git a/container-engine-lib/lib/backend_interface/objects/service/service_config.go b/container-engine-lib/lib/backend_interface/objects/service/service_config.go index 0aef1da1ca..0a9b234f1c 100644 --- a/container-engine-lib/lib/backend_interface/objects/service/service_config.go +++ b/container-engine-lib/lib/backend_interface/objects/service/service_config.go @@ -91,7 +91,7 @@ func CreateServiceConfig( cpuAllocationMillicpus uint64, memoryAllocationMegabytes uint64, privateIPAddrPlaceholder string, - minCpuMilliCores uint64, + minCpuMilliCpus uint64, minMemoryMegaBytes uint64, labels map[string]string, user *service_user.ServiceUser, @@ -120,7 +120,7 @@ func CreateServiceConfig( MemoryAllocationMegabytes: memoryAllocationMegabytes, PrivateIPAddrPlaceholder: privateIPAddrPlaceholder, // The minimum resources specification is only available for kubernetes - MinCpuAllocationMilliCpus: minCpuMilliCores, + MinCpuAllocationMilliCpus: minCpuMilliCpus, MinMemoryAllocationMegabytes: minMemoryMegaBytes, Labels: labels, User: user, @@ -196,10 +196,18 @@ func (serviceConfig *ServiceConfig) GetCPUAllocationMillicpus() uint64 { return serviceConfig.privateServiceConfig.CpuAllocationMillicpus } +func (serviceConfig *ServiceConfig) SetCPUAllocationMillicpus(cpuAllocation uint64) { + serviceConfig.privateServiceConfig.CpuAllocationMillicpus = cpuAllocation +} + func (serviceConfig *ServiceConfig) GetMemoryAllocationMegabytes() uint64 { return serviceConfig.privateServiceConfig.MemoryAllocationMegabytes } +func (serviceConfig *ServiceConfig) SetMemoryAllocationMegabytes(memoryAllocation uint64) { + serviceConfig.privateServiceConfig.MemoryAllocationMegabytes = memoryAllocation +} + func (serviceConfig *ServiceConfig) GetPrivateIPAddrPlaceholder() string { return serviceConfig.privateServiceConfig.PrivateIPAddrPlaceholder } @@ -209,27 +217,51 @@ func (serviceConfig *ServiceConfig) GetMinCPUAllocationMillicpus() uint64 { return serviceConfig.privateServiceConfig.MinCpuAllocationMilliCpus } +func (serviceConfig *ServiceConfig) SetMinCPUAllocationMillicpus(cpuAllocation uint64) { + serviceConfig.privateServiceConfig.MemoryAllocationMegabytes = cpuAllocation +} + // only available for Kubernetes func (serviceConfig *ServiceConfig) GetMinMemoryAllocationMegabytes() uint64 { return serviceConfig.privateServiceConfig.MinMemoryAllocationMegabytes } +func (serviceConfig *ServiceConfig) SetMinMemoryAllocationMegabytes(memoryAllocation uint64) { + serviceConfig.privateServiceConfig.MemoryAllocationMegabytes = memoryAllocation +} + func (serviceConfig *ServiceConfig) GetUser() *service_user.ServiceUser { return serviceConfig.privateServiceConfig.User } +func (serviceConfig *ServiceConfig) SetUser(user *service_user.ServiceUser) { + serviceConfig.privateServiceConfig.User = user +} + func (serviceConfig *ServiceConfig) GetLabels() map[string]string { return serviceConfig.privateServiceConfig.Labels } +func (serviceConfig *ServiceConfig) SetLabels(labels map[string]string) { + serviceConfig.privateServiceConfig.Labels = labels +} + func (serviceConfig *ServiceConfig) GetTolerations() []v1.Toleration { return serviceConfig.privateServiceConfig.Tolerations } +func (serviceConfig *ServiceConfig) SetTolerations(tolerations []v1.Toleration) { + serviceConfig.privateServiceConfig.Tolerations = tolerations +} + func (serviceConfig *ServiceConfig) GetImageDownloadMode() image_download_mode.ImageDownloadMode { return serviceConfig.privateServiceConfig.ImageDownloadMode } +func (serviceConfig *ServiceConfig) SetImageDownloadMode(mode image_download_mode.ImageDownloadMode) { + serviceConfig.privateServiceConfig.ImageDownloadMode = mode +} + func (serviceConfig *ServiceConfig) MarshalJSON() ([]byte, error) { return json.Marshal(serviceConfig.privateServiceConfig) } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index be33954906..02362edf08 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -144,7 +144,12 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred retrieving service config for service: %v'", builtin.serviceName) } - builtin.interpretationTimeStore.PutServiceConfig(serviceName, upsertServiceConfigs(currApiServiceConfig, apiServiceConfigOverride)) + mergedServiceConfig, err := upsertServiceConfigs(currApiServiceConfig, apiServiceConfigOverride) + if err != nil { + return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while overriding service configs in set service for service: %v", builtin.serviceName) + } + + builtin.interpretationTimeStore.PutServiceConfig(serviceName, mergedServiceConfig) builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(descriptionFormatStr, builtin.serviceName)) return starlark.None, nil @@ -183,15 +188,60 @@ func (builtin *SetServiceCapabilities) Description() string { } // Takes values set in [serviceConfigOverride] and sets them on [currServiceConfig], leaving other values of [currServiceConfig] untouched -func upsertServiceConfigs(currServiceConfig, serviceConfigOverride *service.ServiceConfig) *service.ServiceConfig { +func upsertServiceConfigs(currServiceConfig, serviceConfigOverride *service.ServiceConfig) (*service.ServiceConfig, error) { // only one of these image values will be set, the others will be nil or empty string - // as Starlark service config gurantees that the both service config objects has one set + // as the Starlark service config gurantees that the service config objects only has one set currServiceConfig.SetContainerImageName(serviceConfigOverride.GetContainerImageName()) currServiceConfig.SetImageBuildSpec(serviceConfigOverride.GetImageBuildSpec()) currServiceConfig.SetImageRegistrySpec(serviceConfigOverride.GetImageRegistrySpec()) currServiceConfig.SetNixBuildSpec(serviceConfigOverride.GetNixBuildSpec()) - // TODO: impl logic for updating other fields - // TODO: note: entrypoint, cmd, env vars, and ports will require special behavior to handle future references that could be overriden - return currServiceConfig + // for other fields, only override if they are explicitly set on serviceConfigOverride + if cpuAllocationMillicpusOverride := serviceConfigOverride.GetCPUAllocationMillicpus(); cpuAllocationMillicpusOverride != 0 { + currServiceConfig.SetCPUAllocationMillicpus(cpuAllocationMillicpusOverride) + } + if memoryAllocationMegabytesOverride := serviceConfigOverride.GetMemoryAllocationMegabytes(); memoryAllocationMegabytesOverride != 0 { + currServiceConfig.SetMemoryAllocationMegabytes(memoryAllocationMegabytesOverride) + } + if minCPUAllocationMillicpusOverride := serviceConfigOverride.GetMinCPUAllocationMillicpus(); minCPUAllocationMillicpusOverride != 0 { + currServiceConfig.SetMinCPUAllocationMillicpus(minCPUAllocationMillicpusOverride) + } + if minMemoryAllocationMegabytesOverride := serviceConfigOverride.GetMinMemoryAllocationMegabytes(); minMemoryAllocationMegabytesOverride != 0 { + currServiceConfig.SetMinMemoryAllocationMegabytes(minMemoryAllocationMegabytesOverride) + } + if userOverride := serviceConfigOverride.GetUser(); userOverride != nil { + currServiceConfig.SetUser(userOverride) + } + if labelsOverride := serviceConfigOverride.GetLabels(); len(labelsOverride) > 0 { + currServiceConfig.SetLabels(labelsOverride) + } + if tolerationsOverride := serviceConfigOverride.GetTolerations(); len(tolerationsOverride) > 0 { + currServiceConfig.SetTolerations(tolerationsOverride) + } + + // TODO: impl logic for overriding entrypoint, cmd, env vars, and ports + // TODO: note: these will require careful handling of future references that could be potentially be overriden and affect behavior + if len(serviceConfigOverride.GetEnvVars()) != 0 { + return nil, startosis_errors.NewInterpretationError("Overriding environment variables is currently not supported.") + } + if len(serviceConfigOverride.GetEntrypointArgs()) != 0 { + return nil, startosis_errors.NewInterpretationError("Overriding entrypoint args is currently not supported.") + } + if len(serviceConfigOverride.GetCmdArgs()) != 0 { + return nil, startosis_errors.NewInterpretationError("Overriding cmd args is currently not supported.") + } + if len(serviceConfigOverride.GetPrivatePorts()) != 0 { + return nil, startosis_errors.NewInterpretationError("Overriding private ports is currently not supported. ") + } + if len(serviceConfigOverride.GetPublicPorts()) != 0 { + return nil, startosis_errors.NewInterpretationError("Overriding public ports is currently not supported.") + } + if serviceConfigOverride.GetFilesArtifactsExpansion() != nil { + return nil, startosis_errors.NewInterpretationError("Overriding files artifacts is currently not supported.") + } + if serviceConfigOverride.GetPersistentDirectories() != nil { + return nil, startosis_errors.NewInterpretationError("Overriding persistent directories is currently not supported.") + } + + return currServiceConfig, nil } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go index 08a66aa314..d820918520 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/set_service_framework_test.go @@ -3,9 +3,11 @@ package test_engine import ( "fmt" "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" + "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/interpretation_time_value_store" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service" + "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/kurtosis_plan_instruction" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages/mock_package_content_provider" "github.com/stretchr/testify/require" @@ -21,6 +23,37 @@ type setServiceTestCase struct { } func (suite *KurtosisPlanInstructionTestSuite) TestSetService() { + dummySerde := shared_helpers.NewDummyStarlarkValueSerDeForTest() + enclaveDb := getEnclaveDBForTest(suite.T()) + interpretationTimeValueStore, err := interpretation_time_value_store.CreateInterpretationTimeValueStore(enclaveDb, dummySerde) + require.NoError(suite.T(), err) + suite.interpretationTimeValueStore = interpretationTimeValueStore + + testServiceConfig, err := service.CreateServiceConfig( + testContainerImageName, + nil, + nil, + nil, + nil, + nil, + []string{}, + []string{}, + map[string]string{}, + nil, + nil, + 0, + 0, + "IP-ADDRESS", + 0, + 0, + map[string]string{}, + nil, + nil, + nil, + image_download_mode.ImageDownloadMode_Always) + require.NoError(suite.T(), err) + suite.interpretationTimeValueStore.PutServiceConfig(testServiceName, testServiceConfig) + suite.run(&setServiceTestCase{ T: suite.T(), serviceNetwork: suite.serviceNetwork, @@ -51,6 +84,6 @@ func (t *setServiceTestCase) GetStarlarkCodeForAssertion() string { func (t *setServiceTestCase) Assert(interpretationResult starlark.Value, executionResult *string) { require.Equal(t, starlark.None, interpretationResult) - expectedExecutionResult := fmt.Sprintf("Set service config on service'%s'.", testServiceName) + expectedExecutionResult := fmt.Sprintf("Set service config on service '%s'.", testServiceName) require.Regexp(t, expectedExecutionResult, *executionResult) } diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go index a2eceac26f..d2bea6d789 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_test.go @@ -19,6 +19,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/kurtosis_print" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/remove_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/render_templates" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service" "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_instruction/store_service_files" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store" @@ -396,37 +397,6 @@ def run(plan): require.Nil(suite.T(), instructionsPlan) } -//func (suite *StartosisInterpreterTestSuite) TestInterpreter() { -// script := ` -//def run(plan): -// config = ServiceConfig( -// image = "someContainer", -// ports = { -// "grpc": PortSpec(number=1234, transport_protocol = "TCP") # port number should be an int -// } -// ) -// plan.add_service(name = "tedi", config = config) -// updatedConfig = ServiceConfig( -// image = "someNewContainer", -// ) -// plan.update_service(name="tedi", config=updatedConfig) -//` -// -// _, instructionsPlan, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) -// require.Nil(suite.T(), interpretationError) -// require.Equal(suite.T(), 2, instructionsPlan.Size()) -// //expectedError := startosis_errors.NewInterpretationErrorWithCauseAndCustomMsg( -// // startosis_errors.NewInterpretationError(`The following argument(s) could not be parsed or did not pass validation: {"number":"Value for 'number' was expected to be an integer between 1 and 65535, but it was 'starlark.String'"}`), -// // []startosis_errors.CallFrame{ -// // *startosis_errors.NewCallFrame("run", startosis_errors.NewScriptPosition(startosis_constants.PackageIdPlaceholderForStandaloneScript, 11, 20)), -// // *startosis_errors.NewCallFrame("PortSpec", startosis_errors.NewScriptPosition("", 0, 0)), -// // }, -// // "Evaluation error: Cannot construct 'PortSpec' from the provided arguments.", -// //).ToAPIType() -// //require.Equal(suite.T(), expectedError, interpretationError) -// //require.Nil(suite.T(), instructionsPlan) -//} - func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_ValidSimpleScriptWithInstructionPortNumberAsString() { script := ` def run(plan): @@ -926,6 +896,49 @@ The service example-datastore-server has been removed validateScriptOutputFromPrintInstructions(suite.T(), instructionsPlan, expectedOutput) } +func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_SetService() { + script := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name = "example-datastore-server", config = config) + + newConfig = ServiceConfig( + image = "someNewContainer", + ) + plan.set_service(name="example-datastore-server", config=newConfig) +` + + _, instructionsPlan, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) + require.Nil(suite.T(), interpretationError) + require.Equal(suite.T(), 2, instructionsPlan.Size()) + + assertInstructionTypeAndPosition(suite.T(), instructionsPlan, 0, add_service.AddServiceBuiltinName, startosis_constants.PackageIdPlaceholderForStandaloneScript, 6, 18) + assertInstructionTypeAndPosition(suite.T(), instructionsPlan, 1, set_service.SetServiceBuiltinName, startosis_constants.PackageIdPlaceholderForStandaloneScript, 11, 18) +} + +func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_SetServiceErrorsOnUnsupportedFields() { + script := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name = "example-datastore-server", config = config) + + newConfig = ServiceConfig( + image= "datastore-image", + env_vars = { + "SOME": "THING" + } + ) + plan.set_service(name="example-datastore-server", config=newConfig) +` + + _, _, interpretationError := suite.interpreter.Interpret(context.Background(), startosis_constants.PackageIdPlaceholderForStandaloneScript, useDefaultMainFunctionName, noPackageReplaceOptions, startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, script, startosis_constants.EmptyInputArgs, defaultNonBlockingMode, emptyEnclaveComponents, emptyInstructionsPlanMask, defaultImageDownloadMode) + require.NotNil(suite.T(), interpretationError) +} + func (suite *StartosisInterpreterTestSuite) TestStartosisInterpreter_NoPanicIfUploadIsPassedAPathNotOnDisk() { filePath := "github.com/kurtosis/module/lib/lib.star" script := ` From f25e53f627f7b57b5ed8448a84d72ba51ece1d51 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 09:56:10 -0400 Subject: [PATCH 06/23] updated -> override --- .../kurtosis_instruction/set_service/set_service.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 02362edf08..a892a93223 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -113,11 +113,11 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt builtin.serviceName = serviceName - updatedServiceConfig, err := builtin_argument.ExtractArgumentValue[*service_config.ServiceConfig](arguments, SetServiceConfigArgName) + serviceConfigOverride, err := builtin_argument.ExtractArgumentValue[*service_config.ServiceConfig](arguments, SetServiceConfigArgName) if err != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract value for '%s' argument", SetServiceConfigArgName) } - rawImageVal, found, interpretationErr := kurtosis_type_constructor.ExtractAttrValue[starlark.Value](updatedServiceConfig.KurtosisValueTypeDefault, service_config.ImageAttr) + rawImageVal, found, interpretationErr := kurtosis_type_constructor.ExtractAttrValue[starlark.Value](serviceConfigOverride.KurtosisValueTypeDefault, service_config.ImageAttr) if interpretationErr != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "Unable to extract raw image attribute.") } @@ -127,7 +127,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt builtin.imageVal = rawImageVal apiServiceConfigOverride, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, - updatedServiceConfig, + serviceConfigOverride, locatorOfModuleInWhichThisBuiltInIsBeingCalled, builtin.packageId, builtin.packageContentProvider, From af8880d6b541e3c86318fbe116a79e0f0324ea26 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 10:05:15 -0400 Subject: [PATCH 07/23] add newline --- .../interpretation_time_value_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go index 2b74f30834..ffa148977d 100644 --- a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go @@ -65,4 +65,4 @@ func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceN return nil, stacktrace.NewError("Did not find service config for '%v' in interpretation time value store.", name) } return serviceConfig, nil -} \ No newline at end of file +} From 881379068c1010f9fc6cae2facad3a25f182a4f4 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 10:09:54 -0400 Subject: [PATCH 08/23] fmt --- .../api_container/server/startosis_engine/kurtosis_builtins.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go index bd4abfea1c..2fec5f5555 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_builtins.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_builtins.go @@ -73,7 +73,7 @@ func KurtosisPlanInstructions( add_service.NewAddService(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), add_service.NewAddServices(serviceNetwork, runtimeValueStore, packageId, packageContentProvider, packageReplaceOptions, interpretationTimeValueStore, imageDownloadMode), get_service.NewGetService(interpretationTimeValueStore), - get_services.NewGetServices(interpretationTimeValueStore), + get_services.NewGetServices(interpretationTimeValueStore), set_service.NewSetService(serviceNetwork, interpretationTimeValueStore, packageId, packageContentProvider, packageReplaceOptions, imageDownloadMode), get_files_artifact.NewGetFilesArtifact(), verify.NewVerify(runtimeValueStore), From 096f35146d0b4fc2fa6a6300a516848ed0ea6212 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 10:32:23 -0400 Subject: [PATCH 09/23] add docs --- .../api-reference/starlark-reference/plan.md | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/docs/docs/api-reference/starlark-reference/plan.md b/docs/docs/api-reference/starlark-reference/plan.md index 96c1bc8a35..c9b99ad471 100644 --- a/docs/docs/api-reference/starlark-reference/plan.md +++ b/docs/docs/api-reference/starlark-reference/plan.md @@ -128,7 +128,7 @@ service = plan.get_service( get_services ----------- -The `get_services` instruction allows you to get the [Service][service-starlark-reference] objects of running services in an enclave. This is +The `get_services` instruction allows retrieving the [Service][service-starlark-reference] objects of running services in an enclave. This is useful in situations where you are running Starlark against an existing enclave or need information about the services that an imported package started. ```python @@ -144,6 +144,51 @@ for service in services: plan.print(service.hostname) plan.print(service.ip_address) ``` + +set_service +----------- + +`set_service` instruction allows overriding the [ServiceConfig][service-config] of a service that exists in the plan. This is useful especially in cases where you are +importing a [package][package] published by somebody else, but you want to modify aspects of it. For example, if you want to change the image that a service +started by the imported package is using, but the package author has not parameterized the service image via the `run` function, you can use `set_service` to change the image! + +```python +# Returns a Service object (see the Service page in the sidebar) +plan.set_service( + # The name of the service to set the config of + # MANDATORY + name = "my-service", + + # A ServiceConfig object populated with fields to override the existing ServiceConfig object with + # OPTIONAL (Default: Fetching service 'SERVICE_NAME') + config = ServiceConfig(...), + + # A human friendly description for the end user of the package + # OPTIONAL (Default: Set service on service "my-service") + description = "Setting new image on my-service" +) +``` +Below is an example usage: +```python +# Postgres package starts a pg db off image "postgres:latest" +postgres = import_module("github.com/kurtosis-tech/postgres-package/main.star") + +def run(plan, args): + postgres.run(plan) + + # Set service detects the "postgres" service in the plan and + # Overrides the ServiceConfig set in the postgres package to use the provided image value + plan.set_service( + name="postgres", + config=ServiceConfig( + image=ImageBuildSpec( + image_name="my-pg-db", # use my custom db instead! + build_context_dir="./database" + ) + ) + ) +``` + get_files_artifact ----------- From 1d8226b209e4dc9ed913594250f84513b675f95d Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Tue, 23 Apr 2024 10:40:46 -0400 Subject: [PATCH 10/23] add caveats --- docs/docs/api-reference/starlark-reference/plan.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/docs/api-reference/starlark-reference/plan.md b/docs/docs/api-reference/starlark-reference/plan.md index c9b99ad471..56c43da6b6 100644 --- a/docs/docs/api-reference/starlark-reference/plan.md +++ b/docs/docs/api-reference/starlark-reference/plan.md @@ -149,7 +149,7 @@ set_service ----------- `set_service` instruction allows overriding the [ServiceConfig][service-config] of a service that exists in the plan. This is useful especially in cases where you are -importing a [package][package] published by somebody else, but you want to modify aspects of it. For example, if you want to change the image that a service +importing a package published by somebody else, but you want to modify aspects of it. For example, if you want to change the image that a service started by the imported package is using, but the package author has not parameterized the service image via the `run` function, you can use `set_service` to change the image! ```python @@ -168,7 +168,7 @@ plan.set_service( description = "Setting new image on my-service" ) ``` -Below is an example usage: +Example usage: ```python # Postgres package starts a pg db off image "postgres:latest" postgres = import_module("github.com/kurtosis-tech/postgres-package/main.star") @@ -188,6 +188,8 @@ def run(plan, args): ) ) ``` +Note: currently, setting the ServiceConfig image, user, labels, tolerations, and resource allocation values are supported. +Overriding the ports, env vars, cmd/entrypoint args, and files are not supported. If this is something you'd like support for please let us know on [GitHub](https://github.com/kurtosis-tech/kurtosis/issues)! get_files_artifact ----------- From f8eda0309f3bfc49704c17e593ea5943fcde0cb7 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Wed, 24 Apr 2024 22:54:11 -0400 Subject: [PATCH 11/23] make set service work with enclave edits --- .../interpretation_time_value_store.go | 43 ++++++++--- .../add_service/add_service.go | 6 ++ .../get_services/get_services.go | 1 + .../set_service/set_service.go | 4 +- .../kurtosis_plan_instruction.go | 2 +- .../startosis_engine/startosis_interpreter.go | 3 +- .../startosis_interpreter_idempotent_test.go | 74 +++++++++++++++++++ 7 files changed, 118 insertions(+), 15 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go index ffa148977d..6cf901ef8d 100644 --- a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go @@ -8,9 +8,10 @@ import ( ) type InterpretationTimeValueStore struct { - serviceConfigValues map[service.ServiceName]*service.ServiceConfig - serviceValues *serviceInterpretationValueRepository - serde *kurtosis_types.StarlarkValueSerde + serviceConfigValues map[service.ServiceName]*service.ServiceConfig + setServiceConfigValues map[service.ServiceName]*service.ServiceConfig + serviceValues *serviceInterpretationValueRepository + serde *kurtosis_types.StarlarkValueSerde } func CreateInterpretationTimeValueStore(enclaveDb *enclave_db.EnclaveDB, serde *kurtosis_types.StarlarkValueSerde) (*InterpretationTimeValueStore, error) { @@ -19,9 +20,10 @@ func CreateInterpretationTimeValueStore(enclaveDb *enclave_db.EnclaveDB, serde * return nil, stacktrace.Propagate(err, "An error occurred creating interpretation time value store") } return &InterpretationTimeValueStore{ - serviceConfigValues: map[service.ServiceName]*service.ServiceConfig{}, - serviceValues: serviceValuesRepository, - serde: serde}, nil + serviceConfigValues: map[service.ServiceName]*service.ServiceConfig{}, + setServiceConfigValues: map[service.ServiceName]*service.ServiceConfig{}, + serviceValues: serviceValuesRepository, + serde: serde}, nil } func (itvs *InterpretationTimeValueStore) PutService(name service.ServiceName, service *kurtosis_types.Service) error { @@ -56,13 +58,32 @@ func (itvs *InterpretationTimeValueStore) RemoveService(name service.ServiceName } func (itvs *InterpretationTimeValueStore) PutServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { - itvs.serviceConfigValues[name] = serviceConfig + if _, ok := itvs.serviceConfigValues[name]; !ok { + itvs.serviceConfigValues[name] = serviceConfig + } else { + itvs.setServiceConfigValues[name] = serviceConfig + } +} + +// +//func (itvs *InterpretationTimeValueStore) SetServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { +// itvs.serviceConfigValues[name] = serviceConfig +// itvs.setServiceConfigValues[name] = serviceConfig +//} + +func (itvs *InterpretationTimeValueStore) ExistsUpdatedServiceConfigForService(name service.ServiceName) bool { + _, doesConfigFromSetServiceInstructionExists := itvs.setServiceConfigValues[name] + return doesConfigFromSetServiceInstructionExists } func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { - serviceConfig, ok := itvs.serviceConfigValues[name] - if !ok { - return nil, stacktrace.NewError("Did not find service config for '%v' in interpretation time value store.", name) + if maybeLatestServiceConfig, ok := itvs.setServiceConfigValues[name]; ok { + return maybeLatestServiceConfig, nil + } else { + serviceConfig, ok := itvs.serviceConfigValues[name] + if !ok { + return nil, stacktrace.NewError("Did not find service config for '%v' in interpretation time value store.", name) + } + return serviceConfig, nil } - return serviceConfig, nil } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 549cb52cfe..165b076535 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -263,6 +263,12 @@ func (builtin *AddServiceCapabilities) TryResolveWith(instructionsAreEqual bool, } } + // We check if service config was changed by a set_service instruction. If that's the case, it should be rerun + if builtin.interpretationTimeValueStore.ExistsUpdatedServiceConfigForService(builtin.serviceName) { + enclaveComponents.AddService(builtin.serviceName, enclave_structure.ComponentIsUpdated) + return enclave_structure.InstructionIsUpdate + } + enclaveComponents.AddService(builtin.serviceName, enclave_structure.ComponentWasLeftIntact) return enclave_structure.InstructionIsEqual } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/get_services/get_services.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/get_services/get_services.go index 403f7e1771..05d2667a24 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/get_services/get_services.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/get_services/get_services.go @@ -84,6 +84,7 @@ func (builtin *GetServicesCapabilities) TryResolveWith(instructionsAreEqual bool if instructionsAreEqual { return enclave_structure.InstructionIsEqual } + return enclave_structure.InstructionIsUnknown } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index a892a93223..d05dcad0c1 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -168,7 +168,9 @@ func (builtin *SetServiceCapabilities) Execute(_ context.Context, _ *builtin_arg } func (builtin *SetServiceCapabilities) TryResolveWith(instructionsAreEqual bool, _ *enclave_plan_persistence.EnclavePlanInstruction, enclaveComponents *enclave_structure.EnclaveComponents) enclave_structure.InstructionResolutionStatus { - if instructionsAreEqual { + if instructionsAreEqual && enclaveComponents.HasServiceBeenUpdated(builtin.serviceName) { + return enclave_structure.InstructionIsUpdate + } else if instructionsAreEqual { return enclave_structure.InstructionIsEqual } return enclave_structure.InstructionIsUnknown diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go index 3a11db48c7..c767707d6f 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/kurtosis_plan_instruction/kurtosis_plan_instruction.go @@ -110,7 +110,7 @@ func (builtin *KurtosisPlanInstructionWrapper) CreateBuiltin() func(thread *star instructionWrapper.String(), instructionWrapper.GetPositionInOriginalScript().String()) } - if enclavePlanInstructionPulledFromMaskMaybe != nil { + if enclavePlanInstructionPulledFromMaskMaybe != nil { // why is it that the mask is invalid if this is the case? and why not make this check before adding the instruction to the plan? builtin.instructionPlanMask.MarkAsInvalid() logrus.Debugf("Marking the plan as invalid as instruction '%s' differs from '%s'", instructionWrapper.String(), enclavePlanInstructionPulledFromMaskMaybe.StarlarkCode) diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter.go b/core/server/api_container/server/startosis_engine/startosis_interpreter.go index 6b37e26add..8660df5ce4 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter.go @@ -136,7 +136,6 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan( // - if it's not successful, then the mask is not compatible with the package. Go back to step 1 var firstPossibleIndexForMatchingInstruction int if currentEnclavePlan.Size() > naiveInstructionsPlan.Size() { - firstPossibleIndexForMatchingInstruction = currentEnclavePlan.Size() - naiveInstructionsPlan.Size() } for { @@ -149,7 +148,7 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan( if matchingInstructionIdx >= 0 { logrus.Debugf("Found an instruction in enclave state at index %d which matches the first instruction of the new instructions plan", matchingInstructionIdx) // we found a match - // -> First recopy store that index into the plan so that all instructions prior to this match will be + // -> First store that index into the plan so that all instructions prior to this match will be // kept in the enclave plan logrus.Debugf("Stored index of matching instructions: %d into the new plan. The instructions prior to this index in the enclave plan won't be executed but need to be kept in the enclave plan", matchingInstructionIdx) optimizedPlan.SetIndexOfFirstInstruction(matchingInstructionIdx) diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go index 7aebb3e0cc..f57fa0a4d2 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go @@ -479,6 +479,80 @@ func (suite *StartosisInterpreterIdempotentTestSuite) TestInterpretAndOptimize_U require.True(suite.T(), scheduledInstruction4.IsExecuted()) // this instruction is not affected, i.e. it won't be re-run } +func (suite *StartosisInterpreterIdempotentTestSuite) TestStartosisInterpreterIdempotent_SetService() { + initialScript := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name = "example-datastore-server", config = config) +` + + // Interpretation of the initial script to generate the current enclave plan + _, currentEnclavePlan, interpretationApiErr := suite.interpreter.Interpret( + context.Background(), + startosis_constants.PackageIdPlaceholderForStandaloneScript, + useDefaultMainFunctionName, + noPackageReplaceOptions, + startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, + initialScript, + noInputParams, + defaultNonBlockingMode, + enclave_structure.NewEnclaveComponents(), + resolver.NewInstructionsPlanMask(0), + image_download_mode.ImageDownloadMode_Missing) + require.Nil(suite.T(), interpretationApiErr) + require.Equal(suite.T(), 1, currentEnclavePlan.Size()) + convertedEnclavePlan := suite.convertInstructionPlanToEnclavePlan(currentEnclavePlan) + + updatedScript := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name="example-datastore-server", config=config) + + newConfig = ServiceConfig( + image = "datastore-image:latest", + ) + plan.set_service(name="example-datastore-server", config=newConfig) +` + // updatedScript := ` + //def run(plan): + // newConfig = ServiceConfig( + // image = "datastore-image:latest", + // ) + // plan.set_service(name="example-datastore-server", config=newConfig) + //` + // Interpret the updated script against the current enclave plan + _, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan( + context.Background(), + startosis_constants.PackageIdPlaceholderForStandaloneScript, + noPackageReplaceOptions, + useDefaultMainFunctionName, + startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, + updatedScript, + noInputParams, + defaultNonBlockingMode, + convertedEnclavePlan, + image_download_mode.ImageDownloadMode_Missing, + ) + require.Nil(suite.T(), interpretationError) + + instructionSequence, err := instructionsPlan.GeneratePlan() + require.Nil(suite.T(), err) + require.Equal(suite.T(), 0, instructionsPlan.GetIndexOfFirstInstruction()) + require.Equal(suite.T(), 2, len(instructionSequence)) + + scheduledInstruction1 := instructionSequence[0] + require.Equal(suite.T(), `add_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image"))`, scheduledInstruction1.GetInstruction().String()) + require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the underlying service config + + scheduledInstruction2 := instructionSequence[1] + require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest"))`, scheduledInstruction2.GetInstruction().String()) + require.False(suite.T(), scheduledInstruction2.IsExecuted()) // set service should also be re-executed because it hasn't been run, but its a noop - its effect is to swap out the service config during interpretation time +} + func (suite *StartosisInterpreterIdempotentTestSuite) convertInstructionPlanToEnclavePlan(instructionPlan *instructions_plan.InstructionsPlan) *enclave_plan_persistence.EnclavePlan { enclavePlan := enclave_plan_persistence.NewEnclavePlan() instructionPlanSequence, interpretationErr := instructionPlan.GeneratePlan() From 13054aedf5e3e26f2526dcdce5ea5b3e54d1c0f1 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 00:43:21 -0400 Subject: [PATCH 12/23] only update service config if new one exists in add service exec --- .../interpretation_time_value_store.go | 36 +++++++++---------- .../add_service/add_service.go | 14 +++++--- .../set_service/set_service.go | 2 +- .../startosis_interpreter_idempotent_test.go | 10 ++---- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go index 6cf901ef8d..7ea9035a14 100644 --- a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store.go @@ -58,32 +58,30 @@ func (itvs *InterpretationTimeValueStore) RemoveService(name service.ServiceName } func (itvs *InterpretationTimeValueStore) PutServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { - if _, ok := itvs.serviceConfigValues[name]; !ok { - itvs.serviceConfigValues[name] = serviceConfig - } else { - itvs.setServiceConfigValues[name] = serviceConfig + itvs.serviceConfigValues[name] = serviceConfig +} + +func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { + serviceConfig, ok := itvs.serviceConfigValues[name] + if !ok { + return nil, stacktrace.NewError("Did not find new service config for '%v' in interpretation time value store.", name) } + return serviceConfig, nil } -// -//func (itvs *InterpretationTimeValueStore) SetServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { -// itvs.serviceConfigValues[name] = serviceConfig -// itvs.setServiceConfigValues[name] = serviceConfig -//} +func (itvs *InterpretationTimeValueStore) SetServiceConfig(name service.ServiceName, serviceConfig *service.ServiceConfig) { + itvs.setServiceConfigValues[name] = serviceConfig +} -func (itvs *InterpretationTimeValueStore) ExistsUpdatedServiceConfigForService(name service.ServiceName) bool { +func (itvs *InterpretationTimeValueStore) ExistsNewServiceConfigForService(name service.ServiceName) bool { _, doesConfigFromSetServiceInstructionExists := itvs.setServiceConfigValues[name] return doesConfigFromSetServiceInstructionExists } -func (itvs *InterpretationTimeValueStore) GetServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { - if maybeLatestServiceConfig, ok := itvs.setServiceConfigValues[name]; ok { - return maybeLatestServiceConfig, nil - } else { - serviceConfig, ok := itvs.serviceConfigValues[name] - if !ok { - return nil, stacktrace.NewError("Did not find service config for '%v' in interpretation time value store.", name) - } - return serviceConfig, nil +func (itvs *InterpretationTimeValueStore) GetNewServiceConfig(name service.ServiceName) (*service.ServiceConfig, error) { + newServiceConfig, ok := itvs.setServiceConfigValues[name] + if !ok { + return nil, stacktrace.NewError("Did not find new service config for '%v' in interpretation time value store.", name) } + return newServiceConfig, nil } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 165b076535..bbbb8f72b9 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -185,11 +185,15 @@ func (builtin *AddServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu } func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { - serviceConfig, err := builtin.interpretationTimeValueStore.GetServiceConfig(builtin.serviceName) - if err != nil { - return "", stacktrace.Propagate(err, "An error occurred getting service config value from interpretation time value store for service '%v'", builtin.serviceName) + // update service config to use new service config set by a set_service instruction, if one exists + if builtin.interpretationTimeValueStore.ExistsNewServiceConfigForService(builtin.serviceName) { + newServiceConfig, err := builtin.interpretationTimeValueStore.GetNewServiceConfig(builtin.serviceName) + if err != nil { + return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '$s'.", builtin.serviceName) + } + builtin.serviceConfig = newServiceConfig } - replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, serviceConfig) + replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) if err != nil { return "", stacktrace.Propagate(err, "An error occurred replace a magic string in '%s' instruction arguments for service '%s'. Execution cannot proceed", AddServiceBuiltinName, builtin.serviceName) } @@ -264,7 +268,7 @@ func (builtin *AddServiceCapabilities) TryResolveWith(instructionsAreEqual bool, } // We check if service config was changed by a set_service instruction. If that's the case, it should be rerun - if builtin.interpretationTimeValueStore.ExistsUpdatedServiceConfigForService(builtin.serviceName) { + if builtin.interpretationTimeValueStore.ExistsNewServiceConfigForService(builtin.serviceName) { enclaveComponents.AddService(builtin.serviceName, enclave_structure.ComponentIsUpdated) return enclave_structure.InstructionIsUpdate } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index d05dcad0c1..592aa71435 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -149,7 +149,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while overriding service configs in set service for service: %v", builtin.serviceName) } - builtin.interpretationTimeStore.PutServiceConfig(serviceName, mergedServiceConfig) + builtin.interpretationTimeStore.SetServiceConfig(serviceName, mergedServiceConfig) builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(descriptionFormatStr, builtin.serviceName)) return starlark.None, nil diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go index f57fa0a4d2..196c079956 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go @@ -517,13 +517,7 @@ def run(plan): ) plan.set_service(name="example-datastore-server", config=newConfig) ` - // updatedScript := ` - //def run(plan): - // newConfig = ServiceConfig( - // image = "datastore-image:latest", - // ) - // plan.set_service(name="example-datastore-server", config=newConfig) - //` + // Interpret the updated script against the current enclave plan _, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan( context.Background(), @@ -546,7 +540,7 @@ def run(plan): scheduledInstruction1 := instructionSequence[0] require.Equal(suite.T(), `add_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image"))`, scheduledInstruction1.GetInstruction().String()) - require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the underlying service config + require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the service config scheduledInstruction2 := instructionSequence[1] require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest"))`, scheduledInstruction2.GetInstruction().String()) From bcc08dc6cade4bb057d50242a51f53d1fc368dcd Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 00:45:02 -0400 Subject: [PATCH 13/23] add debug log --- .../kurtosis_instruction/add_service/add_service.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index bbbb8f72b9..c20cab6ffc 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -21,6 +21,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages" "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" "reflect" ) @@ -191,6 +192,7 @@ func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_a if err != nil { return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '$s'.", builtin.serviceName) } + logrus.Info("USING NEW SERVICE CONFIG") builtin.serviceConfig = newServiceConfig } replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) From 99b1b03c4c12ca0b462daec6683c88bbf8052fbb Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 01:22:04 -0400 Subject: [PATCH 14/23] add multi set service test --- .../objects/service/service_config.go | 2 +- .../add_service/add_service.go | 4 +- .../set_service/set_service.go | 2 +- .../startosis_interpreter_idempotent_test.go | 143 ++++++++++++++++++ 4 files changed, 146 insertions(+), 5 deletions(-) diff --git a/container-engine-lib/lib/backend_interface/objects/service/service_config.go b/container-engine-lib/lib/backend_interface/objects/service/service_config.go index 0a9b234f1c..94560c1f22 100644 --- a/container-engine-lib/lib/backend_interface/objects/service/service_config.go +++ b/container-engine-lib/lib/backend_interface/objects/service/service_config.go @@ -218,7 +218,7 @@ func (serviceConfig *ServiceConfig) GetMinCPUAllocationMillicpus() uint64 { } func (serviceConfig *ServiceConfig) SetMinCPUAllocationMillicpus(cpuAllocation uint64) { - serviceConfig.privateServiceConfig.MemoryAllocationMegabytes = cpuAllocation + serviceConfig.privateServiceConfig.MinCpuAllocationMilliCpus = cpuAllocation } // only available for Kubernetes diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index c20cab6ffc..924bbf66b5 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -21,7 +21,6 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/startosis_packages" "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" "reflect" ) @@ -190,9 +189,8 @@ func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_a if builtin.interpretationTimeValueStore.ExistsNewServiceConfigForService(builtin.serviceName) { newServiceConfig, err := builtin.interpretationTimeValueStore.GetNewServiceConfig(builtin.serviceName) if err != nil { - return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '$s'.", builtin.serviceName) + return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '%s'.", builtin.serviceName) } - logrus.Info("USING NEW SERVICE CONFIG") builtin.serviceConfig = newServiceConfig } replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 592aa71435..591725edcb 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -148,7 +148,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt if err != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred while overriding service configs in set service for service: %v", builtin.serviceName) } - + builtin.serviceConfig = mergedServiceConfig builtin.interpretationTimeStore.SetServiceConfig(serviceName, mergedServiceConfig) builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(descriptionFormatStr, builtin.serviceName)) diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go index 196c079956..abc3b898eb 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go @@ -547,6 +547,149 @@ def run(plan): require.False(suite.T(), scheduledInstruction2.IsExecuted()) // set service should also be re-executed because it hasn't been run, but its a noop - its effect is to swap out the service config during interpretation time } +func (suite *StartosisInterpreterIdempotentTestSuite) TestStartosisInterpreterIdempotent_MultipleSetService() { + initialScript := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name = "example-datastore-server", config = config) +` + + // Interpretation of the initial script to generate the current enclave plan + _, currentEnclavePlan, interpretationApiErr := suite.interpreter.Interpret( + context.Background(), + startosis_constants.PackageIdPlaceholderForStandaloneScript, + useDefaultMainFunctionName, + noPackageReplaceOptions, + startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, + initialScript, + noInputParams, + defaultNonBlockingMode, + enclave_structure.NewEnclaveComponents(), + resolver.NewInstructionsPlanMask(0), + image_download_mode.ImageDownloadMode_Missing) + require.Nil(suite.T(), interpretationApiErr) + require.Equal(suite.T(), 1, currentEnclavePlan.Size()) + convertedEnclavePlan := suite.convertInstructionPlanToEnclavePlan(currentEnclavePlan) + + updatedScript := ` +def run(plan): + config = ServiceConfig( + image = "datastore-image", + ) + plan.add_service(name="example-datastore-server", config=config) + + newConfig = ServiceConfig( + image = "datastore-image:latest", + ) + plan.set_service(name="example-datastore-server", config=newConfig) + + newerConfig = ServiceConfig( + image = "datastore-image:latest", + min_cpu=1 + ) + plan.set_service(name="example-datastore-server", config=newerConfig) +` + + // Interpret the updated script against the current enclave plan + _, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan( + context.Background(), + startosis_constants.PackageIdPlaceholderForStandaloneScript, + noPackageReplaceOptions, + useDefaultMainFunctionName, + startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, + updatedScript, + noInputParams, + defaultNonBlockingMode, + convertedEnclavePlan, + image_download_mode.ImageDownloadMode_Missing, + ) + require.Nil(suite.T(), interpretationError) + + instructionSequence, err := instructionsPlan.GeneratePlan() + require.Nil(suite.T(), err) + require.Equal(suite.T(), 0, instructionsPlan.GetIndexOfFirstInstruction()) + require.Equal(suite.T(), 3, len(instructionSequence)) + + scheduledInstruction1 := instructionSequence[0] + require.Equal(suite.T(), `add_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image"))`, scheduledInstruction1.GetInstruction().String()) + require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the service config + + scheduledInstruction2 := instructionSequence[1] + require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest"))`, scheduledInstruction2.GetInstruction().String()) + require.False(suite.T(), scheduledInstruction2.IsExecuted()) // set service should be executed because it hasn't been run, but it's a noop - its effect is to swap out the service config during interpretation time + + scheduledInstruction3 := instructionSequence[2] + require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest", min_cpu=1))`, scheduledInstruction3.GetInstruction().String()) + require.False(suite.T(), scheduledInstruction3.IsExecuted()) // set service should be re-executed because it hasn't been run, but itss a noop - its effect is to swap out the service config during interpretation time + // TODO: ideally, we'd test that updated config for add_service is the most recent but we can't do that well now but we will be able to when making_set service work with ports + // TODO: in which case you can `get_service` and assert ports are from the most recent config +} + +//func (suite *StartosisInterpreterIdempotentTestSuite) TestStartosisInterpreterIdempotent_SetServiceNewScript() { +// initialScript := ` +//def run(plan): +// config = ServiceConfig( +// image = "datastore-image", +// ) +// plan.add_service(name = "example-datastore-server", config = config) +//` +// +// // Interpretation of the initial script to generate the current enclave plan +// _, currentEnclavePlan, interpretationApiErr := suite.interpreter.Interpret( +// context.Background(), +// startosis_constants.PackageIdPlaceholderForStandaloneScript, +// useDefaultMainFunctionName, +// noPackageReplaceOptions, +// startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, +// initialScript, +// noInputParams, +// defaultNonBlockingMode, +// enclave_structure.NewEnclaveComponents(), +// resolver.NewInstructionsPlanMask(0), +// image_download_mode.ImageDownloadMode_Missing) +// require.Nil(suite.T(), interpretationApiErr) +// require.Equal(suite.T(), 1, currentEnclavePlan.Size()) +// convertedEnclavePlan := suite.convertInstructionPlanToEnclavePlan(currentEnclavePlan) +// +// updatedScript := ` +//def run(plan): +// newConfig = ServiceConfig( +// image = "datastore-image:latest", +// ) +// plan.set_service(name="example-datastore-server", config=newConfig) +//` +// +// // Interpret the updated script against the current enclave plan +// _, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan( +// context.Background(), +// startosis_constants.PackageIdPlaceholderForStandaloneScript, +// noPackageReplaceOptions, +// useDefaultMainFunctionName, +// startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, +// updatedScript, +// noInputParams, +// defaultNonBlockingMode, +// convertedEnclavePlan, +// image_download_mode.ImageDownloadMode_Missing, +// ) +// require.Nil(suite.T(), interpretationError) +// +// instructionSequence, err := instructionsPlan.GeneratePlan() +// require.Nil(suite.T(), err) +// require.Equal(suite.T(), 0, instructionsPlan.GetIndexOfFirstInstruction()) +// require.Equal(suite.T(), 2, len(instructionSequence)) +// +// scheduledInstruction1 := instructionSequence[0] +// require.Equal(suite.T(), `add_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image"))`, scheduledInstruction1.GetInstruction().String()) +// require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the service config +// +// scheduledInstruction2 := instructionSequence[1] +// require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest"))`, scheduledInstruction2.GetInstruction().String()) +// require.False(suite.T(), scheduledInstruction2.IsExecuted()) // set service should also be re-executed because it hasn't been run, but its a noop - its effect is to swap out the service config during interpretation time +//} + func (suite *StartosisInterpreterIdempotentTestSuite) convertInstructionPlanToEnclavePlan(instructionPlan *instructions_plan.InstructionsPlan) *enclave_plan_persistence.EnclavePlan { enclavePlan := enclave_plan_persistence.NewEnclavePlan() instructionPlanSequence, interpretationErr := instructionPlan.GeneratePlan() From 74fab490d29a182694d7f88835c5a1c41ec5fc38 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 01:28:37 -0400 Subject: [PATCH 15/23] remove set service new script test --- .../startosis_interpreter_idempotent_test.go | 63 ------------------- 1 file changed, 63 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go index abc3b898eb..96ddcb8680 100644 --- a/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_interpreter_idempotent_test.go @@ -627,69 +627,6 @@ def run(plan): // TODO: in which case you can `get_service` and assert ports are from the most recent config } -//func (suite *StartosisInterpreterIdempotentTestSuite) TestStartosisInterpreterIdempotent_SetServiceNewScript() { -// initialScript := ` -//def run(plan): -// config = ServiceConfig( -// image = "datastore-image", -// ) -// plan.add_service(name = "example-datastore-server", config = config) -//` -// -// // Interpretation of the initial script to generate the current enclave plan -// _, currentEnclavePlan, interpretationApiErr := suite.interpreter.Interpret( -// context.Background(), -// startosis_constants.PackageIdPlaceholderForStandaloneScript, -// useDefaultMainFunctionName, -// noPackageReplaceOptions, -// startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, -// initialScript, -// noInputParams, -// defaultNonBlockingMode, -// enclave_structure.NewEnclaveComponents(), -// resolver.NewInstructionsPlanMask(0), -// image_download_mode.ImageDownloadMode_Missing) -// require.Nil(suite.T(), interpretationApiErr) -// require.Equal(suite.T(), 1, currentEnclavePlan.Size()) -// convertedEnclavePlan := suite.convertInstructionPlanToEnclavePlan(currentEnclavePlan) -// -// updatedScript := ` -//def run(plan): -// newConfig = ServiceConfig( -// image = "datastore-image:latest", -// ) -// plan.set_service(name="example-datastore-server", config=newConfig) -//` -// -// // Interpret the updated script against the current enclave plan -// _, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan( -// context.Background(), -// startosis_constants.PackageIdPlaceholderForStandaloneScript, -// noPackageReplaceOptions, -// useDefaultMainFunctionName, -// startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript, -// updatedScript, -// noInputParams, -// defaultNonBlockingMode, -// convertedEnclavePlan, -// image_download_mode.ImageDownloadMode_Missing, -// ) -// require.Nil(suite.T(), interpretationError) -// -// instructionSequence, err := instructionsPlan.GeneratePlan() -// require.Nil(suite.T(), err) -// require.Equal(suite.T(), 0, instructionsPlan.GetIndexOfFirstInstruction()) -// require.Equal(suite.T(), 2, len(instructionSequence)) -// -// scheduledInstruction1 := instructionSequence[0] -// require.Equal(suite.T(), `add_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image"))`, scheduledInstruction1.GetInstruction().String()) -// require.False(suite.T(), scheduledInstruction1.IsExecuted()) // the add service needs to be re-executed as the `set_service` changed the service config -// -// scheduledInstruction2 := instructionSequence[1] -// require.Equal(suite.T(), `set_service(name="example-datastore-server", config=ServiceConfig(image="datastore-image:latest"))`, scheduledInstruction2.GetInstruction().String()) -// require.False(suite.T(), scheduledInstruction2.IsExecuted()) // set service should also be re-executed because it hasn't been run, but its a noop - its effect is to swap out the service config during interpretation time -//} - func (suite *StartosisInterpreterIdempotentTestSuite) convertInstructionPlanToEnclavePlan(instructionPlan *instructions_plan.InstructionsPlan) *enclave_plan_persistence.EnclavePlan { enclavePlan := enclave_plan_persistence.NewEnclavePlan() instructionPlanSequence, interpretationErr := instructionPlan.GeneratePlan() From 41763583e0b896f827349e479e23b4a89d3e8d63 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 01:46:40 -0400 Subject: [PATCH 16/23] make validate a shared helper --- .../add_service/add_service.go | 35 +---------------- .../set_service/set_service.go | 4 +- .../shared_helpers/service_helpers.go | 38 +++++++++++++++++++ 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 924bbf66b5..903b30c358 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -9,6 +9,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" + "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" @@ -140,7 +141,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfig, readyCondition, interpretationErr := ValidateAndConvertConfigAndReadyCondition( + apiServiceConfig, readyCondition, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfig, locatorOfModuleInWhichThisBuiltInIsBeingCalled, @@ -323,35 +324,3 @@ func (builtin *AddServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) func (builtin *AddServiceCapabilities) Description() string { return builtin.description } - -func ValidateAndConvertConfigAndReadyCondition( - serviceNetwork service_network.ServiceNetwork, - rawConfig starlark.Value, - locatorOfModuleInWhichThisBuiltInIsBeingCalled string, - packageId string, - packageContentProvider startosis_packages.PackageContentProvider, - packageReplaceOptions map[string]string, - imageDownloadMode image_download_mode.ImageDownloadMode, -) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { - config, ok := rawConfig.(*service_config.ServiceConfig) - if !ok { - return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", ConfigsArgName, reflect.TypeOf(rawConfig)) - } - apiServiceConfig, interpretationErr := config.ToKurtosisType( - serviceNetwork, - locatorOfModuleInWhichThisBuiltInIsBeingCalled, - packageId, - packageContentProvider, - packageReplaceOptions, - imageDownloadMode) - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - readyCondition, interpretationErr := config.GetReadyCondition() - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - return apiServiceConfig, readyCondition, nil -} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 591725edcb..eaedadec5a 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -9,7 +9,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service" + "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" @@ -125,7 +125,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfigOverride, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( + apiServiceConfigOverride, _, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfigOverride, locatorOfModuleInWhichThisBuiltInIsBeingCalled, diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go index 706a067989..cfd1e69330 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go @@ -2,14 +2,20 @@ package shared_helpers import ( "context" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" "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/add_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/verify" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types/service_config" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/recipe" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store" + "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_packages" "github.com/kurtosis-tech/stacktrace" "go.starlark.net/starlark" + "reflect" "time" ) @@ -147,3 +153,35 @@ func executeServiceAssertionWithRecipeWithTicker( } } } + +func ValidateAndConvertConfigAndReadyCondition( + serviceNetwork service_network.ServiceNetwork, + rawConfig starlark.Value, + locatorOfModuleInWhichThisBuiltInIsBeingCalled string, + packageId string, + packageContentProvider startosis_packages.PackageContentProvider, + packageReplaceOptions map[string]string, + imageDownloadMode image_download_mode.ImageDownloadMode, +) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { + config, ok := rawConfig.(*service_config.ServiceConfig) + if !ok { + return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", add_service.ConfigsArgName, reflect.TypeOf(rawConfig)) + } + apiServiceConfig, interpretationErr := config.ToKurtosisType( + serviceNetwork, + locatorOfModuleInWhichThisBuiltInIsBeingCalled, + packageId, + packageContentProvider, + packageReplaceOptions, + imageDownloadMode) + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + readyCondition, interpretationErr := config.GetReadyCondition() + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + return apiServiceConfig, readyCondition, nil +} From 582e268778fa6550e3d65108537a700379612081 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 01:46:56 -0400 Subject: [PATCH 17/23] clarify docs --- docs/docs/api-reference/starlark-reference/plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/api-reference/starlark-reference/plan.md b/docs/docs/api-reference/starlark-reference/plan.md index 56c43da6b6..85ef55eb1d 100644 --- a/docs/docs/api-reference/starlark-reference/plan.md +++ b/docs/docs/api-reference/starlark-reference/plan.md @@ -177,7 +177,7 @@ def run(plan, args): postgres.run(plan) # Set service detects the "postgres" service in the plan and - # Overrides the ServiceConfig set in the postgres package to use the provided image value + # Overrides the ServiceConfig set in the `add_service` instruction of postgres package to use the new image value plan.set_service( name="postgres", config=ServiceConfig( From 4a7cce8c503ea238f5d736cc391fe43a07ec3caa Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 01:49:45 -0400 Subject: [PATCH 18/23] update description of add service --- .../kurtosis_instruction/add_service/add_service.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 903b30c358..8b685f5ab0 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -185,7 +185,7 @@ func (builtin *AddServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu return nil } -func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { +func (builtin *AddServiceCapabilities) Execute(ctx context.Context, arguments *builtin_argument.ArgumentValuesSet) (string, error) { // update service config to use new service config set by a set_service instruction, if one exists if builtin.interpretationTimeValueStore.ExistsNewServiceConfigForService(builtin.serviceName) { newServiceConfig, err := builtin.interpretationTimeValueStore.GetNewServiceConfig(builtin.serviceName) @@ -193,6 +193,7 @@ func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_a return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '%s'.", builtin.serviceName) } builtin.serviceConfig = newServiceConfig + builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(addServiceDescriptionFormatStr, builtin.serviceName, newServiceConfig.GetContainerImageName())) } replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) if err != nil { From ab4291af62ee78e0f63571f374575a35baa7a1b4 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 02:00:39 -0400 Subject: [PATCH 19/23] Revert "make validate a shared helper" This reverts commit 41763583e0b896f827349e479e23b4a89d3e8d63. --- .../add_service/add_service.go | 35 ++++++++++++++++- .../set_service/set_service.go | 4 +- .../shared_helpers/service_helpers.go | 38 ------------------- 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 8b685f5ab0..c51d73fb4f 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -9,7 +9,6 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" - "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" @@ -141,7 +140,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfig, readyCondition, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( + apiServiceConfig, readyCondition, interpretationErr := ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfig, locatorOfModuleInWhichThisBuiltInIsBeingCalled, @@ -325,3 +324,35 @@ func (builtin *AddServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) func (builtin *AddServiceCapabilities) Description() string { return builtin.description } + +func ValidateAndConvertConfigAndReadyCondition( + serviceNetwork service_network.ServiceNetwork, + rawConfig starlark.Value, + locatorOfModuleInWhichThisBuiltInIsBeingCalled string, + packageId string, + packageContentProvider startosis_packages.PackageContentProvider, + packageReplaceOptions map[string]string, + imageDownloadMode image_download_mode.ImageDownloadMode, +) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { + config, ok := rawConfig.(*service_config.ServiceConfig) + if !ok { + return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", ConfigsArgName, reflect.TypeOf(rawConfig)) + } + apiServiceConfig, interpretationErr := config.ToKurtosisType( + serviceNetwork, + locatorOfModuleInWhichThisBuiltInIsBeingCalled, + packageId, + packageContentProvider, + packageReplaceOptions, + imageDownloadMode) + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + readyCondition, interpretationErr := config.GetReadyCondition() + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + return apiServiceConfig, readyCondition, nil +} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index eaedadec5a..591725edcb 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -9,7 +9,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" - "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_instruction/add_service" "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" @@ -125,7 +125,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfigOverride, _, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( + apiServiceConfigOverride, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfigOverride, locatorOfModuleInWhichThisBuiltInIsBeingCalled, diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go index cfd1e69330..706a067989 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go @@ -2,20 +2,14 @@ package shared_helpers import ( "context" - "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" "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/add_service" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/verify" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types/service_config" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/recipe" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store" - "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_packages" "github.com/kurtosis-tech/stacktrace" "go.starlark.net/starlark" - "reflect" "time" ) @@ -153,35 +147,3 @@ func executeServiceAssertionWithRecipeWithTicker( } } } - -func ValidateAndConvertConfigAndReadyCondition( - serviceNetwork service_network.ServiceNetwork, - rawConfig starlark.Value, - locatorOfModuleInWhichThisBuiltInIsBeingCalled string, - packageId string, - packageContentProvider startosis_packages.PackageContentProvider, - packageReplaceOptions map[string]string, - imageDownloadMode image_download_mode.ImageDownloadMode, -) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { - config, ok := rawConfig.(*service_config.ServiceConfig) - if !ok { - return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", add_service.ConfigsArgName, reflect.TypeOf(rawConfig)) - } - apiServiceConfig, interpretationErr := config.ToKurtosisType( - serviceNetwork, - locatorOfModuleInWhichThisBuiltInIsBeingCalled, - packageId, - packageContentProvider, - packageReplaceOptions, - imageDownloadMode) - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - readyCondition, interpretationErr := config.GetReadyCondition() - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - return apiServiceConfig, readyCondition, nil -} From 3c2dd90a48f9fb34ca649e6c19bcaf45cea38893 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 02:31:04 -0400 Subject: [PATCH 20/23] Revert "update description of add service" This reverts commit 4a7cce8c503ea238f5d736cc391fe43a07ec3caa. --- .../kurtosis_instruction/add_service/add_service.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index c51d73fb4f..924bbf66b5 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -184,7 +184,7 @@ func (builtin *AddServiceCapabilities) Validate(_ *builtin_argument.ArgumentValu return nil } -func (builtin *AddServiceCapabilities) Execute(ctx context.Context, arguments *builtin_argument.ArgumentValuesSet) (string, error) { +func (builtin *AddServiceCapabilities) Execute(ctx context.Context, _ *builtin_argument.ArgumentValuesSet) (string, error) { // update service config to use new service config set by a set_service instruction, if one exists if builtin.interpretationTimeValueStore.ExistsNewServiceConfigForService(builtin.serviceName) { newServiceConfig, err := builtin.interpretationTimeValueStore.GetNewServiceConfig(builtin.serviceName) @@ -192,7 +192,6 @@ func (builtin *AddServiceCapabilities) Execute(ctx context.Context, arguments *b return "", stacktrace.Propagate(err, "An error occurred retrieving a new service config '%s'.", builtin.serviceName) } builtin.serviceConfig = newServiceConfig - builtin.description = builtin_argument.GetDescriptionOrFallBack(arguments, fmt.Sprintf(addServiceDescriptionFormatStr, builtin.serviceName, newServiceConfig.GetContainerImageName())) } replacedServiceName, replacedServiceConfig, err := replaceMagicStrings(builtin.runtimeValueStore, builtin.serviceName, builtin.serviceConfig) if err != nil { From 0bba39d387eecb1c1c29105181ff9550b6874123 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 09:36:35 -0400 Subject: [PATCH 21/23] add tests --- .../interpretation_time_value_store_test.go | 107 ++++++++++++++++++ .../set_service/set_service.go | 2 +- 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store_test.go diff --git a/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store_test.go b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store_test.go new file mode 100644 index 0000000000..49ef32590d --- /dev/null +++ b/core/server/api_container/server/startosis_engine/interpretation_time_value_store/interpretation_time_value_store_test.go @@ -0,0 +1,107 @@ +package interpretation_time_value_store + +import ( + "fmt" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/database_accessors/enclave_db" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers" + "github.com/stretchr/testify/require" + bolt "go.etcd.io/bbolt" + "os" + "testing" +) + +const ( + testServiceName = service.ServiceName("datastore-service") + testContainerImageName = "datastore-image" + enclaveDbFilePerm = 0666 +) + +func TestGetServiceConfigReturnsError(t *testing.T) { + enclaveDb := getEnclaveDBForTest(t) + dummySerde := shared_helpers.NewDummyStarlarkValueSerDeForTest() + itvs, err := CreateInterpretationTimeValueStore(enclaveDb, dummySerde) + require.NoError(t, err) + + // no service config exists in store + _, err = itvs.GetServiceConfig(testServiceName) + require.Error(t, err) +} + +func TestPutServiceConfig(t *testing.T) { + enclaveDb := getEnclaveDBForTest(t) + dummySerde := shared_helpers.NewDummyStarlarkValueSerDeForTest() + itvs, err := CreateInterpretationTimeValueStore(enclaveDb, dummySerde) + require.NoError(t, err) + + expectedServiceConfig, err := getTestServiceConfigForService(testServiceName, "latest") + require.NoError(t, err) + + itvs.PutServiceConfig(testServiceName, expectedServiceConfig) + + actualServiceConfig, err := itvs.GetServiceConfig(testServiceName) + require.NoError(t, err) + require.Equal(t, expectedServiceConfig.GetContainerImageName(), actualServiceConfig.GetContainerImageName()) +} + +func TestPutNewServiceConfig(t *testing.T) { + enclaveDb := getEnclaveDBForTest(t) + dummySerde := shared_helpers.NewDummyStarlarkValueSerDeForTest() + itvs, err := CreateInterpretationTimeValueStore(enclaveDb, dummySerde) + require.NoError(t, err) + + oldServiceConfig, err := getTestServiceConfigForService(testServiceName, "older") + require.NoError(t, err) + itvs.PutServiceConfig(testServiceName, oldServiceConfig) + + newerServiceConfig, err := getTestServiceConfigForService(testServiceName, "latest") + require.NoError(t, err) + itvs.SetServiceConfig(testServiceName, newerServiceConfig) + + actualNewerServiceConfig, err := itvs.GetNewServiceConfig(testServiceName) + require.NoError(t, err) + require.Equal(t, newerServiceConfig.GetContainerImageName(), actualNewerServiceConfig.GetContainerImageName()) +} + +func getTestServiceConfigForService(name service.ServiceName, imageTag string) (*service.ServiceConfig, error) { + return service.CreateServiceConfig( + fmt.Sprintf("%v-%v:%v", name, testContainerImageName, imageTag), + nil, + nil, + nil, + nil, + nil, + []string{}, + []string{}, + map[string]string{}, + nil, + nil, + 0, + 0, + "IP-ADDRESS", + 0, + 0, + map[string]string{}, + nil, + nil, + nil, + image_download_mode.ImageDownloadMode_Always) +} + +func getEnclaveDBForTest(t *testing.T) *enclave_db.EnclaveDB { + file, err := os.CreateTemp("/tmp", "*.db") + defer func() { + err = os.Remove(file.Name()) + require.NoError(t, err) + }() + + require.NoError(t, err) + db, err := bolt.Open(file.Name(), enclaveDbFilePerm, nil) + require.NoError(t, err) + enclaveDb := &enclave_db.EnclaveDB{ + DB: db, + } + + return enclaveDb +} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 591725edcb..36f98d41b5 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -28,7 +28,7 @@ const ( ServiceNameArgName = "name" SetServiceConfigArgName = "config" - descriptionFormatStr = "Updating config of service '%v'" + descriptionFormatStr = "Setting config of service '%v'" ) func NewSetService( From 35aa6239f0f2ccd7f58720fea5fe88af6aaa1c6a Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 09:42:12 -0400 Subject: [PATCH 22/23] add note --- .../kurtosis_instruction/set_service/set_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 36f98d41b5..7cd8e7e5cb 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -138,7 +138,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, interpretationErr } - // get existing service config for service and merge with apiServiceConfigOverride + // get original service config for service and merge with apiServiceConfigOverride currApiServiceConfig, err := builtin.interpretationTimeStore.GetServiceConfig(builtin.serviceName) if err != nil { return nil, startosis_errors.WrapWithInterpretationError(err, "An error occurred retrieving service config for service: %v'", builtin.serviceName) From 881e8362b410f53cf4582126e463c15bfba9b177 Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 25 Apr 2024 09:50:26 -0400 Subject: [PATCH 23/23] make validate a shared helper --- .../add_service/add_service.go | 35 +---------------- .../set_service/set_service.go | 4 +- .../shared_helpers/service_helpers.go | 38 +++++++++++++++++++ 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go index 924bbf66b5..903b30c358 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service/add_service.go @@ -9,6 +9,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" + "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" @@ -140,7 +141,7 @@ func (builtin *AddServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfig, readyCondition, interpretationErr := ValidateAndConvertConfigAndReadyCondition( + apiServiceConfig, readyCondition, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfig, locatorOfModuleInWhichThisBuiltInIsBeingCalled, @@ -323,35 +324,3 @@ func (builtin *AddServiceCapabilities) UpdatePlan(planYaml *plan_yaml.PlanYaml) func (builtin *AddServiceCapabilities) Description() string { return builtin.description } - -func ValidateAndConvertConfigAndReadyCondition( - serviceNetwork service_network.ServiceNetwork, - rawConfig starlark.Value, - locatorOfModuleInWhichThisBuiltInIsBeingCalled string, - packageId string, - packageContentProvider startosis_packages.PackageContentProvider, - packageReplaceOptions map[string]string, - imageDownloadMode image_download_mode.ImageDownloadMode, -) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { - config, ok := rawConfig.(*service_config.ServiceConfig) - if !ok { - return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", ConfigsArgName, reflect.TypeOf(rawConfig)) - } - apiServiceConfig, interpretationErr := config.ToKurtosisType( - serviceNetwork, - locatorOfModuleInWhichThisBuiltInIsBeingCalled, - packageId, - packageContentProvider, - packageReplaceOptions, - imageDownloadMode) - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - readyCondition, interpretationErr := config.GetReadyCondition() - if interpretationErr != nil { - return nil, nil, interpretationErr - } - - return apiServiceConfig, readyCondition, nil -} diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go index 7cd8e7e5cb..340efc7c9e 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/set_service/set_service.go @@ -9,7 +9,7 @@ import ( "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_plan_persistence" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/enclave_structure" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/interpretation_time_value_store" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/add_service" + "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" @@ -125,7 +125,7 @@ func (builtin *SetServiceCapabilities) Interpret(locatorOfModuleInWhichThisBuilt return nil, startosis_errors.NewInterpretationError("Unable to extract image attribute off of service config.") } builtin.imageVal = rawImageVal - apiServiceConfigOverride, _, interpretationErr := add_service.ValidateAndConvertConfigAndReadyCondition( + apiServiceConfigOverride, _, interpretationErr := shared_helpers.ValidateAndConvertConfigAndReadyCondition( builtin.serviceNetwork, serviceConfigOverride, locatorOfModuleInWhichThisBuiltInIsBeingCalled, diff --git a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go index 706a067989..1f209cc600 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_instruction/shared_helpers/service_helpers.go @@ -2,20 +2,26 @@ package shared_helpers import ( "context" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/image_download_mode" "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/verify" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_types/service_config" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/recipe" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/runtime_value_store" + "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_packages" "github.com/kurtosis-tech/stacktrace" "go.starlark.net/starlark" + "reflect" "time" ) const ( bufferedChannelSize = 2 starlarkThreadName = "starlark-value-serde-for-test-thread" + configArgName = "config" ) func NewDummyStarlarkValueSerDeForTest() *kurtosis_types.StarlarkValueSerde { @@ -147,3 +153,35 @@ func executeServiceAssertionWithRecipeWithTicker( } } } + +func ValidateAndConvertConfigAndReadyCondition( + serviceNetwork service_network.ServiceNetwork, + rawConfig starlark.Value, + locatorOfModuleInWhichThisBuiltInIsBeingCalled string, + packageId string, + packageContentProvider startosis_packages.PackageContentProvider, + packageReplaceOptions map[string]string, + imageDownloadMode image_download_mode.ImageDownloadMode, +) (*service.ServiceConfig, *service_config.ReadyCondition, *startosis_errors.InterpretationError) { + config, ok := rawConfig.(*service_config.ServiceConfig) + if !ok { + return nil, nil, startosis_errors.NewInterpretationError("The '%s' argument is not a ServiceConfig (was '%s').", configArgName, reflect.TypeOf(rawConfig)) + } + apiServiceConfig, interpretationErr := config.ToKurtosisType( + serviceNetwork, + locatorOfModuleInWhichThisBuiltInIsBeingCalled, + packageId, + packageContentProvider, + packageReplaceOptions, + imageDownloadMode) + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + readyCondition, interpretationErr := config.GetReadyCondition() + if interpretationErr != nil { + return nil, nil, interpretationErr + } + + return apiServiceConfig, readyCondition, nil +}