Skip to content

Commit

Permalink
Remove executor config variable from task env
Browse files Browse the repository at this point in the history
Fixes: #84
  • Loading branch information
janisz committed Feb 13, 2018
1 parent dd59253 commit 80df525
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 30 deletions.
8 changes: 3 additions & 5 deletions cmd/executor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
"github.com/allegro/mesos-executor/runenv"
)

const environmentPrefix = "allegro_executor"

// Version designates the version of application.
var Version string

Expand All @@ -32,7 +30,7 @@ var debug = *flag.Bool("debug", false, "Forces executor debug mode")
func init() {
flag.Parse()

if err := envconfig.Process(environmentPrefix, &Config); err != nil {
if err := envconfig.Process(executor.EnvironmentPrefix, &Config); err != nil {
log.WithError(err).Fatal("Failed to load executor configuration")
}

Expand Down Expand Up @@ -87,7 +85,7 @@ func initSentry(config executor.Config) error {

func createHooks() []hook.Hook {
var consulConfig consul.Config
if err := envconfig.Process(environmentPrefix, &consulConfig); err != nil {
if err := envconfig.Process(executor.EnvironmentPrefix, &consulConfig); err != nil {
log.WithError(err).Fatal("Failed to load Consul hook configuration")
}
consulHook, err := consul.NewHook(consulConfig)
Expand All @@ -96,7 +94,7 @@ func createHooks() []hook.Hook {
}

var vaasConfig vaas.Config
if err := envconfig.Process(environmentPrefix, &vaasConfig); err != nil {
if err := envconfig.Process(executor.EnvironmentPrefix, &vaasConfig); err != nil {
log.WithError(err).Fatal("Failed to load VaaS hook configuration")
}
vaasHook, err := vaas.NewHook(vaasConfig)
Expand Down
22 changes: 13 additions & 9 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@ import (
"fmt"
"os"
"os/exec"
"strings"
"syscall"
"time"

"github.com/mesos/mesos-go/api/v1/lib"
log "github.com/sirupsen/logrus"

osutil "github.com/allegro/mesos-executor/os"
"github.com/allegro/mesos-executor/servicelog"
"github.com/allegro/mesos-executor/servicelog/appender"
"github.com/allegro/mesos-executor/servicelog/scraper"
"github.com/mesos/mesos-go/api/v1/lib"
)

// TaskExitState is a type describing reason of program execution interuption.
Expand Down Expand Up @@ -136,7 +137,7 @@ func NewCommand(commandInfo mesos.CommandInfo, env []string, options ...func(*ex
// similar to how POSIX exec families launch processes (i.e.,
// execlp(value, arguments(0), arguments(1), ...)).
cmd := exec.Command("sh", "-c", commandInfo.GetValue()) // #nosec
cmd.Env = combineExecutorAndTaskEnv(env, commandInfo.GetEnvironment())
cmd.Env = envWithoutExecutorConfig()
for _, option := range options {
if err := option(cmd); err != nil {
return nil, fmt.Errorf("invalid config option: %s", err)
Expand Down Expand Up @@ -171,13 +172,16 @@ func ScrapCmdOutput(s scraper.Scraper, a appender.Appender, extenders ...service
}
}

func combineExecutorAndTaskEnv(env []string, mesosEnv *mesos.Environment) []string {
var combined []string
combined = append(combined, env...)
// envWithoutExecutorConfig returns os.Environ without executor specific entries.
// Marathon does not support custom executor env and all task env are passed
// as executor env. This means environment are setup before executor startup.
func envWithoutExecutorConfig() (env []string) {
for _, variable := range os.Environ() {
if !strings.HasPrefix(variable, strings.ToUpper(EnvironmentPrefix)) {
env = append(env, variable)
} else {

for _, variable := range mesosEnv.GetVariables() {
combined = append(combined, fmt.Sprintf("%s=%s", variable.Name, variable.Value))
}
}

return combined
return env
}
35 changes: 19 additions & 16 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,38 @@ import (
"path/filepath"
"testing"

mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/mesos/mesos-go/api/v1/lib"
"github.com/stretchr/testify/assert"
)

func TestIfNewCancellableCommandReturnsCommand(t *testing.T) {
os.Environ()
commandInfo := newCommandInfo("./sleep 100", "ignored", false, []string{"ignored"}, map[string]string{"one": "1", "two": "2"})
func TestIfNewCancellableCommandReturnsCommandWithoutExecutorEnv(t *testing.T) {
os.Setenv("ALLEGRO_EXECUTOR_TEST_1", "x")
os.Setenv("allegro_executor_TEST_2", "y")
os.Setenv("TEST", "z")
os.Setenv("_ALLEGRO_EXECUTOR_", "0")

defer os.Unsetenv("ALLEGRO_EXECUTOR_TEST_1")
defer os.Unsetenv("allegro_executor_TEST_2")
defer os.Unsetenv("TEST")
defer os.Unsetenv("_ALLEGRO_EXECUTOR_")

commandInfo := newCommandInfo("./sleep 100", "ignored", false, []string{"ignored"})
command, err := NewCommand(commandInfo, nil)
cmd := command.(*cancellableCommand).cmd

assert.NoError(t, err)
assert.Equal(t, []string{"sh", "-c", "./sleep 100"}, cmd.Args)
assert.Equal(t, filepath.Base(cmd.Path), "sh")
assert.Contains(t, cmd.Env, "one=1")
assert.Contains(t, cmd.Env, "two=2")
assert.True(t, cmd.SysProcAttr.Setpgid, "should have pgid flag set to true")

assert.NotContains(t, cmd.Env, "ALLEGRO_EXECUTOR_TEST_1=x")
assert.Contains(t, cmd.Env, "allegro_executor_TEST_2=y")
assert.Contains(t, cmd.Env, "TEST=z")
assert.Contains(t, cmd.Env, "_ALLEGRO_EXECUTOR_=0")
}

func newCommandInfo(command, user string, shell bool, args []string, environment map[string]string) mesos.CommandInfo {
env := mesos.Environment{}
for key, value := range environment {
// Go uses a copy of the value instead of the value itself within a range clause.
k := key
v := value
env.Variables = append(env.Variables, mesos.Environment_Variable{Name: k, Value: v})
}
func newCommandInfo(command, user string, shell bool, args []string) mesos.CommandInfo {
return mesos.CommandInfo{
URIs: nil,
Environment: &env,
Shell: &shell,
Value: &command,
Arguments: args,
Expand Down
4 changes: 4 additions & 0 deletions executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import (
"github.com/allegro/mesos-executor/state"
)

// EnvironmentPrefix is a prefix for environmental configuration
const EnvironmentPrefix = "allegro_executor"

// Config settable from the environment
type Config struct {
// Sets logging level to `debug` when true, `info` otherwise
Expand Down Expand Up @@ -474,6 +477,7 @@ func taskExitToEvent(exitStateChan <-chan TaskExitState, events chan<- Event) {
}

// Hack: For Marathon #4952
// https://jira.mesosphere.com/browse/MARATHON-4210
func prepareCommandInfo(commandInfo *mesos.CommandInfo) {
marathonPrefix := fmt.Sprintf("chmod ug+rx '%s' && exec '%s' ", os.Args[0], os.Args[0])
commandLine := strings.TrimPrefix(commandInfo.GetValue(), marathonPrefix)
Expand Down

0 comments on commit 80df525

Please sign in to comment.