Skip to content

Commit

Permalink
Fix idempotent plan resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
guillaume.bouvignies committed Aug 9, 2023
1 parent 80623cf commit c7ef5c2
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 101 deletions.
5 changes: 1 addition & 4 deletions cli/cli/commands/files/rendertemplate/rendertemplate.go
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/google/uuid"
"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/engine/kurtosis_engine_rpc_api_bindings"
Expand Down Expand Up @@ -54,7 +53,6 @@ def run(plan, args):
),
}
)
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single render_templates above won't get cached
`

Expand All @@ -68,7 +66,6 @@ def run(plan, args):
),
}
)
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single render_templates above won't get cached
`

doNotDryRun = false
Expand Down Expand Up @@ -227,7 +224,7 @@ func renderTemplateStarlarkCommand(ctx context.Context, enclaveCtx *enclaves.Enc
if err != nil {
return "", stacktrace.Propagate(err, "An error has occurred when parsing input params to render template Starlark command")
}
params := fmt.Sprintf(`{"file_name": "%s", "template": "%s", "template_data": %s, "name": "%s", "uuid": "%s"}`, destRelFilepath, templateFileContents, string(templateDataBytes), artifactName, uuid.New().String())
params := fmt.Sprintf(`{"file_name": "%s", "template": "%s", "template_data": %s, "name": "%s"}`, destRelFilepath, templateFileContents, string(templateDataBytes), artifactName)
runResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, template, params, doNotDryRun, noParallelism, noExperimentalFeature)
if runResult.ExecutionError != nil {
return "", stacktrace.NewError("An error occurred during Starlark script execution for rendering template: %s", runResult.ExecutionError.GetErrorMessage())
Expand Down
5 changes: 1 addition & 4 deletions cli/cli/commands/files/storeservice/storeservice.go
Expand Up @@ -3,7 +3,6 @@ package storeservice
import (
"context"
"fmt"
"github.com/google/uuid"
"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/services"
Expand Down Expand Up @@ -44,7 +43,6 @@ def run(plan, args):
name = args["name"],
service_name = args["service_name"],
)
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single store_service_files above won't get cached
`

starlarkTemplateWithoutArtifactName = `
Expand All @@ -53,7 +51,6 @@ def run(plan, args):
src = args["src"],
service_name = args["service_name"],
)
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single store_service_files above won't get cached
`
noParallelism = 1

Expand Down Expand Up @@ -163,7 +160,7 @@ func storeServiceFileStarlarkCommand(ctx context.Context, enclaveCtx *enclaves.E
if artifactName == defaultName {
template = starlarkTemplateWithoutArtifactName
}
params := fmt.Sprintf(`{"service_name": "%s", "src": "%s", "name": "%s", "uuid": "%s"}`, serviceName, filePath, artifactName, uuid.New().String())
params := fmt.Sprintf(`{"service_name": "%s", "src": "%s", "name": "%s"}`, serviceName, filePath, artifactName)
runResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, template, params, false, noParallelism, noExperimentalFeature)
if err != nil {
return nil, stacktrace.Propagate(
Expand Down
1 change: 0 additions & 1 deletion cli/cli/commands/service/add/add.go
Expand Up @@ -311,7 +311,6 @@ func run(

starlarkScript := fmt.Sprintf(`def run(plan):

Check failure on line 312 in cli/cli/commands/service/add/add.go

View workflow job for this annotation

GitHub Actions / golang-lint

printf: fmt.Sprintf call needs 2 args but has 3 args (govet)
plan.add_service(name = "%s", config = %s)
plan.print("%s") # we add this print of a random UUID to make sure the single add_service above won't get cached
`, serviceName, serviceConfigStarlark, uuid.New().String())
starlarkRunResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, starlarkScript, "{}", false, defaultParallelism, noExperimentalFeature)
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions cli/cli/commands/service/rm/rm.go
Expand Up @@ -3,7 +3,6 @@ package rm
import (
"context"
"fmt"
"github.com/google/uuid"
"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/services"
Expand Down Expand Up @@ -35,7 +34,6 @@ const (
starlarkScript = `
def run(plan, args):
plan.remove_service(name=args["service_name"])
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single remove_service above won't get cached
`
doNotDryRun = false
defaultParallelism = 4
Expand Down Expand Up @@ -112,7 +110,7 @@ func run(
}

func removeServiceStarlarkCommand(ctx context.Context, enclaveCtx *enclaves.EnclaveContext, serviceName services.ServiceName) error {
params := fmt.Sprintf(`{"service_name": "%s", "uuid": "%s"}`, serviceName, uuid.New().String())
params := fmt.Sprintf(`{"service_name": "%s"}`, serviceName)
runResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, starlarkScript, params, doNotDryRun, defaultParallelism, noExperimentalFeature)
if err != nil {
return stacktrace.Propagate(err, "An unexpected error occurred on Starlark for rendering template")
Expand Down
4 changes: 1 addition & 3 deletions cli/cli/commands/service/start/start.go
Expand Up @@ -3,7 +3,6 @@ package start
import (
"context"
"fmt"
"github.com/google/uuid"
"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/services"
Expand Down Expand Up @@ -35,7 +34,6 @@ const (
starlarkScript = `
def run(plan, args):
plan.start_service(name=args["service_name"])
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single start_service above won't get cached
`
doNotDryRun = false
defaultParallelism = 4
Expand Down Expand Up @@ -112,7 +110,7 @@ func run(
}

func startServiceStarlarkCommand(ctx context.Context, enclaveCtx *enclaves.EnclaveContext, serviceName services.ServiceName) error {
serviceNameString := fmt.Sprintf(`{"service_name": "%s", "uuid": "%s"}`, serviceName, uuid.New().String())
serviceNameString := fmt.Sprintf(`{"service_name": "%s"}`, serviceName)
runResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, starlarkScript, serviceNameString, doNotDryRun, defaultParallelism, noExperimentalFeature)
if err != nil {
return stacktrace.Propagate(err, "An unexpected error occurred on Starlark for starting service")
Expand Down
4 changes: 1 addition & 3 deletions cli/cli/commands/service/stop/stop.go
Expand Up @@ -3,7 +3,6 @@ package stop
import (
"context"
"fmt"
"github.com/google/uuid"
"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/services"
Expand Down Expand Up @@ -35,7 +34,6 @@ const (
starlarkScript = `
def run(plan, args):
plan.stop_service(name=args["service_name"])
plan.print(args["uuid"]) # we add this print of a random UUID to make sure the single stop_service above won't get cached
`
doNotDryRun = false
defaultParallelism = 4
Expand Down Expand Up @@ -112,7 +110,7 @@ func run(
}

func stopServiceStarlarkCommand(ctx context.Context, enclaveCtx *enclaves.EnclaveContext, serviceName services.ServiceName) error {
params := fmt.Sprintf(`{"service_name": "%s", "uuid": "%s"}`, serviceName, uuid.New().String())
params := fmt.Sprintf(`{"service_name": "%s"}`, serviceName)
runResult, err := enclaveCtx.RunStarlarkScriptBlocking(ctx, useDefaultMainFile, starlarkScript, params, doNotDryRun, defaultParallelism, noExperimentalFeature)
if err != nil {
return stacktrace.Propagate(err, "An unexpected error occurred on Starlark for stopping service")
Expand Down
Expand Up @@ -122,7 +122,11 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan(
// 4. Run the interpretation with the mask.
// - If it's successful, then we've found the optimized plan
// - if it's not successful, then the mask is not compatible with the package. Go back to step 1
firstPossibleIndexForMatchingInstruction := 0
var firstPossibleIndexForMatchingInstruction int
if currentEnclavePlan.Size() > naiveInstructionsPlan.Size() {

firstPossibleIndexForMatchingInstruction = currentEnclavePlan.Size() - naiveInstructionsPlan.Size()
}
for {
// initialize an empty optimized plan and an empty the mask
potentialMask := resolver.NewInstructionsPlanMask(len(naiveInstructionsPlanSequence))
Expand Down Expand Up @@ -193,28 +197,6 @@ func (interpreter *StartosisInterpreter) InterpretAndOptimizePlan(
optimizedPlan.AddScheduledInstruction(scheduledInstruction)
}

// there might be still be instructions in the current enclave plan that have not been imported to the
// optimized plan
// for now, we support this only if no new instructions will be executed for this run. If that's not the case
// continue the loop in the hope of finding another mask
if len(currentEnclavePlanSequence) > matchingInstructionIdx+optimizedPlan.Size() {
logrus.Debugf("There are %d instructions remaining in the current state that have not been transferred to the new plan. Transferring them now", len(currentEnclavePlanSequence)-matchingInstructionIdx+optimizedPlan.Size())
atLeastOneInstructionWillBeExecuted := false
for _, instructionThatWillPotentiallyBeRun := range attemptInstructionsPlanSequence {
if !instructionThatWillPotentiallyBeRun.IsExecuted() {
atLeastOneInstructionWillBeExecuted = true
}
}
if atLeastOneInstructionWillBeExecuted {
logrus.Debugf("The remaining instructions in the current enclave plan cannot be transferred to the new plan because this plan contains instructions that will be executed." +
"The remaining instructions might depend on those and Kurtosis cannot re-run them (this is unsupported for now)")
continue
}
// recopy all remaining instructions into the optimized plan
for _, remainingInstructionFromCurrentEnclaveState := range currentEnclavePlanSequence[matchingInstructionIdx+optimizedPlan.Size():] {
optimizedPlan.AddScheduledInstruction(remainingInstructionFromCurrentEnclaveState).ImportedFromCurrentEnclavePlan(true).Executed(true)
}
}
// finally we can return the optimized plan as well as the serialized script output returned by the last
// interpretation attempt
return attemptSerializedScriptOutput, optimizedPlan, nil
Expand Down
Expand Up @@ -215,66 +215,6 @@ func (suite *StartosisInterpreterIdempotentTestSuite) TestInterpretAndOptimize_D
require.False(suite.T(), scheduledInstruction3.IsExecuted())
}

// This is a bit of an edge case here, we run only part of the instruction that are identical to what was already run
// Current plan -> [`print("instruction1")` `print("instruction2")` `print("instruction3")`]
// Package to run -> [`print("instruction1")` `print("instruction2")` ]
// Check that instruction 1 and 2 are part of the new plan, already executed, and instruction3 is ALSO part of the
// plan, but marked as imported from a previous plan
func (suite *StartosisInterpreterIdempotentTestSuite) TestInterpretAndOptimize_ReplacePartOfInstructionWithIdenticalInstruction() {
initialScript := `def run(plan, args):
plan.print(msg="instruction1")
plan.print(msg="instruction2")
plan.print(msg="instruction3")
`
// Interpretation of the initial script to generate the current enclave plan
_, currentEnclavePlan, interpretationApiErr := suite.interpreter.Interpret(
context.Background(),
startosis_constants.PackageIdPlaceholderForStandaloneScript,
useDefaultMainFunctionName,
startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript,
initialScript,
noInputParams,
enclave_structure.NewEnclaveComponents(),
resolver.NewInstructionsPlanMask(0))
require.Nil(suite.T(), interpretationApiErr)
require.Equal(suite.T(), 3, currentEnclavePlan.Size())

updatedScript := `def run(plan, args):
plan.print(msg="instruction1")
plan.print(msg="instruction2")
`
// Interpret the updated script against the current enclave plan
_, instructionsPlan, interpretationError := suite.interpreter.InterpretAndOptimizePlan(
context.Background(),
startosis_constants.PackageIdPlaceholderForStandaloneScript,
useDefaultMainFunctionName,
startosis_constants.PlaceHolderMainFileForPlaceStandAloneScript,
updatedScript,
noInputParams,
currentEnclavePlan,
)
require.Nil(suite.T(), interpretationError)

instructionSequence, err := instructionsPlan.GeneratePlan()
require.Nil(suite.T(), err)
require.Equal(suite.T(), 3, len(instructionSequence))

scheduledInstruction1 := instructionSequence[0]
require.Equal(suite.T(), `print(msg="instruction1")`, scheduledInstruction1.GetInstruction().String())
require.False(suite.T(), scheduledInstruction1.IsImportedFromCurrentEnclavePlan())
require.True(suite.T(), scheduledInstruction1.IsExecuted())

scheduledInstruction2 := instructionSequence[1]
require.Equal(suite.T(), `print(msg="instruction2")`, scheduledInstruction2.GetInstruction().String())
require.False(suite.T(), scheduledInstruction2.IsImportedFromCurrentEnclavePlan())
require.True(suite.T(), scheduledInstruction2.IsExecuted())

scheduledInstruction3 := instructionSequence[2]
require.Equal(suite.T(), `print(msg="instruction3")`, scheduledInstruction3.GetInstruction().String())
require.True(suite.T(), scheduledInstruction3.IsImportedFromCurrentEnclavePlan())
require.True(suite.T(), scheduledInstruction3.IsExecuted())
}

// Submit a package with a update on an instruction located "in the middle" of the package
// Current plan -> [`print("instruction1")` `print("instruction2")` `print("instruction3")`]
// Package to run -> [`print("instruction1")` `print("instruction2_NEW")` `print("instruction3")`]
Expand Down

0 comments on commit c7ef5c2

Please sign in to comment.