From 09214f21da8e5805a4628d555324e0d2e8a8f4c3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Jan 2022 15:30:49 -0800 Subject: [PATCH 1/2] list: getContainers: less indentation Instead of a huge if {} block, use continue. Best reviewed with --ignore-all-space. Signed-off-by: Kir Kolyshkin (cherry picked from commit 095929b15eb21bfedfbcd46600c6fd3d1ea9d1da) Signed-off-by: Kir Kolyshkin --- list.go | 85 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/list.go b/list.go index 3503dcd2f5e..bf6ce792e69 100644 --- a/list.go +++ b/list.go @@ -127,50 +127,51 @@ func getContainers(context *cli.Context) ([]containerState, error) { var s []containerState for _, item := range list { - if item.IsDir() { - st, err := os.Stat(filepath.Join(absRoot, item.Name())) - if err != nil { - fatal(err) - } - // This cast is safe on Linux. - uid := st.Sys().(*syscall.Stat_t).Uid - owner, err := user.LookupUid(int(uid)) - if err != nil { - owner.Name = fmt.Sprintf("#%d", uid) - } + if !item.IsDir() { + continue + } + st, err := os.Stat(filepath.Join(absRoot, item.Name())) + if err != nil { + fatal(err) + } + // This cast is safe on Linux. + uid := st.Sys().(*syscall.Stat_t).Uid + owner, err := user.LookupUid(int(uid)) + if err != nil { + owner.Name = fmt.Sprintf("#%d", uid) + } - container, err := factory.Load(item.Name()) - if err != nil { - fmt.Fprintf(os.Stderr, "load container %s: %v\n", item.Name(), err) - continue - } - containerStatus, err := container.Status() - if err != nil { - fmt.Fprintf(os.Stderr, "status for %s: %v\n", item.Name(), err) - continue - } - state, err := container.State() - if err != nil { - fmt.Fprintf(os.Stderr, "state for %s: %v\n", item.Name(), err) - continue - } - pid := state.BaseState.InitProcessPid - if containerStatus == libcontainer.Stopped { - pid = 0 - } - bundle, annotations := utils.Annotations(state.Config.Labels) - s = append(s, containerState{ - Version: state.BaseState.Config.Version, - ID: state.BaseState.ID, - InitProcessPid: pid, - Status: containerStatus.String(), - Bundle: bundle, - Rootfs: state.BaseState.Config.Rootfs, - Created: state.BaseState.Created, - Annotations: annotations, - Owner: owner.Name, - }) + container, err := factory.Load(item.Name()) + if err != nil { + fmt.Fprintf(os.Stderr, "load container %s: %v\n", item.Name(), err) + continue + } + containerStatus, err := container.Status() + if err != nil { + fmt.Fprintf(os.Stderr, "status for %s: %v\n", item.Name(), err) + continue + } + state, err := container.State() + if err != nil { + fmt.Fprintf(os.Stderr, "state for %s: %v\n", item.Name(), err) + continue + } + pid := state.BaseState.InitProcessPid + if containerStatus == libcontainer.Stopped { + pid = 0 } + bundle, annotations := utils.Annotations(state.Config.Labels) + s = append(s, containerState{ + Version: state.BaseState.Config.Version, + ID: state.BaseState.ID, + InitProcessPid: pid, + Status: containerStatus.String(), + Bundle: bundle, + Rootfs: state.BaseState.Config.Rootfs, + Created: state.BaseState.Created, + Annotations: annotations, + Owner: owner.Name, + }) } return s, nil } From 986edbe60ff9f87be578a503a941ed7bb95f9206 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Jan 2022 15:33:25 -0800 Subject: [PATCH 2/2] list: use Info(), fix race with delete Since commit 551629417 we can (and should) use Info() to get access to file stat. Do this. While going over directory entries, a parallel runc delete can remove an entry, and with the current code it results in a fatal error (which was not observed in practice, but looks quite possible). To fix, add a special case to continue on ErrNotExist. Signed-off-by: Kir Kolyshkin (cherry picked from commit 1a3ee4966ce58abd243cb6aeb55fb561dca31a56) Signed-off-by: Kir Kolyshkin --- list.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/list.go b/list.go index bf6ce792e69..3fb9917248f 100644 --- a/list.go +++ b/list.go @@ -130,8 +130,12 @@ func getContainers(context *cli.Context) ([]containerState, error) { if !item.IsDir() { continue } - st, err := os.Stat(filepath.Join(absRoot, item.Name())) + st, err := item.Info() if err != nil { + if errors.Is(err, os.ErrNotExist) { + // Possible race with runc delete. + continue + } fatal(err) } // This cast is safe on Linux.