Skip to content

Commit

Permalink
Merge pull request containers#19727 from vrothberg/fix-19715
Browse files Browse the repository at this point in the history
kube: notifyproxy: close once
  • Loading branch information
openshift-merge-robot authored Aug 24, 2023
2 parents 6009d16 + a5f6a4a commit 32f7bb1
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 14 deletions.
8 changes: 2 additions & 6 deletions pkg/domain/infra/abi/play.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,8 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
initContainers = append(initContainers, ctr)
}

var sdNotifyProxies []*notifyproxy.NotifyProxy // containers' sd-notify proxies
// Callers are expected to close the proxies
var sdNotifyProxies []*notifyproxy.NotifyProxy

for _, container := range podYAML.Spec.Containers {
// Error out if the same name is used for more than one container
Expand Down Expand Up @@ -915,11 +916,6 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
errors := make([]error, len(sdNotifyProxies))
for i := range sdNotifyProxies {
wg.Add(1)
defer func() {
if err := sdNotifyProxies[i].Close(); err != nil {
logrus.Errorf("Closing sdnotify proxy %q: %v", sdNotifyProxies[i].SocketPath(), err)
}
}()
go func(i int) {
err := sdNotifyProxies[i].Wait()
if err != nil {
Expand Down
15 changes: 7 additions & 8 deletions pkg/systemd/notifyproxy/notifyproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,16 @@ func New(tmpDir string) (*NotifyProxy, error) {
// Start waiting for the READY message in the background. This way,
// the proxy can be created prior to starting the container and
// circumvents a race condition on writing/reading on the socket.
proxy.waitForReady()
proxy.listen()

return proxy, nil
}

// waitForReady waits for the READY message in the background. The goroutine
// returns on receiving READY or when the socket is closed.
func (p *NotifyProxy) waitForReady() {
// listen waits for the READY message in the background, and process file
// descriptors and barriers send over the NOTIFY_SOCKET. The goroutine returns
// when the socket is closed.
func (p *NotifyProxy) listen() {
go func() {
// Read until the `READY` message is received or the connection
// is closed.

// See https://github.com/containers/podman/issues/16515 for a description of the protocol.
fdSize := unix.CmsgSpace(4)
buffer := make([]byte, _notifyBufferMax)
Expand All @@ -128,6 +126,7 @@ func (p *NotifyProxy) waitForReady() {
return
}
logrus.Errorf("Error reading unix message on socket %q: %v", p.socketPath, err)
continue
}

if n > _notifyBufferMax || oobn > _notifyFdMax*fdSize {
Expand Down Expand Up @@ -207,7 +206,7 @@ type Container interface {
ID() string
}

// WaitAndClose waits until receiving the `READY` notify message. Note that the
// Wait waits until receiving the `READY` notify message. Note that the
// this function must only be executed inside a systemd service which will kill
// the process after a given timeout. If the (optional) container stopped
// running before the `READY` is received, the waiting gets canceled and
Expand Down
2 changes: 2 additions & 0 deletions test/system/260-sdnotify.bats
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ none | false | false | 0
podman_exit=0
fi
run_podman $podman_exit kube play --service-exit-code-propagation="$exit_code_prop" --service-container $fname
# Make sure that there are no error logs (e.g., #19715)
assert "$output" !~ "error msg="
run_podman container inspect --format '{{.KubeExitCodePropagation}}' $service_container
is "$output" "$exit_code_prop" "service container has the expected policy set in its annotations"
run_podman wait $service_container
Expand Down

0 comments on commit 32f7bb1

Please sign in to comment.