This repository has been archived by the owner on Nov 20, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 520
Add podman support #570
Closed
Closed
Add podman support #570
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
2df06a3
DockerRunner: prefer using host network over creating custom network.
tmds a2973ca
Add UseHostNetwork property to Hosting Application
tmds 9291d97
Make tests pass
tmds 6f5dd43
Make PortAssigner host network friendly
tmds d2b2caa
Remove unused SkipIfNotPodmanAttribute
tmds 2b690e2
Move UseHostNetwork setting to PortAssigner
tmds 5240f37
Run FrontendBackendRunTestWithDocker also on podman systems
tmds 9d9c0cf
Enable MultiProjectDockerStoppingTests
tmds be553a6
cleanup
tmds feb5df3
Merge branch 'master' into podman
tmds File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,9 @@ public Application(FileInfo source, Dictionary<string, Service> services) | |
|
||
public string? Network { get; set; } | ||
|
||
// All services and application run on the container host. | ||
public bool UseHostNetwork { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need to parse this in config, right? Do you intend for this to be part of tye.yaml and/or a command line arg to tye run? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah nvrm, you use podman existing as that check. Little bit confusing, should this variable be called IsPodman for now? |
||
|
||
public void PopulateEnvironment(Service service, Action<string, string> set, string defaultHost = "localhost") | ||
{ | ||
var bindings = ComputeBindings(service, defaultHost); | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,8 +21,16 @@ public PortAssigner(ILogger logger) | |
_logger = logger; | ||
} | ||
|
||
public Task StartAsync(Application application) | ||
public async Task StartAsync(Application application) | ||
{ | ||
// rootless podman doesn't permit creation of networks. | ||
// Use the host network instead, and perform communication between applications using "localhost". | ||
if (string.IsNullOrEmpty(application.Network)) | ||
{ | ||
bool isPodman = await DockerDetector.Instance.IsPodman.Value; | ||
application.UseHostNetwork = isPodman; | ||
} | ||
|
||
foreach (var service in application.Services.Values) | ||
{ | ||
if (service.Description.RunInfo == null) | ||
|
@@ -44,10 +52,22 @@ static int GetNextPort() | |
|
||
foreach (var binding in service.Description.Bindings) | ||
{ | ||
// Auto assign a port | ||
// We assign a port to each binding. | ||
// When we use the host network, port mapping is not supported. | ||
// The ContainerPort and Port need to match. | ||
if (binding.Port == null) | ||
{ | ||
binding.Port = GetNextPort(); | ||
// UseHostNetwork: ContainerPort exposes the service on localhost | ||
// Set Port to match ContainerPort. | ||
if (application.UseHostNetwork && binding.ContainerPort.HasValue) | ||
{ | ||
binding.Port = binding.ContainerPort.Value; | ||
} | ||
else | ||
{ | ||
// Pick a random port. | ||
binding.Port = GetNextPort(); | ||
} | ||
} | ||
|
||
if (service.Description.Readiness == null && service.Description.Replicas == 1) | ||
|
@@ -72,22 +92,23 @@ static int GetNextPort() | |
binding.Name ?? binding.Protocol); | ||
} | ||
|
||
// Set ContainerPort for the first http and https port. | ||
// For ASP.NET we'll match the Port when UseHostNetwork. ASPNETCORE_URLS will configure the application. | ||
// For other applications, we use the default ports 80 and 443. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why should we default to 80 and 443 for non-aspnet services? |
||
var httpBinding = service.Description.Bindings.FirstOrDefault(b => b.Protocol == "http"); | ||
var httpsBinding = service.Description.Bindings.FirstOrDefault(b => b.Protocol == "https"); | ||
|
||
// Default the first http and https port to 80 and 443 | ||
bool isAspNetWithHostNetwork = application.UseHostNetwork && | ||
(service.Description.RunInfo as DockerRunInfo)?.IsAspNet == true; | ||
if (httpBinding != null) | ||
{ | ||
httpBinding.ContainerPort ??= 80; | ||
httpBinding.ContainerPort ??= isAspNetWithHostNetwork ? httpBinding.Port : 80; | ||
} | ||
|
||
if (httpsBinding != null) | ||
{ | ||
httpsBinding.ContainerPort ??= 443; | ||
httpsBinding.ContainerPort ??= isAspNetWithHostNetwork ? httpsBinding.Port : 443; | ||
} | ||
} | ||
|
||
return Task.CompletedTask; | ||
} | ||
|
||
public Task StopAsync(Application application) | ||
|
Oops, something went wrong.
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.
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.
What does this mean for shutdown?
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.
This affect what happens when the system restarts.
For "docker", "unless-stopped" means only containers which weren't stopped will be started at boot.
"podman" doesn't start containers at boot. With podman v2, "always" and "unless-stopped" are aliases.