fix: handle container optimization for default images when pure shell…

… script is run at last (#5958) * fix: handle container optimization for default images when pure shell script is run at last * chore: add constant for busybox binaries in default images
kubeshop · Oct 22, 2024 · b8265f8 · b8265f8
1 parent 068d044
commit b8265f8
Show file tree

Hide file tree

Showing 9 changed files with 192 additions and 48 deletions.
diff --git a/cmd/testworkflow-init/commands/setup.go b/cmd/testworkflow-init/commands/setup.go
@@ -9,6 +9,7 @@ import (
 	"github.com/kubeshop/testkube/cmd/testworkflow-init/data"
 	"github.com/kubeshop/testkube/cmd/testworkflow-init/output"
 	"github.com/kubeshop/testkube/pkg/testworkflows/testworkflowprocessor/action/actiontypes/lite"
+	constants2 "github.com/kubeshop/testkube/pkg/testworkflows/testworkflowprocessor/constants"
 	"github.com/kubeshop/testkube/pkg/version"
 )
 
@@ -49,7 +50,7 @@ func Setup(config lite.ActionSetup) error {
 	if config.CopyBinaries {
 		// Use `cp` on the whole directory, as it has plenty of files, which lead to the same FS block.
 		// Copying individual files will lead to high FS usage
-		err := exec.Command("cp", "-rf", "/.tktw-bin", data.InternalBinPath).Run()
+		err := exec.Command("cp", "-rf", constants2.DefaultInitImageBusyboxBinaryPath, data.InternalBinPath).Run()
 		if err != nil {
 			stdoutUnsafe.Error(" error\n")
 			stdoutUnsafe.Errorf("  failed to copy the binaries: %s\n", err.Error())

diff --git a/pkg/testworkflows/testworkflowprocessor/action/actiontypes/lite/utils.go b/pkg/testworkflows/testworkflowprocessor/action/actiontypes/lite/utils.go
@@ -34,8 +34,8 @@ func (a LiteActionList) Result(ref, expression string) LiteActionList {
 	return append(a, LiteAction{Result: &ActionResult{Ref: ref, Value: expression}})
 }
 
-func (a LiteActionList) Execute(ref string, negative bool) LiteActionList {
-	return append(a, LiteAction{Execute: &ActionExecute{Ref: ref, Negative: negative}})
+func (a LiteActionList) Execute(ref string, negative, pure bool) LiteActionList {
+	return append(a, LiteAction{Execute: &ActionExecute{Ref: ref, Negative: negative, Pure: pure}})
 }
 
 func (a LiteActionList) MutateContainer(config LiteContainerConfig) LiteActionList {

diff --git a/pkg/testworkflows/testworkflowprocessor/action/actiontypes/utils.go b/pkg/testworkflows/testworkflowprocessor/action/actiontypes/utils.go
@@ -68,14 +68,23 @@ func (a ActionList) Result(ref, expression string) ActionList {
 	return append(a, Action{Result: &lite.ActionResult{Ref: ref, Value: expression}})
 }
 
-func (a ActionList) Execute(ref string, negative bool) ActionList {
-	return append(a, Action{Execute: &lite.ActionExecute{Ref: ref, Negative: negative}})
+func (a ActionList) Execute(ref string, negative, pure bool) ActionList {
+	return append(a, Action{Execute: &lite.ActionExecute{Ref: ref, Negative: negative, Pure: pure}})
 }
 
 func (a ActionList) MutateContainer(ref string, config testworkflowsv1.ContainerConfig) ActionList {
 	return append(a, Action{Container: &ActionContainer{Ref: ref, Config: config}})
 }
 
+func (a ActionList) Image() string {
+	for i := range a {
+		if a[i].Container != nil {
+			return a[i].Container.Config.Image
+		}
+	}
+	return ""
+}
+
 type ActionGroups []ActionList
 
 func (a ActionGroups) Append(fn func(list ActionList) ActionList) ActionGroups {

diff --git a/pkg/testworkflows/testworkflowprocessor/action/group.go b/pkg/testworkflows/testworkflowprocessor/action/group.go
@@ -169,5 +169,15 @@ merging:
 		i++
 	}
 
+	// Fix the auto-merged internal images
+	for i := range groups {
+		image := groups[i].Image()
+		// Re-use /.tktw/bin/sh when the internal image has been merged into different container
+		if image != constants.DefaultToolkitImage && image != constants.DefaultInitImage {
+			groups[i] = groups[i].RewireCommandDirectory(constants.DefaultInitImage, constants.DefaultInitImageBusyboxBinaryPath, constants.InternalBinPath)
+			groups[i] = groups[i].RewireCommandDirectory(constants.DefaultToolkitImage, constants.DefaultInitImageBusyboxBinaryPath, constants.InternalBinPath)
+		}
+	}
+
 	return groups
 }
diff --git a/pkg/testworkflows/testworkflowprocessor/action/group_test.go b/pkg/testworkflows/testworkflowprocessor/action/group_test.go
@@ -33,7 +33,7 @@ func TestGroup_Basic(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 		CurrentStatus("init").
 
@@ -43,7 +43,7 @@ func TestGroup_Basic(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step3").
-		Execute("step3", false).
+		Execute("step3", false, false).
 		End("step3").
 		End("init").
 		End("")

diff --git a/pkg/testworkflows/testworkflowprocessor/action/optimize.go b/pkg/testworkflows/testworkflowprocessor/action/optimize.go
@@ -29,8 +29,8 @@ func optimize(actions actiontypes.ActionList) (actiontypes.ActionList, error) {
 	actions = actions.Skip(skipped)
 
 	// Avoid using /.tktw/bin/sh when it is internal image used, with direct binaries
-	actions = actions.RewireCommandDirectory(constants.DefaultInitImage, constants.InternalBinPath, "/.tktw-bin")
-	actions = actions.RewireCommandDirectory(constants.DefaultToolkitImage, constants.InternalBinPath, "/.tktw-bin")
+	actions = actions.RewireCommandDirectory(constants.DefaultInitImage, constants.InternalBinPath, constants.DefaultInitImageBusyboxBinaryPath)
+	actions = actions.RewireCommandDirectory(constants.DefaultToolkitImage, constants.InternalBinPath, constants.DefaultInitImageBusyboxBinaryPath)
 
 	return actions, nil
 }
diff --git a/pkg/testworkflows/testworkflowprocessor/action/process_test.go b/pkg/testworkflows/testworkflowprocessor/action/process_test.go
@@ -8,6 +8,7 @@ import (
 	testworkflowsv1 "github.com/kubeshop/testkube-operator/api/testworkflows/v1"
 	"github.com/kubeshop/testkube/internal/common"
 	"github.com/kubeshop/testkube/pkg/testworkflows/testworkflowprocessor/action/actiontypes"
+	"github.com/kubeshop/testkube/pkg/testworkflows/testworkflowprocessor/constants"
 	"github.com/kubeshop/testkube/pkg/testworkflows/testworkflowprocessor/stage"
 )
 
@@ -40,7 +41,7 @@ func TestProcess_BasicSteps(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -50,7 +51,7 @@ func TestProcess_BasicSteps(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -100,7 +101,7 @@ func TestProcess_Grouping(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Start the group 1
@@ -114,7 +115,7 @@ func TestProcess_Grouping(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Run the step 2
@@ -124,7 +125,7 @@ func TestProcess_Grouping(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step3").
-		Execute("step3", false).
+		Execute("step3", false, false).
 		End("step3").
 
 		// End the group 1
@@ -137,7 +138,7 @@ func TestProcess_Grouping(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step4").
-		Execute("step4", false).
+		Execute("step4", false, false).
 		End("step4").
 
 		// Finish
@@ -184,7 +185,7 @@ func TestProcess_Pause(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -194,7 +195,7 @@ func TestProcess_Pause(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -238,7 +239,7 @@ func TestProcess_NegativeStep(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", true).
+		Execute("step1", true, false).
 		End("step1").
 
 		// Run the step 2
@@ -248,7 +249,7 @@ func TestProcess_NegativeStep(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -291,7 +292,7 @@ func TestProcess_NegativeGroup(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -301,7 +302,7 @@ func TestProcess_NegativeGroup(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -345,7 +346,7 @@ func TestProcess_OptionalStep(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -355,7 +356,7 @@ func TestProcess_OptionalStep(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -404,7 +405,7 @@ func TestProcess_OptionalGroup(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -414,7 +415,7 @@ func TestProcess_OptionalGroup(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -464,7 +465,7 @@ func TestProcess_IgnoreExecutionOfStaticSkip(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -604,7 +605,7 @@ func TestProcess_IgnoreExecutionOfStaticSkip_PauseGroup(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
 		End("step2").
 
 		// Finish
@@ -650,7 +651,7 @@ func TestProcess_ConsecutiveAlways(t *testing.T) {
 			Command: common.Ptr([]string{"a", "b"}),
 		}).
 		Start("step1").
-		Execute("step1", false).
+		Execute("step1", false, false).
 		End("step1").
 
 		// Run the step 2
@@ -660,7 +661,64 @@ func TestProcess_ConsecutiveAlways(t *testing.T) {
 			Command: common.Ptr([]string{"c", "d"}),
 		}).
 		Start("step2").
-		Execute("step2", false).
+		Execute("step2", false, false).
+		End("step2").
+
+		// Finish
+		End("init").
+		End("")
+
+	// Assert
+	got, err := Process(root, nil)
+	assert.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+func TestProcess_PureShellAtTheEnd(t *testing.T) {
+	// Build the structure
+	root := stage.NewGroupStage("init", false)
+	step1 := stage.NewContainerStage("step1", stage.NewContainer().SetImage("image:1.2.3").SetCommand(constants.DefaultShellPath))
+	step1.SetPure(true)
+	step1.SetCondition("always")
+	root.Add(step1)
+	step2 := stage.NewContainerStage("step2", stage.NewContainer().SetCommand(constants.DefaultShellPath))
+	step2.SetPure(true)
+	step2.SetCondition("always")
+	root.Add(step2)
+
+	// Build the expectations
+	want := actiontypes.NewActionList().
+		// Declare stage conditions
+		Declare("init", "true").
+		Declare("step1", "true", "init").
+		Declare("step2", "true", "init").
+
+		// Declare group resolutions
+		Result("init", "step1&&step2").
+		Result("", "init").
+
+		// Initialize
+		Start("").
+		CurrentStatus("true").
+		Start("init").
+
+		// Run the step 1
+		CurrentStatus("init").
+		MutateContainer("step1", testworkflowsv1.ContainerConfig{
+			Image:   "image:1.2.3",
+			Command: common.Ptr([]string{constants.DefaultShellPath}),
+		}).
+		Start("step1").
+		Execute("step1", false, true).
+		End("step1").
+
+		// Run the step 2
+		CurrentStatus("step1&&init").
+		MutateContainer("step2", testworkflowsv1.ContainerConfig{
+			Command: common.Ptr([]string{constants.DefaultShellPath}),
+		}).
+		Start("step2").
+		Execute("step2", false, true).
 		End("step2").
 
 		// Finish

diff --git a/pkg/testworkflows/testworkflowprocessor/constants/constants.go b/pkg/testworkflows/testworkflowprocessor/constants/constants.go
@@ -49,6 +49,7 @@ var (
 	}
 	DefaultInitImage                             = getInitImage()
 	DefaultToolkitImage                          = getToolkitImage()
+	DefaultInitImageBusyboxBinaryPath            = "/.tktw-bin"
 	ErrOpenSourceExecuteOperationIsNotAvailable  = errors.New(`"execute" ` + OpenSourceOperationErrorMessage)
 	ErrOpenSourceParallelOperationIsNotAvailable = errors.New(`"parallel" ` + OpenSourceOperationErrorMessage)
 	ErrOpenSourceServicesOperationIsNotAvailable = errors.New(`"services" ` + OpenSourceOperationErrorMessage)