Merge pull request #15717 from Luap99/events

fix several podman events issues

cmd/podman/system/events.go

@@ -99,25 +99,33 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 		errChannel <- err
 	}()
 
-	for event := range eventChannel {
-		switch {
-		case event == nil:
-			// no-op
-		case doJSON:
-			jsonStr, err := event.ToJSONString()
-			if err != nil {
-				return err
+	for {
+		select {
+		case event, ok := <-eventChannel:
+			if !ok {
+				// channel was closed we can exit
+				return nil
+			}
+			switch {
+			case doJSON:
+				jsonStr, err := event.ToJSONString()
+				if err != nil {
+					return err
+				}
+				fmt.Println(jsonStr)
+			case cmd.Flags().Changed("format"):
+				if err := rpt.Execute(event); err != nil {
+					return err
+				}
+			default:
+				fmt.Println(event.ToHumanReadable(!noTrunc))
 			}
-			fmt.Println(jsonStr)
-		case cmd.Flags().Changed("format"):
-			if err := rpt.Execute(event); err != nil {
+		case err := <-errChannel:
+			// only exit in case of an error,
+			// otherwise keep reading events until the event channel is closed
+			if err != nil {
 				return err
 			}
-			os.Stdout.WriteString("\n")
-		default:
-			fmt.Println(event.ToHumanReadable(!noTrunc))
 		}
 	}
-
-	return <-errChannel
 }

docs/source/markdown/podman.1.md

@@ -43,8 +43,8 @@ Remote connections use local containers.conf for default.
 #### **--events-backend**=*type*
 
 Backend to use for storing events. Allowed values are **file**, **journald**, and
-**none**. When *file* is specified, the events are stored under a subdirectory
-of the *tmpdir* location (see **--tmpdir** below).
+**none**. When *file* is specified, the events are stored under
+`<tmpdir>/events/events.log` (see **--tmpdir** below).
 
 #### **--help**, **-h**
 
@@ -158,7 +158,7 @@ On remote clients, including Mac and Windows (excluding WSL2) machines, logging
 
 #### **--tmpdir**
 
-Path to the tmp directory, for libpod runtime content.
+Path to the tmp directory, for libpod runtime content. Defaults to `$XDG\_RUNTIME\_DIR/libpod/tmp` as rootless and `run/libpod/tmp` as rootful.
 
 NOTE --tmpdir is not used for the temporary storage of downloaded images.  Use the environment variable `TMPDIR` to change the temporary storage location of downloaded container images. Podman defaults to use `/var/tmp`.
 

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/containernetworking/cni v1.1.2
 	github.com/containernetworking/plugins v1.1.1
 	github.com/containers/buildah v1.27.1-0.20220907121344-97a52b13bb27
-	github.com/containers/common v0.49.2-0.20220908074553-1a09baf471c4
+	github.com/containers/common v0.49.2-0.20220909190843-e5685792b5d7
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/image/v5 v5.22.1-0.20220907162003-651744379993
 	github.com/containers/ocicrypt v1.1.5

go.sum

@@ -424,8 +424,8 @@ github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19
 github.com/containers/buildah v1.27.1-0.20220907121344-97a52b13bb27 h1:LRgKJ/JUd6iTocPg/q7oMZ9ilnbew50JXClXgiEoR9Q=
 github.com/containers/buildah v1.27.1-0.20220907121344-97a52b13bb27/go.mod h1:0iWhIkE70dkoVuwpmZy5/DXpBdI3C23iYmBQccTDWMU=
 github.com/containers/common v0.49.1/go.mod h1:ueM5hT0itKqCQvVJDs+EtjornAQtrHYxQJzP2gxeGIg=
-github.com/containers/common v0.49.2-0.20220908074553-1a09baf471c4 h1:+Z/KvBR34ihTFkliEGuj+kNX+8G/OEv1n8Nv4OiAXkI=
-github.com/containers/common v0.49.2-0.20220908074553-1a09baf471c4/go.mod h1:HaPvle8BvLTyjtY9B4HJoNCl60DpHwCDLA2FsZTWaak=
+github.com/containers/common v0.49.2-0.20220909190843-e5685792b5d7 h1:iSrqOya92AllZSA7y64Aamfcr4iOxgf4iatc9uFeL0U=
+github.com/containers/common v0.49.2-0.20220909190843-e5685792b5d7/go.mod h1:HaPvle8BvLTyjtY9B4HJoNCl60DpHwCDLA2FsZTWaak=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/image/v5 v5.22.0/go.mod h1:D8Ksv2RNB8qLJ7xe1P3rgJJOSQpahA6amv2Ax++/YO4=

libpod/events.go

@@ -3,6 +3,7 @@ package libpod
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"sync"
 
 	"github.com/containers/podman/v4/libpod/events"
@@ -11,6 +12,10 @@ import (
 
 // newEventer returns an eventer that can be used to read/write events
 func (r *Runtime) newEventer() (events.Eventer, error) {
+	if r.config.Engine.EventsLogFilePath == "" {
+		// default, use path under tmpdir when none was explicitly set by the user
+		r.config.Engine.EventsLogFilePath = filepath.Join(r.config.Engine.TmpDir, "events", "events.log")
+	}
 	options := events.EventerOptions{
 		EventerType:    r.config.Engine.EventsLogger,
 		LogFilePath:    r.config.Engine.EventsLogFilePath,
@@ -133,11 +138,7 @@ func (v *Volume) newVolumeEvent(status events.Status) {
 // Events is a wrapper function for everyone to begin tailing the events log
 // with options
 func (r *Runtime) Events(ctx context.Context, options events.ReadOptions) error {
-	eventer, err := r.newEventer()
-	if err != nil {
-		return err
-	}
-	return eventer.Read(ctx, options)
+	return r.eventer.Read(ctx, options)
 }
 
 // GetEvents reads the event log and returns events based on input filters
@@ -149,10 +150,6 @@ func (r *Runtime) GetEvents(ctx context.Context, filters []string) ([]*events.Ev
 		FromStart:    true,
 		Stream:       false,
 	}
-	eventer, err := r.newEventer()
-	if err != nil {
-		return nil, err
-	}
 
 	logEvents := make([]*events.Event, 0, len(eventChannel))
 	readLock := sync.Mutex{}
@@ -164,7 +161,7 @@ func (r *Runtime) GetEvents(ctx context.Context, filters []string) ([]*events.Ev
 		readLock.Unlock()
 	}()
 
-	readErr := eventer.Read(ctx, options)
+	readErr := r.eventer.Read(ctx, options)
 	readLock.Lock() // Wait for the events to be consumed.
 	return logEvents, readErr
 }

libpod/events/events_linux.go

@@ -18,9 +18,9 @@ func NewEventer(options EventerOptions) (Eventer, error) {
 		}
 		return eventer, nil
 	case strings.ToUpper(LogFile.String()):
-		return EventLogFile{options}, nil
+		return newLogFileEventer(options)
 	case strings.ToUpper(Null.String()):
-		return NewNullEventer(), nil
+		return newNullEventer(), nil
 	case strings.ToUpper(Memory.String()):
 		return NewMemoryEventer(), nil
 	default:

libpod/events/journal_linux.go

@@ -112,57 +112,16 @@ func (e EventJournalD) Read(ctx context.Context, options ReadOptions) error {
 		}
 	}
 
-	// the api requires a next|prev before getting a cursor
-	if _, err := j.Next(); err != nil {
-		return fmt.Errorf("failed to move journal cursor to next entry: %w", err)
-	}
-
-	prevCursor, err := j.GetCursor()
-	if err != nil {
-		return fmt.Errorf("failed to get journal cursor: %w", err)
-	}
 	for {
-		select {
-		case <-ctx.Done():
-			// the consumer has cancelled
-			return nil
-		default:
-			// fallthrough
-		}
-
-		if _, err := j.Next(); err != nil {
-			return fmt.Errorf("failed to move journal cursor to next entry: %w", err)
-		}
-		newCursor, err := j.GetCursor()
+		entry, err := getNextEntry(ctx, j, options.Stream, untilTime)
 		if err != nil {
-			return fmt.Errorf("failed to get journal cursor: %w", err)
+			return err
 		}
-		if prevCursor == newCursor {
-			if !options.Stream || (len(options.Until) > 0 && time.Now().After(untilTime)) {
-				break
-			}
-
-			// j.Wait() is blocking, this would cause the goroutine to hang forever
-			// if no more journal entries are generated and thus if the client
-			// has closed the connection in the meantime to leak memory.
-			// Waiting only 5 seconds makes sure we can check if the client closed in the
-			// meantime at least every 5 seconds.
-			t := 5 * time.Second
-			if len(options.Until) > 0 {
-				until := time.Until(untilTime)
-				if until < t {
-					t = until
-				}
-			}
-			_ = j.Wait(t)
-			continue
+		// no entry == we hit the end
+		if entry == nil {
+			return nil
 		}
-		prevCursor = newCursor
 
-		entry, err := j.GetEntry()
-		if err != nil {
-			return fmt.Errorf("failed to read journal entry: %w", err)
-		}
 		newEvent, err := newEventFromJournalEntry(entry)
 		if err != nil {
 			// We can't decode this event.
@@ -177,7 +136,6 @@ func (e EventJournalD) Read(ctx context.Context, options ReadOptions) error {
 			options.EventChannel <- newEvent
 		}
 	}
-	return nil
 }
 
 func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) {
@@ -238,3 +196,51 @@ func newEventFromJournalEntry(entry *sdjournal.JournalEntry) (*Event, error) {
 func (e EventJournalD) String() string {
 	return Journald.String()
 }
+
+// getNextEntry returns the next entry in the journal. If the end  of the
+// journal is reached and stream is not set or the current time is after
+// the until time this function return nil,nil.
+func getNextEntry(ctx context.Context, j *sdjournal.Journal, stream bool, untilTime time.Time) (*sdjournal.JournalEntry, error) {
+	for {
+		select {
+		case <-ctx.Done():
+			// the consumer has cancelled
+			return nil, nil
+		default:
+			// fallthrough
+		}
+		// the api requires a next|prev before reading the event
+		ret, err := j.Next()
+		if err != nil {
+			return nil, fmt.Errorf("failed to move journal cursor to next entry: %w", err)
+		}
+		// ret == 0 equals EOF, see sd_journal_next(3)
+		if ret == 0 {
+			if !stream || (!untilTime.IsZero() && time.Now().After(untilTime)) {
+				// we hit the end and should not keep streaming
+				return nil, nil
+			}
+			// keep waiting for the next entry
+			// j.Wait() is blocking, this would cause the goroutine to hang forever
+			// if no more journal entries are generated and thus if the client
+			// has closed the connection in the meantime to leak memory.
+			// Waiting only 5 seconds makes sure we can check if the client closed in the
+			// meantime at least every 5 seconds.
+			t := 5 * time.Second
+			if !untilTime.IsZero() {
+				until := time.Until(untilTime)
+				if until < t {
+					t = until
+				}
+			}
+			_ = j.Wait(t)
+			continue
+		}
+
+		entry, err := j.GetEntry()
+		if err != nil {
+			return nil, fmt.Errorf("failed to read journal entry: %w", err)
+		}
+		return entry, nil
+	}
+}

libpod/events/logfile.go

@@ -12,6 +12,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"time"
 
 	"github.com/containers/podman/v4/pkg/util"
@@ -27,6 +28,21 @@ type EventLogFile struct {
 	options EventerOptions
 }
 
+// newLogFileEventer creates a new EventLogFile eventer
+func newLogFileEventer(options EventerOptions) (*EventLogFile, error) {
+	// Create events log dir
+	if err := os.MkdirAll(filepath.Dir(options.LogFilePath), 0700); err != nil {
+		return nil, fmt.Errorf("creating events dirs: %w", err)
+	}
+	// We have to make sure the file is created otherwise reading events will hang.
+	// https://github.com/containers/podman/issues/15688
+	fd, err := os.OpenFile(options.LogFilePath, os.O_RDONLY|os.O_CREATE, 0700)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create event log file: %w", err)
+	}
+	return &EventLogFile{options: options}, fd.Close()
+}
+
 // Writes to the log file
 func (e EventLogFile) Write(ee Event) error {
 	// We need to lock events file
@@ -108,6 +124,8 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
 			}
 		}()
 	}
+	logrus.Debugf("Reading events from file %q", e.options.LogFilePath)
+
 	var line *tail.Line
 	var ok bool
 	for {

libpod/events/nullout.go

@@ -2,25 +2,26 @@ package events
 
 import (
 	"context"
+	"errors"
 )
 
-// EventToNull is an eventer type that only performs write operations
-// and only writes to /dev/null. It is meant for unittests only
+// EventToNull is an eventer type that does nothing.
+// It is meant for unittests only
 type EventToNull struct{}
 
 // Write eats the event and always returns nil
 func (e EventToNull) Write(ee Event) error {
 	return nil
 }
 
-// Read does nothing. Do not use it.
+// Read does nothing and returns an error.
 func (e EventToNull) Read(ctx context.Context, options ReadOptions) error {
-	return nil
+	return errors.New("cannot read events with the \"none\" backend")
 }
 
-// NewNullEventer returns a new null eventer.  You should only do this for
+// newNullEventer returns a new null eventer.  You should only do this for
 // the purposes of internal libpod testing.
-func NewNullEventer() Eventer {
+func newNullEventer() Eventer {
 	return EventToNull{}
 }
 

libpod/runtime.go

@@ -466,14 +466,6 @@ func makeRuntime(runtime *Runtime) (retErr error) {
 		}
 	}
 
-	// Create events log dir
-	if err := os.MkdirAll(filepath.Dir(runtime.config.Engine.EventsLogFilePath), 0700); err != nil {
-		// The directory is allowed to exist
-		if !errors.Is(err, os.ErrExist) {
-			return fmt.Errorf("creating events dirs: %w", err)
-		}
-	}
-
 	// Get us at least one working OCI runtime.
 	runtime.ociRuntimes = make(map[string]OCIRuntime)
 
@@ -1038,9 +1030,6 @@ func (r *Runtime) mergeDBConfig(dbConfig *DBConfig) {
 			logrus.Debugf("Overriding tmp dir %q with %q from database", c.TmpDir, dbConfig.LibpodTmp)
 		}
 		c.TmpDir = dbConfig.LibpodTmp
-		if c.EventsLogFilePath == "" {
-			c.EventsLogFilePath = filepath.Join(dbConfig.LibpodTmp, "events", "events.log")
-		}
 	}
 
 	if !r.storageSet.VolumePathSet && dbConfig.VolumePath != "" {

pkg/domain/infra/abi/play.go

@@ -16,6 +16,7 @@ import (
 	"github.com/containers/common/libimage"
 	nettypes "github.com/containers/common/libnetwork/types"
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/libpod"
 	"github.com/containers/podman/v4/libpod/define"
@@ -1110,7 +1111,13 @@ func (ic *ContainerEngine) playKubeSecret(secret *v1.Secret) (*entities.SecretCr
 	if secret.Immutable != nil && *secret.Immutable {
 		meta["immutable"] = "true"
 	}
-	secretID, err := secretsManager.Store(secret.Name, data, "file", opts, meta)
+
+	storeOpts := secrets.StoreOptions{
+		DriverOpts: opts,
+		Metadata:   meta,
+	}
+
+	secretID, err := secretsManager.Store(secret.Name, data, "file", storeOpts)
 	if err != nil {
 		return nil, err
 	}