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.

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

Introduce Workload Clean: Garbage Collection Component #30266

Merged
merged 49 commits into from
Mar 23, 2023

Conversation

nagilson
Copy link
Member

@nagilson nagilson commented Feb 1, 2023

What's this?

Resolves #19603
Doc: #30480

  • Adds PART ONE of dotnet workload clean:
    (So not the stuff in 2) shown below.)

  • Make workload list show all VS workloads regardless of feature bands.

  • Workload --info now correctly shows msi or filebased install type.

  • Fixes Incorrect customer facing logging messages.

  • Refactor MSI Garbage Collection into separate functions to make it easier to use

  • Note that workloads will still appear after workload clean --> workload list, but not for workload clean --all, because we don't remove the workload installation record itself in GC. The old GC also does not remove workload installation records for workloads of uninstalled featurebands. To do this, we would need the manifest or some sort of history ledger to map N-1 packs to a workload id. Or maybe we could include the workload id in the workload pack records, but not sure if we want to do that.

  • Evidence of VS Workload Behavior:
    image

  • Evidence of MSI Clean up:
    image
    image
    image

  • Shows that MSI clean up works for older feature bands:

C:\Users\WDAGUtilityAccount>dotnet --info
.NET SDK:
 Version:   7.0.103
 Commit:    276c71d299
...

C:\Users\WDAGUtilityAccount>dotnet workload install wasm-tools

The machine has a pending reboot. Installation will continue, but you may need to restart.
...

Successfully installed workload(s) wasm-tools.


C:\Users\WDAGUtilityAccount>dotnet workload list

Installed Workload Id      Manifest Version      Installation Source
--------------------------------------------------------------------
wasm-tools                 7.0.3/7.0.100         SDK 7.0.100
maui-windows               7.0.59/7.0.100        VS 17.4.33403.182

...


--> Then in an 8.0 terminal:

PS C:\Users\WDAGUtilityAccount\sdk> dotnet workload clean --all

--> Then back to 7:

C:\Users\WDAGUtilityAccount>dotnet workload list

Installed Workload Id      Manifest Version      Installation Source
--------------------------------------------------------------------
maui-windows               7.0.59/7.0.100        VS 17.4.33403.182
[2023-02-09 16:25:41.122] [00001D28] Microsoft.Maui.Resizetizer.Sdk (7.0.59) will not be removed because other dependents remain: VS.{AEF703B8-D2CC-4343-915C-F54A30B90937}.
[2023-02-09 16:25:41.123] [00001D28] Microsoft.Maui.Sdk (6.0.552) will not be removed because other dependents remain: VS.{AEF703B8-D2CC-4343-915C-F54A30B90937}.
[2023-02-09 16:25:41.124] [00001D28] Microsoft.Maui.Sdk (7.0.59) will not be removed because other dependents remain: VS.{AEF703B8-D2CC-4343-915C-F54A30B90937}.
[2023-02-09 16:25:41.124] [00001D28] Microsoft.Maui.Templates.net6 (6.0.552) will not be removed because other dependents remain: VS.{AEF703B8-D2CC-4343-915C-F54A30B90937}.
[2023-02-09 16:25:41.124] [00001D28] Microsoft.Maui.Templates.net7 (7.0.59) will not be removed because other dependents remain: VS.{AEF703B8-D2CC-4343-915C-F54A30B90937}.
  • Show that VS workloads across versions are displayed:
PS C:\Users\source\repos\sdk> dotnet workload clean --all
Workload 'maui-windows (VS 17.4.33213.308, VS 17.4.33205.214)' was not removed because it is installed and managed by Visual Studio. Please uninstall this workload through Visual Studio to fully remove it.
Workload 'maui-ios (VS 17.4.33205.214)' was not removed because it is installed and managed by Visual Studio. Please uninstall this workload through Visual Studio to fully remove it.
Workload 'ios (VS 17.4.33205.214)' was not removed because it is installed and managed by Visual Studio. Please uninstall this workload through Visual Studio to fully remove it.
  • Updated test plan for MSIs for vendors.

(Note that I added this into my own copy of main so it won't go into prod.)

Should

  1. Run garbage collection for the SDK feature band under execution
    2) Restore Manifests to 'fresh' state, to whatever baseline manifests were initially shipped with the current SDK feature band, however, including edits that VS made to said manifest, assuming a ledger is available. (Daniel is working on the ledger.)

Add dotnet workload clean --all:
Should

  1. Remove ALL CLI installed workloads for the SDK feature band under execution and for all feature bands of the SDK below the current version of the SDK. It should also give a message for VS installed workloads and say that these workloads weren't installed because they are managed by VS.

Note that this is different from running dotnet workload uninstall en masse. Workload uninstall needs the manifest to know what has been orphaned.

2) Restore manifests to 'fresh' state, like above, but for all lower SDK feature bands if possible. (They may not have a ledger available.)

Not in this PR:

Make workload list show all workloads regardless of feature band.

@nagilson nagilson changed the base branch from main to nagilson-main February 6, 2023 18:02
@nagilson nagilson changed the title [WIP] Workload Clean Workload Clean -> Garbage Collection Component Feb 6, 2023
@nagilson nagilson changed the title Workload Clean -> Garbage Collection Component Introduce Workload Clean: Garbage Collection Component Feb 6, 2023
@nagilson nagilson marked this pull request as ready for review February 9, 2023 00:27
nagilson added a commit that referenced this pull request Feb 9, 2023
@nagilson
Copy link
Member Author

nagilson commented Mar 6, 2023

@dsplaisted @joeloff I have responded to all feedback, including making it work for > feature bands. If we would like to revert that part, it is a simple commit revert. Thank you.

…lean. Include dotnetDir with newer info changes.
… with an if check instead of in the interface as it would be more confusing to have an msi api in a file based interface
@nagilson
Copy link
Member Author

nagilson commented Mar 8, 2023

@dsplaisted I have responded to all of the feedback, thanks for taking a look. I know you still need to look @ the tests.

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Looks good.

@nagilson
Copy link
Member Author

@marcpopMSFT pls merge on red, arcade outage seems to be blocking this pr

@marcpopMSFT marcpopMSFT merged commit 2e3a93c into dotnet:main Mar 23, 2023
@marcpopMSFT
Copy link
Member

Do we have a link to a known outage we can ping them on?

@nagilson
Copy link
Member Author

I believe it's fixed now, so no action needed :)

JL03-Yue pushed a commit to JL03-Yue/sdk that referenced this pull request May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add workload force clean option
5 participants