Skip to content

Commit 8ad9dd8

Browse files
committed
Ensure Instance specialized on assignment failure
1 parent 14f7d69 commit 8ad9dd8

File tree

3 files changed

+53
-8
lines changed

3 files changed

+53
-8
lines changed

src/WebJobs.Script.WebHost/Management/InstanceManager.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,6 @@ private async Task Assign(HostAssignmentContext assignmentContext)
103103
// first make all environment and file system changes required for
104104
// the host to be specialized
105105
await ApplyContext(assignmentContext);
106-
107-
// all assignment settings/files have been applied so we can flip
108-
// the switch now on specialization
109-
_logger.LogInformation("Triggering specialization");
110-
_webHostEnvironment.FlagAsSpecializedAndReady();
111106
}
112107
catch (Exception ex)
113108
{
@@ -116,6 +111,13 @@ private async Task Assign(HostAssignmentContext assignmentContext)
116111
}
117112
finally
118113
{
114+
// all assignment settings/files have been applied so we can flip
115+
// the switch now on specialization
116+
// even if there are failures applying context above, we want to
117+
// leave placeholder mode
118+
_logger.LogInformation("Triggering specialization");
119+
_webHostEnvironment.FlagAsSpecializedAndReady();
120+
119121
_webHostEnvironment.ResumeRequests();
120122
}
121123
}
@@ -173,5 +175,11 @@ public IDictionary<string, string> GetInstanceInfo()
173175
{ "WEBSITE_NODE_DEFAULT_VERSION", "8.5.0" }
174176
};
175177
}
178+
179+
// for testing
180+
internal static void Reset()
181+
{
182+
_assignmentContext = null;
183+
}
176184
}
177185
}

test/WebJobs.Script.Tests.Integration/Management/InstanceManagerTests.cs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace Microsoft.Azure.WebJobs.Script.Tests.Managment
2121
public class InstanceManagerTests : IDisposable
2222
{
2323
private readonly TestLoggerProvider _loggerProvider;
24-
private readonly TestEnvironment _environment;
24+
private readonly TestEnvironmentEx _environment;
2525
private readonly ScriptWebHostEnvironment _scriptWebEnvironment;
2626
private readonly InstanceManager _instanceManager;
2727
private readonly HttpClient _httpClient;
@@ -34,11 +34,13 @@ public InstanceManagerTests()
3434
var loggerFactory = new LoggerFactory();
3535
loggerFactory.AddProvider(_loggerProvider);
3636

37-
_environment = new TestEnvironment();
37+
_environment = new TestEnvironmentEx();
3838
_scriptWebEnvironment = new ScriptWebHostEnvironment(_environment);
3939

4040
var optionsFactory = new TestOptionsFactory<ScriptApplicationHostOptions>(new ScriptApplicationHostOptions());
4141
_instanceManager = new InstanceManager(optionsFactory, _httpClient, _scriptWebEnvironment, _environment, loggerFactory.CreateLogger<InstanceManager>());
42+
43+
InstanceManager.Reset();
4244
}
4345

4446
[Fact]
@@ -86,6 +88,29 @@ public async Task StartAssignment_AppliesAssignmentContext()
8688
p => Assert.StartsWith("Assign called while host is not in placeholder mode", p));
8789
}
8890

91+
[Fact]
92+
public async Task StartAssignment_Failure_ExitsPlaceholderMode()
93+
{
94+
_environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "1");
95+
var context = new HostAssignmentContext
96+
{
97+
Environment = new Dictionary<string, string>
98+
{
99+
// force the assignment to fail
100+
{ "throw", "test" }
101+
}
102+
};
103+
bool result = _instanceManager.StartAssignment(context);
104+
Assert.True(result);
105+
Assert.True(_scriptWebEnvironment.InStandbyMode);
106+
107+
await TestHelpers.Await(() => !_scriptWebEnvironment.InStandbyMode, timeout: 5000);
108+
109+
var error = _loggerProvider.GetAllLogMessages().Where(p => p.Level == LogLevel.Error).First();
110+
Assert.Equal("Assign failed", error.FormattedMessage);
111+
Assert.Equal("Kaboom!", error.Exception.Message);
112+
}
113+
89114
[Fact]
90115
public void StartAssignment_ReturnsFalse_WhenNotInStandbyMode()
91116
{
@@ -145,5 +170,17 @@ public void Dispose()
145170
{
146171
Environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, null);
147172
}
173+
174+
private class TestEnvironmentEx : TestEnvironment
175+
{
176+
public override void SetEnvironmentVariable(string name, string value)
177+
{
178+
if (name == "throw")
179+
{
180+
throw new InvalidOperationException("Kaboom!");
181+
}
182+
base.SetEnvironmentVariable(name, value);
183+
}
184+
}
148185
}
149186
}

test/WebJobs.Script.Tests.Shared/TestEnvironment.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public string GetEnvironmentVariable(string name)
3535
return result;
3636
}
3737

38-
public void SetEnvironmentVariable(string name, string value)
38+
public virtual void SetEnvironmentVariable(string name, string value)
3939
{
4040
if (value == null && _variables.ContainsKey(name))
4141
{

0 commit comments

Comments
 (0)