diff --git a/internal/worker/command_blender.go b/internal/worker/command_blender.go index 827de638..b0550697 100644 --- a/internal/worker/command_blender.go +++ b/internal/worker/command_blender.go @@ -25,13 +25,12 @@ package worker import ( "context" "fmt" - "os/exec" "github.com/rs/zerolog" "gitlab.com/blender/flamenco-ng-poc/pkg/api" ) -type BlenderSettings struct { +type BlenderParameters struct { exe string // Expansion of `{blender}`: executable path + its CLI parameters defined by the Manager. argsBefore []string // Additional CLI arguments defined by the job compiler script, to go before the blend file name. blendfile string // Path of the file to open. @@ -40,40 +39,46 @@ type BlenderSettings struct { // cmdBlender executes the "blender-render" command. func (ce *CommandExecutor) cmdBlenderRender(ctx context.Context, logger zerolog.Logger, taskID string, cmd api.Command) error { - settings := BlenderSettings{ - exe: cmd.Parameters["exe"].(string), - blendfile: cmd.Parameters["blendfile"].(string), + var ( + parameters BlenderParameters + ok bool + ) + + if parameters.exe, ok = cmdParameter[string](cmd, "exe"); !ok || parameters.exe == "" { + logger.Warn().Interface("command", cmd).Msg("missing 'exe' parameter") + return fmt.Errorf("missing 'exe' parameter: %+v", cmd.Parameters) } - var ok bool - if settings.exe == "" { - logger.Warn().Interface("command", cmd).Msg("missing 'exe' setting") - return fmt.Errorf("missing 'exe' setting: %+v", cmd) + if parameters.argsBefore, ok = cmdParameterAsStrings(cmd, "argsBefore"); !ok { + logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' parameter") + return fmt.Errorf("invalid 'argsBefore' parameter: %+v", cmd.Parameters) } - if settings.argsBefore, ok = cmdSettingAsStrings(cmd, "argsBefore"); !ok { - logger.Warn().Interface("command", cmd).Msg("invalid 'argsBefore' setting") - return fmt.Errorf("invalid 'argsBefore' setting: %+v", cmd) + if parameters.blendfile, ok = cmdParameter[string](cmd, "blendfile"); !ok || parameters.blendfile == "" { + logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' parameter") + return fmt.Errorf("missing 'blendfile' parameter: %+v", cmd.Parameters) } - if settings.blendfile == "" { - logger.Warn().Interface("command", cmd).Msg("missing 'blendfile' setting") - return fmt.Errorf("missing 'blendfile' setting: %+v", cmd) - } - if settings.args, ok = cmdSettingAsStrings(cmd, "args"); !ok { - logger.Warn().Interface("command", cmd).Msg("invalid 'args' setting") - return fmt.Errorf("invalid 'args' setting: %+v", cmd) + if parameters.args, ok = cmdParameterAsStrings(cmd, "args"); !ok { + logger.Warn().Interface("command", cmd).Msg("invalid 'args' parameter") + return fmt.Errorf("invalid 'args' parameter: %+v", cmd.Parameters) } cliArgs := make([]string, 0) - cliArgs = append(cliArgs, settings.argsBefore...) - cliArgs = append(cliArgs, settings.blendfile) - cliArgs = append(cliArgs, settings.args...) - execCmd := exec.CommandContext(ctx, settings.exe, cliArgs...) + cliArgs = append(cliArgs, parameters.argsBefore...) + cliArgs = append(cliArgs, parameters.blendfile) + cliArgs = append(cliArgs, parameters.args...) + execCmd := ce.cli.CommandContext(ctx, parameters.exe, cliArgs...) + if execCmd == nil { + logger.Error(). + Str("cmdName", cmd.Name). + Msg("unable to create command executor") + return ErrNoExecCmd + } logger.Info(). Str("cmdName", cmd.Name). Str("execCmd", execCmd.String()). Msg("going to execute Blender") - // if err := ce.listener.LogProduced(ctx, taskID, fmt.Sprintf("echo: %q", messageStr)); err != nil { - // return err - // } + if err := ce.listener.LogProduced(ctx, taskID, fmt.Sprintf("going to run: %s", cliArgs)); err != nil { + return err + } return nil } diff --git a/internal/worker/command_blender_test.go b/internal/worker/command_blender_test.go new file mode 100644 index 00000000..a48ef834 --- /dev/null +++ b/internal/worker/command_blender_test.go @@ -0,0 +1,56 @@ +package worker + +import ( + "context" + "testing" + + "github.com/golang/mock/gomock" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "gitlab.com/blender/flamenco-ng-poc/pkg/api" +) + +/* ***** BEGIN GPL LICENSE BLOCK ***** + * + * Original Code Copyright (C) 2022 Blender Foundation. + * + * This file is part of Flamenco. + * + * Flamenco is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, either version 3 of the License, or (at your option) any later + * version. + * + * Flamenco is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR + * A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * Flamenco. If not, see . + * + * ***** END GPL LICENSE BLOCK ***** */ + +func TestCmdBlenderSimpleCliArgs(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + ce, mocks := testCommandExecutor(t, mockCtrl) + + ctx := context.Background() + taskID := "1d54c6fe-1242-4c8f-bd63-5a09e358d7b6" + cmd := api.Command{ + Name: "blender", + Parameters: map[string]interface{}{ + "exe": "/path/to/blender", + "argsBefore": []string{"--background"}, + "blendfile": "file.blend", + "args": []string{"--render-output", "/frames"}, + }, + } + + cliArgs := []string{"--background", "file.blend", "--render-output", "/frames"} + mocks.cli.EXPECT().CommandContext(ctx, "/path/to/blender", cliArgs).Return(nil) + + err := ce.cmdBlenderRender(ctx, zerolog.Nop(), taskID, cmd) + assert.Equal(t, ErrNoExecCmd, err, "nil *exec.Cmd should result in ErrNoExecCmd") +} diff --git a/internal/worker/command_exe.go b/internal/worker/command_exe.go index aec4f90f..5c6cb41e 100644 --- a/internal/worker/command_exe.go +++ b/internal/worker/command_exe.go @@ -22,6 +22,7 @@ package worker import ( "context" + "errors" "fmt" "os/exec" "time" @@ -37,6 +38,7 @@ import ( // CommandListener sends the result of commands (log, output files) to the Manager. type CommandListener interface { // LogProduced sends any logging to whatever service for storing logging. + // logLines are concatenated. LogProduced(ctx context.Context, taskID string, logLines ...string) error // OutputProduced tells the Manager there has been some output (most commonly a rendered frame or video). OutputProduced(ctx context.Context, taskID string, outputLocation string) error @@ -66,6 +68,12 @@ type CommandLineRunner interface { CommandContext(ctx context.Context, name string, arg ...string) *exec.Cmd } +// ErrNoExecCmd means CommandLineRunner.CommandContext() returned nil. +// This shouldn't happen in production, but can happen in unit tests when the +// test just wants to check the CLI arguments that are supposed to be executed, +// without actually executing anything. +var ErrNoExecCmd = errors.New("no exec.Cmd could be created") + func NewCommandExecutor(cli CommandLineRunner, listener CommandListener, timeService TimeService) *CommandExecutor { ce := &CommandExecutor{ cli: cli, @@ -97,18 +105,19 @@ func (ce *CommandExecutor) Run(ctx context.Context, taskID string, cmd api.Comma return runner(ctx, logger, taskID, cmd) } -// cmdSettingAsStrings converts an array setting ([]interface{}) to a []string slice. -func cmdSettingAsStrings(cmd api.Command, key string) ([]string, bool) { - setting, found := cmd.Parameters[key] +// cmdParameterAsStrings converts an array parameter ([]interface{}) to a []string slice. +// A missing parameter is ok and returned as empty slice. +func cmdParameterAsStrings(cmd api.Command, key string) ([]string, bool) { + parameter, found := cmd.Parameters[key] if !found { - return []string{}, false + return []string{}, true } - if asStrSlice, ok := setting.([]string); ok { + if asStrSlice, ok := parameter.([]string); ok { return asStrSlice, true } - interfSlice, ok := setting.([]interface{}) + interfSlice, ok := parameter.([]interface{}) if !ok { return []string{}, false } @@ -126,3 +135,15 @@ func cmdSettingAsStrings(cmd api.Command, key string) ([]string, bool) { } return strSlice, true } + +// cmdParameter retrieves a single parameter of a certain type. +func cmdParameter[T any](cmd api.Command, key string) (T, bool) { + setting, found := cmd.Parameters[key] + if !found { + var zeroValue T + return zeroValue, false + } + + value, ok := setting.(T) + return value, ok +}