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: make node tool non volatile #2372

Merged
merged 4 commits into from
Jul 10, 2024
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
6 changes: 3 additions & 3 deletions pkg/runner/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil {
return err
}
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)}
containerArgs := []string{rc.GetNodeToolFullPath(ctx), path.Join(containerActionDir, action.Runs.Main)}
logger.Debugf("executing remote job container: %s", containerArgs)

rc.ApplyExtraPath(ctx, step.getEnv())
Expand Down Expand Up @@ -533,7 +533,7 @@ func runPreStep(step actionStep) common.Executor {
return err
}

containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Pre)}
containerArgs := []string{rc.GetNodeToolFullPath(ctx), path.Join(containerActionDir, action.Runs.Pre)}
logger.Debugf("executing remote job container: %s", containerArgs)

rc.ApplyExtraPath(ctx, step.getEnv())
Expand Down Expand Up @@ -627,7 +627,7 @@ func runPostStep(step actionStep) common.Executor {
populateEnvsFromSavedState(step.getEnv(), step, rc)
populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc)

containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)}
containerArgs := []string{rc.GetNodeToolFullPath(ctx), path.Join(containerActionDir, action.Runs.Post)}
logger.Debugf("executing remote job container: %s", containerArgs)

rc.ApplyExtraPath(ctx, step.getEnv())
Expand Down
21 changes: 11 additions & 10 deletions pkg/runner/action_composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,17 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action
},
},
},
Config: &configCopy,
StepResults: map[string]*model.StepResult{},
JobContainer: parent.JobContainer,
ActionPath: actionPath,
Env: env,
GlobalEnv: parent.GlobalEnv,
Masks: parent.Masks,
ExtraPath: parent.ExtraPath,
Parent: parent,
EventJSON: parent.EventJSON,
Config: &configCopy,
StepResults: map[string]*model.StepResult{},
JobContainer: parent.JobContainer,
ActionPath: actionPath,
Env: env,
GlobalEnv: parent.GlobalEnv,
Masks: parent.Masks,
ExtraPath: parent.ExtraPath,
Parent: parent,
EventJSON: parent.EventJSON,
nodeToolFullPath: parent.nodeToolFullPath,
}
compositerc.ExprEval = compositerc.NewExpressionEvaluator(ctx)

Expand Down
2 changes: 2 additions & 0 deletions pkg/runner/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ func TestActionRunner(t *testing.T) {
},
},
},
nodeToolFullPath: "node",
},
action: &model.Action{
Inputs: map[string]model.Input{
Expand Down Expand Up @@ -208,6 +209,7 @@ func TestActionRunner(t *testing.T) {
"name": "state value",
},
},
nodeToolFullPath: "node",
},
action: &model.Action{
Runs: model.ActionRuns{
Expand Down
2 changes: 1 addition & 1 deletion pkg/runner/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
Mode: 0o644,
Body: hashfiles,
}).
Then(rc.execJobContainer([]string{"node", path.Join(rc.JobContainer.GetActPath(), name)},
Then(rc.execJobContainer([]string{rc.GetNodeToolFullPath(ctx), path.Join(rc.JobContainer.GetActPath(), name)},

Check warning on line 199 in pkg/runner/expression.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/expression.go#L199

Added line #L199 was not covered by tests
env, "", "")).
Finally(func(context.Context) error {
rc.JobContainer.ReplaceLogWriter(stdout, stderr)
Expand Down
1 change: 1 addition & 0 deletions pkg/runner/job_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
})

pipeline := make([]common.Executor, 0)
pipeline = append(pipeline, rc.InitializeNodeTool())
pipeline = append(pipeline, preSteps...)
pipeline = append(pipeline, steps...)

Expand Down
3 changes: 2 additions & 1 deletion pkg/runner/job_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ func TestNewJobExecutor(t *testing.T) {
},
},
},
Config: &Config{},
Config: &Config{},
nodeToolFullPath: "node",
}
rc.ExprEval = rc.NewExpressionEvaluator(ctx)
executorOrder := make([]string, 0)
Expand Down
44 changes: 44 additions & 0 deletions pkg/runner/run_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package runner
import (
"archive/tar"
"bufio"
"bytes"
"context"
"crypto/rand"
"crypto/sha256"
Expand Down Expand Up @@ -50,6 +51,7 @@ type RunContext struct {
Masks []string
cleanUpJobContainer common.Executor
caller *caller // job calling this RunContext (reusable workflows)
nodeToolFullPath string
}

func (rc *RunContext) AddMask(mask string) {
Expand Down Expand Up @@ -432,6 +434,48 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user
}
}

func (rc *RunContext) InitializeNodeTool() common.Executor {
return func(ctx context.Context) error {
rc.GetNodeToolFullPath(ctx)
return nil
}
}

func (rc *RunContext) GetNodeToolFullPath(ctx context.Context) string {
if rc.nodeToolFullPath == "" {
timeed, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
path := rc.JobContainer.GetPathVariableName()
cenv := map[string]string{}
var cpath string
if err := rc.JobContainer.UpdateFromImageEnv(&cenv)(ctx); err == nil {
if p, ok := cenv[path]; ok {
cpath = p
}
}
if len(cpath) == 0 {
cpath = rc.JobContainer.DefaultPathVariable()
}
cenv[path] = cpath
hout := &bytes.Buffer{}
herr := &bytes.Buffer{}
stdout, stderr := rc.JobContainer.ReplaceLogWriter(hout, herr)
err := rc.execJobContainer([]string{"node", "--no-warnings", "-e", "console.log(process.execPath)"},
cenv, "", "").
Finally(func(context.Context) error {
rc.JobContainer.ReplaceLogWriter(stdout, stderr)
return nil
})(timeed)
rawStr := strings.Trim(hout.String(), "\r\n")
if err == nil && !strings.ContainsAny(rawStr, "\r\n") {
rc.nodeToolFullPath = rawStr
} else {
rc.nodeToolFullPath = "node"
}
}
return rc.nodeToolFullPath
}

func (rc *RunContext) ApplyExtraPath(ctx context.Context, env *map[string]string) {
if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
path := rc.JobContainer.GetPathVariableName()
Expand Down
3 changes: 2 additions & 1 deletion pkg/runner/step_action_local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ func TestStepActionLocalPost(t *testing.T) {
},
},
},
StepResults: tt.initialStepResults,
StepResults: tt.initialStepResults,
nodeToolFullPath: "node",
},
Step: tt.stepModel,
action: tt.actionModel,
Expand Down
1 change: 1 addition & 0 deletions pkg/runner/step_action_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ func TestStepActionRemotePost(t *testing.T) {
},
StepResults: tt.initialStepResults,
IntraActionState: tt.IntraActionState,
nodeToolFullPath: "node",
},
Step: tt.stepModel,
action: tt.actionModel,
Expand Down
Loading