-
Notifications
You must be signed in to change notification settings - Fork 956
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
Job container + container services #23
Conversation
did you talk with Chris about removing the logic to handle file permission different? |
_pathMappings[hostContext.GetDirectory(WellKnownDirectory.Tools)] = "/__t"; // Tool cache folder may come from ENV, so we need a unique folder to avoid collision | ||
_pathMappings[hostContext.GetDirectory(WellKnownDirectory.Work)] = "/__w"; | ||
_pathMappings[hostContext.GetDirectory(WellKnownDirectory.Root)] = "/__a"; |
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.
do we still need the __a
? we had it because we have variables like Agent.HomeDirectory
, so i have to make it shows reasonable when you get inside container, i don't think we need it anymore
Conflicts: src/Runner.Worker/ExecutionContext.cs
@@ -246,7 +246,9 @@ private async Task StartContainerAsync(IExecutionContext executionContext, Conta | |||
container.AddPortMappings(await _dockerManger.DockerPort(executionContext, container.ContainerId)); | |||
foreach (var port in container.PortMappings) | |||
{ | |||
executionContext.SetRunnerContext($"service.{container.ContainerNetworkAlias}.ports.{port.ContainerPort}", port.HostPort); | |||
var contextVarName = $"service_{container.ContainerNetworkAlias}_ports_{port.ContainerPort}"; |
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.
@TingluoHuang Is this still the right pattern? Ive tested it and it still works, the ports show up in the host process env as e.g. AGENT_SERVICE_REDIS_PORTS_6379=32771
I am just wondering if this is the best way to expose information to the runner at runtime.
Should we also change the prefix to RUNNER instead of AGENT?
Avoid needing to fumble paths together by splitting org/repo name later
in container workdir is relative path from workdir which is concat from host work dir and source dir
* Fix parallel 1, no runner available actions#23 * Make runner early available * Check if parallel 1 works
release workflow update
Removes node intermediate handler script
docker
is exec'd with the environment, which maps the environment into the command run bydocker exec
Script is run directly via
docker exec
rather than execing a node script that then runs the scriptNeed to iterate on mapping in path