Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Workload feature band fallback with rollback files #24545

Merged
merged 5 commits into from
Mar 30, 2022
Merged

Workload feature band fallback with rollback files #24545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8d3378
trying to update print-rollback to include feature band
gkulin Mar 24, 2022
2002195
update rollback to include feature band
gkulin Mar 24, 2022
8814b96
adding some tests
gkulin Mar 28, 2022
7a1a8e7
adding test for rollback across feature band
gkulin Mar 28, 2022
aa8622b
update failing test to add unique identifier and add another test for…
gkulin Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/Cli/dotnet/commands/dotnet-workload/install/IWorkloadManifestUpdater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.DotNet.Workloads.Workload.Install.InstallRecord;
using Microsoft.Extensions.EnvironmentAbstractions;
using Microsoft.NET.Sdk.WorkloadManifestReader;

Expand All @@ -16,11 +17,14 @@ internal interface IWorkloadManifestUpdater

IEnumerable<(
ManifestId manifestId,
ManifestVersion existingVersion,
ManifestVersion existingVersion,
SdkFeatureBand existingFeatureBand,
ManifestVersion newVersion,
Dictionary<WorkloadId, WorkloadDefinition> Workloads)> CalculateManifestUpdates();
SdkFeatureBand newFeatureBand,
Dictionary<WorkloadId, WorkloadDefinition> Workloads
)> CalculateManifestUpdates();

IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, ManifestVersion newVersion)>
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand)>
CalculateManifestRollbacks(string rollbackDefinitionFilePath);

Task<IEnumerable<string>> DownloadManifestPackagesAsync(bool includePreviews, DirectoryPath downloadPath);
Expand Down
10 changes: 5 additions & 5 deletions src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public void InstallWorkloads(IEnumerable<WorkloadId> workloadIds, bool skipManif
{
_reporter.WriteLine();

IEnumerable<(ManifestId, ManifestVersion, ManifestVersion)> manifestsToUpdate = new List<(ManifestId, ManifestVersion, ManifestVersion)>();
var manifestsToUpdate = Enumerable.Empty<(ManifestId manifestId, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand)>();
if (!skipManifestUpdate)
{
if (_verbosity != VerbosityOptions.quiet && _verbosity != VerbosityOptions.q)
Expand All @@ -200,7 +200,7 @@ public void InstallWorkloads(IEnumerable<WorkloadId> workloadIds, bool skipManif

_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(includePreviews, offlineCache).Wait();
manifestsToUpdate = string.IsNullOrWhiteSpace(_fromRollbackDefinition) ?
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => (m.manifestId, m.existingVersion, m.newVersion)) :
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => (m.manifestId, m.existingVersion, m.existingFeatureBand, m.newVersion, m.newFeatureBand)) :
_workloadManifestUpdater.CalculateManifestRollbacks(_fromRollbackDefinition);
}

Expand Down Expand Up @@ -233,7 +233,7 @@ internal static void TryRunGarbageCollection(IInstaller workloadInstaller, IRepo
private void InstallWorkloadsWithInstallRecord(
IEnumerable<WorkloadId> workloadIds,
SdkFeatureBand sdkFeatureBand,
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, ManifestVersion newVersion)> manifestsToUpdate,
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand)> manifestsToUpdate,
DirectoryPath? offlineCache)
{
if (_workloadInstaller.GetInstallationUnit().Equals(InstallationUnit.Packs))
Expand All @@ -249,7 +249,7 @@ private void InstallWorkloadsWithInstallRecord(

foreach (var manifest in manifestsToUpdate)
{
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.newVersion, sdkFeatureBand, offlineCache, rollback);
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.newVersion, manifest.newFeatureBand, offlineCache, rollback);
}

_workloadResolver.RefreshWorkloadManifests();
Expand All @@ -273,7 +273,7 @@ private void InstallWorkloadsWithInstallRecord(

foreach (var manifest in manifestsToUpdate)
{
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.existingVersion, sdkFeatureBand, offlineCache: null, isRollback: true);
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.existingVersion, manifest.existingFeatureBand, offlineCache: null, isRollback: true);
}

foreach (var packId in workloadPackToInstall)
Expand Down
56 changes: 43 additions & 13 deletions src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,19 @@ public static void AdvertiseWorkloadUpdates()
public IEnumerable<(
ManifestId manifestId,
ManifestVersion existingVersion,
SdkFeatureBand existingFeatureBand,
ManifestVersion newVersion,
Dictionary<WorkloadId, WorkloadDefinition> Workloads)> CalculateManifestUpdates()
SdkFeatureBand newFeatureBand,
Dictionary<WorkloadId, WorkloadDefinition> Workloads
)>
CalculateManifestUpdates()
{
var manifestUpdates =
new List<(ManifestId, ManifestVersion, ManifestVersion,
new List<(ManifestId manifestId,
ManifestVersion existingVersion,
SdkFeatureBand existingFeatureBand,
ManifestVersion newVersion,
SdkFeatureBand newFeatureBand,
Dictionary<WorkloadId, WorkloadDefinition> Workloads)>();
var currentManifestIds = GetInstalledManifestIds();
foreach (var manifestId in currentManifestIds)
Expand All @@ -182,10 +190,10 @@ public static void AdvertiseWorkloadUpdates()
}

if (advertisingManifestVersionAndWorkloads != null &&
advertisingManifestVersionAndWorkloads.Value.ManifestVersion.CompareTo(currentManifestVersion) > 0)
advertisingManifestVersionAndWorkloads.Value.ManifestVersion.CompareTo(currentManifestVersion.Item1) > 0)
{
manifestUpdates.Add((manifestId, currentManifestVersion,
advertisingManifestVersionAndWorkloads.Value.ManifestVersion,
manifestUpdates.Add((manifestId, currentManifestVersion.Item1, currentManifestVersion.Item2,
advertisingManifestVersionAndWorkloads.Value.ManifestVersion, advertisingManifestVersionAndWorkloads.Value.ManifestFeatureBand,
advertisingManifestVersionAndWorkloads.Value.Workloads));
}
}
Expand All @@ -207,7 +215,7 @@ public IEnumerable<WorkloadId> GetUpdatableWorkloadsToAdvertise(IEnumerable<Work
}
}

public IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, ManifestVersion newVersion)> CalculateManifestRollbacks(string rollbackDefinitionFilePath)
public IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand)> CalculateManifestRollbacks(string rollbackDefinitionFilePath)
{
var currentManifestIds = GetInstalledManifestIds();
var manifestRollbacks = ParseRollbackDefinitionFile(rollbackDefinitionFilePath);
Expand All @@ -220,7 +228,11 @@ public IEnumerable<WorkloadId> GetUpdatableWorkloadsToAdvertise(IEnumerable<Work
}

var manifestUpdates = manifestRollbacks
.Select(manifest => (manifest.Item1, GetInstalledManifestVersion(manifest.Item1), manifest.Item2));
.Select(manifest =>
{
var installedManifestInfo = GetInstalledManifestVersion(manifest.id);
return (manifest.id, installedManifestInfo.manifestVersion, installedManifestInfo.sdkFeatureBand, manifest.version, manifest.featureBand);
});

return manifestUpdates;
}
Expand Down Expand Up @@ -365,7 +377,7 @@ private async Task UpdateAdvertisingManifestAsync(ManifestId manifestId, bool in
}
}

private (ManifestVersion ManifestVersion, Dictionary<WorkloadId, WorkloadDefinition> Workloads)?
private (ManifestVersion ManifestVersion, SdkFeatureBand ManifestFeatureBand, Dictionary<WorkloadId, WorkloadDefinition> Workloads)?
GetAdvertisingManifestVersionAndWorkloads(ManifestId manifestId)
{
var manifestPath = Path.Combine(GetAdvertisingManifestPath(_sdkFeatureBand, manifestId),
Expand All @@ -378,11 +390,14 @@ private async Task UpdateAdvertisingManifestAsync(ManifestId manifestId, bool in
using (FileStream fsSource = new FileStream(manifestPath, FileMode.Open, FileAccess.Read))
{
var manifest = WorkloadManifestReader.ReadWorkloadManifest(manifestId.ToString(), fsSource, manifestPath);
return (new ManifestVersion(manifest.Version), manifest.Workloads.Values.OfType<WorkloadDefinition>().ToDictionary(w => w.Id));

// TODO: figure out how to differentiate between the feature band an advertising manifest is branded as and the feature band of the SDK
// it's advertised to
Comment on lines +394 to +395
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we avoid leaving TODO comments in the code when we merge. If there is follow-up work to do, it's better to track it in an issue than a code comment.

If there are no other changes, I'm OK with leaving this in so we don't have to reset the CI checks. I've made a note about this in #23403.

return (new ManifestVersion(manifest.Version), _sdkFeatureBand, manifest.Workloads.Values.OfType<WorkloadDefinition>().ToDictionary(w => w.Id));
}
}

private ManifestVersion GetInstalledManifestVersion(ManifestId manifestId)
private (ManifestVersion manifestVersion, SdkFeatureBand sdkFeatureBand) GetInstalledManifestVersion(ManifestId manifestId)
{

var manifest = _workloadResolver.GetInstalledManifests()
Expand All @@ -391,7 +406,7 @@ private ManifestVersion GetInstalledManifestVersion(ManifestId manifestId)
{
throw new Exception(string.Format(LocalizableStrings.ManifestDoesNotExist, manifestId.ToString()));
}
return new ManifestVersion(manifest.Version);
return (new ManifestVersion(manifest.Version), new SdkFeatureBand(manifest.ManifestFeatureBand));
}

private bool AdManifestSentinelIsDueForUpdate()
Expand Down Expand Up @@ -437,7 +452,7 @@ private async Task<bool> NewerManifestPackageExists(ManifestId manifest)
}
}

private IEnumerable<(ManifestId, ManifestVersion)> ParseRollbackDefinitionFile(string rollbackDefinitionFilePath)
private IEnumerable<(ManifestId id, ManifestVersion version, SdkFeatureBand featureBand)> ParseRollbackDefinitionFile(string rollbackDefinitionFilePath)
{
string fileContent;

Expand All @@ -457,7 +472,22 @@ private async Task<bool> NewerManifestPackageExists(ManifestId manifest)
}
}
return JsonSerializer.Deserialize<IDictionary<string, string>>(fileContent)
.Select(manifest => (new ManifestId(manifest.Key), new ManifestVersion(manifest.Value)));
.Select(manifest =>
{
ManifestVersion manifestVersion;
SdkFeatureBand manifestFeatureBand;
var parts = manifest.Value.Split('/');
manifestVersion = new ManifestVersion(parts[0]);
if (parts.Length == 1)
{
manifestFeatureBand = _sdkFeatureBand;
}
else
{
manifestFeatureBand = new SdkFeatureBand(parts[1]);
}
return (new ManifestId(manifest.Key), manifestVersion, manifestFeatureBand);
});
}

private bool BackgroundUpdatesAreDisabled() =>
Expand Down
6 changes: 3 additions & 3 deletions src/Cli/dotnet/commands/dotnet-workload/list/WorkloadListCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,19 @@ internal UpdateAvailableEntry[] GetUpdateAvailable(IEnumerable<WorkloadId> insta
{
HashSet<WorkloadId> installedWorkloads = installedList.ToHashSet();
_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(_includePreviews).Wait();
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, ManifestVersion newVersion,
Dictionary<WorkloadId, WorkloadDefinition> Workloads)> manifestsToUpdate =
var manifestsToUpdate =
_workloadManifestUpdater.CalculateManifestUpdates();

List<UpdateAvailableEntry> updateList = new();
foreach ((ManifestId _, ManifestVersion existingVersion, ManifestVersion newVersion,
foreach ((ManifestId _, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand,
Dictionary<WorkloadId, WorkloadDefinition> workloads) in manifestsToUpdate)
{
foreach ((WorkloadId WorkloadId, WorkloadDefinition workloadDefinition) in
workloads)
{
if (installedWorkloads.Contains(new WorkloadId(WorkloadId.ToString())))
{
// TODO: Potentially show existing and new feature bands
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another TODO which should ideally be removed.

updateList.Add(new UpdateAvailableEntry(existingVersion.ToString(),
newVersion.ToString(),
workloadDefinition.Description, WorkloadId.ToString()));
Expand Down
8 changes: 4 additions & 4 deletions src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public override int Execute()
}
else if (_printRollbackDefinitionOnly)
{
var manifests = _workloadResolver.GetInstalledManifests().ToDictionary(m => m.Id, m => m.Version, StringComparer.OrdinalIgnoreCase);
var manifests = _workloadResolver.GetInstalledManifests().ToDictionary(m => m.Id, m => m.Version + "/" + m.ManifestFeatureBand, StringComparer.OrdinalIgnoreCase);

_reporter.WriteLine("==workloadRollbackDefinitionJsonOutputStart==");
_reporter.WriteLine(JsonSerializer.Serialize(manifests));
Expand Down Expand Up @@ -159,7 +159,7 @@ public void UpdateWorkloads(bool includePreviews = false, DirectoryPath? offline
_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(includePreviews, offlineCache).Wait();

var manifestsToUpdate = string.IsNullOrWhiteSpace(_fromRollbackDefinition) ?
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => (m.manifestId, m.existingVersion, m.newVersion)) :
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => (m.manifestId, m.existingVersion, m.existingFeatureBand, m.newVersion, m.newFeatureBand)) :
_workloadManifestUpdater.CalculateManifestRollbacks(_fromRollbackDefinition);

UpdateWorkloadsWithInstallRecord(workloadIds, featureBand, manifestsToUpdate, offlineCache);
Expand All @@ -176,7 +176,7 @@ public void UpdateWorkloads(bool includePreviews = false, DirectoryPath? offline
private void UpdateWorkloadsWithInstallRecord(
IEnumerable<WorkloadId> workloadIds,
SdkFeatureBand sdkFeatureBand,
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, ManifestVersion newVersion)> manifestsToUpdate,
IEnumerable<(ManifestId manifestId, ManifestVersion existingVersion, SdkFeatureBand existingFeatureBand, ManifestVersion newVersion, SdkFeatureBand newFeatureBand)> manifestsToUpdate,
DirectoryPath? offlineCache = null)
{
if (_workloadInstaller.GetInstallationUnit().Equals(InstallationUnit.Packs))
Expand All @@ -191,7 +191,7 @@ private void UpdateWorkloadsWithInstallRecord(

foreach (var manifest in manifestsToUpdate)
{
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.newVersion, sdkFeatureBand, offlineCache, rollback);
_workloadInstaller.InstallWorkloadManifest(manifest.manifestId, manifest.newVersion, manifest.newFeatureBand, offlineCache, rollback);
}

_workloadResolver.RefreshWorkloadManifests();
Expand Down
Loading