-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Refactor workload set and workload install/update logic #39991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
marcpopMSFT
merged 23 commits into
dotnet:release/8.0.4xx
from
dsplaisted:refactor-workload-sets
Jul 1, 2024
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
513a744
Refactor workload update and install commands
dsplaisted a571bba
Handle different workload update / install scenarios
dsplaisted c10f8df
Add Aspire test
dsplaisted 5c6bf09
Roll back install state changes on workload install / update failures
dsplaisted b2ed34c
Fix workload set version prerelease handling
dsplaisted 80557a6
Move WorkloadSetVersionToWorkloadSetPackageVersion to WorkloadSet class
dsplaisted 45d2e5b
Support loading workload sets from different feature bands
dsplaisted 22452a2
Fix issue where on some machines, VM snapshots couldn't be found
dsplaisted e47104b
Fix some tests not to skip manifest updates when installing workloads
dsplaisted e2b177f
Don't try to roll back installation of workload set package that wasn…
dsplaisted d316d96
Don't update pinned workload set on install, refactor InstallingWorkl…
dsplaisted 768ec44
Fix some cases of pinning / unpinning workload version in global inst…
dsplaisted 42019de
Add message for workload set installation failure
dsplaisted 1d87f96
Remove unused isRollback parameter
dsplaisted 515544c
Resolve TODOs in WorkloadInstallCommand
dsplaisted 4bb9d50
Refactor ManifestVersionUpdate
dsplaisted 49a9b98
Fix tests
dsplaisted 0742587
Apply code review feedback
dsplaisted e6e20ad
Fix tests
dsplaisted 708f383
Add tests for --skip-manifest-update, switch to robocopy for VM direc…
dsplaisted 904eb64
Fix issue garbage collecting cross-feature band workload set, add err…
dsplaisted 62c9743
Apply code review feedback and fix test
dsplaisted 979ac00
Fix combinations of workload install, global install state, and globa…
dsplaisted File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
using Microsoft.DotNet.Cli.Utils; | ||
using Microsoft.DotNet.ToolPackage; | ||
using Microsoft.DotNet.Workloads.Workload.Install; | ||
using Microsoft.DotNet.Workloads.Workload.Update; | ||
using Microsoft.Extensions.EnvironmentAbstractions; | ||
using Microsoft.NET.Sdk.WorkloadManifestReader; | ||
using NuGet.Versioning; | ||
|
@@ -34,7 +35,8 @@ internal abstract class InstallingWorkloadCommand : WorkloadCommandBase | |
protected readonly SdkFeatureBand _sdkFeatureBand; | ||
protected readonly ReleaseVersion _targetSdkVersion; | ||
protected readonly string _fromRollbackDefinition; | ||
protected string _workloadSetVersion; | ||
protected string _workloadSetVersionFromCommandLine; | ||
dsplaisted marked this conversation as resolved.
Show resolved
Hide resolved
|
||
protected string _globalJsonPath; | ||
protected string _workloadSetVersionFromGlobalJson; | ||
protected readonly PackageSourceLocation _packageSourceLocation; | ||
protected readonly IWorkloadResolverFactory _workloadResolverFactory; | ||
|
@@ -44,6 +46,10 @@ internal abstract class InstallingWorkloadCommand : WorkloadCommandBase | |
protected IInstaller _workloadInstaller; | ||
protected IWorkloadManifestUpdater _workloadManifestUpdater; | ||
|
||
protected bool UseRollback => !string.IsNullOrWhiteSpace(_fromRollbackDefinition); | ||
protected bool SpecifiedWorkloadSetVersionOnCommandLine => !string.IsNullOrWhiteSpace(_workloadSetVersionFromCommandLine); | ||
protected bool SpecifiedWorkloadSetVersionInGlobalJson => !string.IsNullOrWhiteSpace(_workloadSetVersionFromGlobalJson); | ||
|
||
public InstallingWorkloadCommand( | ||
ParseResult parseResult, | ||
IReporter reporter, | ||
|
@@ -60,6 +66,8 @@ public InstallingWorkloadCommand( | |
_downloadToCacheOption = parseResult.GetValue(InstallingWorkloadCommandParser.DownloadToCacheOption); | ||
|
||
_fromRollbackDefinition = parseResult.GetValue(InstallingWorkloadCommandParser.FromRollbackFileOption); | ||
_workloadSetVersionFromCommandLine = parseResult.GetValue(InstallingWorkloadCommandParser.WorkloadSetVersionOption); | ||
|
||
var configOption = parseResult.GetValue(InstallingWorkloadCommandParser.ConfigOption); | ||
var sourceOption = parseResult.GetValue(InstallingWorkloadCommandParser.SourceOption); | ||
_packageSourceLocation = string.IsNullOrEmpty(configOption) && (sourceOption == null || !sourceOption.Any()) ? null : | ||
|
@@ -91,70 +99,162 @@ public InstallingWorkloadCommand( | |
|
||
_workloadInstallerFromConstructor = workloadInstaller; | ||
_workloadManifestUpdaterFromConstructor = workloadManifestUpdater; | ||
|
||
_globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory); | ||
_workloadSetVersionFromGlobalJson = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(_globalJsonPath); | ||
|
||
if (SpecifiedWorkloadSetVersionInGlobalJson && (SpecifiedWorkloadSetVersionOnCommandLine || UseRollback)) | ||
{ | ||
throw new GracefulException(string.Format(Strings.CannotSpecifyVersionOnCommandLineAndInGlobalJson, _globalJsonPath), isUserError: true); | ||
} | ||
|
||
if (SpecifiedWorkloadSetVersionOnCommandLine && UseRollback) | ||
{ | ||
throw new GracefulException(string.Format(Update.LocalizableStrings.CannotCombineOptions, | ||
InstallingWorkloadCommandParser.FromRollbackFileOption.Name, | ||
InstallingWorkloadCommandParser.WorkloadSetVersionOption.Name), isUserError: true); | ||
} | ||
|
||
// At this point, at most one of SpecifiedWorkloadSetVersionOnCommandLine, UseRollback, and SpecifiedWorkloadSetVersionInGlobalJson is true | ||
} | ||
|
||
protected static Dictionary<string, string> GetInstallStateContents(IEnumerable<ManifestVersionUpdate> manifestVersionUpdates) => | ||
WorkloadSet.FromManifests( | ||
manifestVersionUpdates.Select(update => new WorkloadManifestInfo(update.ManifestId.ToString(), update.NewVersion.ToString(), /* We don't actually use the directory here */ string.Empty, update.NewFeatureBand)) | ||
).ToDictionaryForJson(); | ||
|
||
public static bool ShouldUseWorkloadSetMode(SdkFeatureBand sdkFeatureBand, string dotnetDir) | ||
InstallStateContents GetCurrentInstallState() | ||
{ | ||
string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, dotnetDir), "default.json"); | ||
var installStateContents = File.Exists(path) ? InstallStateContents.FromString(File.ReadAllText(path)) : new InstallStateContents(); | ||
return installStateContents.UseWorkloadSets ?? false; | ||
return GetCurrentInstallState(_sdkFeatureBand, _dotnetPath); | ||
} | ||
|
||
protected void ErrorIfGlobalJsonAndCommandLineMismatch(string globaljsonPath) | ||
static InstallStateContents GetCurrentInstallState(SdkFeatureBand sdkFeatureBand, string dotnetDir) | ||
{ | ||
if (!string.IsNullOrWhiteSpace(_workloadSetVersionFromGlobalJson) && !string.IsNullOrWhiteSpace(_workloadSetVersion) && !_workloadSetVersion.Equals(_workloadSetVersionFromGlobalJson)) | ||
{ | ||
throw new Exception(string.Format(Strings.CannotSpecifyVersionOnCommandLineAndInGlobalJson, globaljsonPath)); | ||
} | ||
string path = Path.Combine(WorkloadInstallType.GetInstallStateFolder(sdkFeatureBand, dotnetDir), "default.json"); | ||
return InstallStateContents.FromPath(path); | ||
} | ||
|
||
protected bool TryHandleWorkloadUpdateFromVersion(ITransactionContext context, DirectoryPath? offlineCache, out IEnumerable<ManifestVersionUpdate> updates) | ||
public static bool ShouldUseWorkloadSetMode(SdkFeatureBand sdkFeatureBand, string dotnetDir) | ||
{ | ||
// Ensure workload set mode is set to 'workloadset' | ||
// Do not skip checking the mode first, as setting it triggers | ||
// an admin authorization popup for MSI-based installs. | ||
if (!ShouldUseWorkloadSetMode(_sdkFeatureBand, _dotnetPath)) | ||
{ | ||
_workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true); | ||
} | ||
|
||
_workloadManifestUpdater.DownloadWorkloadSet(_workloadSetVersionFromGlobalJson ?? _workloadSetVersion, offlineCache); | ||
return TryInstallWorkloadSet(context, out updates, throwOnFailure: true); | ||
return GetCurrentInstallState(sdkFeatureBand, dotnetDir).UseWorkloadSets ?? false; | ||
} | ||
|
||
public bool TryInstallWorkloadSet(ITransactionContext context, out IEnumerable<ManifestVersionUpdate> updates, bool throwOnFailure = false) | ||
protected void UpdateWorkloadManifests(ITransactionContext context, DirectoryPath? offlineCache) | ||
{ | ||
var advertisingPackagePath = Path.Combine(_userProfileDir, "sdk-advertising", _sdkFeatureBand.ToString(), "microsoft.net.workloads"); | ||
if (File.Exists(Path.Combine(advertisingPackagePath, Constants.workloadSetVersionFileName))) | ||
var updateToLatestWorkloadSet = ShouldUseWorkloadSetMode(_sdkFeatureBand, _dotnetPath); | ||
if (UseRollback && updateToLatestWorkloadSet) | ||
{ | ||
// This file isn't created in tests. | ||
var version = File.ReadAllText(Path.Combine(advertisingPackagePath, Constants.workloadSetVersionFileName)); | ||
PrintWorkloadSetTransition(version); | ||
// Rollback files are only for loose manifests. Update the mode to be loose manifests. | ||
Reporter.WriteLine(Update.LocalizableStrings.UpdateFromRollbackSwitchesModeToLooseManifests); | ||
_workloadInstaller.UpdateInstallMode(_sdkFeatureBand, false); | ||
updateToLatestWorkloadSet = false; | ||
} | ||
else if (_workloadInstaller is FileBasedInstaller || _workloadInstaller is NetSdkMsiInstallerClient) | ||
|
||
if (SpecifiedWorkloadSetVersionOnCommandLine) | ||
{ | ||
// No workload sets found | ||
if (throwOnFailure) | ||
updateToLatestWorkloadSet = false; | ||
|
||
// If a workload set version is specified, then switch to workload set update mode | ||
// Check to make sure the value needs to be changed, as updating triggers a UAC prompt | ||
// for MSI-based installs. | ||
if (!ShouldUseWorkloadSetMode(_sdkFeatureBand, _dotnetPath)) | ||
dsplaisted marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
throw new NuGetPackageNotFoundException(string.Format(Update.LocalizableStrings.WorkloadVersionRequestedNotFound, _workloadSetVersionFromGlobalJson ?? _workloadSetVersion)); | ||
_workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true); | ||
} | ||
else | ||
} | ||
|
||
string resolvedWorkloadSetVersion = _workloadSetVersionFromGlobalJson ??_workloadSetVersionFromCommandLine; | ||
dsplaisted marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (string.IsNullOrWhiteSpace(resolvedWorkloadSetVersion) && !UseRollback) | ||
{ | ||
_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(_includePreviews, updateToLatestWorkloadSet, offlineCache).Wait(); | ||
if (updateToLatestWorkloadSet) | ||
{ | ||
Reporter.WriteLine(Update.LocalizableStrings.NoWorkloadUpdateFound); | ||
resolvedWorkloadSetVersion = _workloadManifestUpdater.GetAdvertisedWorkloadSetVersion(); | ||
} | ||
updates = null; | ||
return false; | ||
} | ||
|
||
var workloadSetPath = _workloadInstaller.InstallWorkloadSet(context, advertisingPackagePath); | ||
var files = Directory.EnumerateFiles(workloadSetPath, "*.workloadset.json"); | ||
updates = _workloadManifestUpdater.ParseRollbackDefinitionFiles(files); | ||
return true; | ||
if (updateToLatestWorkloadSet && resolvedWorkloadSetVersion == null) | ||
{ | ||
Reporter.WriteLine(Update.LocalizableStrings.NoWorkloadUpdateFound); | ||
return; | ||
} | ||
|
||
IEnumerable<ManifestVersionUpdate> manifestsToUpdate; | ||
if (resolvedWorkloadSetVersion != null) | ||
{ | ||
manifestsToUpdate = InstallWorkloadSet(context, resolvedWorkloadSetVersion); | ||
} | ||
else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably make this whole thing a ternary; I only didn't because the return value was different for workload sets. |
||
{ | ||
manifestsToUpdate = UseRollback ? _workloadManifestUpdater.CalculateManifestRollbacks(_fromRollbackDefinition) : | ||
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => m.ManifestUpdate); | ||
} | ||
|
||
InstallStateContents oldInstallState = GetCurrentInstallState(); | ||
|
||
context.Run( | ||
action: () => | ||
{ | ||
foreach (var manifestUpdate in manifestsToUpdate) | ||
{ | ||
_workloadInstaller.InstallWorkloadManifest(manifestUpdate, context, offlineCache); | ||
} | ||
|
||
if (!SpecifiedWorkloadSetVersionInGlobalJson) | ||
{ | ||
if (UseRollback) | ||
{ | ||
_workloadInstaller.SaveInstallStateManifestVersions(_sdkFeatureBand, GetInstallStateContents(manifestsToUpdate)); | ||
} | ||
else if (SpecifiedWorkloadSetVersionOnCommandLine) | ||
{ | ||
_workloadInstaller.AdjustWorkloadSetInInstallState(_sdkFeatureBand, resolvedWorkloadSetVersion); | ||
} | ||
else if (this is WorkloadUpdateCommand) | ||
{ | ||
// For workload updates, if you don't specify a rollback file, or a workload version then we should update to a new version of the manifests or workload set, and | ||
// should remove the install state that pins to the other version | ||
_workloadInstaller.RemoveManifestsFromInstallState(_sdkFeatureBand); | ||
_workloadInstaller.AdjustWorkloadSetInInstallState(_sdkFeatureBand, null); | ||
} | ||
} | ||
|
||
_workloadResolver.RefreshWorkloadManifests(); | ||
}, | ||
rollback: () => | ||
{ | ||
// Reset install state | ||
var currentInstallState = GetCurrentInstallState(); | ||
if (currentInstallState.UseWorkloadSets != oldInstallState.UseWorkloadSets) | ||
{ | ||
_workloadInstaller.UpdateInstallMode(_sdkFeatureBand, oldInstallState.UseWorkloadSets); | ||
} | ||
|
||
if ((currentInstallState.Manifests == null && oldInstallState.Manifests != null) || | ||
(currentInstallState.Manifests != null && oldInstallState.Manifests == null) || | ||
(currentInstallState.Manifests != null && oldInstallState.Manifests != null && | ||
(currentInstallState.Manifests.Count != oldInstallState.Manifests.Count || | ||
!currentInstallState.Manifests.All(m => oldInstallState.Manifests.TryGetValue(m.Key, out var val) && val.Equals(m.Value))))) | ||
{ | ||
_workloadInstaller.SaveInstallStateManifestVersions(_sdkFeatureBand, oldInstallState.Manifests); | ||
} | ||
|
||
if (currentInstallState.WorkloadVersion != oldInstallState.WorkloadVersion) | ||
{ | ||
_workloadInstaller.AdjustWorkloadSetInInstallState(_sdkFeatureBand, oldInstallState.WorkloadVersion); | ||
} | ||
|
||
// We will refresh the workload manifests to make sure that the resolver has the updated state after the rollback | ||
_workloadResolver.RefreshWorkloadManifests(); | ||
}); | ||
} | ||
|
||
private IEnumerable<ManifestVersionUpdate> InstallWorkloadSet(ITransactionContext context, string workloadSetVersion) | ||
{ | ||
PrintWorkloadSetTransition(workloadSetVersion); | ||
var workloadSet = _workloadInstaller.InstallWorkloadSet(context, workloadSetVersion); | ||
|
||
return _workloadManifestUpdater.CalculateManifestUpdatesForWorkloadSet(workloadSet); | ||
} | ||
|
||
private void PrintWorkloadSetTransition(string newVersion) | ||
|
@@ -258,6 +358,17 @@ protected IEnumerable<WorkloadId> GetInstalledWorkloads(bool fromPreviousSdk) | |
return workloads ?? Enumerable.Empty<WorkloadId>(); | ||
} | ||
} | ||
|
||
protected IEnumerable<WorkloadId> WriteSDKInstallRecordsForVSWorkloads(IEnumerable<WorkloadId> workloadsWithExistingInstallRecords) | ||
{ | ||
#if !DOT_NET_BUILD_FROM_SOURCE | ||
if (OperatingSystem.IsWindows()) | ||
{ | ||
return VisualStudioWorkloads.WriteSDKInstallRecordsForVSWorkloads(_workloadInstaller, _workloadResolver, workloadsWithExistingInstallRecords, Reporter); | ||
} | ||
#endif | ||
return workloadsWithExistingInstallRecords; | ||
} | ||
} | ||
|
||
internal static class InstallingWorkloadCommandParser | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a concern that we'll get stuck in this loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really—we'll start out a finite number of directories deep, and that provides an upper limit to how many times it'll run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to use this as part of cleaning up the workload install records, but I didn't want it to delete directories further than a certain level. It probably would have been OK to delete empty directories and they would have been recreated if needed, but it felt safer to me to limit it.