From c5bdb6afe741d34c32f779b6ef9508b6f1d05794 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 9 Sep 2022 11:07:07 +0200 Subject: [PATCH 1/9] fix hang with podman events file logger podman --events-backend file events --stream=false should never hang. The problem is that our tail library will wait for the file to be created which makes sense when we do not run with --stream=false. To fix this we can just always create the file when the logger is initialized. This would also help to report errors early on in case the file is not accessible. Fixes part one from #15688 Signed-off-by: Paul Holzinger --- libpod/events/events_linux.go | 2 +- libpod/events/logfile.go | 18 ++++++++++++++++++ libpod/runtime.go | 8 -------- test/system/090-events.bats | 6 +++++- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/libpod/events/events_linux.go b/libpod/events/events_linux.go index e7801af5b2..b11467acaf 100644 --- a/libpod/events/events_linux.go +++ b/libpod/events/events_linux.go @@ -18,7 +18,7 @@ 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 case strings.ToUpper(Memory.String()): diff --git a/libpod/events/logfile.go b/libpod/events/logfile.go index 519e16629b..1b06e22e76 100644 --- a/libpod/events/logfile.go +++ b/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, 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 { diff --git a/libpod/runtime.go b/libpod/runtime.go index fe90b6df1d..03610c687d 100644 --- a/libpod/runtime.go +++ b/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) diff --git a/test/system/090-events.bats b/test/system/090-events.bats index cd1bf327bf..51a327865a 100644 --- a/test/system/090-events.bats +++ b/test/system/090-events.bats @@ -147,7 +147,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 < Date: Fri, 9 Sep 2022 11:43:20 +0200 Subject: [PATCH 2/9] event backend none: return an error when reading events podman --events-backend none events should return with an error since it will never be able to actually list events. Fixes part three of #15688 Signed-off-by: Paul Holzinger --- cmd/podman/system/events.go | 39 +++++++++++++++++++---------------- libpod/events/events_linux.go | 2 +- libpod/events/nullout.go | 13 ++++++------ test/system/090-events.bats | 9 ++++++++ 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/cmd/podman/system/events.go b/cmd/podman/system/events.go index 3c6a35e832..57279bb021 100644 --- a/cmd/podman/system/events.go +++ b/cmd/podman/system/events.go @@ -99,25 +99,28 @@ 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 err := <-errChannel: + return err + case event := <-eventChannel: + switch { + case event == nil: + // no-op + 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 + } + os.Stdout.WriteString("\n") + default: + fmt.Println(event.ToHumanReadable(!noTrunc)) } - 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 } diff --git a/libpod/events/events_linux.go b/libpod/events/events_linux.go index b11467acaf..66b125dd5f 100644 --- a/libpod/events/events_linux.go +++ b/libpod/events/events_linux.go @@ -20,7 +20,7 @@ func NewEventer(options EventerOptions) (Eventer, error) { case strings.ToUpper(LogFile.String()): return newLogFileEventer(options) case strings.ToUpper(Null.String()): - return NewNullEventer(), nil + return newNullEventer(), nil case strings.ToUpper(Memory.String()): return NewMemoryEventer(), nil default: diff --git a/libpod/events/nullout.go b/libpod/events/nullout.go index 587a1b98b3..da3820c23c 100644 --- a/libpod/events/nullout.go +++ b/libpod/events/nullout.go @@ -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{} } diff --git a/test/system/090-events.bats b/test/system/090-events.bats index 51a327865a..509e7a3067 100644 --- a/test/system/090-events.bats +++ b/test/system/090-events.bats @@ -217,3 +217,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" +} From 76980a2226278eca1c0b31224ae7bdce59d5eabb Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 9 Sep 2022 13:10:10 +0200 Subject: [PATCH 3/9] event backend journald: fix problem with empty journal Currently podman events will just fail with `Error: failed to get journal cursor: failed to get cursor: cannot assign requested address` when the journal contains zero podman events. The problem is that we are using the journal accessors wrong. There is no need to call GetCursor() and compare them manually. The Next() return an integer which tells if it moved to the next or not. This means the we can remove GetCursor() which would fail when there is no entry. This also includes another bug fix. Previously the logic called Next() twice for the first entry which caused us to miss the first entry. To reproduce this issue you can run the following commands: ``` sudo journalctl --rotate sudo journalctl --vacuum-time=1s ``` Note that this will delete the full journal. Now run podman events and it fails but with this patch it works. Now generate a single event, i.e. podman pull alpine, and run podman events --until 1s. I am not sure how to get a reliable test into CI, I really do not want to delete the journal and developer or CI systems. Fixes second part of #15688 Signed-off-by: Paul Holzinger --- libpod/events/journal_linux.go | 100 +++++++++++++++++---------------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 16ef6504f2..4986502a27 100644 --- a/libpod/events/journal_linux.go +++ b/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 + } +} From cd32b929e35cdb2d6b49853a7b0e5d93921b0979 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 9 Sep 2022 13:31:03 +0200 Subject: [PATCH 4/9] libpod: runtime newEventer() cleanup There is no reason to create a new eventer every time. The libpod runtime already has one attached which should be used instead. Signed-off-by: Paul Holzinger --- libpod/events.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/libpod/events.go b/libpod/events.go index 60142cb605..ad9c5eafed 100644 --- a/libpod/events.go +++ b/libpod/events.go @@ -133,11 +133,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 +145,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 +156,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 } From 72e715a1109426114ef054042be28014380a246d Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Fri, 9 Sep 2022 10:57:45 -0400 Subject: [PATCH 5/9] Use new secret store API Refactored secrets API in common for stability purposes. Move podman to said API. [NO NEW TESTS NEEDED] Signed-off-by: Ashley Cui --- go.mod | 2 +- go.sum | 4 +- pkg/domain/infra/abi/play.go | 9 +++- pkg/domain/infra/abi/secrets.go | 8 +++- pkg/specgen/generate/kube/play_test.go | 6 ++- .../common/libnetwork/network/interface.go | 42 ++++++++++++------- .../containers/common/pkg/config/default.go | 2 - .../containers/common/pkg/secrets/secrets.go | 33 +++++++++++---- vendor/modules.txt | 2 +- 9 files changed, 77 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 2ee4df39a6..a12ef2760c 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index f35770f31c..7c718b5a87 100644 --- a/go.sum +++ b/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= diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index db72bb3556..d447b4d001 100644 --- a/pkg/domain/infra/abi/play.go +++ b/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 } diff --git a/pkg/domain/infra/abi/secrets.go b/pkg/domain/infra/abi/secrets.go index e17de5a8cf..47159d65ab 100644 --- a/pkg/domain/infra/abi/secrets.go +++ b/pkg/domain/infra/abi/secrets.go @@ -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 diff --git a/pkg/specgen/generate/kube/play_test.go b/pkg/specgen/generate/kube/play_test.go index 470c0c39c6..ec0dc4bcdf 100644 --- a/pkg/specgen/generate/kube/play_test.go +++ b/pkg/specgen/generate/kube/play_test.go @@ -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) } diff --git a/vendor/github.com/containers/common/libnetwork/network/interface.go b/vendor/github.com/containers/common/libnetwork/network/interface.go index 545655fd3e..2093e10496 100644 --- a/vendor/github.com/containers/common/libnetwork/network/interface.go +++ b/vendor/github.com/containers/common/libnetwork/network/interface.go @@ -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 { + 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 + } + } + + // 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(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(nets) > 1 { + // we do not have a fresh system so use CNI + return types.CNI, nil } } - return types.CNI, nil + return types.Netavark, nil } func getCniInterface(conf *config.Config) (types.ContainerNetwork, error) { diff --git a/vendor/github.com/containers/common/pkg/config/default.go b/vendor/github.com/containers/common/pkg/config/default.go index b0d62779b1..3a3a558a1a 100644 --- a/vendor/github.com/containers/common/pkg/config/default.go +++ b/vendor/github.com/containers/common/pkg/config/default.go @@ -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 diff --git a/vendor/github.com/containers/common/pkg/secrets/secrets.go b/vendor/github.com/containers/common/pkg/secrets/secrets.go index ff12fa7999..705da3dda9 100644 --- a/vendor/github.com/containers/common/pkg/secrets/secrets.go +++ b/vendor/github.com/containers/common/pkg/secrets/secrets.go @@ -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 } diff --git a/vendor/modules.txt b/vendor/modules.txt index 43fa24b56f..a2ac343f36 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -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 From 12a1483e7fdf3d1d1526f3631dc4ad2cbe1bcd37 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 12 Sep 2022 11:04:27 +0200 Subject: [PATCH 6/9] Improve --tmpdir and --events-backend docs List the default paths to the event log file and the tmpdir option. Signed-off-by: Paul Holzinger --- docs/source/markdown/podman.1.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/source/markdown/podman.1.md b/docs/source/markdown/podman.1.md index 3b3974dcc1..7a8dd7043b 100644 --- a/docs/source/markdown/podman.1.md +++ b/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 +`/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`. From b3212a6802b9a9dd5311dcbebe68c9c67fd96218 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 12 Sep 2022 11:15:35 +0200 Subject: [PATCH 7/9] set default EventsLogFilePath on first run The current code only sets EventsLogFilePath when the tmp is overwritten from the db. We should always set the default when no path was set in containers.conf. Signed-off-by: Paul Holzinger --- libpod/events.go | 5 +++++ libpod/events/logfile.go | 2 +- libpod/runtime.go | 3 --- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/libpod/events.go b/libpod/events.go index ad9c5eafed..2f97991144 100644 --- a/libpod/events.go +++ b/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, diff --git a/libpod/events/logfile.go b/libpod/events/logfile.go index 1b06e22e76..d749a0d4d0 100644 --- a/libpod/events/logfile.go +++ b/libpod/events/logfile.go @@ -38,7 +38,7 @@ func newLogFileEventer(options EventerOptions) (*EventLogFile, error) { // 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, err + return nil, fmt.Errorf("failed to create event log file: %w", err) } return &EventLogFile{options: options}, fd.Close() } diff --git a/libpod/runtime.go b/libpod/runtime.go index 03610c687d..83c9f53e26 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -1030,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 != "" { From 2ae4ce79996097fb0a403abbb9be9ef7b5c02d80 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 12 Sep 2022 14:22:27 +0200 Subject: [PATCH 8/9] fix race where podman events exits to early In order to display all events we have to read until the event channel is closed. Signed-off-by: Paul Holzinger --- cmd/podman/system/events.go | 16 +++++++++++----- test/system/090-events.bats | 1 + 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cmd/podman/system/events.go b/cmd/podman/system/events.go index 57279bb021..693af3e60d 100644 --- a/cmd/podman/system/events.go +++ b/cmd/podman/system/events.go @@ -101,12 +101,12 @@ func eventsCmd(cmd *cobra.Command, _ []string) error { for { select { - case err := <-errChannel: - return err - case event := <-eventChannel: + case event, ok := <-eventChannel: + if !ok { + // channel was closed we can exit + return nil + } switch { - case event == nil: - // no-op case doJSON: jsonStr, err := event.ToJSONString() if err != nil { @@ -121,6 +121,12 @@ func eventsCmd(cmd *cobra.Command, _ []string) error { 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 + } } } } diff --git a/test/system/090-events.bats b/test/system/090-events.bats index 509e7a3067..3fac519389 100644 --- a/test/system/090-events.bats +++ b/test/system/090-events.bats @@ -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" From a63a40c3eece95e96e5b52a00646998606c5f82c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 12 Sep 2022 18:02:45 +0200 Subject: [PATCH 9/9] podman events --format: fix duplicated newline The --format changes caused a duplicated newline. PR #15678 should have a test for this. Signed-off-by: Paul Holzinger --- cmd/podman/system/events.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/podman/system/events.go b/cmd/podman/system/events.go index 693af3e60d..290f5b0fa2 100644 --- a/cmd/podman/system/events.go +++ b/cmd/podman/system/events.go @@ -117,7 +117,6 @@ func eventsCmd(cmd *cobra.Command, _ []string) error { if err := rpt.Execute(event); err != nil { return err } - os.Stdout.WriteString("\n") default: fmt.Println(event.ToHumanReadable(!noTrunc)) }