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.

Sign up for GitHub

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

Set runner environment in context and env #2518

Merged
merged 2 commits into from
May 26, 2023
Merged

Set runner environment in context and env #2518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c40884
Set runner environment in runner context and env
feelepxyz Apr 3, 2023
361d7be
encoding.
TingluoHuang May 26, 2023
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
5 changes: 5 additions & 0 deletions src/Runner.Worker/JobRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ public async Task<TaskResult> RunAsync(AgentJobRequestMessage message, Cancellat
_runnerSettings = HostContext.GetService<IConfigurationStore>().GetSettings();
jobContext.SetRunnerContext("name", _runnerSettings.AgentName);

if (jobContext.Global.Variables.TryGetValue(WellKnownDistributedTaskVariables.RunnerEnvironment, out var runnerEnvironment))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TingluoHuang presuming this tryGetValue should make it safe to deploy this change independently of the backend change?

Also had a poke around and doesn't look like there's any allow list for what goes in the RUNNER_{KEY} ENV values: https://github.com/actions/runner/blob/main/src/Runner.Worker/RunnerContext.cs

Looks like the GitHub context does have an allow list: https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an allow list before for the RUNNER_* since before this change all RUNNER_* are set by the runner, while most GITHUB_* are coming from the service.

{
jobContext.SetRunnerContext("environment", runnerEnvironment);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TingluoHuang another q: Looking at these docs: https://docs.github.com/en/actions/learn-github-actions/variables#detecting-the-operating-system - it seems you can access the runner context before the job runs, e.g. if: runner.os == 'Windows'. From our sync yesterday, is this a problem for us? Presuming it should be ok to set it here as everything else is set here too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if: runner.os == 'Windows' in the example in under steps (not before job land on the runner), which will be on the runner for sure.

}

string toolsDirectory = HostContext.GetDirectory(WellKnownDirectory.Tools);
Directory.CreateDirectory(toolsDirectory);
jobContext.SetRunnerContext("tool_cache", toolsDirectory);
Expand Down
3 changes: 2 additions & 1 deletion src/Sdk/DTWebApi/WebApi/WellKnownDistributedTaskVariables.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
using System;
using System;

namespace GitHub.DistributedTask.WebApi
{
public static class WellKnownDistributedTaskVariables
{
public static readonly String JobId = "system.jobId";
public static readonly String RunnerLowDiskspaceThreshold = "system.runner.lowdiskspacethreshold";
public static readonly String RunnerEnvironment = "system.runnerEnvironment";
}
}