From b1df3c7ec6f14920c6b5d6b40d10d1ec00497ebd Mon Sep 17 00:00:00 2001 From: Tedi Mitiku Date: Thu, 21 Mar 2024 15:14:51 -0400 Subject: [PATCH] address comments --- .../user_services_functions/start_user_services.go | 9 ++++----- .../docker/docker_manager/docker_manager.go | 2 +- .../test_engine/service_config_full_framework_test.go | 5 +++-- .../test_engine/static_constants.go | 2 ++ .../kurtosis_types/service_config/service_config.go | 3 --- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go index 7b78e4d407..d229461293 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go @@ -555,13 +555,12 @@ func createStartServiceOperation( envVars[key] = strings.Replace(envVars[key], privateIPAddrPlaceholder, privateIPAddrStr, unlimitedReplacements) } - // THIS IS A HACK - // TODO clean this up + // TODO clean this hack up // this path will only be hit if `files_to_be_moved` is set in Starlark; which is a hidden property // used by compose transpilation if len(filesToBeMoved) > 0 { var err error - cmdArgs, entrypointArgs, err = handleFilesToBeMovedForDockerCompose(ctx, dockerManager, containerImageName, cmdArgs, entrypointArgs, filesToBeMoved) + cmdArgs, entrypointArgs, err = getUpdatedEntrypointAndCmdFromFilesToBeMoved(ctx, dockerManager, containerImageName, cmdArgs, entrypointArgs, filesToBeMoved) if err != nil { return nil, stacktrace.Propagate(err, "an error occurred while handling files for compose") } @@ -769,7 +768,7 @@ func createStartServiceOperation( } // TODO - clean this up this is super janky, a way to handle compose & volume -func handleFilesToBeMovedForDockerCompose(ctx context.Context, dockerManager *docker_manager.DockerManager, containerImageName string, cmdArgs []string, entrypointArgs []string, filesToBeMoved map[string]string) ([]string, []string, error) { +func getUpdatedEntrypointAndCmdFromFilesToBeMoved(ctx context.Context, dockerManager *docker_manager.DockerManager, containerImageName string, cmdArgs []string, entrypointArgs []string, filesToBeMoved map[string]string) ([]string, []string, error) { concatenatedFilesToBeMoved := []string{} for source, destination := range filesToBeMoved { // TODO improve this; the first condition handles files the other folders @@ -777,7 +776,7 @@ func handleFilesToBeMovedForDockerCompose(ctx context.Context, dockerManager *do } concatenatedFilesToBeMovedAsStr := strings.Join(concatenatedFilesToBeMoved, " && ") - originalEntrypointArgs, originalCmdArgs, err := dockerManager.GetOriginalEntryPointAndCommand(ctx, containerImageName) + originalEntrypointArgs, originalCmdArgs, err := dockerManager.GetEntryPointAndCommand(ctx, containerImageName) if err != nil { return nil, nil, stacktrace.Propagate(err, "an error occurred fetching data about image '%v'", containerImageName) } diff --git a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go index fe20e4c0b7..35f9567ea8 100644 --- a/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go +++ b/container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go @@ -1660,7 +1660,7 @@ func (manager *DockerManager) getImagePlatform(ctx context.Context, imageName st return imageInspect.Architecture, nil } -func (manager *DockerManager) GetOriginalEntryPointAndCommand(ctx context.Context, imageName string) ([]string, []string, error) { +func (manager *DockerManager) GetEntryPointAndCommand(ctx context.Context, imageName string) ([]string, []string, error) { imageInspect, _, err := manager.dockerClient.ImageInspectWithRaw(ctx, imageName) if err != nil { return nil, nil, stacktrace.Propagate(err, "an error occurred while running image inspect on image '%v'", imageName) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/service_config_full_framework_test.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/service_config_full_framework_test.go index c755afb263..535fcdfda8 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/service_config_full_framework_test.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/service_config_full_framework_test.go @@ -39,7 +39,7 @@ func (t *serviceConfigFullTestCase) GetStarlarkCode() string { fileArtifact1 := fmt.Sprintf("%s(%s=[%q])", directory.DirectoryTypeName, directory.ArtifactNamesAttr, testFilesArtifactName1) fileArtifact2 := fmt.Sprintf("%s(%s=[%q])", directory.DirectoryTypeName, directory.ArtifactNamesAttr, testFilesArtifactName2) persistentDirectory := fmt.Sprintf("%s(%s=%q)", directory.DirectoryTypeName, directory.PersistentKeyAttr, testPersistentDirectoryKey) - starlarkCode := fmt.Sprintf("%s(%s=%q, %s=%s, %s=%s, %s=%s, %s=%s, %s=%s, %s=%s, %s=%q, %s=%d, %s=%d, %s=%d, %s=%d, %s=%s, %s=%v, %s=%v, files_to_be_moved={\"/foo/bar\": \"/doo/dar\"})", + starlarkCode := fmt.Sprintf("%s(%s=%q, %s=%s, %s=%s, %s=%s, %s=%s, %s=%s, %s=%s, %s=%q, %s=%d, %s=%d, %s=%d, %s=%d, %s=%s, %s=%v, %s=%v, %s=%v)", service_config.ServiceConfigTypeName, service_config.ImageAttr, testContainerImageName, service_config.PortsAttr, fmt.Sprintf("{%q: PortSpec(number=%d, transport_protocol=%q, application_protocol=%q, wait=%q)}", testPrivatePortId, testPrivatePortNumber, testPrivatePortProtocolStr, testPrivateApplicationProtocol, testWaitConfiguration), @@ -56,7 +56,8 @@ func (t *serviceConfigFullTestCase) GetStarlarkCode() string { service_config.ReadyConditionsAttr, getDefaultReadyConditionsScriptPart(), service_config.LabelsAttr, fmt.Sprintf("{%q: %q, %q: %q}", testServiceConfigLabelsKey1, testServiceConfigLabelsValue1, testServiceConfigLabelsKey2, testServiceConfigLabelsValue2), - service_config.NodeSelectorsAttr, fmt.Sprintf("{%q: %q}", testNodeSelectorKey1, testNodeSelectorValue1)) + service_config.NodeSelectorsAttr, fmt.Sprintf("{%q: %q}", testNodeSelectorKey1, testNodeSelectorValue1), + service_config.FilesToBeMovedAttr, fmt.Sprintf("{%q: %q}", testFilesToBeMoved, testFilesToBeMoved)) return starlarkCode } diff --git a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/static_constants.go b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/static_constants.go index 3602872ab8..9037358714 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/static_constants.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_starlark_framework/test_engine/static_constants.go @@ -148,4 +148,6 @@ var ( testTolerationKey = "test-key" testTolerationValue = "test-value" testTolerationSeconds = int64(64) + + testFilesToBeMoved = "test.txt" ) diff --git a/core/server/api_container/server/startosis_engine/kurtosis_types/service_config/service_config.go b/core/server/api_container/server/startosis_engine/kurtosis_types/service_config/service_config.go index 16adbe3e4d..54145d6145 100644 --- a/core/server/api_container/server/startosis_engine/kurtosis_types/service_config/service_config.go +++ b/core/server/api_container/server/startosis_engine/kurtosis_types/service_config/service_config.go @@ -4,7 +4,6 @@ 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/image_registry_spec" - "github.com/sirupsen/logrus" "math" "path" @@ -520,13 +519,11 @@ func (config *ServiceConfig) ToKurtosisType( return nil, interpretationErr } if found && filesToBeMovedStarlark.Len() > 0 { - logrus.Info("this is confusing") filesToBeMoved, interpretationErr = kurtosis_types.SafeCastToMapStringString(filesToBeMovedStarlark, FilesToBeMovedAttr) if interpretationErr != nil { return nil, interpretationErr } } - logrus.Infof("the attribute is '%v'", filesToBeMoved) serviceConfig, err := service.CreateServiceConfig( imageName,