Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle container optimization for default images when pure shell script is run at last #5958

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/testworkflow-init/commands/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions pkg/testworkflows/testworkflowprocessor/action/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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").

Expand All @@ -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("")
Expand Down
4 changes: 2 additions & 2 deletions pkg/testworkflows/testworkflowprocessor/action/optimize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
98 changes: 78 additions & 20 deletions pkg/testworkflows/testworkflowprocessor/action/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading