Use mountpoint as image fs identifier.

Signed-off-by: Lantao Liu <lantaol@google.com>

cmd/cri-containerd/options/options.go

@@ -83,9 +83,6 @@ type PluginConfig struct {
 	StatsCollectPeriod int `toml:"stats_collect_period" json:"statsCollectPeriod,omitempty"`
 	// SystemdCgroup enables systemd cgroup support.
 	SystemdCgroup bool `toml:"systemd_cgroup" json:"systemdCgroup,omitempty"`
-	// SkipImageFSUUID skips retrieving imagefs uuid.
-	// TODO(random-liu): Remove this after we find a generic way to get imagefs uuid.
-	SkipImageFSUUID bool `toml:"skip_imagefs_uuid" json:"skipImageFSUUID,omitempty"`
 	// EnableIPv6DAD enables IPv6 DAD.
 	// TODO(random-liu): Use optimistic_dad when it's GA.
 	EnableIPv6DAD bool `toml:"enable_ipv6_dad" json:"enableIPv6DAD,omitempty"`
@@ -186,8 +183,6 @@ func (c *CRIContainerdOptions) AddFlags(fs *pflag.FlagSet) {
 		defaults.ProfilingPort, "Profiling port for web interface host:port/debug/pprof/.")
 	fs.StringVar(&c.ProfilingAddress, "profiling-addr",
 		defaults.ProfilingAddress, "Profiling address for web interface host:port/debug/pprof/.")
-	fs.BoolVar(&c.SkipImageFSUUID, "skip-imagefs-uuid",
-		defaults.SkipImageFSUUID, "Skip retrieval of imagefs uuid. When turned on, kubelet will not be able to get imagefs capacity or perform imagefs disk eviction.")
 	fs.BoolVar(&c.EnableIPv6DAD, "enable-ipv6-dad",
 		defaults.EnableIPv6DAD, "Enable IPv6 DAD (duplicate address detection) for pod sandbox network. Enabling this will increase pod sandbox start latency by several seconds.")
 	fs.Var(&c.Registry, "registry",
@@ -251,7 +246,6 @@ func DefaultConfig() Config {
 			SandboxImage:        "gcr.io/google_containers/pause:3.0",
 			StatsCollectPeriod:  10,
 			SystemdCgroup:       false,
-			SkipImageFSUUID:     false,
 			EnableIPv6DAD:       false,
 			Registry: Registry{
 				Mirrors: map[string]Mirror{

integration/container_stats_test.go

@@ -338,7 +338,7 @@ func testStats(t *testing.T,
 	require.NotEmpty(t, s.GetMemory().GetTimestamp())
 	require.NotEmpty(t, s.GetMemory().GetWorkingSetBytes().GetValue())
 	require.NotEmpty(t, s.GetWritableLayer().GetTimestamp())
-	require.NotEmpty(t, s.GetWritableLayer().GetStorageId().GetUuid())
+	require.NotEmpty(t, s.GetWritableLayer().GetFsId().GetMountpoint())
 	require.NotEmpty(t, s.GetWritableLayer().GetUsedBytes().GetValue())
 	require.NotEmpty(t, s.GetWritableLayer().GetInodesUsed().GetValue())
 

integration/imagefs_info_test.go

@@ -18,7 +18,7 @@ package integration
 
 import (
 	"fmt"
-	"io/ioutil"
+	"os"
 	"testing"
 	"time"
 
@@ -63,18 +63,13 @@ func TestImageFSInfo(t *testing.T) {
 		if info.GetTimestamp() != 0 &&
 			info.GetUsedBytes().GetValue() != 0 &&
 			info.GetInodesUsed().GetValue() != 0 &&
-			info.GetStorageId().GetUuid() != "" {
+			info.GetFsId().GetMountpoint() != "" {
 			return true, nil
 		}
 		return false, nil
 	}, time.Second, 30*time.Second))
 
-	t.Logf("Device uuid should exist")
+	t.Logf("Image filesystem mountpath should exist")
-	files, err := ioutil.ReadDir("/dev/disk/by-uuid")
+	_, err = os.Stat(info.GetFsId().GetMountpoint())
-	require.NoError(t, err)
+	assert.NoError(t, err)
-	var names []string
-	for _, f := range files {
-		names = append(names, f.Name())
-	}
-	assert.Contains(t, names, info.GetStorageId().GetUuid())
 }

pkg/os/os.go

@@ -17,12 +17,10 @@ limitations under the License.
 package os
 
 import (
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"syscall"
 
 	containerdmount "github.com/containerd/containerd/mount"
 	"github.com/containerd/fifo"
@@ -44,7 +42,6 @@ type OS interface {
 	Mount(source string, target string, fstype string, flags uintptr, data string) error
 	Unmount(target string, flags int) error
 	LookupMount(path string) (containerdmount.Info, error)
-	DeviceUUID(device uint64) (string, error)
 }
 
 // RealOS is used to dispatch the real system level operations.
@@ -124,39 +121,3 @@ func (RealOS) Unmount(target string, flags int) error {
 func (RealOS) LookupMount(path string) (containerdmount.Info, error) {
 	return containerdmount.Lookup(path)
 }
-
-// blkdev returns the rdev of a block device or an error if not a block device
-func blkrdev(device string) (uint64, error) {
-	info, err := os.Stat(device)
-	if err != nil {
-		return 0, err
-	}
-	stat := info.Sys().(*syscall.Stat_t)
-	if (stat.Mode & syscall.S_IFMT) != syscall.S_IFBLK {
-		return 0, fmt.Errorf("%s is not a block device", device)
-	}
-	return stat.Rdev, nil
-}
-
-// DeviceUUID gets device uuid of a device. The passed in rdev should be
-// linux device number.
-func (RealOS) DeviceUUID(rdev uint64) (string, error) {
-	const uuidDir = "/dev/disk/by-uuid"
-	files, err := ioutil.ReadDir(uuidDir)
-	if err != nil {
-		return "", err
-	}
-	for _, file := range files {
-		path := filepath.Join(uuidDir, file.Name())
-
-		trdev, err := blkrdev(path)
-		if err != nil {
-			continue
-		}
-
-		if rdev == trdev {
-			return file.Name(), nil
-		}
-	}
-	return "", fmt.Errorf("device %d not found", rdev)
-}

pkg/os/testing/fake_os.go

@@ -50,7 +50,6 @@ type FakeOS struct {
 	MountFn               func(source string, target string, fstype string, flags uintptr, data string) error
 	UnmountFn             func(target string, flags int) error
 	LookupMountFn         func(path string) (containerdmount.Info, error)
-	DeviceUUIDFn          func(device uint64) (string, error)
 	calls                 []CalledDetail
 	errors                map[string]error
 }
@@ -241,16 +240,3 @@ func (f *FakeOS) LookupMount(path string) (containerdmount.Info, error) {
 	}
 	return containerdmount.Info{}, nil
 }
-
-// DeviceUUID is a fake call that invodes DeviceUUIDFn or just return nil.
-func (f *FakeOS) DeviceUUID(device uint64) (string, error) {
-	f.appendCalls("DeviceUUID", device)
-	if err := f.getError("DeviceUUID"); err != nil {
-		return "", err
-	}
-
-	if f.DeviceUUIDFn != nil {
-		return f.DeviceUUIDFn(device)
-	}
-	return "", nil
-}

pkg/server/container_stats_list.go

@@ -83,8 +83,8 @@ func (c *criContainerdService) getContainerMetrics(
 	}
 	cs.WritableLayer = &runtime.FilesystemUsage{
 		Timestamp: sn.Timestamp,
-		StorageId: &runtime.StorageIdentifier{
+		FsId: &runtime.FilesystemIdentifier{
-			Uuid: c.imageFSUUID,
+			Mountpoint: c.imageFSPath,
 		},
 		UsedBytes:  &runtime.UInt64Value{Value: usedBytes},
 		InodesUsed: &runtime.UInt64Value{Value: inodesUsed},

pkg/server/imagefs_info.go

@@ -42,7 +42,7 @@ func (c *criContainerdService) ImageFsInfo(ctx context.Context, r *runtime.Image
 		ImageFilesystems: []*runtime.FilesystemUsage{
 			{
 				Timestamp:  timestamp,
-				StorageId:  &runtime.StorageIdentifier{Uuid: c.imageFSUUID},
+				FsId:       &runtime.FilesystemIdentifier{Mountpoint: c.imageFSPath},
 				UsedBytes:  &runtime.UInt64Value{Value: usedBytes},
 				InodesUsed: &runtime.UInt64Value{Value: inodesUsed},
 			},

pkg/server/imagefs_info_test.go

@@ -55,7 +55,7 @@ func TestImageFsInfo(t *testing.T) {
 	}
 	expected := &runtime.FilesystemUsage{
 		Timestamp:  123456,
-		StorageId:  &runtime.StorageIdentifier{Uuid: testImageFSUUID},
+		FsId:       &runtime.FilesystemIdentifier{Mountpoint: testImageFSPath},
 		UsedBytes:  &runtime.UInt64Value{Value: 30},
 		InodesUsed: &runtime.UInt64Value{Value: 300},
 	}

pkg/server/service.go

@@ -33,7 +33,6 @@ import (
 	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
-	"golang.org/x/sys/unix"
 	"google.golang.org/grpc"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 	"k8s.io/kubernetes/pkg/kubelet/server/streaming"
@@ -76,8 +75,8 @@ type CRIContainerdService interface {
 type criContainerdService struct {
 	// config contains all configurations.
 	config options.Config
-	// imageFSUUID is the device uuid of image filesystem.
+	// imageFSPath is the path to image filesystem.
-	imageFSUUID string
+	imageFSPath string
 	// apparmorEnabled indicates whether apparmor is enabled.
 	apparmorEnabled bool
 	// seccompEnabled indicates whether seccomp is enabled.
@@ -138,16 +137,8 @@ func NewCRIContainerdService(config options.Config) (CRIContainerdService, error
 		selinux.SetDisabled()
 	}
 
-	if !c.config.SkipImageFSUUID {
+	c.imageFSPath = imageFSPath(config.ContainerdRootDir, config.ContainerdConfig.Snapshotter)
-		imageFSPath := imageFSPath(config.ContainerdRootDir, config.ContainerdConfig.Snapshotter)
+	logrus.Infof("Get image filesystem path %q", c.imageFSPath)
-		c.imageFSUUID, err = c.getDeviceUUID(imageFSPath)
-		if err != nil {
-			return nil, fmt.Errorf("failed to get imagefs uuid of %q: %v", imageFSPath, err)
-		}
-		logrus.Infof("Get device uuid %q for image filesystem %q", c.imageFSUUID, imageFSPath)
-	} else {
-		logrus.Warn("Skip retrieving imagefs UUID, kubelet will not be able to get imagefs capacity or perform imagefs disk eviction.")
-	}
 
 	c.netPlugin, err = ocicni.InitCNI(config.NetworkPluginConfDir, config.NetworkPluginBinDir)
 	if err != nil {
@@ -293,16 +284,6 @@ func (c *criContainerdService) Close() error {
 	return nil
 }
 
-// getDeviceUUID gets device uuid for a given path.
-func (c *criContainerdService) getDeviceUUID(path string) (string, error) {
-	mount, err := c.os.LookupMount(path)
-	if err != nil {
-		return "", err
-	}
-	rdev := unix.Mkdev(uint32(mount.Major), uint32(mount.Minor))
-	return c.os.DeviceUUID(rdev)
-}
-
 // imageFSPath returns containerd image filesystem path.
 // Note that if containerd changes directory layout, we also needs to change this.
 func imageFSPath(rootDir, snapshotter string) string {

pkg/server/service_test.go

@@ -33,7 +33,7 @@ const (
 	// TODO(random-liu): Change this to image name after we have complete image
 	// management unit test framework.
 	testSandboxImage = "sha256:c75bebcdd211f41b3a460c7bf82970ed6c75acaab9cd4c9a4e125b03ca113798"
-	testImageFSUUID  = "test-image-fs-uuid"
+	testImageFSPath  = "/test/image/fs/path"
 )
 
 // newTestCRIContainerdService creates a fake criContainerdService for test.
@@ -45,7 +45,7 @@ func newTestCRIContainerdService() *criContainerdService {
 				SandboxImage: testSandboxImage,
 			},
 		},
-		imageFSUUID:        testImageFSUUID,
+		imageFSPath:        testImageFSPath,
 		os:                 ostesting.NewFakeOS(),
 		sandboxStore:       sandboxstore.NewStore(),
 		imageStore:         imagestore.NewStore(),