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
Self-contained instances #3970
Conversation
This file contains 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
bording
force-pushed
the
self-contained-instances
branch
3 times, most recently
from
February 27, 2024 16:15
9b09a65
to
e89babe
Compare
bording
force-pushed
the
self-contained-instances
branch
from
March 1, 2024 17:33
a254a2b
to
b779358
Compare
bording
force-pushed
the
self-contained-instances
branch
from
March 1, 2024 17:54
b779358
to
bcb9055
Compare
bording
force-pushed
the
self-contained-instances
branch
from
March 1, 2024 18:26
bcb9055
to
5129540
Compare
src/ServiceControl.MultiInstance.AcceptanceTests/ServiceControl.runsettings
Show resolved
Hide resolved
danielmarbach
approved these changes
Mar 4, 2024
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.
@bording discussed the feedback on the PR and will follow through on a bunch of those nitpicks. Giving the approval already
mauroservienti
approved these changes
Mar 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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:Persisters
andTransports
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.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 updatingexe.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 existingexe.config
file and reads those settings intoConfigurationManager
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 theConnectionStrings
properly not read-only. Given that theConfigurationManager
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.