Skip to content

Commit

Permalink
Merge pull request #24794 from ruihe774/fix-quadlet-container-network
Browse files Browse the repository at this point in the history
quadlet: fix inter-dependency of containers in `Network=`
  • Loading branch information
openshift-merge-bot[bot] authored Dec 11, 2024
2 parents 58bbe14 + cf505fe commit b60df61
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 13 deletions.
2 changes: 2 additions & 0 deletions cmd/quadlet/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,8 @@ func generateUnitsInfoMap(units []*parser.UnitFile) map[string]*quadlet.UnitInfo
switch {
case strings.HasSuffix(unit.Filename, ".container"):
serviceName = quadlet.GetContainerServiceName(unit)
// Prefill resouceNames for .container files. This solves network reusing.
resourceName = quadlet.GetContainerResourceName(unit)
case strings.HasSuffix(unit.Filename, ".volume"):
serviceName = quadlet.GetVolumeServiceName(unit)
case strings.HasSuffix(unit.Filename, ".kube"):
Expand Down
41 changes: 28 additions & 13 deletions pkg/systemd/quadlet/quadlet.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,15 +559,7 @@ func ConvertContainer(container *parser.UnitFile, isUser bool, unitsInfoMap map[
}
}

containerName, ok := container.Lookup(ContainerGroup, KeyContainerName)
if !ok || len(containerName) == 0 {
// By default, We want to name the container by the service name
if strings.Contains(container.Filename, "@") {
containerName = "systemd-%p_%i"
} else {
containerName = "systemd-%N"
}
}
containerName := getContainerName(container)

// Set PODMAN_SYSTEMD_UNIT so that podman auto-update can restart the service.
service.Add(ServiceGroup, "Environment", "PODMAN_SYSTEMD_UNIT=%n")
Expand Down Expand Up @@ -868,14 +860,37 @@ func ConvertContainer(container *parser.UnitFile, isUser bool, unitsInfoMap map[

service.AddCmdline(ServiceGroup, "ExecStart", podman.Args)

return service, nil
}

// Get the unresolved container name that may contain '%'.
func getContainerName(container *parser.UnitFile) string {
containerName, ok := container.Lookup(ContainerGroup, KeyContainerName)
if !ok || len(containerName) == 0 {
// By default, We want to name the container by the service name.
if strings.Contains(container.Filename, "@") {
containerName = "systemd-%p_%i"
} else {
containerName = "systemd-%N"
}
}
return containerName
}

// Get the resolved container name that contains no '%'.
// Returns an empty string if not resolvable.
func GetContainerResourceName(container *parser.UnitFile) string {
containerName := getContainerName(container)

// XXX: only %N is handled.
// it is difficult to properly implement specifiers handling without consulting systemd.
resourceName := strings.ReplaceAll(containerName, "%N", unitInfo.ServiceName)
resourceName := strings.ReplaceAll(containerName, "%N", GetContainerServiceName(container))

if !strings.Contains(resourceName, "%") {
unitInfo.ResourceName = resourceName
return resourceName
} else {
return ""
}

return service, nil
}

func defaultOneshotServiceGroup(service *parser.UnitFile, remainAfterExit bool) {
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/quadlet/a.network.reuse.container
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## assert-podman-args "--network" "container:systemd-basic"
## assert-key-is "Unit" "Requires" "basic.service"
## assert-key-is-regex "Unit" "After" "network-online.target|podman-user-wait-network-online.service" "basic.service"

[Container]
Image=localhost/imagename
Network=basic.container
7 changes: 7 additions & 0 deletions test/e2e/quadlet/a.network.reuse.name.container
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## assert-podman-args "--network" "container:foobar"
## assert-key-is "Unit" "Requires" "name.service"
## assert-key-is-regex "Unit" "After" "network-online.target|podman-user-wait-network-online.service" "name.service"

[Container]
Image=localhost/imagename
Network=name.container
2 changes: 2 additions & 0 deletions test/e2e/quadlet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,8 @@ BOGUS=foo
Entry("Container - Quadlet build with multiple tags", "build.multiple-tags.container", []string{"multiple-tags.build"}),
Entry("Container - Reuse another container's network", "network.reuse.container", []string{"basic.container"}),
Entry("Container - Reuse another named container's network", "network.reuse.name.container", []string{"name.container"}),
Entry("Container - Reuse another container's network", "a.network.reuse.container", []string{"basic.container"}),
Entry("Container - Reuse another named container's network", "a.network.reuse.name.container", []string{"name.container"}),

Entry("Volume - Quadlet image (.build)", "build.quadlet.volume", []string{"basic.build"}),
Entry("Volume - Quadlet image (.image)", "image.quadlet.volume", []string{"basic.image"}),
Expand Down

0 comments on commit b60df61

Please sign in to comment.