codingworkshop/act
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: reworked container architecture (#619)

Browse Source 
- Don't set architecture, let Docker host decide it's own platform,
  remove `runtime` dependency and don't show default in `--help`
- Remove most tests, we need to check only once if it works on
  different platform
- Rename `DeleteImage` to `RemoveImage` to conform to existing
  function in `docker` cli, added options to specify `force` and
  `pruneChildren`
This commit is contained in:
Ryan (hackercat)
2021-05-02 17:15:13 +02:00
committed by GitHub
parent 3e768cd916
commit f32babb51d
8 changed files with 73 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/runner/run_context.go
View File

@@ -96,10 +96,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
binds = append(binds, fmt.Sprintf("%s:%s%s", rc.Config.Workdir, rc.Config.Workdir, bindModifiers))
}
if rc.Config.ContainerArchitecture == "" {
rc.Config.ContainerArchitecture = fmt.Sprintf("%s/%s", "linux", runtime.GOARCH)
}
rc.JobContainer = container.NewContainer(&container.NewContainerInput{
Cmd: nil,
Entrypoint: []string{"/usr/bin/tail", "-f", "/dev/null"},

57
pkg/runner/runner_test.go
View File

@@ -79,45 +79,28 @@ func TestRunEvent(t *testing.T) {
"ubuntu-latest": "node:12.20.1-buster-slim",
}
tables := []TestJobFileInfo{
// {"testdata", "powershell", "push", "", platforms}, // Powershell is not available on default act test runner (yet) but preserving here for posterity
{"testdata", "basic", "push", "", platforms, "linux/amd64"},
{"testdata", "fail", "push", "exit with `FAILURE`: 1", platforms, "linux/amd64"},
{"testdata", "runs-on", "push", "", platforms, "linux/amd64"},
{"testdata", "job-container", "push", "", platforms, "linux/amd64"},
{"testdata", "job-container-non-root", "push", "", platforms, "linux/amd64"},
{"testdata", "uses-docker-url", "push", "", platforms, "linux/amd64"},
{"testdata", "remote-action-docker", "push", "", platforms, "linux/amd64"},
{"testdata", "remote-action-js", "push", "", platforms, "linux/amd64"},
{"testdata", "local-action-docker-url", "push", "", platforms, "linux/amd64"},
{"testdata", "local-action-dockerfile", "push", "", platforms, "linux/amd64"},
{"testdata", "local-action-js", "push", "", platforms, "linux/amd64"},
{"testdata", "matrix", "push", "", platforms, "linux/amd64"},
{"testdata", "matrix-include-exclude", "push", "", platforms, "linux/amd64"},
{"testdata", "commands", "push", "", platforms, "linux/amd64"},
{"testdata", "workdir", "push", "", platforms, "linux/amd64"},
// {"testdata", "issue-228", "push", "", platforms, "linux/amd64"}, // TODO [igni]: Remove this once everything passes
{"testdata", "defaults-run", "push", "", platforms, "linux/amd64"},
{"testdata", "uses-composite", "push", "", platforms, "linux/amd64"},
{"testdata", "basic", "push", "", platforms, ""},
{"testdata", "fail", "push", "exit with `FAILURE`: 1", platforms, ""},
{"testdata", "runs-on", "push", "", platforms, ""},
{"testdata", "job-container", "push", "", platforms, ""},
{"testdata", "job-container-non-root", "push", "", platforms, ""},
{"testdata", "uses-docker-url", "push", "", platforms, ""},
{"testdata", "remote-action-docker", "push", "", platforms, ""},
{"testdata", "remote-action-js", "push", "", platforms, ""},
{"testdata", "local-action-docker-url", "push", "", platforms, ""},
{"testdata", "local-action-dockerfile", "push", "", platforms, ""},
{"testdata", "local-action-js", "push", "", platforms, ""},
{"testdata", "matrix", "push", "", platforms, ""},
{"testdata", "matrix-include-exclude", "push", "", platforms, ""},
{"testdata", "commands", "push", "", platforms, ""},
{"testdata", "workdir", "push", "", platforms, ""},
{"testdata", "defaults-run", "push", "", platforms, ""},
{"testdata", "uses-composite", "push", "", platforms, ""},
// {"testdata", "powershell", "push", "", platforms, ""}, // Powershell is not available on default act test runner (yet) but preserving here for posterity
// {"testdata", "issue-228", "push", "", platforms, ""}, // TODO [igni]: Remove this once everything passes
// linux/arm64
// single test for different architecture: linux/arm64
{"testdata", "basic", "push", "", platforms, "linux/arm64"},
{"testdata", "fail", "push", "exit with `FAILURE`: 1", platforms, "linux/arm64"},
{"testdata", "runs-on", "push", "", platforms, "linux/arm64"},
{"testdata", "job-container", "push", "", platforms, "linux/arm64"},
{"testdata", "job-container-non-root", "push", "", platforms, "linux/arm64"},
{"testdata", "uses-docker-url", "push", "", platforms, "linux/arm64"},
{"testdata", "remote-action-docker", "push", "", platforms, "linux/arm64"},
{"testdata", "remote-action-js", "push", "", platforms, "linux/arm64"},
{"testdata", "local-action-docker-url", "push", "", platforms, "linux/arm64"},
{"testdata", "local-action-dockerfile", "push", "", platforms, "linux/arm64"},
{"testdata", "local-action-js", "push", "", platforms, "linux/arm64"},
{"testdata", "matrix", "push", "", platforms, "linux/arm64"},
{"testdata", "matrix-include-exclude", "push", "", platforms, "linux/arm64"},
{"testdata", "commands", "push", "", platforms, "linux/arm64"},
{"testdata", "workdir", "push", "", platforms, "linux/arm64"},
// {"testdata", "issue-228", "push", "", platforms, "linux/arm64"}, // TODO [igni]: Remove this once everything passes
{"testdata", "defaults-run", "push", "", platforms, "linux/arm64"},
{"testdata", "uses-composite", "push", "", platforms, "linux/arm64"},
}
log.SetLevel(log.DebugLevel)

82
pkg/runner/step_context.go
View File

@@ -252,10 +252,6 @@ func (sc *StepContext) newStepContainer(ctx context.Context, image string, cmd [
binds = append(binds, fmt.Sprintf("%s:%s%s", rc.Config.Workdir, rc.Config.Workdir, bindModifiers))
}
if rc.Config.ContainerArchitecture == "" {
rc.Config.ContainerArchitecture = fmt.Sprintf("%s/%s", "linux", runtime.GOARCH)
}
stepContainer := container.NewContainer(&container.NewContainerInput{
Cmd: cmd,
Entrypoint: entrypoint,
@@ -438,33 +434,35 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe
image = strings.ToLower(image)
contextDir := filepath.Join(actionDir, actionPath, action.Runs.Main)
exists, err := container.ImageExistsLocally(ctx, image, "any")
anyArchExists, err := container.ImageExistsLocally(ctx, image, "any")
if err != nil {
return err
}
if exists {
wasRemoved, err := container.DeleteImage(ctx, image)
correctArchExists, err := container.ImageExistsLocally(ctx, image, rc.Config.ContainerArchitecture)
if err != nil {
return err
}
if anyArchExists && !correctArchExists {
wasRemoved, err := container.RemoveImage(ctx, image, true, true)
if err != nil {
return err
}
if !wasRemoved {
return fmt.Errorf("failed to delete image '%s'", image)
return fmt.Errorf("failed to remove image '%s'", image)
}
}
prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{
ContextDir: contextDir,
ImageTag: image,
Platform: rc.Config.ContainerArchitecture,
})
exists, err = container.ImageExistsLocally(ctx, image, rc.Config.ContainerArchitecture)
if err != nil {
return err
}
if !exists {
return err
if !correctArchExists {
log.Debugf("image '%s' for architecture '%s' will be built from context '%s", image, rc.Config.ContainerArchitecture, contextDir)
prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{
ContextDir: contextDir,
ImageTag: image,
Platform: rc.Config.ContainerArchitecture,
})
} else {
log.Debugf("image '%s' for architecture '%s' already exists", image, rc.Config.ContainerArchitecture)
}
}
@@ -508,27 +506,27 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe
stepID := 0
for _, compositeStep := range action.Runs.Steps {
stepClone := compositeStep
// Take a copy of the run context structure (rc is a pointer)
// Then take the address of the new structure
rcCloneStr := *rc
rcClone := &rcCloneStr
// Take a copy of the run context structure (rc is a pointer)
// Then take the address of the new structure
rcCloneStr := *rc
rcClone := &rcCloneStr
if stepClone.ID == "" {
stepClone.ID = fmt.Sprintf("composite-%d", stepID)
stepID++
}
rcClone.CurrentStep = stepClone.ID
rcClone.CurrentStep = stepClone.ID
if err := compositeStep.Validate(); err != nil {
return err
}
if err := compositeStep.Validate(); err != nil {
return err
}
// Setup the outputs for the composite steps
if _, ok := rcClone.StepResults[stepClone.ID]; ! ok {
rcClone.StepResults[stepClone.ID] = &stepResult{
Success: true,
Outputs: make(map[string]string),
}
}
// Setup the outputs for the composite steps
if _, ok := rcClone.StepResults[stepClone.ID]; !ok {
rcClone.StepResults[stepClone.ID] = &stepResult{
Success: true,
Outputs: make(map[string]string),
}
}
stepClone.Run = strings.ReplaceAll(stepClone.Run, "${{ github.action_path }}", filepath.Join(containerActionDir, actionName))
@@ -538,14 +536,14 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe
Env: mergeMaps(sc.Env, stepClone.Env),
}
// Interpolate the outer inputs into the composite step with items
exprEval := sc.NewExpressionEvaluator()
for k, v := range stepContext.Step.With {
// Interpolate the outer inputs into the composite step with items
exprEval := sc.NewExpressionEvaluator()
for k, v := range stepContext.Step.With {
if strings.Contains(v, "inputs") {
stepContext.Step.With[k] = exprEval.Interpolate(v)
}
}
if strings.Contains(v, "inputs") {
stepContext.Step.With[k] = exprEval.Interpolate(v)
}
}
executors = append(executors, stepContext.Executor())
}

16
pkg/runner/step_context_test.go
View File

@@ -12,17 +12,11 @@ func TestStepContextExecutor(t *testing.T) {
"ubuntu-latest": "node:12.20.1-buster-slim",
}
tables := []TestJobFileInfo{
{"testdata", "uses-and-run-in-one-step", "push", "Invalid run/uses syntax for job:test step:Test", platforms, "linux/amd64"},
{"testdata", "uses-github-empty", "push", "Expected format {org}/{repo}[/path]@ref", platforms, "linux/amd64"},
{"testdata", "uses-github-noref", "push", "Expected format {org}/{repo}[/path]@ref", platforms, "linux/amd64"},
{"testdata", "uses-github-root", "push", "", platforms, "linux/amd64"},
{"testdata", "uses-github-path", "push", "", platforms, "linux/amd64"},
{"testdata", "uses-and-run-in-one-step", "push", "Invalid run/uses syntax for job:test step:Test", platforms, "linux/arm64"},
{"testdata", "uses-github-empty", "push", "Expected format {org}/{repo}[/path]@ref", platforms, "linux/arm64"},
{"testdata", "uses-github-noref", "push", "Expected format {org}/{repo}[/path]@ref", platforms, "linux/arm64"},
{"testdata", "uses-github-root", "push", "", platforms, "linux/arm64"},
{"testdata", "uses-github-path", "push", "", platforms, "linux/arm64"},
{"testdata", "uses-and-run-in-one-step", "push", "Invalid run/uses syntax for job:test step:Test", platforms, ""},
{"testdata", "uses-github-empty", "push", "Expected format {org}/{repo}[/path]@ref", platforms, ""},
{"testdata", "uses-github-noref", "push", "Expected format {org}/{repo}[/path]@ref", platforms, ""},
{"testdata", "uses-github-root", "push", "", platforms, ""},
{"testdata", "uses-github-path", "push", "", platforms, ""},
}
// These tests are sufficient to only check syntax.
ctx := common.WithDryrun(context.Background(), true)