Revert "Integrate CRI and containerd file system stats "

Author: bobbypage

Makefile

@@ -91,8 +91,6 @@ docker-build:
 presubmit: vet
 	@echo ">> checking go formatting"
 	@./build/check_gofmt.sh
-	@echo ">> checking CRI definitions"
-	@./build/check_cri_defs.sh
 	@echo ">> checking go mod tidy"
 	@./build/check_gotidy.sh
 	@echo ">> checking file boilerplate"

build/boilerplate/boilerplate.py

@@ -95,7 +95,7 @@ def file_passes(filename, refs, regexs):
 def file_extension(filename):
     return os.path.splitext(filename)[1].split(".")[-1].lower()
 
-skipped_dirs = ['Godeps', 'vendor', 'third_party', '_gopath', '_output', '.git', 'cri-api']
+skipped_dirs = ['Godeps', 'vendor', 'third_party', '_gopath', '_output', '.git']
 def normalize_files(files):
     newfiles = []
     for pathname in files:

build/check_cri_defs.sh

@@ -21,7 +21,7 @@ require (
 	github.com/mesos/mesos-go v0.0.7-0.20180413204204-29de6ff97b48
 	github.com/pquerna/ffjson v0.0.0-20171002144729-d49c2bc1aa13 // indirect
 	github.com/prometheus/client_golang v1.8.0
-	github.com/stretchr/testify v1.7.0
+	github.com/stretchr/testify v1.6.1
 	golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43
 	google.golang.org/api v0.34.0
 	gopkg.in/olivere/elastic.v2 v2.0.12

build/update_cri_defs.sh

@@ -454,9 +454,8 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
-github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 h1:kdXcSzyDtseVEc4yCz2qF8ZrQvIDBJLl4S1c3GCXmoI=
 github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=

cmd/go.mod

@@ -37,21 +37,15 @@ type FsUsage struct {
 	InodeUsage      uint64
 }
 
-type FsUsageProvider interface {
-	// Usage returns the fs usage
-	Usage() (*FsUsage, error)
-	// Targets returns where the fs usage metric is collected,it maybe a directory, a file or some
-	// information about the snapshotter(for containerd)
-	Targets() []string
-}
-
 type realFsHandler struct {
 	sync.RWMutex
-	lastUpdate    time.Time
-	usage         FsUsage
-	period        time.Duration
-	minPeriod     time.Duration
-	usageProvider FsUsageProvider
+	lastUpdate time.Time
+	usage      FsUsage
+	period     time.Duration
+	minPeriod  time.Duration
+	rootfs     string
+	extraDir   string
+	fsInfo     fs.FsInfo
 	// Tells the container to stop.
 	stopChan chan struct{}
 }
@@ -64,33 +58,56 @@ const DefaultPeriod = time.Minute
 
 var _ FsHandler = &realFsHandler{}
 
-func NewFsHandler(period time.Duration, provider FsUsageProvider) FsHandler {
+func NewFsHandler(period time.Duration, rootfs, extraDir string, fsInfo fs.FsInfo) FsHandler {
 	return &realFsHandler{
-		lastUpdate:    time.Time{},
-		usage:         FsUsage{},
-		period:        period,
-		minPeriod:     period,
-		usageProvider: provider,
-		stopChan:      make(chan struct{}, 1),
+		lastUpdate: time.Time{},
+		usage:      FsUsage{},
+		period:     period,
+		minPeriod:  period,
+		rootfs:     rootfs,
+		extraDir:   extraDir,
+		fsInfo:     fsInfo,
+		stopChan:   make(chan struct{}, 1),
 	}
 }
 
 func (fh *realFsHandler) update() error {
+	var (
+		rootUsage, extraUsage fs.UsageInfo
+		rootErr, extraErr     error
+	)
+	// TODO(vishh): Add support for external mounts.
+	if fh.rootfs != "" {
+		rootUsage, rootErr = fh.fsInfo.GetDirUsage(fh.rootfs)
+	}
 
-	usage, err := fh.usageProvider.Usage()
-
-	if err != nil {
-		return err
+	if fh.extraDir != "" {
+		extraUsage, extraErr = fh.fsInfo.GetDirUsage(fh.extraDir)
 	}
 
+	// Wait to handle errors until after all operartions are run.
+	// An error in one will not cause an early return, skipping others
 	fh.Lock()
 	defer fh.Unlock()
 	fh.lastUpdate = time.Now()
+	if fh.rootfs != "" && rootErr == nil {
+		fh.usage.InodeUsage = rootUsage.Inodes
+		fh.usage.BaseUsageBytes = rootUsage.Bytes
+		fh.usage.TotalUsageBytes = rootUsage.Bytes
+	}
+	if fh.extraDir != "" && extraErr == nil {
+		if fh.rootfs != "" {
+			fh.usage.TotalUsageBytes += extraUsage.Bytes
+		} else {
+			// rootfs is empty, totalUsageBytes use extra usage bytes
+			fh.usage.TotalUsageBytes = extraUsage.Bytes
+		}
+	}
 
-	fh.usage.InodeUsage = usage.InodeUsage
-	fh.usage.BaseUsageBytes = usage.BaseUsageBytes
-	fh.usage.TotalUsageBytes = usage.TotalUsageBytes
-
+	// Combine errors into a single error to return
+	if rootErr != nil || extraErr != nil {
+		return fmt.Errorf("rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)
+	}
 	return nil
 }
 
@@ -113,8 +130,7 @@ func (fh *realFsHandler) trackUsage() {
 			// if the long duration is persistent either because of slow
 			// disk or lots of containers.
 			longOp = longOp + time.Second
-			klog.V(2).Infof(`fs: disk usage and inodes count on targets took %v: %v; `+
-				`will not log again for this container unless duration exceeds %v`, duration, fh.usageProvider.Targets(), longOp)
+			klog.V(2).Infof("fs: disk usage and inodes count on following dirs took %v: %v; will not log again for this container unless duration exceeds %v", duration, []string{fh.rootfs, fh.extraDir}, longOp)
 		}
 		select {
 		case <-fh.stopChan:
@@ -137,56 +153,3 @@ func (fh *realFsHandler) Usage() FsUsage {
 	defer fh.RUnlock()
 	return fh.usage
 }
-
-type fsUsageProvider struct {
-	fsInfo fs.FsInfo
-	rootFs string
-	// The directory consumed by the container but outside rootFs, e.g. directory of saving logs
-	extraDir string
-}
-
-func NewGeneralFsUsageProvider(fsInfo fs.FsInfo, rootFs, extraDir string) FsUsageProvider {
-	return &fsUsageProvider{
-		fsInfo:   fsInfo,
-		rootFs:   rootFs,
-		extraDir: extraDir,
-	}
-}
-
-func (f *fsUsageProvider) Targets() []string {
-	return []string{f.rootFs, f.extraDir}
-}
-
-func (f *fsUsageProvider) Usage() (*FsUsage, error) {
-	var (
-		rootUsage, extraUsage fs.UsageInfo
-		rootErr, extraErr     error
-	)
-
-	if f.rootFs != "" {
-		rootUsage, rootErr = f.fsInfo.GetDirUsage(f.rootFs)
-	}
-
-	if f.extraDir != "" {
-		extraUsage, extraErr = f.fsInfo.GetDirUsage(f.extraDir)
-	}
-
-	usage := &FsUsage{}
-
-	if f.rootFs != "" && rootErr == nil {
-		usage.InodeUsage = rootUsage.Inodes
-		usage.BaseUsageBytes = rootUsage.Bytes
-		usage.TotalUsageBytes = rootUsage.Bytes
-	}
-
-	if f.extraDir != "" && extraErr == nil {
-		usage.TotalUsageBytes += extraUsage.Bytes
-	}
-
-	// Combine errors into a single error to return
-	if rootErr != nil || extraErr != nil {
-		return nil, fmt.Errorf("failed to obtain filesystem usage; rootDiskErr: %v, extraDiskErr: %v", rootErr, extraErr)
-	}
-
-	return usage, nil
-}

cmd/go.sum

@@ -22,15 +22,12 @@ import (
 	"time"
 
 	containersapi "github.com/containerd/containerd/api/services/containers/v1"
-	snapshotapi "github.com/containerd/containerd/api/services/snapshots/v1"
 	tasksapi "github.com/containerd/containerd/api/services/tasks/v1"
 	versionapi "github.com/containerd/containerd/api/services/version/v1"
-	types "github.com/containerd/containerd/api/types"
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/pkg/dialer"
 	ptypes "github.com/gogo/protobuf/types"
-	criapi "github.com/google/cadvisor/cri-api/pkg/apis/runtime/v1alpha2"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/backoff"
 )
@@ -39,17 +36,12 @@ type client struct {
 	containerService containersapi.ContainersClient
 	taskService      tasksapi.TasksClient
 	versionService   versionapi.VersionClient
-	snapshotService  snapshotapi.SnapshotsClient
-	criService       criapi.RuntimeServiceClient
 }
 
 type ContainerdClient interface {
 	LoadContainer(ctx context.Context, id string) (*containers.Container, error)
 	TaskPid(ctx context.Context, id string) (uint32, error)
 	Version(ctx context.Context) (string, error)
-	SnapshotMounts(ctx context.Context, snapshotter, key string) ([]*types.Mount, error)
-	ContainerStatus(ctx context.Context, id string) (*criapi.ContainerStatus, error)
-	ContainerStats(ctx context.Context, id string) (*criapi.ContainerStats, error)
 }
 
 var once sync.Once
@@ -100,8 +92,6 @@ func Client(address, namespace string) (ContainerdClient, error) {
 			containerService: containersapi.NewContainersClient(conn),
 			taskService:      tasksapi.NewTasksClient(conn),
 			versionService:   versionapi.NewVersionClient(conn),
-			snapshotService:  snapshotapi.NewSnapshotsClient(conn),
-			criService:       criapi.NewRuntimeServiceClient(conn),
 		}
 	})
 	return ctrdClient, retErr
@@ -135,38 +125,6 @@ func (c *client) Version(ctx context.Context) (string, error) {
 	return response.Version, nil
 }
 
-func (c *client) SnapshotMounts(ctx context.Context, snapshotter, key string) ([]*types.Mount, error) {
-	response, err := c.snapshotService.Mounts(ctx, &snapshotapi.MountsRequest{
-		Snapshotter: snapshotter,
-		Key:         key,
-	})
-	if err != nil {
-		return nil, errdefs.FromGRPC(err)
-	}
-	return response.Mounts, nil
-}
-
-func (c *client) ContainerStatus(ctx context.Context, id string) (*criapi.ContainerStatus, error) {
-	response, err := c.criService.ContainerStatus(ctx, &criapi.ContainerStatusRequest{
-		ContainerId: id,
-		Verbose:     false,
-	})
-	if err != nil {
-		return nil, err
-	}
-	return response.Status, nil
-}
-
-func (c *client) ContainerStats(ctx context.Context, id string) (*criapi.ContainerStats, error) {
-	response, err := c.criService.ContainerStats(ctx, &criapi.ContainerStatsRequest{
-		ContainerId: id,
-	})
-	if err != nil {
-		return nil, err
-	}
-	return response.Stats, nil
-}
-
 func containerFromProto(containerpb containersapi.Container) *containers.Container {
 	var runtime containers.RuntimeInfo
 	if containerpb.Runtime != nil {