The Wayback Machine - https://web.archive.org/web/20210903200344/https://github.com/hashicorp/waypoint/pull/2201
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

Runners support config files and dynamic config #2201

Merged
merged 11 commits into from Sep 2, 2021
Merged

Conversation

@mitchellh
Copy link
Member

@mitchellh mitchellh commented Sep 2, 2021

Runner configuration has always supported env vars (using the -runner flag on waypoint config). This PR enhances runners to also support dynamic config and config files, which apps have supported for a couple releases. This brings parity between runner and app config.

Implementation Details

Months ago I extracted internal/appconfig from our entrypoint so that the "app config" logic is reusable. We never refactored runners to be built on top of that, so I did that in this PR. The rest mostly comes "for free" from that.

I did have to update internal/appconfig to support detecting unset env vars and to replace unset env vars with their original value. This is important for runners because we run the jobs directly in the same process. We never had to do this before because entrypoints subprocess, so keeping track of unset env vars was unnecessary and so was replacing old values, since if they don't exist anymore, the next subprocess would just pick up the parent process old values.

@mitchellh mitchellh added this to the 0.6.0 milestone Sep 2, 2021
@mitchellh mitchellh requested a review from hashicorp/waypoint-core Sep 2, 2021
@github-actions github-actions bot added the core label Sep 2, 2021
@mitchellh mitchellh force-pushed the feature/runner-appconfig branch from 8a4953c to 62ccb82 Sep 2, 2021
Copy link
Member

@briancain briancain left a comment

Nice, makes sense! 👍🏻

@evanphx
evanphx approved these changes Sep 2, 2021
Copy link
Contributor

@evanphx evanphx left a comment

:shipit:

if key == envLogLevel {
value := pair[idx+1:]
level := hclog.LevelFromString(value)
if level == hclog.NoLevel {
// We warn this
log.Warn("log level provided in env var is invalid", value)
} else {
// We set the log level on the root logger so it
// affects all runner logs.
r.logger.SetLevel(level)
Comment on lines +182 to +191

This comment has been minimized.

// If this is a runner, we don't support dynamic values currently.
if value.Target.Runner != nil {
if _, ok := value.Value.(*pb.ConfigVar_Static); !ok {
return status.Errorf(codes.FailedPrecondition,
"runner-scoped configuration must be static")
}
}

Comment on lines -105 to -112

This comment has been minimized.

@mitchellh mitchellh merged commit 1770505 into main Sep 2, 2021
9 checks passed
9 checks passed
@github-actions
changelog-check
Details
@github-actions
triage
Details
@vercel
Vercel Canceled by Ignored Build Step
Details
@circleci-checks
frontend Workflow: frontend
Details
@circleci-checks
go-tests Workflow: go-tests
Details
@circleci-checks
integration Workflow: integration
Details
license/cla Contributor License Agreement is signed.
Details
@percy
percy/waypoint Visual review automatically approved, no visual changes found.
Details
@circleci-checks
website-mdx Workflow: website-mdx
Details
@mitchellh mitchellh deleted the feature/runner-appconfig branch Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants