Merge pull request #15717 from Luap99/events

fix several podman events issues
This commit is contained in:
OpenShift Merge Robot
2022-09-12 19:55:15 +02:00
committed by GitHub
18 changed files with 209 additions and 130 deletions

View File

@ -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()
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))
}
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
}
fmt.Println(jsonStr)
case cmd.Flags().Changed("format"):
if err := rpt.Execute(event); err != nil {
return err
}
os.Stdout.WriteString("\n")
default:
fmt.Println(event.ToHumanReadable(!noTrunc))
}
}
return <-errChannel
}

View File

@ -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`.

2
go.mod
View File

@ -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

4
go.sum
View File

@ -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=

View File

@ -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
}

View File

@ -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:

View File

@ -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
entry, err := getNextEntry(ctx, j, options.Stream, untilTime)
if err != nil {
return err
}
// no entry == we hit the end
if entry == nil {
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()
if err != nil {
return fmt.Errorf("failed to get journal cursor: %w", 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
}
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
}
}

View File

@ -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 {

View File

@ -2,10 +2,11 @@ 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
@ -13,14 +14,14 @@ 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{}
}

View File

@ -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 != "" {

View File

@ -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
}

View File

@ -8,6 +8,7 @@ import (
"path/filepath"
"strings"
"github.com/containers/common/pkg/secrets"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/domain/utils"
)
@ -42,10 +43,15 @@ func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader
}
}
secretID, err := manager.Store(name, data, options.Driver, options.DriverOpts, nil)
storeOpts := secrets.StoreOptions{
DriverOpts: options.DriverOpts,
}
secretID, err := manager.Store(name, data, options.Driver, storeOpts)
if err != nil {
return nil, err
}
return &entities.SecretCreateReport{
ID: secretID,
}, nil

View File

@ -24,11 +24,15 @@ func createSecrets(t *testing.T, d string) *secrets.SecretsManager {
"path": d,
}
storeOpts := secrets.StoreOptions{
DriverOpts: driverOpts,
}
for _, s := range k8sSecrets {
data, err := json.Marshal(s.Data)
assert.NoError(t, err)
_, err = secretsManager.Store(s.ObjectMeta.Name, data, driver, driverOpts, nil)
_, err = secretsManager.Store(s.ObjectMeta.Name, data, driver, storeOpts)
assert.NoError(t, err)
}

View File

@ -74,6 +74,7 @@ load helpers
.*image tag $imageID $tag
.*image untag $imageID $tag:latest
.*image tag $imageID $tag
.*image untag $imageID $IMAGE
.*image untag $imageID $tag:latest
.*image remove $imageID $imageID" \
"podman events"
@ -147,7 +148,6 @@ function _populate_events_file() {
# Config without a limit
eventsFile=$PODMAN_TMPDIR/events.txt
_populate_events_file $eventsFile
containersConf=$PODMAN_TMPDIR/containers.conf
cat >$containersConf <<EOF
[engine]
@ -155,6 +155,11 @@ events_logger="file"
events_logfile_path="$eventsFile"
EOF
# Check that a non existing event file does not cause a hang (#15688)
CONTAINERS_CONF=$containersConf run_podman events --stream=false
_populate_events_file $eventsFile
# Create events *without* a limit and make sure that it has not been
# rotated/truncated.
contentBefore=$(head -n100 $eventsFile)
@ -213,3 +218,12 @@ EOF
--format="{{.Attributes.$lname}}"
assert "$output" = "$lvalue" "podman-events output includes container label"
}
@test "events - backend none should error" {
skip_if_remote "remote does not support --events-backend"
run_podman 125 --events-backend none events
is "$output" "Error: cannot read events with the \"none\" backend" "correct error message"
run_podman 125 --events-backend none events --stream=false
is "$output" "Error: cannot read events with the \"none\" backend" "correct error message"
}

View File

@ -132,29 +132,41 @@ func defaultNetworkBackend(store storage.Store, conf *config.Config) (backend ty
return types.CNI, nil
}
// now check if there are already containers, images and CNI networks (new install?)
// If there are any containers then return CNI
cons, err := store.Containers()
if err != nil {
return "", err
}
if len(cons) == 0 {
imgs, err := store.Images()
if err != nil {
return "", err
}
if len(imgs) == 0 {
cniInterface, err := getCniInterface(conf)
if err == nil {
nets, err := cniInterface.NetworkList()
// there is always a default network so check <= 1
if err == nil && len(nets) <= 1 {
// we have a fresh system so use netavark
return types.Netavark, nil
}
}
if len(cons) != 0 {
return types.CNI, nil
}
// If there are any non ReadOnly images then return CNI
imgs, err := store.Images()
if err != nil {
return "", err
}
for _, i := range imgs {
if !i.ReadOnly {
return types.CNI, nil
}
}
return types.CNI, nil
// If there are CNI Networks then return CNI
cniInterface, err := getCniInterface(conf)
if err == nil {
nets, err := cniInterface.NetworkList()
// there is always a default network so check > 1
if err != nil && !errors.Is(err, os.ErrNotExist) {
return "", err
}
if len(nets) > 1 {
// we do not have a fresh system so use CNI
return types.CNI, nil
}
}
return types.Netavark, nil
}
func getCniInterface(conf *config.Config) (types.ContainerNetwork, error) {

View File

@ -280,8 +280,6 @@ func defaultConfigFromMemory() (*EngineConfig, error) {
}
c.TmpDir = tmp
c.EventsLogFilePath = filepath.Join(c.TmpDir, "events", "events.log")
c.EventsLogFileMaxSize = eventsLogMaxSize(DefaultEventsLogSizeMax)
c.CompatAPIEnforceDockerHub = true

View File

@ -72,13 +72,15 @@ type Secret struct {
Name string `json:"name"`
// ID is the unique secret ID
ID string `json:"id"`
// Labels are labels on the secret
Labels map[string]string `json:"labels,omitempty"`
// Metadata stores other metadata on the secret
Metadata map[string]string `json:"metadata,omitempty"`
// CreatedAt is when the secret was created
CreatedAt time.Time `json:"createdAt"`
// Driver is the driver used to store secret data
Driver string `json:"driver"`
// DriverOptions is other metadata needed to use the driver
// DriverOptions are extra options used to run this driver
DriverOptions map[string]string `json:"driverOptions"`
}
@ -100,6 +102,16 @@ type SecretsDriver interface {
Delete(id string) error
}
// StoreOptions are optional metadata fields that can be set when storing a new secret
type StoreOptions struct {
// DriverOptions are extra options used to run this driver
DriverOpts map[string]string
// Metadata stores extra metadata on the secret
Metadata map[string]string
// Labels are labels on the secret
Labels map[string]string
}
// NewManager creates a new secrets manager
// rootPath is the directory where the secrets data file resides
func NewManager(rootPath string) (*SecretsManager, error) {
@ -129,7 +141,7 @@ func NewManager(rootPath string) (*SecretsManager, error) {
// Store takes a name, creates a secret and stores the secret metadata and the secret payload.
// It returns a generated ID that is associated with the secret.
// The max size for secret data is 512kB.
func (s *SecretsManager) Store(name string, data []byte, driverType string, driverOpts map[string]string, metadata map[string]string) (string, error) {
func (s *SecretsManager) Store(name string, data []byte, driverType string, options StoreOptions) (string, error) {
err := validateSecretName(name)
if err != nil {
return "", err
@ -168,16 +180,23 @@ func (s *SecretsManager) Store(name string, data []byte, driverType string, driv
}
}
if metadata == nil {
metadata = make(map[string]string)
if options.Metadata == nil {
options.Metadata = make(map[string]string)
}
if options.Labels == nil {
options.Labels = make(map[string]string)
}
if options.DriverOpts == nil {
options.DriverOpts = make(map[string]string)
}
secr.Driver = driverType
secr.Metadata = metadata
secr.Metadata = options.Metadata
secr.CreatedAt = time.Now()
secr.DriverOptions = driverOpts
secr.DriverOptions = options.DriverOpts
secr.Labels = options.Labels
driver, err := getDriver(driverType, driverOpts)
driver, err := getDriver(driverType, options.DriverOpts)
if err != nil {
return "", err
}

2
vendor/modules.txt vendored
View File

@ -110,7 +110,7 @@ github.com/containers/buildah/pkg/rusage
github.com/containers/buildah/pkg/sshagent
github.com/containers/buildah/pkg/util
github.com/containers/buildah/util
# github.com/containers/common v0.49.2-0.20220908074553-1a09baf471c4
# github.com/containers/common v0.49.2-0.20220909190843-e5685792b5d7
## explicit
github.com/containers/common/libimage
github.com/containers/common/libimage/define