Skip to content

Commit 8a5f98c

Browse files
Revert container startup command in container provider (#5556)
* container startup windows command fix * updated knob name and L0 tests * added debug logs
1 parent a3f05f2 commit 8a5f98c

7 files changed

Lines changed: 33 additions & 27 deletions

File tree

src/Agent.Sdk/Knob/AgentKnobs.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -964,12 +964,12 @@ public class AgentKnobs
964964
new EnvironmentKnobSource("AGENT_ENABLE_DOCKER_EXEC_DIAGNOSTICS"),
965965
new BuiltInDefaultKnobSource("false"));
966966

967-
public static readonly Knob UseNodeVersionStrategy = new Knob(
968-
nameof(UseNodeVersionStrategy),
969-
"If true, use the strategy pattern for Node.js version selection (both host and container). This provides centralized node selection logic with EOL policy enforcement. Set to false to use legacy node selection logic.",
970-
new PipelineFeatureSource("UseNodeVersionStrategy"),
971-
new RuntimeKnobSource("AGENT_USE_NODE_STRATEGY"),
972-
new EnvironmentKnobSource("AGENT_USE_NODE_STRATEGY"),
967+
public static readonly Knob UseEnhancedNodeSelection = new Knob(
968+
nameof(UseEnhancedNodeSelection),
969+
"If true, use the enhanced Node.js version selection logic (both host and container). This provides centralized node selection with EOL policy enforcement and correct container keepalive. Set to false to use legacy node selection logic.",
970+
new PipelineFeatureSource("UseEnhancedNodeSelection"),
971+
new RuntimeKnobSource("AGENT_USE_ENHANCED_NODE_SELECTION"),
972+
new EnvironmentKnobSource("AGENT_USE_ENHANCED_NODE_SELECTION"),
973973
new BuiltInDefaultKnobSource("false"));
974974
}
975975
}

src/Agent.Worker/ContainerOperationProvider.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ private async Task StartContainerAsync(IExecutionContext executionContext, Conta
553553
container.MountVolumes.Add(new MountVolume(taskKeyFile, container.TranslateToContainerPath(taskKeyFile)));
554554
}
555555

556-
bool UseNodeVersionStrategy = AgentKnobs.UseNodeVersionStrategy.GetValue(executionContext).AsBoolean();
556+
bool useEnhancedNodeSelection = AgentKnobs.UseEnhancedNodeSelection.GetValue(executionContext).AsBoolean();
557557
bool useNode20ToStartContainer = AgentKnobs.UseNode20ToStartContainer.GetValue(executionContext).AsBoolean();
558558
bool useNode24ToStartContainer = AgentKnobs.UseNode24ToStartContainer.GetValue(executionContext).AsBoolean();
559559
bool useAgentNode = false;
@@ -581,17 +581,18 @@ string containerNodePath(string nodeFolder)
581581
dockerObject: container.ContainerImage,
582582
options: $"--format=\"{{{{index .Config.Labels \\\"{_nodeJsPathLabel}\\\"}}}}\"");
583583

584-
if (UseNodeVersionStrategy)
584+
if (useEnhancedNodeSelection)
585585
{
586+
executionContext.Debug("[ContainerSetup] Using enhanced node selection path for container startup.");
586587
bool isWindowsContainer = container.ImageOS == PlatformUtil.OS.Windows;
587588

588589
container.ContainerCommand = isWindowsContainer
589-
? "cmd.exe /c timeout /t -1 /nobreak > nul"
590+
? "cmd.exe /c ping -t localhost > nul"
590591
: "sleep infinity";
591592
}
592593
else
593594
{
594-
595+
executionContext.Debug("[ContainerSetup] Using legacy node selection path for container startup.");
595596
// Legacy approach: Use node-based startup command
596597
string nodeSetInterval(string node)
597598
{
@@ -674,10 +675,11 @@ string useDoubleQuotes(string value)
674675

675676
executionContext.Warning($"Docker container {container.ContainerId} is not in running state.");
676677
}
677-
else if (UseNodeVersionStrategy && container.IsJobContainer)
678+
else if (useEnhancedNodeSelection && container.IsJobContainer)
678679
{
679680
try
680681
{
682+
executionContext.Debug("[ContainerSetup] Calling enhanced node selection path for container startup.");
681683
SetContainerNodePathWithOrchestrator(executionContext, container);
682684
}
683685
catch (Exception ex)

src/Agent.Worker/ContainerOperationProviderEnhanced.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ private async Task StartContainerAsync(IExecutionContext executionContext, Conta
662662
$"{m.SourceVolumePath ?? "anonymous"}:{m.TargetVolumePath}{(m.ReadOnly ? ":ro" : "")}");
663663
trace.Info($"Configured {container.MountVolumes.Count} volume mounts: {string.Join(", ", mountSummary)}");
664664

665-
bool useNodeVersionStrategy = AgentKnobs.UseNodeVersionStrategy.GetValue(executionContext).AsBoolean();
665+
bool useEnhancedNodeSelection = AgentKnobs.UseEnhancedNodeSelection.GetValue(executionContext).AsBoolean();
666666
bool useNode20ToStartContainer = AgentKnobs.UseNode20ToStartContainer.GetValue(executionContext).AsBoolean();
667667
bool useNode24ToStartContainer = AgentKnobs.UseNode24ToStartContainer.GetValue(executionContext).AsBoolean();
668668
bool useAgentNode = false;
@@ -690,17 +690,18 @@ string containerNodePath(string nodeFolder)
690690
dockerObject: container.ContainerImage,
691691
options: $"--format=\"{{{{index .Config.Labels \\\"{_nodeJsPathLabel}\\\"}}}}\"");
692692

693-
if (useNodeVersionStrategy)
693+
if (useEnhancedNodeSelection)
694694
{
695+
executionContext.Debug("[ContainerSetup] Using enhanced node selection path for container startup.");
695696
bool isWindowsContainer = container.ImageOS == PlatformUtil.OS.Windows;
696697

697698
container.ContainerCommand = isWindowsContainer
698-
? "cmd.exe /c timeout /t -1 /nobreak > nul"
699+
? "cmd.exe /c ping -t localhost > nul"
699700
: "sleep infinity";
700701
}
701702
else
702703
{
703-
704+
executionContext.Debug("[ContainerSetup] Using legacy node selection path for container startup.");
704705
// Legacy approach: Use node-based startup command
705706
string nodeSetInterval(string node)
706707
{
@@ -814,10 +815,11 @@ string useDoubleQuotes(string value)
814815

815816
executionContext.Warning($"Docker container {container.ContainerId} is not in running state.");
816817
}
817-
else if (useNodeVersionStrategy && container.IsJobContainer)
818+
else if (useEnhancedNodeSelection && container.IsJobContainer)
818819
{
819820
try
820821
{
822+
executionContext.Debug("[ContainerSetup] Calling enhanced node selection path for container startup.");
821823
SetContainerNodePathWithOrchestrator(executionContext, container);
822824
}
823825
catch (Exception ex)

src/Agent.Worker/Handlers/NodeHandler.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,15 @@ private string GetNodeFolderWithFallback(string preferredNodeFolder, bool node20
421421

422422
public string GetNodeLocation(bool node20ResultsInGlibCError, bool node24ResultsInGlibCError, bool inContainer)
423423
{
424-
bool useStrategyPattern = AgentKnobs.UseNodeVersionStrategy.GetValue(ExecutionContext).AsBoolean();
425-
424+
bool useStrategyPattern = AgentKnobs.UseEnhancedNodeSelection.GetValue(ExecutionContext).AsBoolean();
425+
426426
if (useStrategyPattern)
427427
{
428+
ExecutionContext.Debug("Using enhanced node selection path for handler node resolution.");
428429
return GetNodeLocationUsingStrategy(inContainer).GetAwaiter().GetResult();
429430
}
430431

432+
ExecutionContext.Debug("Using legacy node selection path for handler node resolution.");
431433
return GetNodeLocationLegacy(node20ResultsInGlibCError, node24ResultsInGlibCError, inContainer);
432434
}
433435

src/Test/L0/NodeHandlerTestBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useStrategy)
5858
Environment.SetEnvironmentVariable(knob.Key, knob.Value);
5959
}
6060

61-
Environment.SetEnvironmentVariable("AGENT_USE_NODE_STRATEGY", useStrategy ? "true" : "false");
61+
Environment.SetEnvironmentVariable("AGENT_USE_ENHANCED_NODE_SELECTION", useStrategy ? "true" : "false");
6262

6363
try
6464
{
@@ -476,7 +476,7 @@ protected void ResetEnvironment()
476476

477477
// EOL and strategy control
478478
Environment.SetEnvironmentVariable("AGENT_RESTRICT_EOL_NODE_VERSIONS", null);
479-
Environment.SetEnvironmentVariable("AGENT_USE_NODE_STRATEGY", null);
479+
Environment.SetEnvironmentVariable("AGENT_USE_ENHANCED_NODE_SELECTION", null);
480480

481481
// System-specific knobs
482482
Environment.SetEnvironmentVariable("AGENT_USE_NODE20_IN_UNSUPPORTED_SYSTEM", null);

src/Test/L0/Worker/ContainerOperationProviderEnhancedL0.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace Microsoft.VisualStudio.Services.Agent.Tests.Worker
1616
public sealed class ContainerOperationProviderEnhancedL0 : ContainerOperationProviderL0Base
1717
{
1818
// =============================================
19-
// Legacy path tests (UseNodeVersionStrategy = false)
19+
// Legacy path tests (UseEnhancedNodeSelection = false)
2020
// =============================================
2121

2222
[Fact]
@@ -175,7 +175,7 @@ public async Task StartContainer_WithoutDockerLabel_OnLinux_UsesAgentNode()
175175
}
176176

177177
// =============================================
178-
// UseNodeVersionStrategy path tests
178+
// UseEnhancedNodeSelection path tests
179179
// Knob activated via Moq-specific matcher overrides
180180
// in CreateExecutionContextMock(hc, useNodeVersionStrategy: true).
181181
// No env vars used — avoids parallel test pollution.
@@ -184,7 +184,7 @@ public async Task StartContainer_WithoutDockerLabel_OnLinux_UsesAgentNode()
184184
[Fact]
185185
[Trait("Level", "L0")]
186186
[Trait("Category", "Worker")]
187-
public async Task StartContainer_UseNodeVersionStrategy_WithDockerLabel_UsesSleepOrTimeout()
187+
public async Task StartContainer_UseNodeVersionStrategy_WithDockerLabel_UsesSleepOrPing()
188188
{
189189
using (var hc = new TestHostContext(this))
190190
{
@@ -209,7 +209,7 @@ public async Task StartContainer_UseNodeVersionStrategy_WithDockerLabel_UsesSlee
209209
Assert.Equal(NodePathFromLabel, container.CustomNodePath);
210210
if (PlatformUtil.RunningOnWindows)
211211
{
212-
Assert.Contains("timeout", container.ContainerCommand);
212+
Assert.Contains("ping -t localhost", container.ContainerCommand);
213213
}
214214
else
215215
{
@@ -255,7 +255,7 @@ public async Task StartContainer_UseNodeVersionStrategy_OnWindows_LinuxContainer
255255
[Trait("Category", "Worker")]
256256
[Trait("SkipOn", "darwin")]
257257
[Trait("SkipOn", "linux")]
258-
public async Task StartContainer_UseNodeVersionStrategy_OnWindows_WindowsContainer_UsesTimeout()
258+
public async Task StartContainer_UseNodeVersionStrategy_OnWindows_WindowsContainer_UsesPing()
259259
{
260260
if (!PlatformUtil.RunningOnWindows)
261261
{
@@ -278,7 +278,7 @@ public async Task StartContainer_UseNodeVersionStrategy_OnWindows_WindowsContain
278278

279279
await provider.StartContainersAsync(executionContext.Object, new List<ContainerInfo> { container });
280280

281-
Assert.Contains("timeout", container.ContainerCommand);
281+
Assert.Contains("ping -t localhost", container.ContainerCommand);
282282
Assert.Contains("nul", container.ContainerCommand);
283283
}
284284
}

src/Test/L0/Worker/ContainerOperationProviderL0Base.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ protected Mock<IExecutionContext> CreateExecutionContextMock(TestHostContext hc,
6464
executionContext.Setup(x => x.GetScopedEnvironment()).Returns(new SystemEnvironment());
6565

6666
string knobValue = useNodeVersionStrategy ? "true" : "false";
67-
executionContext.Setup(x => x.GetVariableValueOrDefault("AGENT_USE_NODE_STRATEGY")).Returns(knobValue);
67+
executionContext.Setup(x => x.GetVariableValueOrDefault("AGENT_USE_ENHANCED_NODE_SELECTION")).Returns(knobValue);
6868

6969
return executionContext;
7070
}

0 commit comments

Comments
 (0)