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

Self-contained instances #3970

Merged
merged 21 commits into from Mar 4, 2024
Merged

Self-contained instances #3970

merged 21 commits into from Mar 4, 2024

Conversation

bording
Copy link
Member

@bording bording commented Feb 19, 2024

This PR makes the SC instances fully self-contained for win-x64, so there is no dependency on what .NET runtimes are installed on the machine.

By default, building the solution will still produce framework-dependent output. The WindowsSelfContained property needs to be true to get a self-contained build. The release workflows have been updated to set this property.

This change also ensures that when the release workflow builds the PowerShell module, only the win-x64 versions of the dependencies are included.

To keep the file size of SCMU from growing dramatically by having to bundle 3 additional copies of the .NET runtime, some changes have been made to the deploy artifact layout and packaging:

  • Each instance folder now contains the Persisters and Transports folders, and this entire file structure is deployed on disk for an instance. The manifest discovery features are then leveraged to choose the correct component based on the config value.
  • An MSBuild task has been added to Particular.Packaging that is used to detect duplicate files. Any file that is duplicated in all 3 folders is excluded from the instance zip and is instead put into an InstanceShared zip. The actual duplicate files are not removed from the deploy folders, only the zip files are impacted.

ServiceControlInstaller.Engine has been updated to understand the new deployment layout and zip file structure.

Some misc. cleanup and small fixes have also been included as part of this PR.


One of these is important enough to call out separately here:

Since we're still using ConfigurationManager to read app.config files after our move to .NET, there is a problem that is introduced. On .NET, the config file being looked for is {assembly}.dll.config, but on .NET Framework the file is named {assembly}.exe.config. All of our existing installer logic is built around creating and updating exe.config files, and this is way too embedded into the code to safely change.

Instead, what I've introduced here is an ReadExeConfiguration method that the instances call first thing on startup that opens up an existing exe.config file and reads those settings into ConfigurationManager so that they will be available to the rest of the application. Unfortunately, to update the connection strings in this manner, some private reflection was required to make the ConnectionStrings properly not read-only. Given that the ConfigurationManager code is a shim to let app.config files be used on .NET, this code is not likely to break/change, but I have added a test to let us know if the reflection starts failing for some reason.

The end result of this change is that ServiceControl instances continue to use the existing exe.config file just like all previous versions of SC do.

@bording bording changed the title Self contained instances Self-contained instances Feb 19, 2024
@bording bording force-pushed the self-contained-instances branch 3 times, most recently from 9b09a65 to e89babe Compare February 27, 2024 16:15
@bording bording marked this pull request as ready for review March 1, 2024 18:35
src/ServiceControl.Audit/Program.cs Outdated Show resolved Hide resolved
src/ServiceControl.Audit/Program.cs Outdated Show resolved Hide resolved
src/ServiceControl.Audit/Program.cs Outdated Show resolved Hide resolved
src/ServiceControl.Audit/Program.cs Outdated Show resolved Hide resolved
src/ServiceControl.Monitoring/Program.cs Outdated Show resolved Hide resolved
src/ServiceControl/Program.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@danielmarbach danielmarbach left a comment

Choose a reason for hiding this comment

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

@bording discussed the feedback on the PR and will follow through on a bunch of those nitpicks. Giving the approval already

@bording bording merged commit b1b6eea into master Mar 4, 2024
19 checks passed
@bording bording deleted the self-contained-instances branch March 4, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants