-
Notifications
You must be signed in to change notification settings - Fork 9
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(executor/local): do not override env with machine details for local exec #522
Conversation
Codecov Report
@@ Coverage Diff @@
## main #522 +/- ##
==========================================
- Coverage 83.44% 83.42% -0.02%
==========================================
Files 69 69
Lines 5381 5371 -10
==========================================
- Hits 4490 4481 -9
Misses 746 746
+ Partials 145 144 -1
|
…ela/worker into enhance/executor/local-exec-env-fix
@@ -90,6 +91,7 @@ func (c *client) ExecStage(ctx context.Context, s *pipeline.Stage, m *sync.Map) | |||
// | |||
// https://pkg.go.dev/github.com/go-vela/worker/internal/step#Skip | |||
if step.Skip(_step, c.build, c.repo) { | |||
logrus.Infof("Skipping step %s due to ruleset", _step.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disclaimer i havent tested the PR to see what the logging actually looks like mid-execution. but we usually print to stdout during pipeline execution, should we do that here to follow the pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks the same as all the start up debug logs:
vela exec pipeline
DEBU[0000] validating pipeline configuration
DEBU[0000] Creating registry clients from CLI configuration
...
[step: init] > Preparing service images...
[step: init] > Preparing stage images...
[step: init] > Preparing step images...
DEBU[0000] streaming build logs
[step: init] $ docker image inspect golang:latest
sha256:2159148dcc081245165b2aa99fc5a94ca9818bece66839d8eb11c9335ae9e688
...
[step: docker-build] INFO[0000] Skipping push to container registry due to --no-push flag
INFO[0004] Skipping step notify_slack due to ruleset
[step: random] > Streaming container 'step_EastonCrupper_myvela_1_random'...
Think that's decent enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sweet, the example is great. it views more like a log from the worker process controlling the execution flow, i like it!
This is the second of three changes that will enhance the
vela exec pipeline
command. These changes fall under theWorker
section below.The Issue
vela exec pipeline
does not work with templates. It also does not work with custom environments if the user is trying to emulate any of the default environment variables, such asVELA_BUILD_COMMIT
.Server Changes
getTemplate
that allows the compiler to read both from local file tree as well as remote using a GitHub token.Worker Changes
CLI Changes
--template
,--template-file
,--compiler.github.token
, and--compiler.github.url
flags to theexec pipeline
command. This will allow users the option to leverage templates in local pipeline execution, both local and remote.--env-file
and--env-file-path
to allow users to source environment variables from a file.