Fix runtime panic for opening lockfile if parent dir got removed
If the lockfile directory gets removed but the container runtime is
still running with an open storage instance, then we can trigger a
runtime panic by:
1. Starting CRI-O
2. Removing the lockfile dir, e.g.:
`sudo rm -rf /var/lib/containers/storage/overlay-images`
3. Running `crictl images`
```
panic: error opening "/var/lib/containers/storage/overlay-images/images.lock": no such file or directory
goroutine 93 [running]:
github.com/containers/storage/pkg/lockfile.(*lockfile).lock(0xc0006b83c0, 0x2000000)
/go/src/github.com/cri-o/cri-o/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go:107 +0x388
github.com/containers/storage/pkg/lockfile.(*lockfile).RLock(0xc0006b83c0)
/go/src/github.com/cri-o/cri-o/vendor/github.com/containers/storage/pkg/lockfile/lockfile_unix.go:146 +0x37
github.com/containers/storage.(*imageStore).RLock(0xc00006e420)
/go/src/github.com/cri-o/cri-o/vendor/github.com/containers/storage/images.go:778 +0x33
github.com/containers/storage.(*store).Images(0xc0003f3b80, 0x0, 0x0, 0x0, 0x0, 0x0)
/go/src/github.com/cri-o/cri-o/vendor/github.com/containers/storage/store.go:3089 +0x1dc
```
We now ensure the parent lockfile directory, but only if it does not
exist to not degrade the performance of c/stroage.
Unit tests around that case have been added as well.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
This commit is contained in:
parent
a52896dea8
commit
16de6d32b4
|
|
@ -5,6 +5,7 @@ package lockfile
|
|||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
|
|
@ -33,11 +34,30 @@ type lockfile struct {
|
|||
// descriptor. Note that the path is opened read-only when ro is set. If ro
|
||||
// is unset, openLock will open the path read-write and create the file if
|
||||
// necessary.
|
||||
func openLock(path string, ro bool) (int, error) {
|
||||
func openLock(path string, ro bool) (fd int, err error) {
|
||||
if ro {
|
||||
return unix.Open(path, os.O_RDONLY|unix.O_CLOEXEC, 0)
|
||||
fd, err = unix.Open(path, os.O_RDONLY|unix.O_CLOEXEC, 0)
|
||||
} else {
|
||||
fd, err = unix.Open(path,
|
||||
os.O_RDWR|unix.O_CLOEXEC|os.O_CREATE,
|
||||
unix.S_IRUSR|unix.S_IWUSR|unix.S_IRGRP|unix.S_IROTH,
|
||||
)
|
||||
}
|
||||
return unix.Open(path, os.O_RDWR|unix.O_CLOEXEC|os.O_CREATE, unix.S_IRUSR|unix.S_IWUSR|unix.S_IRGRP|unix.S_IROTH)
|
||||
|
||||
if err == nil {
|
||||
return
|
||||
}
|
||||
|
||||
// the directory of the lockfile seems to be removed, try to create it
|
||||
if os.IsNotExist(err) {
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
|
||||
return fd, errors.Wrap(err, "creating locker directory")
|
||||
}
|
||||
|
||||
return openLock(path, ro)
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// createLockerForPath returns a Locker object, possibly (depending on the platform)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,65 @@
|
|||
// +build linux solaris darwin freebsd
|
||||
|
||||
package lockfile
|
||||
|
||||
import (
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestOpenLock(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
prepare func() (path string, readOnly bool)
|
||||
}{
|
||||
{
|
||||
name: "file exists (read/write)",
|
||||
prepare: func() (string, bool) {
|
||||
tempFile, err := ioutil.TempFile("", "lock-")
|
||||
require.NoError(t, err)
|
||||
return tempFile.Name(), false
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "file exists readonly (readonly)",
|
||||
prepare: func() (string, bool) {
|
||||
tempFile, err := ioutil.TempFile("", "lock-")
|
||||
require.NoError(t, err)
|
||||
return tempFile.Name(), true
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "base dir exists (read/write)",
|
||||
prepare: func() (string, bool) {
|
||||
tempDir := os.TempDir()
|
||||
require.DirExists(t, tempDir)
|
||||
return filepath.Join(tempDir, "test-1.lock"), false
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "base dir not exists (read/write)",
|
||||
prepare: func() (string, bool) {
|
||||
tempDir, err := ioutil.TempDir("", "lock-")
|
||||
require.NoError(t, err)
|
||||
return filepath.Join(tempDir, "subdir", "test-1.lock"), false
|
||||
},
|
||||
},
|
||||
} {
|
||||
path, readOnly := tc.prepare()
|
||||
|
||||
_, err := openLock(path, readOnly)
|
||||
|
||||
require.NoError(t, err, tc.name)
|
||||
|
||||
_, err = openLock(path, readOnly)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Nil(t, os.RemoveAll(path))
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue