Skip to content

Commit 41b50c9

Browse files
isra-felisra-fel
authored
Fix Importing Az in Parallel (#18483)
* use mutex to protect config init * Fix ConfigKeys reference
1 parent 18b119b commit 41b50c9

18 files changed

+728
-484
lines changed

‎src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ public void OnImport()
670670
try
671671
{
672672
#endif
673-
AzureSessionInitializer.InitializeAzureSession();
673+
AzureSessionInitializer.InitializeAzureSession(WriteInitializationWarnings);
674674
AzureSessionInitializer.MigrateAdalCache(AzureSession.Instance, GetAzureContextContainer, WriteInitializationWarnings);
675675
#if DEBUG
676676
if (!TestMockSupport.RunningMocked)

‎src/Accounts/Accounts/ChangeLog.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@
1919
-->
2020

2121
## Upcoming Release
22-
* Fixed incorrect access token [#18105]
2322
* Supported exporting and importing configurations by `Export-AzConfig` and `Import-AzConfig`.
24-
* Upgraded version of Microsoft.Identity.Client for .NET Framework [#18495]
23+
* Fixed an issue that Az.Accounts may fail to be imported in parallel PowerShell processes. [#18321]
24+
* Fixed incorrect access token [#18105]
25+
* Upgraded version of Microsoft.Identity.Client for .NET Framework. [#18495]
2526
* Fixed an issue that Az.Accounts failed to be imported if multiple environment variables, which only differ by case, are set. [#18304]
2627

2728
## Version 2.8.0

‎src/Accounts/Authentication.Test/ConfigTests/ClearConfigTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using Microsoft.Azure.Commands.Common.Exceptions;
1615
using Microsoft.Azure.Commands.Common.Authentication.Config;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
1717
using Microsoft.Azure.PowerShell.Common.Config;
1818
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
1919
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
20-
using Xunit;
2120
using System.Linq;
21+
using Xunit;
2222

2323
namespace Microsoft.Azure.Authentication.Test.Config
2424
{

‎src/Accounts/Authentication.Test/ConfigTests/ConfigDefinitionTests.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,21 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using Microsoft.Azure.Commands.Common.Exceptions;
1615
using Microsoft.Azure.Commands.Common.Authentication.Config;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
17+
using Microsoft.Azure.PowerShell.Common.Config;
1718
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
18-
using Microsoft.WindowsAzure.Commands.Common;
1919
using System;
20-
using System.Linq;
2120
using Xunit;
22-
using Microsoft.Azure.PowerShell.Common.Config;
2321

2422
namespace Microsoft.Azure.Authentication.Test.Config
2523
{
2624
public class ConfigDefinitionTests : ConfigTestsBase
2725
{
2826
[Fact]
2927
[Trait(TestTraits.AcceptanceType, TestTraits.CheckIn)]
30-
public void CanValidateInput() {
28+
public void CanValidateInput()
29+
{
3130
const string boolKey = "BoolKey";
3231
var boolConfig = new SimpleTypedConfig<bool>(boolKey, "", false);
3332
var rangedIntConfig = new RangedConfig();

‎src/Accounts/Authentication.Test/ConfigTests/ConfigTestsBase.cs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,19 @@ public class ConfigTestsBase
4545
/// <returns>A config manager with initial state, ready to use.</returns>
4646
internal IConfigManager GetConfigManager(InitSettings settings, params ConfigDefinition[] config)
4747
{
48-
var dataStore = settings?.DataStore ?? new MockDataStore();
4948
var configPath = settings?.Path ?? Path.GetRandomFileName();
49+
var dataStore = settings?.DataStore;
50+
if (dataStore == null)
51+
{
52+
dataStore = new MockDataStore();
53+
}
54+
if (!dataStore.FileExists(configPath))
55+
{
56+
dataStore.WriteFile(configPath, @"{}");
57+
}
5058
var environmentVariableProvider = settings?.EnvironmentVariableProvider ?? new MockEnvironmentVariableProvider();
5159

52-
ConfigInitializer ci = new ConfigInitializer(new List<string>() { configPath })
53-
{
54-
DataStore = dataStore,
55-
EnvironmentVariableProvider = environmentVariableProvider
56-
};
57-
IConfigManager icm = ci.GetConfigManager();
60+
IConfigManager icm = new DefaultConfigManager(configPath, dataStore, environmentVariableProvider);
5861
foreach (var configDefinition in config)
5962
{
6063
icm.RegisterConfig(configDefinition);

‎src/Accounts/Authentication.Test/ConfigTests/GetConfigTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using Microsoft.Azure.Commands.Common.Exceptions;
1615
using Microsoft.Azure.Commands.Common.Authentication.Config;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
1717
using Microsoft.Azure.PowerShell.Common.Config;
1818
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
1919
using System.Linq;

‎src/Accounts/Authentication.Test/ConfigTests/RegisterConfigTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using Microsoft.Azure.Commands.Common.Exceptions;
1615
using Microsoft.Azure.Commands.Common.Authentication.Config;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
1717
using Microsoft.Azure.PowerShell.Common.Config;
1818
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
1919
using System;

‎src/Accounts/Authentication.Test/ConfigTests/UpdateConfigTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15-
using Microsoft.Azure.Commands.Common.Exceptions;
1615
using Microsoft.Azure.Commands.Common.Authentication.Config;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
1717
using Microsoft.Azure.PowerShell.Common.Config;
1818
using Microsoft.Rest.ClientRuntime.Azure.TestFramework;
1919
using Moq;

‎src/Accounts/Authentication/AzureSessionInitializer.cs

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
using TraceLevel = System.Diagnostics.TraceLevel;
3131
using System.Collections.Generic;
32+
using System.Threading;
3233

3334
namespace Microsoft.Azure.Commands.Common.Authentication
3435
{
@@ -43,9 +44,9 @@ public static class AzureSessionInitializer
4344
/// <summary>
4445
/// Initialize the azure session if it is not already initialized
4546
/// </summary>
46-
public static void InitializeAzureSession()
47+
public static void InitializeAzureSession(Action<string> writeWarning = null)
4748
{
48-
AzureSession.Initialize(() => CreateInstance());
49+
AzureSession.Initialize(() => CreateInstance(null, writeWarning));
4950
}
5051

5152
/// <summary>
@@ -137,7 +138,7 @@ public static void MigrateAdalCache(IAzureSession session, Func<IAzureContextCon
137138
new AdalTokenMigrator(adalData, getContextContainer).MigrateFromAdalToMsal();
138139
}
139140
}
140-
catch(Exception e)
141+
catch (Exception e)
141142
{
142143
writeWarning(Resources.FailedToMigrateAdal2Msal.FormatInvariant(e.Message));
143144
}
@@ -206,7 +207,7 @@ static void InitializeDataCollection(IAzureSession session)
206207
session.RegisterComponent(DataCollectionController.RegistryKey, () => DataCollectionController.Create(session));
207208
}
208209

209-
static IAzureSession CreateInstance(IDataStore dataStore = null)
210+
static IAzureSession CreateInstance(IDataStore dataStore = null, Action<string> writeWarning = null)
210211
{
211212
string profilePath = Path.Combine(
212213
Environment.GetFolderPath(Environment.SpecialFolder.UserProfile),
@@ -239,23 +240,52 @@ static IAzureSession CreateInstance(IDataStore dataStore = null)
239240
session.TokenCacheDirectory = autoSave.CacheDirectory;
240241
session.TokenCacheFile = autoSave.CacheFile;
241242

242-
InitializeConfigs(session, profilePath);
243+
InitializeConfigs(session, profilePath, writeWarning);
243244
InitializeDataCollection(session);
244245
session.RegisterComponent(HttpClientOperationsFactory.Name, () => HttpClientOperationsFactory.Create());
245246
session.TokenCache = session.TokenCache ?? new AzureTokenCache();
246247
return session;
247248
}
248249

249-
private static void InitializeConfigs(AzureSession session, string profilePath)
250+
private static void InitializeConfigs(AzureSession session, string profilePath, Action<string> writeWarning)
250251
{
252+
if (writeWarning == null) { writeWarning = (string s) => { }; };
251253
var fallbackList = new List<string>()
252254
{
253255
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".Azure", "PSConfig.json"),
254256
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), ".Azure", "PSConfig.json")
255257
};
256258
ConfigInitializer configInitializer = new ConfigInitializer(fallbackList);
257-
configInitializer.MigrateConfigs(profilePath);
258-
configInitializer.InitializeForAzureSession(session);
259+
260+
using (var mutex = new Mutex(false, @"Global\AzurePowerShellConfigInit"))
261+
{
262+
if (mutex.WaitOne(1000))
263+
{
264+
// regular initialization
265+
try
266+
{
267+
configInitializer.MigrateConfigs(profilePath);
268+
configInitializer.Initialize(session);
269+
return; // done, return.
270+
}
271+
catch (Exception ex)
272+
{
273+
writeWarning($"[AzureSessionInitializer] Failed to initialize the configs, reason: {ex.Message}.");
274+
}
275+
finally
276+
{
277+
mutex.ReleaseMutex();
278+
}
279+
}
280+
else
281+
{
282+
writeWarning($"[AzureSessionInitializer] Timed out when initializing the configs.");
283+
}
284+
285+
// initialize in safe mode and do not try to migrate configs
286+
writeWarning($"[AzureSessionInitializer] Config manager will be re-initialized in safe mode. All configs will have only default values.");
287+
configInitializer.SafeInitialize(session);
288+
}
259289
}
260290

261291
public class AdalSession : AzureSession

‎src/Accounts/Authentication/Config/ConfigInitializer.cs

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ namespace Microsoft.Azure.Commands.Common.Authentication.Config
3333
internal class ConfigInitializer
3434
{
3535
internal IDataStore DataStore { get; set; } = new DiskDataStore();
36-
private static readonly object _fsLock = new object();
3736

3837
internal IEnvironmentVariableProvider EnvironmentVariableProvider { get; set; } = new DefaultEnvironmentVariableProvider();
3938

40-
public string ConfigPath
39+
private string ConfigPath
4140
{
4241
get
4342
{
@@ -47,6 +46,10 @@ public string ConfigPath
4746
}
4847
return _cachedConfigPath;
4948
}
49+
set
50+
{
51+
_cachedConfigPath = value;
52+
}
5053
}
5154
private string _cachedConfigPath;
5255
private readonly IEnumerable<string> _configPathCandidates;
@@ -87,16 +90,7 @@ private string GetPathToConfigFile(IEnumerable<string> paths)
8790
continue;
8891
}
8992
}
90-
throw new ApplicationException($"Failed to store the config file. Please make sure any one of the following paths is accessible: {string.Join(", ", paths)}");
91-
}
92-
93-
internal IConfigManager GetConfigManager()
94-
{
95-
lock (_fsLock)
96-
{
97-
ValidateConfigFile();
98-
}
99-
return new ConfigManager(ConfigPath, DataStore, EnvironmentVariableProvider);
93+
throw new ApplicationException($"Failed to create the config file. Please make sure any one of the following paths is accessible: {string.Join(", ", paths)}");
10094
}
10195

10296
private void ValidateConfigFile()
@@ -122,46 +116,62 @@ private void ResetConfigFileToDefault()
122116
}
123117
}
124118

125-
// todo: tests initializes configs in a different way. Maybe there should be an abstraction IConfigInitializer and two concrete classes ConfigInitializer + TestConfigInitializer
126-
internal void InitializeForAzureSession(AzureSession session)
119+
/// <summary>
120+
/// Initializes the config manager and registers it to <see cref="AzureSession"/>.
121+
/// </summary>
122+
/// <param name="session"></param>
123+
public void Initialize(IAzureSession session)
124+
{
125+
ValidateConfigFile();
126+
Initialize(session, new DefaultConfigManager(ConfigPath, DataStore, EnvironmentVariableProvider));
127+
}
128+
129+
private void Initialize(IAzureSession session, IConfigManager configManager)
127130
{
128-
IConfigManager configManager = GetConfigManager();
129-
session.RegisterComponent(nameof(IConfigManager), () => configManager);
131+
session.RegisterComponent(nameof(IConfigManager), () => configManager, true);
130132
RegisterConfigs(configManager);
131133
configManager.BuildConfig();
132134
}
133135

136+
/// <summary>
137+
/// Initializes the config manager with limited capabilities but in a safe manner.
138+
/// Registers it to <see cref="AzureSession"/>.
139+
/// </summary>
140+
/// <param name="session"></param>
141+
public void SafeInitialize(IAzureSession session)
142+
{
143+
ConfigPath = "";
144+
Initialize(session, new SafeConfigManager());
145+
}
146+
134147
/// <summary>
135148
/// Migrates independent configs to the new config framework.
136149
/// </summary>
137150
/// <param name="profilePath">Path of session profile where old config files are stored.</param>
138151
internal void MigrateConfigs(string profilePath)
139152
{
140-
lock (_fsLock)
153+
// Migrate data collection config
154+
string dataCollectionConfigPath = Path.Combine(profilePath, AzurePSDataCollectionProfile.DefaultFileName);
155+
const string legacyConfigKey = "enableAzureDataCollection";
156+
// Migrate only when:
157+
// 1. Old config file exists
158+
// 2. New config file does not exist
159+
if (DataStore.FileExists(dataCollectionConfigPath) && _configPathCandidates.All(path => !DataStore.FileExists(path)))
141160
{
142-
// Migrate data collection config
143-
string dataCollectionConfigPath = Path.Combine(profilePath, AzurePSDataCollectionProfile.DefaultFileName);
144-
const string legacyConfigKey = "enableAzureDataCollection";
145-
// Migrate only when:
146-
// 1. Old config file exists
147-
// 2. New config file does not exist
148-
if (DataStore.FileExists(dataCollectionConfigPath) && _configPathCandidates.All(path => !DataStore.FileExists(path)))
161+
try
149162
{
150-
try
163+
string json = DataStore.ReadFileAsText(dataCollectionConfigPath);
164+
JObject root = JObject.Parse(json);
165+
if (root.TryGetValue(legacyConfigKey, out JToken jToken))
151166
{
152-
string json = DataStore.ReadFileAsText(dataCollectionConfigPath);
153-
JObject root = JObject.Parse(json);
154-
if (root.TryGetValue(legacyConfigKey, out JToken jToken))
155-
{
156-
bool enabled = ((bool)jToken);
157-
new JsonConfigHelper(ConfigPath, DataStore).Update(ConfigPathHelper.GetPathOfConfig(ConfigKeys.EnableDataCollection), enabled);
158-
}
159-
}
160-
catch (Exception)
161-
{
162-
// do not throw for file IO exceptions
167+
bool enabled = ((bool)jToken);
168+
new JsonConfigHelper(ConfigPath, DataStore).Update(ConfigPathHelper.GetPathOfConfig(ConfigKeys.EnableDataCollection), enabled);
163169
}
164170
}
171+
catch (Exception)
172+
{
173+
// do not throw for file IO exceptions
174+
}
165175
}
166176
}
167177

0 commit comments

Comments
 (0)