From 4995c511e581723ee9d441d500eda2e6d2e8a839 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 1 Jul 2020 16:21:57 -0400 Subject: Fix `system service` panic from early hangup in events We weren't actually halting the goroutine that sent events, so it would continue sending even when the channel closed (the most notable cause being early hangup - e.g. Control-c on a curl session). Use a context to cancel the events goroutine and stop sending events. Fixes #6805 Signed-off-by: Matthew Heon --- pkg/api/handlers/compat/events.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'pkg/api/handlers') diff --git a/pkg/api/handlers/compat/events.go b/pkg/api/handlers/compat/events.go index 215d7c972..5acc94153 100644 --- a/pkg/api/handlers/compat/events.go +++ b/pkg/api/handlers/compat/events.go @@ -1,6 +1,7 @@ package compat import ( + "context" "fmt" "net/http" @@ -45,13 +46,15 @@ func GetEvents(w http.ResponseWriter, r *http.Request) { fromStart = true } + eventCtx, eventCancel := context.WithCancel(r.Context()) eventChannel := make(chan *events.Event) go func() { readOpts := events.ReadOptions{FromStart: fromStart, Stream: query.Stream, Filters: libpodFilters, EventChannel: eventChannel, Since: query.Since, Until: query.Until} - eventsError = runtime.Events(readOpts) + eventsError = runtime.Events(eventCtx, readOpts) }() if eventsError != nil { utils.InternalServerError(w, eventsError) + eventCancel() close(eventChannel) return } @@ -59,6 +62,7 @@ func GetEvents(w http.ResponseWriter, r *http.Request) { // If client disappears we need to stop listening for events go func(done <-chan struct{}) { <-done + eventCancel() if _, ok := <-eventChannel; ok { close(eventChannel) } -- cgit v1.2.3-54-g00ecf From eb85f429073d7dca0d2d01a1afe5a972ca9b429b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 24 Jun 2020 14:44:28 +0200 Subject: container: move volume chown after spec generation move the chown for newly created volumes after the spec generation so the correct UID/GID are known. Closes: https://github.com/containers/libpod/issues/5698 Signed-off-by: Giuseppe Scrivano --- libpod/container_internal.go | 48 ++++++++++++++++++++++++++++++++++++++ libpod/options.go | 13 +++++++++++ libpod/runtime_ctr.go | 2 +- libpod/volume.go | 36 ++++++++++++++++++++++++---- libpod/volume_inspect.go | 11 +++++++-- pkg/api/handlers/libpod/volumes.go | 30 ++++++++++++++++++++---- pkg/domain/infra/abi/volumes.go | 26 +++++++++++++++++---- 7 files changed, 151 insertions(+), 15 deletions(-) (limited to 'pkg/api/handlers') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index e86512569..7a547e565 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1015,6 +1015,12 @@ func (c *Container) init(ctx context.Context, retainRetries bool) error { return err } + for _, v := range c.config.NamedVolumes { + if err := c.chownVolume(v.Name); err != nil { + return err + } + } + // With the spec complete, do an OCI create if err := c.ociRuntime.CreateContainer(c, nil); err != nil { // Fedora 31 is carrying a patch to display improved error @@ -1508,6 +1514,48 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) return vol, nil } +// Chown the specified volume if necessary. +func (c *Container) chownVolume(volumeName string) error { + vol, err := c.runtime.state.Volume(volumeName) + if err != nil { + return errors.Wrapf(err, "error retrieving named volume %s for container %s", volumeName, c.ID()) + } + + uid := int(c.config.Spec.Process.User.UID) + gid := int(c.config.Spec.Process.User.GID) + + vol.lock.Lock() + defer vol.lock.Unlock() + + // The volume may need a copy-up. Check the state. + if err := vol.update(); err != nil { + return err + } + + if vol.state.NeedsChown { + vol.state.NeedsChown = false + vol.state.UIDChowned = uid + vol.state.GIDChowned = gid + + if err := vol.save(); err != nil { + return err + } + err := filepath.Walk(vol.MountPoint(), func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if err := os.Chown(path, uid, gid); err != nil { + return err + } + return nil + }) + if err != nil { + return err + } + } + return nil +} + // cleanupStorage unmounts and cleans up the container's root filesystem func (c *Container) cleanupStorage() error { if !c.state.Mounted { diff --git a/libpod/options.go b/libpod/options.go index 573806308..3120a35d7 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1485,6 +1485,19 @@ func WithVolumeGID(gid int) VolumeCreateOption { } } +// WithVolumeNeedsChown sets the NeedsChown flag for the volume. +func WithVolumeNeedsChown() VolumeCreateOption { + return func(volume *Volume) error { + if volume.valid { + return define.ErrVolumeFinalized + } + + volume.state.NeedsChown = true + + return nil + } +} + // withSetAnon sets a bool notifying libpod that this volume is anonymous and // should be removed when containers using it are removed and volumes are // specified for removal. diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 4010c9bea..c073aabb5 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -309,7 +309,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai logrus.Debugf("Creating new volume %s for container", vol.Name) // The volume does not exist, so we need to create it. - volOptions := []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())} + volOptions := []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID()), WithVolumeNeedsChown()} if isAnonymous { volOptions = append(volOptions, withSetAnon()) } diff --git a/libpod/volume.go b/libpod/volume.go index 72b080d1a..438957086 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -64,6 +64,14 @@ type VolumeState struct { // create time, then cleared after the copy up is done and never set // again. NeedsCopyUp bool `json:"notYetMounted,omitempty"` + // NeedsChown indicates that the next time the volume is mounted into + // a container, the container will chown the volume to the container process + // UID/GID. + NeedsChown bool `json:"notYetChowned,omitempty"` + // UIDChowned is the UID the volume was chowned to. + UIDChowned int `json:"uidChowned,omitempty"` + // GIDChowned is the GID the volume was chowned to. + GIDChowned int `json:"gidChowned,omitempty"` } // Name retrieves the volume's name @@ -113,13 +121,33 @@ func (v *Volume) Anonymous() bool { } // UID returns the UID the volume will be created as. -func (v *Volume) UID() int { - return v.config.UID +func (v *Volume) UID() (int, error) { + v.lock.Lock() + defer v.lock.Unlock() + + if !v.valid { + return -1, define.ErrVolumeRemoved + } + + if v.state.UIDChowned > 0 { + return v.state.UIDChowned, nil + } + return v.config.UID, nil } // GID returns the GID the volume will be created as. -func (v *Volume) GID() int { - return v.config.GID +func (v *Volume) GID() (int, error) { + v.lock.Lock() + defer v.lock.Unlock() + + if !v.valid { + return -1, define.ErrVolumeRemoved + } + + if v.state.GIDChowned > 0 { + return v.state.GIDChowned, nil + } + return v.config.GID, nil } // CreatedTime returns the time the volume was created at. It was not tracked diff --git a/libpod/volume_inspect.go b/libpod/volume_inspect.go index 5258792eb..85848f84f 100644 --- a/libpod/volume_inspect.go +++ b/libpod/volume_inspect.go @@ -65,8 +65,15 @@ func (v *Volume) Inspect() (*InspectVolumeData, error) { for k, v := range v.config.Options { data.Options[k] = v } - data.UID = v.config.UID - data.GID = v.config.GID + var err error + data.UID, err = v.UID() + if err != nil { + return nil, err + } + data.GID, err = v.GID() + if err != nil { + return nil, err + } data.Anonymous = v.config.IsAnon return data, nil diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index f9a651c5d..0d83218e3 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/pkg/api/handlers/libpod/volumes.go @@ -86,6 +86,17 @@ func InspectVolume(w http.ResponseWriter, r *http.Request) { utils.VolumeNotFound(w, name, err) return } + var uid, gid int + uid, err = vol.UID() + if err != nil { + utils.Error(w, "Error fetching volume UID", http.StatusInternalServerError, err) + return + } + gid, err = vol.GID() + if err != nil { + utils.Error(w, "Error fetching volume GID", http.StatusInternalServerError, err) + return + } volResponse := entities.VolumeConfigResponse{ Name: vol.Name(), Driver: vol.Driver(), @@ -94,8 +105,8 @@ func InspectVolume(w http.ResponseWriter, r *http.Request) { Labels: vol.Labels(), Scope: vol.Scope(), Options: vol.Options(), - UID: vol.UID(), - GID: vol.GID(), + UID: uid, + GID: gid, } utils.WriteResponse(w, http.StatusOK, volResponse) } @@ -130,6 +141,17 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) { } volumeConfigs := make([]*entities.VolumeListReport, 0, len(vols)) for _, v := range vols { + var uid, gid int + uid, err = v.UID() + if err != nil { + utils.Error(w, "Error fetching volume UID", http.StatusInternalServerError, err) + return + } + gid, err = v.GID() + if err != nil { + utils.Error(w, "Error fetching volume GID", http.StatusInternalServerError, err) + return + } config := entities.VolumeConfigResponse{ Name: v.Name(), Driver: v.Driver(), @@ -138,8 +160,8 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) { Labels: v.Labels(), Scope: v.Scope(), Options: v.Options(), - UID: v.UID(), - GID: v.GID(), + UID: uid, + GID: gid, } volumeConfigs = append(volumeConfigs, &entities.VolumeListReport{VolumeConfigResponse: config}) } diff --git a/pkg/domain/infra/abi/volumes.go b/pkg/domain/infra/abi/volumes.go index 861617eb6..8db89899e 100644 --- a/pkg/domain/infra/abi/volumes.go +++ b/pkg/domain/infra/abi/volumes.go @@ -95,6 +95,15 @@ func (ic *ContainerEngine) VolumeInspect(ctx context.Context, namesOrIds []strin } reports := make([]*entities.VolumeInspectReport, 0, len(vols)) for _, v := range vols { + var uid, gid int + uid, err = v.UID() + if err != nil { + return nil, err + } + gid, err = v.GID() + if err != nil { + return nil, err + } config := entities.VolumeConfigResponse{ Name: v.Name(), Driver: v.Driver(), @@ -103,8 +112,8 @@ func (ic *ContainerEngine) VolumeInspect(ctx context.Context, namesOrIds []strin Labels: v.Labels(), Scope: v.Scope(), Options: v.Options(), - UID: v.UID(), - GID: v.GID(), + UID: uid, + GID: gid, } reports = append(reports, &entities.VolumeInspectReport{VolumeConfigResponse: &config}) } @@ -141,6 +150,15 @@ func (ic *ContainerEngine) VolumeList(ctx context.Context, opts entities.VolumeL } reports := make([]*entities.VolumeListReport, 0, len(vols)) for _, v := range vols { + var uid, gid int + uid, err = v.UID() + if err != nil { + return nil, err + } + gid, err = v.GID() + if err != nil { + return nil, err + } config := entities.VolumeConfigResponse{ Name: v.Name(), Driver: v.Driver(), @@ -149,8 +167,8 @@ func (ic *ContainerEngine) VolumeList(ctx context.Context, opts entities.VolumeL Labels: v.Labels(), Scope: v.Scope(), Options: v.Options(), - UID: v.UID(), - GID: v.GID(), + UID: uid, + GID: gid, } reports = append(reports, &entities.VolumeListReport{VolumeConfigResponse: config}) } -- cgit v1.2.3-54-g00ecf From 1444be3d53dfa677332fe7731fc7e0058741394e Mon Sep 17 00:00:00 2001 From: Maximilian Müller Date: Fri, 26 Jun 2020 13:54:34 +0200 Subject: Fix a bug with APIv2 compat network remove to log an ErrNetworkNotFound instead of nil MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maximilian Müller --- pkg/api/handlers/compat/networks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg/api/handlers') diff --git a/pkg/api/handlers/compat/networks.go b/pkg/api/handlers/compat/networks.go index fe7d8888e..2e11c0edb 100644 --- a/pkg/api/handlers/compat/networks.go +++ b/pkg/api/handlers/compat/networks.go @@ -285,7 +285,7 @@ func RemoveNetwork(w http.ResponseWriter, r *http.Request) { return } if !exists { - utils.Error(w, "network not found", http.StatusNotFound, err) + utils.Error(w, "network not found", http.StatusNotFound, network.ErrNetworkNotFound) return } if err := network.RemoveNetwork(config, name); err != nil { -- cgit v1.2.3-54-g00ecf