From f9baec4b32458d6f0f858a463ba264558d23bddf Mon Sep 17 00:00:00 2001 From: Lokesh Gopu Date: Mon, 13 Apr 2020 21:33:13 -0400 Subject: [PATCH] Added support for custom labels (#414) * Added support for custom labels * ignore case * Added interactive config for labels * Fixing L0s * pr comments --- src/Runner.Common/Constants.cs | 1 + src/Runner.Listener/CommandSettings.cs | 25 +++++++- .../Configuration/ConfigurationManager.cs | 37 ++++++++---- .../Configuration/PromptManager.cs | 24 ++++++-- .../Configuration/Validators.cs | 16 +++++ .../Generated/TaskAgentHttpClientBase.cs | 12 ++-- src/Sdk/DTWebApi/WebApi/AgentLabel.cs | 59 +++++++++++++++++++ src/Sdk/DTWebApi/WebApi/LabelType.cs | 14 +++++ src/Sdk/DTWebApi/WebApi/TaskAgent.cs | 8 +-- src/Test/L0/Listener/CommandSettingsL0.cs | 36 +++++++---- .../Configuration/ConfigurationManagerL0.cs | 10 +++- 11 files changed, 199 insertions(+), 43 deletions(-) create mode 100644 src/Sdk/DTWebApi/WebApi/AgentLabel.cs create mode 100644 src/Sdk/DTWebApi/WebApi/LabelType.cs diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 0a8261c55..e67b6d42e 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -87,6 +87,7 @@ namespace GitHub.Runner.Common public static class Args { public static readonly string Auth = "auth"; + public static readonly string Labels = "labels"; public static readonly string MonitorSocketAddress = "monitorsocketaddress"; public static readonly string Name = "name"; public static readonly string Pool = "pool"; diff --git a/src/Runner.Listener/CommandSettings.cs b/src/Runner.Listener/CommandSettings.cs index 07a6c334b..0d80c1d5e 100644 --- a/src/Runner.Listener/CommandSettings.cs +++ b/src/Runner.Listener/CommandSettings.cs @@ -39,6 +39,7 @@ namespace GitHub.Runner.Listener private readonly string[] validArgs = { Constants.Runner.CommandLine.Args.Auth, + Constants.Runner.CommandLine.Args.Labels, Constants.Runner.CommandLine.Args.MonitorSocketAddress, Constants.Runner.CommandLine.Args.Name, Constants.Runner.CommandLine.Args.Pool, @@ -249,6 +250,24 @@ namespace GitHub.Runner.Listener return GetArg(Constants.Runner.CommandLine.Args.StartupType); } + public ISet GetLabels() + { + var labelSet = new HashSet(StringComparer.OrdinalIgnoreCase); + string labels = GetArgOrPrompt( + name: Constants.Runner.CommandLine.Args.Labels, + description: $"This runner will have the following labels: 'self-hosted', '{VarUtil.OS}', '{VarUtil.OSArchitecture}' \nEnter any additional labels (ex. label-1,label-2):", + defaultValue: string.Empty, + validator: Validators.LabelsValidator, + isOptional: true); + + if (!string.IsNullOrEmpty(labels)) + { + labelSet = labels.Split(',').Where(x => !string.IsNullOrEmpty(x)).ToHashSet(StringComparer.OrdinalIgnoreCase); + } + + return labelSet; + } + // // Private helpers. // @@ -280,7 +299,8 @@ namespace GitHub.Runner.Listener string name, string description, string defaultValue, - Func validator) + Func validator, + bool isOptional = false) { // Check for the arg in the command line parser. ArgUtil.NotNull(validator, nameof(validator)); @@ -311,7 +331,8 @@ namespace GitHub.Runner.Listener secret: Constants.Runner.CommandLine.Args.Secrets.Any(x => string.Equals(x, name, StringComparison.OrdinalIgnoreCase)), defaultValue: defaultValue, validator: validator, - unattended: Unattended); + unattended: Unattended, + isOptional: isOptional); } private string GetEnvArg(string name) diff --git a/src/Runner.Listener/Configuration/ConfigurationManager.cs b/src/Runner.Listener/Configuration/ConfigurationManager.cs index c2d4a60cd..289ccaa1a 100644 --- a/src/Runner.Listener/Configuration/ConfigurationManager.cs +++ b/src/Runner.Listener/Configuration/ConfigurationManager.cs @@ -166,6 +166,9 @@ namespace GitHub.Runner.Listener.Configuration _term.WriteLine(); + var userLabels = command.GetLabels(); + _term.WriteLine(); + var agents = await _runnerServer.GetAgentsAsync(runnerSettings.PoolId, runnerSettings.AgentName); Trace.Verbose("Returns {0} agents", agents.Count); agent = agents.FirstOrDefault(); @@ -175,7 +178,7 @@ namespace GitHub.Runner.Listener.Configuration if (command.GetReplace()) { // Update existing agent with new PublicKey, agent version. - agent = UpdateExistingAgent(agent, publicKey); + agent = UpdateExistingAgent(agent, publicKey, userLabels); try { @@ -198,7 +201,7 @@ namespace GitHub.Runner.Listener.Configuration else { // Create a new agent. - agent = CreateNewAgent(runnerSettings.AgentName, publicKey); + agent = CreateNewAgent(runnerSettings.AgentName, publicKey, userLabels); try { @@ -448,7 +451,7 @@ namespace GitHub.Runner.Listener.Configuration } - private TaskAgent UpdateExistingAgent(TaskAgent agent, RSAParameters publicKey) + private TaskAgent UpdateExistingAgent(TaskAgent agent, RSAParameters publicKey, ISet userLabels) { ArgUtil.NotNull(agent, nameof(agent)); agent.Authorization = new TaskAgentAuthorization @@ -456,18 +459,25 @@ namespace GitHub.Runner.Listener.Configuration PublicKey = new TaskAgentPublicKey(publicKey.Exponent, publicKey.Modulus), }; - // update - update instead of delete so we don't lose labels etc... + // update should replace the existing labels agent.Version = BuildConstants.RunnerPackage.Version; agent.OSDescription = RuntimeInformation.OSDescription; + + agent.Labels.Clear(); - agent.Labels.Add("self-hosted"); - agent.Labels.Add(VarUtil.OS); - agent.Labels.Add(VarUtil.OSArchitecture); + agent.Labels.Add(new AgentLabel("self-hosted", LabelType.System)); + agent.Labels.Add(new AgentLabel(VarUtil.OS, LabelType.System)); + agent.Labels.Add(new AgentLabel(VarUtil.OSArchitecture, LabelType.System)); + foreach (var userLabel in userLabels) + { + agent.Labels.Add(new AgentLabel(userLabel, LabelType.User)); + } + return agent; } - private TaskAgent CreateNewAgent(string agentName, RSAParameters publicKey) + private TaskAgent CreateNewAgent(string agentName, RSAParameters publicKey, ISet userLabels) { TaskAgent agent = new TaskAgent(agentName) { @@ -480,9 +490,14 @@ namespace GitHub.Runner.Listener.Configuration OSDescription = RuntimeInformation.OSDescription, }; - agent.Labels.Add("self-hosted"); - agent.Labels.Add(VarUtil.OS); - agent.Labels.Add(VarUtil.OSArchitecture); + agent.Labels.Add(new AgentLabel("self-hosted", LabelType.System)); + agent.Labels.Add(new AgentLabel(VarUtil.OS, LabelType.System)); + agent.Labels.Add(new AgentLabel(VarUtil.OSArchitecture, LabelType.System)); + + foreach (var userLabel in userLabels) + { + agent.Labels.Add(new AgentLabel(userLabel, LabelType.User)); + } return agent; } diff --git a/src/Runner.Listener/Configuration/PromptManager.cs b/src/Runner.Listener/Configuration/PromptManager.cs index 977786c23..274a24878 100644 --- a/src/Runner.Listener/Configuration/PromptManager.cs +++ b/src/Runner.Listener/Configuration/PromptManager.cs @@ -20,7 +20,8 @@ namespace GitHub.Runner.Listener.Configuration bool secret, string defaultValue, Func validator, - bool unattended); + bool unattended, + bool isOptional = false); } public sealed class PromptManager : RunnerService, IPromptManager @@ -56,7 +57,8 @@ namespace GitHub.Runner.Listener.Configuration bool secret, string defaultValue, Func validator, - bool unattended) + bool unattended, + bool isOptional = false) { Trace.Info(nameof(ReadValue)); ArgUtil.NotNull(validator, nameof(validator)); @@ -85,18 +87,28 @@ namespace GitHub.Runner.Listener.Configuration { _terminal.Write($"[press Enter for {defaultValue}] "); } + else if (isOptional){ + _terminal.Write($"[press Enter to skip] "); + } // Read and trim the value. value = secret ? _terminal.ReadSecret() : _terminal.ReadLine(); value = value?.Trim() ?? string.Empty; // Return the default if not specified. - if (string.IsNullOrEmpty(value) && !string.IsNullOrEmpty(defaultValue)) + if (string.IsNullOrEmpty(value)) { - Trace.Info($"Falling back to the default: '{defaultValue}'"); - return defaultValue; + if (!string.IsNullOrEmpty(defaultValue)) + { + Trace.Info($"Falling back to the default: '{defaultValue}'"); + return defaultValue; + } + else if (isOptional) + { + return string.Empty; + } } - + // Return the value if it is not empty and it is valid. // Otherwise try the loop again. if (!string.IsNullOrEmpty(value)) diff --git a/src/Runner.Listener/Configuration/Validators.cs b/src/Runner.Listener/Configuration/Validators.cs index c0cd1ef0e..79d968221 100644 --- a/src/Runner.Listener/Configuration/Validators.cs +++ b/src/Runner.Listener/Configuration/Validators.cs @@ -1,6 +1,7 @@ using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; using System; +using System.Linq; using System.IO; using System.Security.Principal; @@ -46,6 +47,21 @@ namespace GitHub.Runner.Listener.Configuration string.Equals(value, "N", StringComparison.CurrentCultureIgnoreCase); } + public static bool LabelsValidator(string labels) + { + if (!string.IsNullOrEmpty(labels)) + { + var labelSet = labels.Split(',').Where(x => !string.IsNullOrEmpty(x)).ToHashSet(StringComparer.OrdinalIgnoreCase); + + if (labelSet.Any(x => x.Length > 256)) + { + return false; + } + } + + return true; + } + public static bool NonEmptyValidator(string value) { return !string.IsNullOrEmpty(value); diff --git a/src/Sdk/DTGenerated/Generated/TaskAgentHttpClientBase.cs b/src/Sdk/DTGenerated/Generated/TaskAgentHttpClientBase.cs index 14327a6f8..d5f9e2b7c 100644 --- a/src/Sdk/DTGenerated/Generated/TaskAgentHttpClientBase.cs +++ b/src/Sdk/DTGenerated/Generated/TaskAgentHttpClientBase.cs @@ -82,7 +82,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), userState: userState, cancellationToken: cancellationToken, content: content); @@ -109,7 +109,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), userState: userState, cancellationToken: cancellationToken).ConfigureAwait(false)) { @@ -164,7 +164,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), queryParameters: queryParams, userState: userState, cancellationToken: cancellationToken); @@ -227,7 +227,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), queryParameters: queryParams, userState: userState, cancellationToken: cancellationToken); @@ -257,7 +257,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), userState: userState, cancellationToken: cancellationToken, content: content); @@ -287,7 +287,7 @@ namespace GitHub.DistributedTask.WebApi httpMethod, locationId, routeValues: routeValues, - version: new ApiResourceVersion(5.1, 1), + version: new ApiResourceVersion(6.0, 2), userState: userState, cancellationToken: cancellationToken, content: content); diff --git a/src/Sdk/DTWebApi/WebApi/AgentLabel.cs b/src/Sdk/DTWebApi/WebApi/AgentLabel.cs new file mode 100644 index 000000000..6d98caed1 --- /dev/null +++ b/src/Sdk/DTWebApi/WebApi/AgentLabel.cs @@ -0,0 +1,59 @@ +using System.Runtime.Serialization; +using Newtonsoft.Json; + +namespace GitHub.DistributedTask.WebApi +{ + [DataContract] + public class AgentLabel + { + [JsonConstructor] + public AgentLabel() + { + } + + public AgentLabel(string name) + { + this.Name = name; + this.Type = LabelType.System; + } + + public AgentLabel(string name, LabelType type) + { + this.Name = name; + this.Type = type; + } + + private AgentLabel(AgentLabel labelToBeCloned) + { + this.Id = labelToBeCloned.Id; + this.Name = labelToBeCloned.Name; + this.Type = labelToBeCloned.Type; + } + + [DataMember] + public int Id + { + get; + set; + } + + [DataMember] + public string Name + { + get; + set; + } + + [DataMember] + public LabelType Type + { + get; + set; + } + + public AgentLabel Clone() + { + return new AgentLabel(this); + } + } +} diff --git a/src/Sdk/DTWebApi/WebApi/LabelType.cs b/src/Sdk/DTWebApi/WebApi/LabelType.cs new file mode 100644 index 000000000..dd135020a --- /dev/null +++ b/src/Sdk/DTWebApi/WebApi/LabelType.cs @@ -0,0 +1,14 @@ +using System.Runtime.Serialization; + +namespace GitHub.DistributedTask.WebApi +{ + [DataContract] + public enum LabelType + { + [EnumMember] + System = 0, + + [EnumMember] + User = 1 + } +} diff --git a/src/Sdk/DTWebApi/WebApi/TaskAgent.cs b/src/Sdk/DTWebApi/WebApi/TaskAgent.cs index 1ca8ae94b..f97322e13 100644 --- a/src/Sdk/DTWebApi/WebApi/TaskAgent.cs +++ b/src/Sdk/DTWebApi/WebApi/TaskAgent.cs @@ -51,7 +51,7 @@ namespace GitHub.DistributedTask.WebApi if (agentToBeCloned.m_labels != null && agentToBeCloned.m_labels.Count > 0) { - m_labels = new HashSet(agentToBeCloned.m_labels, StringComparer.OrdinalIgnoreCase); + m_labels = new HashSet(agentToBeCloned.m_labels); } } @@ -118,13 +118,13 @@ namespace GitHub.DistributedTask.WebApi /// /// The labels of the runner /// - public ISet Labels + public ISet Labels { get { if (m_labels == null) { - m_labels = new HashSet(StringComparer.OrdinalIgnoreCase); + m_labels = new HashSet(); } return m_labels; } @@ -164,6 +164,6 @@ namespace GitHub.DistributedTask.WebApi private PropertiesCollection m_properties; [DataMember(IsRequired = false, EmitDefaultValue = false, Name = "Labels")] - private HashSet m_labels; + private HashSet m_labels; } } diff --git a/src/Test/L0/Listener/CommandSettingsL0.cs b/src/Test/L0/Listener/CommandSettingsL0.cs index 9ef40dd0c..b35729d49 100644 --- a/src/Test/L0/Listener/CommandSettingsL0.cs +++ b/src/Test/L0/Listener/CommandSettingsL0.cs @@ -317,7 +317,8 @@ namespace GitHub.Runner.Common.Tests false, // secret Environment.MachineName, // defaultValue Validators.NonEmptyValidator, // validator - true)) // unattended + true, // unattended + false)) // isOptional .Returns("some runner"); // Act. @@ -344,7 +345,8 @@ namespace GitHub.Runner.Common.Tests false, // secret Environment.MachineName, // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some runner"); // Act. @@ -371,7 +373,8 @@ namespace GitHub.Runner.Common.Tests false, // secret "some default auth", // defaultValue Validators.AuthSchemeValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some auth"); // Act. @@ -398,7 +401,8 @@ namespace GitHub.Runner.Common.Tests true, // secret string.Empty, // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some token"); // Act. @@ -475,7 +479,8 @@ namespace GitHub.Runner.Common.Tests true, // secret string.Empty, // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some token"); // Act. @@ -502,7 +507,8 @@ namespace GitHub.Runner.Common.Tests true, // secret string.Empty, // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some token"); // Act. @@ -529,7 +535,8 @@ namespace GitHub.Runner.Common.Tests false, // secret string.Empty, // defaultValue Validators.ServerUrlValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some url"); // Act. @@ -556,7 +563,8 @@ namespace GitHub.Runner.Common.Tests false, // secret "some default account", // defaultValue Validators.NTAccountValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some windows logon account"); // Act. @@ -584,7 +592,8 @@ namespace GitHub.Runner.Common.Tests true, // secret string.Empty, // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some windows logon password"); // Act. @@ -611,7 +620,8 @@ namespace GitHub.Runner.Common.Tests false, // secret "_work", // defaultValue Validators.NonEmptyValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some work"); // Act. @@ -640,7 +650,8 @@ namespace GitHub.Runner.Common.Tests false, // secret string.Empty, // defaultValue Validators.ServerUrlValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some url"); // Act. @@ -669,7 +680,8 @@ namespace GitHub.Runner.Common.Tests false, // secret string.Empty, // defaultValue Validators.ServerUrlValidator, // validator - false)) // unattended + false, // unattended + false)) // isOptional .Returns("some url"); // Act. diff --git a/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs b/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs index c47d5be21..9cf3ad5ad 100644 --- a/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs +++ b/src/Test/L0/Listener/Configuration/ConfigurationManagerL0.cs @@ -145,6 +145,8 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration IConfigurationManager configManager = new ConfigurationManager(); configManager.Initialize(tc); + var userLabels = "userlabel1,userlabel2"; + trace.Info("Preparing command line arguments"); var command = new CommandSettings( tc, @@ -156,7 +158,8 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration "--pool", _expectedPoolName, "--work", _expectedWorkFolder, "--auth", _expectedAuthType, - "--token", _expectedToken + "--token", _expectedToken, + "--labels", userLabels }); trace.Info("Constructed."); _store.Setup(x => x.IsConfigured()).Returns(false); @@ -178,7 +181,10 @@ namespace GitHub.Runner.Common.Tests.Listener.Configuration // validate GetAgentPoolsAsync gets called twice with automation pool type _runnerServer.Verify(x => x.GetAgentPoolsAsync(It.IsAny(), It.Is(p => p == TaskAgentPoolType.Automation)), Times.Exactly(2)); - _runnerServer.Verify(x => x.AddAgentAsync(It.IsAny(), It.Is(a => a.Labels.Contains("self-hosted") && a.Labels.Contains(VarUtil.OS) && a.Labels.Contains(VarUtil.OSArchitecture))), Times.Once); + var expectedLabels = new List() { "self-hosted", VarUtil.OS, VarUtil.OSArchitecture}; + expectedLabels.AddRange(userLabels.Split(",").ToList()); + + _runnerServer.Verify(x => x.AddAgentAsync(It.IsAny(), It.Is(a => a.Labels.Select(x => x.Name).ToHashSet().SetEquals(expectedLabels))), Times.Once); } } }