-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Support refresh runner configs with pipelines service. #3706
base: main
Are you sure you want to change the base?
Conversation
@@ -119,8 +119,11 @@ public interface IConfigurationStore : IRunnerService | |||
CredentialData GetCredentials(); |
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.
this is basic add things back from https://github.com/actions/runner/pull/513/files#diff-8ef9a7b7967d070375a6e4773f2bbeb1ae60e986bff48a4171256ae1b3869872
and introduce migrated config file (.runner) in addition to the migrated credentials file (.credentials).
case WellKnownConfigFile.MigratedRunner: | ||
path = Path.Combine( | ||
GetDirectory(WellKnownDirectory.Root), | ||
".runner_migrated"); |
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.
we are going to reuse .credentials_migrated
.
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.
we already delete the .credentials_migrated
as part of https://github.com/actions/runner/pull/513/files#diff-f965276d94b7d1ede0f58a5e1341987e3cf976566f942e5f96e23717ec52140c
@@ -315,10 +318,17 @@ public Task<PackageMetadata> GetPackageAsync(string packageType, string platform | |||
return _genericTaskAgentClient.GetPackageAsync(packageType, platform, version, includeToken, cancellationToken: cancellationToken); | |||
} | |||
|
|||
public Task<TaskAgent> UpdateAgentUpdateStateAsync(int agentPoolId, ulong agentId, string currentState, string trace) |
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.
only add cancellation token
} | ||
|
||
// runner config refresh | ||
public Task<string> RefreshRunnerConfigAsync(int agentId, string configType, string encodedRunnerConfig, CancellationToken cancellationToken) |
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.
new endpoint added by https://github.com/github/actions-dotnet/pull/18725
var runnerRefreshConfigMessage = JsonUtility.FromString<RunnerRefreshConfigMessage>(message.Body); | ||
Trace.Info($"Received RunnerRefreshConfigMessage for '{runnerRefreshConfigMessage.ConfigType}' config file"); | ||
var configUpdater = HostContext.GetService<IRunnerConfigUpdater>(); | ||
await configUpdater.UpdateRunnerConfigAsync( |
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.
UpdateRunnerConfigAsync
does not throw.
namespace GitHub.DistributedTask.WebApi | ||
{ | ||
[DataContract] | ||
public sealed class RunnerRefreshConfigMessage |
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.
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.
RunnerQualifiedId string `json:"runnerQualifiedId"`
ConfigType string `json:"configType"`
ServiceType string `json:"serviceType"`
ConfigRefreshURL string `json:"configRefreshURL"`
/// <param name="userState"></param> | ||
/// <param name="cancellationToken">The cancellation token to cancel operation.</param> | ||
[EditorBrowsable(EditorBrowsableState.Never)] | ||
public virtual Task<string> RefreshRunnerConfigAsync( |
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.
generated by genclient
from actions-dotnet
{ | ||
using (var tokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30))) | ||
{ | ||
await _runnerServer.UpdateAgentUpdateStateAsync(_settings.PoolId, _settings.AgentId, "RefreshConfig", telemetry, tokenSource.Token); |
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.
work with https://github.com/github/actions-dotnet/pull/18766 to flow telemetry to our log.
7cac6dc
to
5af59de
Compare
The runner would need to exchange its
.runner
and.credentials
with service to finish migration to new backend.We will based on message from service to make request to either pipelines or admin to get a new config file.
The new config will get stored in either
.runner_migrated
or.credentials_migrated
, which are side by side with existing.runner
and.credentials
.We will report telemetry back to service on any failure.
https://github.com/github/actions-runtime/issues/4944