Skip to content

[release/3.1] Allow Hosting Bundle to be upgraded by MU #37969

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
merged 8 commits into from
Nov 5, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 1, 2021

Backport of #37967 to release/3.1

/cc @wtgodbe

Customer Impact

Adds an .msi, dotnet-hosting-options, which writes to the registry the value of the switches used when last installing the hosting bundle for a given Major.Minor version. Subsequent installs of that band of the hosting bundle will choose what the install in the following way:

  • If the user passes any switches on the command line, use exactly those (and write them to the registry, along with 0 for any switches not passed).
  • Else use what's in the registry, if there are values in the registry.
  • Else set all switches to 0 and write 0 to the registry for each switch

Testing

Tested various scenarios with versions of the new hosting bundle across multiple Major.Minor.Patch versions, as well as how it interacts with previous Hosting Bundles without the Registry-writing behavior.

Risk

In December all customers who get this update thru MU will get a 1-time hit where, regardless of what they already had installed, the hosting bundle will install all components. We suspect most customers install everything anyways, and those who don't will be able to work around this by manually re-running the installer with whatever switches they passed the last time they installed the bundle manually. Additionally, there could be a scenario that we didn't think of (and therefore wasn't covered by our manual testing) that will break.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@github-actions github-actions bot requested a review from Pilchie as a code owner November 1, 2021 00:58
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Nov 1, 2021
@ghost ghost added this to the 3.1.x milestone Nov 1, 2021
@ghost
Copy link

ghost commented Nov 1, 2021

Hi @github-actions[bot]. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@wtgodbe wtgodbe requested review from joeloff, jamshedd and a team November 1, 2021 01:01
@wtgodbe wtgodbe added the Servicing-consider Shiproom approval is required for the issue label Nov 1, 2021
@ghost
Copy link

ghost commented Nov 1, 2021

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@wtgodbe wtgodbe changed the title [release/3.1] [release/6.0] Allow Hosting Bundle to be upgraded by MU [release/3.1] Allow Hosting Bundle to be upgraded by MU Nov 1, 2021
<!-- Use values from the registry for switches, if the registry values are set & the user hasn't passed any switches from the command line -->

<util:RegistrySearch Id="opt_no_ancm_should_be_set"
Condition="NOT OPT_NO_ANCM AND NOT OPT_NO_ANCM=0 AND NOT OPT_NO_FTS AND NOT OPT_NO_FTS=0 AND NOT OPT_NO_LTS AND NOT OPT_NO_LTS=0 AND NOT OPT_NO_SHAREDFX AND NOT OPT_NO_SHAREDFX=0 AND NOT OPT_NO_RUNTIME AND NOT OPT_NO_RUNTIME=0 AND NOT OPT_NO_X86 AND NOT OPT_NO_X86=0 AND NOT OPT_NO_SHARED_CONFIG_CHECK AND NOT OPT_NO_SHARED_CONFIG_CHECK=0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Find a way to put this condition in one place

Copy link
Member

Choose a reason for hiding this comment

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

See comment below

<Variable Name="OPT_NO_X86" bal:Overridable="yes"/>
<Variable Name="OPT_NO_SHARED_CONFIG_CHECK" bal:Overridable="yes" />

<!-- Use values from the registry for switches, if the registry values are set & the user hasn't passed any switches from the command line -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain how some registry values could be set while others are not. This seems significantly more complicated than it would be w/ a single check for either the registry key or one value.

Copy link
Member

Choose a reason for hiding this comment

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

The only way that one registry value could be set and not another would be if the user manually modified the registry. But, having to create this conditional based on a registry search is sort of a WiX quirk - there isn't a direct way to assign a variable value based off of user input in a bundle. You can, however, conditionally set a variable to the value of a registry key based off of user input. We could just do this conditional check once based off of the existence of just one arbitrary registry key, but the cost of this is low and its purpose is more explicit. @joeloff do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, and for the initial deployment from MU, there would be no existing options recorded in the registry, so we want a default of 0, but if we set that initially and the user overrides it, then the registry search will win over the user's choice.

@leecow leecow modified the milestones: 3.1.x, 3.1.22 Nov 2, 2021
@wtgodbe wtgodbe added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Nov 2, 2021
@wtgodbe
Copy link
Member

wtgodbe commented Nov 2, 2021

Approved in tactics

@wtgodbe wtgodbe enabled auto-merge (squash) November 4, 2021 21:18
@wtgodbe wtgodbe disabled auto-merge November 4, 2021 22:00
@wtgodbe wtgodbe merged commit c1cdb12 into release/3.1 Nov 5, 2021
@wtgodbe wtgodbe deleted the backport/pr-37967-to-release/3.1 branch November 5, 2021 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants