Skip to content

Commit 618a0ba

Browse files
rishabhmalikMSrishabhmalikMS
authored
Custom Node strategy added | Integrating/updating with Node Handler | Custom Node L0 tests (#5431)
* Adding code for cusotm node handler | container integration with strategy code via feature flag knob * code cleaning * minor fix * contatiner operation provider integration with strategy framework * Adding custom node handler in task manager * minor fix * removing container provider operation integration to cover only node handler and custom node work in this pr
1 parent 54b4a69 commit 618a0ba

File tree

8 files changed

+225
-51
lines changed

8 files changed

+225
-51
lines changed

‎src/Agent.Worker/Handlers/NodeHandler.cs‎

Lines changed: 40 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -190,61 +190,54 @@ public async Task RunAsync()
190190
StepHost.ErrorDataReceived += OnDataReceived;
191191

192192
string file;
193-
if (!string.IsNullOrEmpty(ExecutionContext.StepTarget()?.CustomNodePath))
194-
{
195-
file = ExecutionContext.StepTarget().CustomNodePath;
196-
}
197-
else
198-
{
199-
bool useNode20InUnsupportedSystem = AgentKnobs.UseNode20InUnsupportedSystem.GetValue(ExecutionContext).AsBoolean();
200-
bool useNode24InUnsupportedSystem = AgentKnobs.UseNode24InUnsupportedSystem.GetValue(ExecutionContext).AsBoolean();
201-
bool node20ResultsInGlibCErrorHost = false;
202-
bool node24ResultsInGlibCErrorHost = false;
193+
bool useNode20InUnsupportedSystem = AgentKnobs.UseNode20InUnsupportedSystem.GetValue(ExecutionContext).AsBoolean();
194+
bool useNode24InUnsupportedSystem = AgentKnobs.UseNode24InUnsupportedSystem.GetValue(ExecutionContext).AsBoolean();
195+
bool node20ResultsInGlibCErrorHost = false;
196+
bool node24ResultsInGlibCErrorHost = false;
203197

204-
if (PlatformUtil.HostOS == PlatformUtil.OS.Linux)
198+
if (PlatformUtil.HostOS == PlatformUtil.OS.Linux)
199+
{
200+
if (!useNode20InUnsupportedSystem)
205201
{
206-
if (!useNode20InUnsupportedSystem)
202+
if (supportsNode20.HasValue)
207203
{
208-
if (supportsNode20.HasValue)
209-
{
210-
node20ResultsInGlibCErrorHost = !supportsNode20.Value;
211-
}
212-
else
213-
{
214-
node20ResultsInGlibCErrorHost = await CheckIfNodeResultsInGlibCError(NodeHandler.Node20_1Folder);
215-
ExecutionContext.EmitHostNode20FallbackTelemetry(node20ResultsInGlibCErrorHost);
216-
supportsNode20 = !node20ResultsInGlibCErrorHost;
217-
}
204+
node20ResultsInGlibCErrorHost = !supportsNode20.Value;
218205
}
219-
220-
if (!useNode24InUnsupportedSystem)
206+
else
221207
{
222-
if (supportsNode24.HasValue)
223-
{
224-
node24ResultsInGlibCErrorHost = !supportsNode24.Value;
225-
}
226-
else
227-
{
228-
node24ResultsInGlibCErrorHost = await CheckIfNodeResultsInGlibCError(NodeHandler.Node24Folder);
229-
ExecutionContext.EmitHostNode24FallbackTelemetry(node24ResultsInGlibCErrorHost);
230-
supportsNode24 = !node24ResultsInGlibCErrorHost;
231-
}
208+
node20ResultsInGlibCErrorHost = await CheckIfNodeResultsInGlibCError(NodeHandler.Node20_1Folder);
209+
ExecutionContext.EmitHostNode20FallbackTelemetry(node20ResultsInGlibCErrorHost);
210+
supportsNode20 = !node20ResultsInGlibCErrorHost;
232211
}
233212
}
234-
235-
ContainerInfo container = (ExecutionContext.StepTarget() as ContainerInfo);
236-
if (container == null)
237-
{
238-
file = GetNodeLocation(node20ResultsInGlibCErrorHost, node24ResultsInGlibCErrorHost, inContainer: false);
239-
}
240-
else
213+
214+
if (!useNode24InUnsupportedSystem)
241215
{
242-
file = GetNodeLocation(container.NeedsNode16Redirect, container.NeedsNode20Redirect, inContainer: true);
216+
if (supportsNode24.HasValue)
217+
{
218+
node24ResultsInGlibCErrorHost = !supportsNode24.Value;
219+
}
220+
else
221+
{
222+
node24ResultsInGlibCErrorHost = await CheckIfNodeResultsInGlibCError(NodeHandler.Node24Folder);
223+
ExecutionContext.EmitHostNode24FallbackTelemetry(node24ResultsInGlibCErrorHost);
224+
supportsNode24 = !node24ResultsInGlibCErrorHost;
225+
}
243226
}
227+
}
244228

245-
ExecutionContext.Debug("Using node path: " + file);
229+
ContainerInfo container = (ExecutionContext.StepTarget() as ContainerInfo);
230+
if (container == null)
231+
{
232+
file = GetNodeLocation(node20ResultsInGlibCErrorHost, node24ResultsInGlibCErrorHost, inContainer: false);
233+
}
234+
else
235+
{
236+
file = GetNodeLocation(container.NeedsNode16Redirect, container.NeedsNode20Redirect, inContainer: true);
246237
}
247238

239+
ExecutionContext.Debug("Using node path: " + file);
240+
248241
// Format the arguments passed to node.
249242
// 1) Wrap the script file path in double quotes.
250243
// 2) Escape double quotes within the script file path. Double-quote is a valid
@@ -403,6 +396,10 @@ private async Task<string> GetNodeLocationUsingStrategy(bool inContainer)
403396

404397
private string GetNodeLocationLegacy(bool node20ResultsInGlibCError, bool node24ResultsInGlibCError, bool inContainer)
405398
{
399+
if (!string.IsNullOrEmpty(ExecutionContext.StepTarget()?.CustomNodePath))
400+
{
401+
return ExecutionContext.StepTarget().CustomNodePath;
402+
}
406403
bool useNode10 = AgentKnobs.UseNode10.GetValue(ExecutionContext).AsBoolean();
407404
bool useNode20_1 = AgentKnobs.UseNode20_1.GetValue(ExecutionContext).AsBoolean();
408405
bool UseNode20InUnsupportedSystem = AgentKnobs.UseNode20InUnsupportedSystem.GetValue(ExecutionContext).AsBoolean();

‎src/Agent.Worker/NodeVersionStrategies/CustomNodeStrategy.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.IO;
6+
using Agent.Sdk.Knob;
7+
using Microsoft.TeamFoundation.DistributedTask.WebApi;
8+
using Microsoft.VisualStudio.Services.Agent.Util;
9+
using Microsoft.VisualStudio.Services.Agent.Worker;
10+
11+
namespace Microsoft.VisualStudio.Services.Agent.Worker.NodeVersionStrategies
12+
{
13+
public sealed class CustomNodeStrategy : INodeVersionStrategy
14+
{
15+
public NodeRunnerInfo CanHandle(TaskContext context, IExecutionContext executionContext, GlibcCompatibilityInfo glibcInfo)
16+
{
17+
string customPath = null;
18+
string source = null;
19+
20+
if (context.Container == null && context.StepTarget != null)
21+
{
22+
customPath = context.StepTarget.CustomNodePath;
23+
source = "StepTarget.CustomNodePath";
24+
}
25+
else if (context.Container != null)
26+
{
27+
customPath = context.Container.CustomNodePath;
28+
source = "Container.CustomNodePath";
29+
}
30+
31+
if (string.IsNullOrWhiteSpace(customPath))
32+
{
33+
executionContext.Debug("[CustomNodeStrategy] No custom node path found");
34+
return null;
35+
}
36+
37+
executionContext.Debug($"[CustomNodeStrategy] Found custom node path in {source}: {customPath}");
38+
39+
return new NodeRunnerInfo
40+
{
41+
NodePath = customPath,
42+
NodeVersion = NodeVersion.Custom,
43+
Reason = $"Custom Node.js path specified by user ({source})",
44+
Warning = null
45+
};
46+
}
47+
}
48+
}

‎src/Agent.Worker/NodeVersionStrategies/Node24Strategy.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ private NodeRunnerInfo DetermineNodeVersionSelection(TaskContext context, bool e
8181
{
8282
executionContext.Debug("[Node24Strategy] Would need Node16 but EOL policy being enabled this is not supported");
8383
string handlerType = context.HandlerData != null ? context.HandlerData.GetType().Name : "UnknownHandlerData";
84-
// why this error is here? shouldn't this be part of orchestrator that if no node return throw error?
8584
throw new NotSupportedException($"No compatible Node.js version available for host execution. Handler type: {handlerType}. This may occur if all available versions are blocked by EOL policy. Please update your pipeline to use Node20 or Node24 tasks. To temporarily disable EOL policy: Set AGENT_RESTRICT_EOL_NODE_VERSIONS=false");
8685
}
8786

‎src/Agent.Worker/NodeVersionStrategies/NodeRunnerInfo.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ public enum NodeVersion
1414
Node10,
1515
Node16,
1616
Node20,
17-
Node24
17+
Node24,
18+
Custom
1819
}
1920

2021
/// <summary>
@@ -34,6 +35,7 @@ public static string GetFolderName(NodeVersion version)
3435
NodeVersion.Node16 => "node16",
3536
NodeVersion.Node20 => "node20_1",
3637
NodeVersion.Node24 => "node24",
38+
NodeVersion.Custom => "custom",
3739
_ => throw new ArgumentOutOfRangeException(nameof(version))
3840
};
3941
}

‎src/Agent.Worker/NodeVersionStrategies/NodeVersionOrchestrator.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public NodeVersionOrchestrator(IExecutionContext executionContext, IHostContext
3131
// IMPORTANT: Strategy order determines selection priority
3232
// Add strategies in descending priority order (newest/preferred versions first)
3333
// The orchestrator will try each strategy in order until one can handle the request
34+
_strategies.Add(new CustomNodeStrategy());
3435
_strategies.Add(new Node24Strategy());
3536
_strategies.Add(new Node20Strategy());
3637
_strategies.Add(new Node16Strategy());
@@ -97,6 +98,11 @@ public async Task<NodeRunnerInfo> SelectNodeVersionAsync(TaskContext context)
9798

9899
private NodeRunnerInfo CreateNodeRunnerInfoWithPath(TaskContext context, NodeRunnerInfo selection)
99100
{
101+
if (!string.IsNullOrEmpty(selection.NodePath))
102+
{
103+
return selection;
104+
}
105+
100106
string externalsPath = HostContext.GetDirectory(WellKnownDirectory.Externals);
101107
string nodeFolder = NodeVersionHelper.GetFolderName(selection.NodeVersion);
102108
string hostPath = Path.Combine(externalsPath, nodeFolder, "bin", $"node{IOUtil.ExeExtension}");
@@ -130,7 +136,7 @@ private void PublishNodeVersionSelectionTelemetry(NodeRunnerInfo result, INodeVe
130136
{ "IsContainer", (context.Container != null).ToString() }
131137
};
132138

133-
ExecutionContext.PublishTaskRunnerTelemetry(telemetryData);
139+
ExecutionContext.PublishTaskRunnerTelemetry(telemetryData);
134140
}
135141
catch (Exception ex)
136142
{

‎src/Agent.Worker/TaskManager.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,10 @@ public sealed class Node24HandlerData : BaseNodeHandlerData
906906
{
907907
public override int Priority => 101;
908908
}
909+
public sealed class CustomNodeHandlerData : BaseNodeHandlerData
910+
{
911+
public override int Priority => 100;
912+
}
909913

910914
public sealed class PowerShell3HandlerData : HandlerData
911915
{

‎src/Test/L0/NodeHandlerL0.TestSpecifications.cs‎

Lines changed: 119 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,124 @@ public static class NodeHandlerTestSpecs
1414
{
1515
public static readonly TestScenario[] AllScenarios = new[]
1616
{
17-
17+
// ============================================
18+
// GROUP 0: CUSTOM NODE SCENARIOS
19+
// ============================================
20+
new TestScenario(
21+
name: "CustomNode_Host_OverridesHandlerData",
22+
description: "Custom node path takes priority over handler data type",
23+
handlerData: typeof(Node20_1HandlerData),
24+
customNodePath: "/usr/local/custom/node",
25+
inContainer: false,
26+
expectedNode: "/usr/local/custom/node"
27+
),
28+
29+
new TestScenario(
30+
name: "CustomNode_Host_BypassesAllKnobs",
31+
description: "Custom node path ignores all global node version knobs",
32+
handlerData: typeof(Node10HandlerData),
33+
knobs: new()
34+
{
35+
["AGENT_USE_NODE24"] = "true",
36+
["AGENT_USE_NODE20_1"] = "true",
37+
["AGENT_USE_NODE10"] = "true"
38+
},
39+
customNodePath: "/opt/my-node/bin/node",
40+
inContainer: false,
41+
expectedNode: "/opt/my-node/bin/node"
42+
),
43+
44+
new TestScenario(
45+
name: "CustomNode_Host_BypassesEOLPolicy",
46+
description: "Custom node path bypasses EOL policy restrictions",
47+
handlerData: typeof(Node10HandlerData),
48+
knobs: new()
49+
{
50+
["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true"
51+
},
52+
customNodePath: "/legacy/node6/bin/node",
53+
inContainer: false,
54+
expectedNode: "/legacy/node6/bin/node"
55+
),
56+
57+
new TestScenario(
58+
name: "CustomNode_Container_OverridesHandlerData",
59+
description: "Container custom node path overrides task handler data",
60+
handlerData: typeof(Node24HandlerData),
61+
customNodePath: "/container/node20/bin/node",
62+
inContainer: true,
63+
expectedNode: "/container/node20/bin/node"
64+
),
65+
66+
new TestScenario(
67+
name: "CustomNode_HighestPriority_OverridesEverything",
68+
description: "Custom path has highest priority - overrides all knobs, EOL policy, and glibc errors",
69+
handlerData: typeof(Node10HandlerData),
70+
knobs: new()
71+
{
72+
["AGENT_USE_NODE24"] = "true",
73+
["AGENT_USE_NODE20_1"] = "true",
74+
["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true",
75+
["AGENT_USE_NODE24_WITH_HANDLER_DATA"] = "false"
76+
},
77+
node20GlibcError: true,
78+
node24GlibcError: true,
79+
customNodePath: "/ultimate/override/node",
80+
inContainer: false,
81+
expectedNode: "/ultimate/override/node"
82+
),
83+
84+
new TestScenario(
85+
name: "CustomNode_NullPath_FallsBackToNormalLogic",
86+
description: "Null custom node path falls back to standard node selection",
87+
handlerData: typeof(Node24HandlerData),
88+
knobs: new() { ["AGENT_USE_NODE24_WITH_HANDLER_DATA"] = "true" },
89+
customNodePath: null,
90+
inContainer: false,
91+
expectedNode: "node24"
92+
),
93+
94+
new TestScenario(
95+
name: "CustomNode_EmptyString_IgnoredFallsBackToNormalLogic",
96+
description: "Empty custom node path is ignored, falls back to normal handler logic",
97+
handlerData: typeof(Node20_1HandlerData),
98+
customNodePath: "",
99+
inContainer: false,
100+
expectedNode: "node20_1"
101+
),
102+
103+
new TestScenario(
104+
name: "CustomNode_WhitespaceOnly_IgnoredFallsBackToNormalLogic",
105+
description: "Whitespace-only custom node path is ignored, falls back to normal handler logic",
106+
handlerData: typeof(Node16HandlerData),
107+
customNodePath: " ",
108+
inContainer: false,
109+
expectedNode: "node16"
110+
),
111+
112+
new TestScenario(
113+
name: "CustomNode_Container_OverridesContainerKnobs",
114+
description: "Container custom node path overrides container-specific knobs",
115+
handlerData: typeof(Node20_1HandlerData),
116+
knobs: new()
117+
{
118+
["AGENT_USE_NODE24_TO_START_CONTAINER"] = "true",
119+
["AGENT_USE_NODE20_TO_START_CONTAINER"] = "true"
120+
},
121+
customNodePath: "/container/custom/node",
122+
inContainer: true,
123+
expectedNode: "/container/custom/node"
124+
),
125+
126+
new TestScenario(
127+
name: "CustomNode_MixedEnvironments_ContainerTakesPrecedence",
128+
description: "When in container environment, Container.CustomNodePath is used over StepTarget.CustomNodePath",
129+
handlerData: typeof(Node24HandlerData),
130+
customNodePath: "/container/custom/node",
131+
inContainer: true,
132+
expectedNode: "/container/custom/node"
133+
),
134+
18135
// ========================================================================================
19136
// GROUP 1: NODE6 SCENARIOS (Node6HandlerData - EOL)
20137
// ========================================================================================
@@ -734,4 +851,4 @@ public TestScenario(
734851
CustomNodePath = customNodePath;
735852
}
736853
}
737-
}
854+
}

‎src/Test/L0/NodeHandlerTestBase.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,21 +77,22 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useStrategy)
7777
nodeHandler.Data = CreateHandlerData(scenario.HandlerDataType);
7878

7979
var expectations = GetScenarioExpectations(scenario, useStrategy);
80+
81+
try{
8082

81-
try
82-
{
8383
string actualLocation = nodeHandler.GetNodeLocation(
8484
node20ResultsInGlibCError: scenario.Node20GlibcError,
8585
node24ResultsInGlibCError: scenario.Node24GlibcError,
8686
inContainer: scenario.InContainer);
87+
8788
string expectedLocation = GetExpectedNodeLocation(expectations.ExpectedNode, scenario, thc);
8889
Assert.Equal(expectedLocation, actualLocation);
8990
}
9091
catch (Exception ex)
9192
{
9293
Assert.NotNull(ex);
9394
Assert.IsType(scenario.ExpectedErrorType, ex);
94-
95+
9596
if (!string.IsNullOrEmpty(expectations.ExpectedError))
9697
{
9798
Assert.Contains(expectations.ExpectedError, ex.Message);

0 commit comments

Comments
 (0)