Merge pull request #1335 from dchen1107/release-v0.23

Release v0.23.4 for kubernetes 1.3

Author: dchen1107

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+### 0.23.4 (2016-06-16)
+- Cherry-pick release:
+  - Check for thin_is binary in path for devicemapper when using ThinPoolWatcher
+  - Fix uint64 overflow issue for CPU stats
+
 ### 0.23.3 (2016-06-08)
 - Cherry-pick release:
   - Cap the maximum consecutive du commands

container/docker/factory.go

@@ -197,24 +197,38 @@ func Register(factory info.MachineInfoFactory, fsInfo fs.FsInfo, ignoreMetrics c
 	)
 
 	if dockerStorageDriver == devicemapperStorageDriver {
-		// If the storage drive is devicemapper, create and start a
-		// ThinPoolWatcher to monitor the size of container CoW layers with
-		// thin_ls.
-		dockerThinPoolName, err := dockerutil.DockerThinPoolName(*dockerInfo)
-		if err != nil {
-			return fmt.Errorf("couldn't find device mapper thin pool name: %v", err)
-		}
-
-		dockerMetadataDevice, err := dockerutil.DockerMetadataDevice(*dockerInfo)
-		if err != nil {
-			return fmt.Errorf("couldn't determine devicemapper metadata device")
+		_, err := devicemapper.ThinLsBinaryPresent()
+		if err == nil {
+			// If the storage driver is devicemapper, create and start a
+			// ThinPoolWatcher to monitor the size of container CoW layers
+			// with thin_ls.
+			dockerThinPoolName, err := dockerutil.DockerThinPoolName(*dockerInfo)
+			if err != nil {
+				return fmt.Errorf("couldn't find device mapper thin pool name: %v", err)
+			}
+
+			dockerMetadataDevice, err := dockerutil.DockerMetadataDevice(*dockerInfo)
+			if err != nil {
+				return fmt.Errorf("couldn't determine devicemapper metadata device: %v", err)
+			}
+
+			thinPoolWatcher, err = devicemapper.NewThinPoolWatcher(dockerThinPoolName, dockerMetadataDevice)
+			if err != nil {
+				return fmt.Errorf("couldn't create thin pool watcher: %v", err)
+			}
+
+			go thinPoolWatcher.Start()
+		} else {
+			msg := []string{
+				"Couldn't locate thin_ls binary; not starting thin pool watcher.",
+				"Containers backed by thin pools will not show accurate usage.",
+				"err: %v",
+			}
+			glog.Errorf(strings.Join(msg, " "), err)
 		}
-
-		thinPoolWatcher = devicemapper.NewThinPoolWatcher(dockerThinPoolName, dockerMetadataDevice)
-		go thinPoolWatcher.Start()
 	}
 
-	glog.Infof("registering Docker factory")
+	glog.Infof("Registering Docker factory")
 	f := &dockerFactory{
 		cgroupSubsystems:   cgroupSubsystems,
 		client:             client,

devicemapper/dmsetup_client.go

@@ -21,19 +21,26 @@ import (
 	"github.com/golang/glog"
 )
 
-// DmsetupClient is a low-level client for interacting with devicemapper via
-// the dmsetup utility.
+// DmsetupClient is a low-level client for interacting with device mapper via
+// the `dmsetup` utility, which is provided by the `device-mapper` package.
 type DmsetupClient interface {
+	// Table runs `dmsetup table` on the given device name and returns the
+	// output or an error.
 	Table(deviceName string) ([]byte, error)
+	// Message runs `dmsetup message` on the given device, passing the given
+	// message to the given sector, and returns the output or an error.
 	Message(deviceName string, sector int, message string) ([]byte, error)
+	// Status runs `dmsetup status` on the given device and returns the output
+	// or an error.
 	Status(deviceName string) ([]byte, error)
 }
 
+// NewDmSetupClient returns a new DmsetupClient.
 func NewDmsetupClient() DmsetupClient {
 	return &defaultDmsetupClient{}
 }
 
-// defaultDmsetupClient implements the standard behavior for interacting with dmsetup.
+// defaultDmsetupClient is a functional DmsetupClient
 type defaultDmsetupClient struct{}
 
 var _ DmsetupClient = &defaultDmsetupClient{}

devicemapper/fake/dmsetup_client_fake.go

@@ -23,14 +23,16 @@ type DmsetupCommand struct {
 	Err    error
 }
 
+// NewFakeDmsetupClient returns a new fake DmsetupClient.
 func NewFakeDmsetupClient(t *testing.T, commands ...DmsetupCommand) *FakeDmsetupClient {
 	if len(commands) == 0 {
 		commands = make([]DmsetupCommand, 0)
 	}
 	return &FakeDmsetupClient{t: t, commands: commands}
 }
 
-// FakeDmsetupClient is a thread-unsafe fake implementation of the DmsetupClient interface
+// FakeDmsetupClient is a thread-unsafe fake implementation of the
+// DmsetupClient interface
 type FakeDmsetupClient struct {
 	t        *testing.T
 	commands []DmsetupCommand

devicemapper/fake/thin_ls_client_fake.go

@@ -18,6 +18,7 @@ type FakeThinLsClient struct {
 	err    error
 }
 
+// NewFakeThinLsClient returns a new fake ThinLsClient.
 func NewFakeThinLsClient(result map[string]uint64, err error) *FakeThinLsClient {
 	return &FakeThinLsClient{result, err}
 }

devicemapper/thin_ls_client.go

@@ -24,32 +24,47 @@ import (
 	"github.com/golang/glog"
 )
 
-// thinLsClient knows how to run a thin_ls very specific to CoW usage for containers.
+// thinLsClient knows how to run a thin_ls very specific to CoW usage for
+// containers.
 type thinLsClient interface {
+	// ThinLs runs a thin ls on the given device, which is expected to be a
+	// metadata device. The caller must hold the metadata snapshot for the
+	// device.
 	ThinLs(deviceName string) (map[string]uint64, error)
 }
 
-func newThinLsClient() thinLsClient {
-	return &defaultThinLsClient{}
+// newThinLsClient returns a thinLsClient or an error if the thin_ls binary
+// couldn't be located.
+func newThinLsClient() (thinLsClient, error) {
+	thinLsPath, err := ThinLsBinaryPresent()
+	if err != nil {
+		return nil, fmt.Errorf("error creating thin_ls client: %v", err)
+	}
+
+	return &defaultThinLsClient{thinLsPath}, nil
 }
 
-type defaultThinLsClient struct{}
+// defaultThinLsClient is a functional thinLsClient
+type defaultThinLsClient struct {
+	thinLsPath string
+}
 
 var _ thinLsClient = &defaultThinLsClient{}
 
-func (*defaultThinLsClient) ThinLs(deviceName string) (map[string]uint64, error) {
+func (c *defaultThinLsClient) ThinLs(deviceName string) (map[string]uint64, error) {
 	args := []string{"--no-headers", "-m", "-o", "DEV,EXCLUSIVE_BYTES", deviceName}
 	glog.V(4).Infof("running command: thin_ls %v", strings.Join(args, " "))
 
-	output, err := exec.Command("thin_ls", args...).Output()
+	output, err := exec.Command(c.thinLsPath, args...).Output()
 	if err != nil {
 		return nil, fmt.Errorf("Error running command `thin_ls %v`: %v\noutput:\n\n%v", strings.Join(args, " "), err, string(output))
 	}
 
 	return parseThinLsOutput(output), nil
 }
 
-// parseThinLsOutput parses the output returned by thin_ls to build a map of device id -> usage.
+// parseThinLsOutput parses the output returned by thin_ls to build a map of
+// device id -> usage.
 func parseThinLsOutput(output []byte) map[string]uint64 {
 	cache := map[string]uint64{}
 

devicemapper/thin_pool_watcher.go

@@ -22,7 +22,8 @@ import (
 	"github.com/golang/glog"
 )
 
-// ThinPoolWatcher maintains a cache of device name -> usage stats for a devicemapper thin-pool using thin_ls.
+// ThinPoolWatcher maintains a cache of device name -> usage stats for a
+// devicemapper thin-pool using thin_ls.
 type ThinPoolWatcher struct {
 	poolName       string
 	metadataDevice string
@@ -34,20 +35,26 @@ type ThinPoolWatcher struct {
 	thinLsClient   thinLsClient
 }
 
-// NewThinPoolWatcher returns a new ThinPoolWatcher for the given devicemapper thin pool name and metadata device.
-func NewThinPoolWatcher(poolName, metadataDevice string) *ThinPoolWatcher {
+// NewThinPoolWatcher returns a new ThinPoolWatcher for the given devicemapper
+// thin pool name and metadata device or an error.
+func NewThinPoolWatcher(poolName, metadataDevice string) (*ThinPoolWatcher, error) {
+	thinLsClient, err := newThinLsClient()
+	if err != nil {
+		return nil, fmt.Errorf("encountered error creating thin_ls client: %v", err)
+	}
+
 	return &ThinPoolWatcher{poolName: poolName,
 		metadataDevice: metadataDevice,
 		lock:           &sync.RWMutex{},
 		cache:          make(map[string]uint64),
 		period:         15 * time.Second,
 		stopChan:       make(chan struct{}),
 		dmsetup:        NewDmsetupClient(),
-		thinLsClient:   newThinLsClient(),
-	}
+		thinLsClient:   thinLsClient,
+	}, nil
 }
 
-// Start starts the thin pool watcher.
+// Start starts the ThinPoolWatcher.
 func (w *ThinPoolWatcher) Start() {
 	err := w.Refresh()
 	if err != nil {
@@ -72,6 +79,7 @@ func (w *ThinPoolWatcher) Start() {
 	}
 }
 
+// Stop stops the ThinPoolWatcher.
 func (w *ThinPoolWatcher) Stop() {
 	close(w.stopChan)
 }
@@ -80,6 +88,7 @@ func (w *ThinPoolWatcher) Stop() {
 func (w *ThinPoolWatcher) GetUsage(deviceId string) (uint64, error) {
 	w.lock.RLock()
 	defer w.lock.RUnlock()
+
 	v, ok := w.cache[deviceId]
 	if !ok {
 		return 0, fmt.Errorf("no cached value for usage of device %v", deviceId)
@@ -115,7 +124,8 @@ func (w *ThinPoolWatcher) Refresh() error {
 	}
 
 	glog.Infof("reserving metadata snapshot for thin-pool %v", w.poolName)
-	// NOTE: "0" in the call below is for the 'sector' argument to 'dmsetup message'.  It's not needed for thin pools.
+	// NOTE: "0" in the call below is for the 'sector' argument to 'dmsetup
+	// message'.  It's not needed for thin pools.
 	if output, err := w.dmsetup.Message(w.poolName, 0, reserveMetadataMessage); err != nil {
 		err = fmt.Errorf("error reserving metadata for thin-pool %v: %v output: %v", w.poolName, err, string(output))
 		return err
@@ -144,7 +154,8 @@ const (
 	thinPoolDmsetupStatusHeldMetadataRoot = 6
 )
 
-// checkReservation checks to see whether the thin device is currently holding userspace metadata.
+// checkReservation checks to see whether the thin device is currently holding
+// userspace metadata.
 func (w *ThinPoolWatcher) checkReservation(poolName string) (bool, error) {
 	glog.V(5).Infof("checking whether the thin-pool is holding a metadata snapshot")
 	output, err := w.dmsetup.Status(poolName)
@@ -153,7 +164,8 @@ func (w *ThinPoolWatcher) checkReservation(poolName string) (bool, error) {
 	}
 
 	tokens := strings.Split(string(output), " ")
-	// Split returns the input as the last item in the result, adjust the number of tokens by one
+	// Split returns the input as the last item in the result, adjust the
+	// number of tokens by one
 	if len(tokens) != thinPoolDmsetupStatusTokens+1 {
 		return false, fmt.Errorf("unexpected output of dmsetup status command; expected 11 fields, got %v; output: %v", len(tokens), string(output))
 	}

devicemapper/util.go

@@ -0,0 +1,60 @@
+// Copyright 2016 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package devicemapper
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// ThinLsBinaryPresent returns the location of the thin_ls binary in the mount
+// namespace cadvisor is running in or an error.  The locations checked are:
+//
+// - /bin/
+// - /usr/sbin/
+// - /usr/bin/
+//
+// ThinLsBinaryPresent checks these paths relative to:
+//
+// 1. For non-containerized operation - `/`
+// 2. For containerized operation - `/rootfs`
+//
+// The thin_ls binary is provided by the device-mapper-persistent-data
+// package.
+func ThinLsBinaryPresent() (string, error) {
+	var (
+		thinLsPath string
+		err        error
+	)
+
+	for _, path := range []string{"/bin", "/usr/sbin/", "/usr/bin"} {
+		// try paths for non-containerized operation
+		// note: thin_ls is most likely a symlink to pdata_tools
+		thinLsPath = filepath.Join(path, "thin_ls")
+		_, err = os.Stat(thinLsPath)
+		if err == nil {
+			return thinLsPath, nil
+		}
+
+		// try paths for containerized operation
+		thinLsPath = filepath.Join("/rootfs", thinLsPath)
+		_, err = os.Stat(thinLsPath)
+		if err == nil {
+			return thinLsPath, nil
+		}
+	}
+
+	return "", fmt.Errorf("unable to find thin_ls binary")
+}

info/v2/conversion.go

@@ -206,7 +206,8 @@ func InstCpuStats(last, cur *v1.ContainerStats) (*CpuInstStats, error) {
 			return 0, fmt.Errorf("cumulative stats decrease")
 		}
 		valueDelta := curValue - lastValue
-		return (valueDelta * 1e9) / timeDeltaNs, nil
+		// Use float64 to keep precision
+		return uint64(float64(valueDelta) / float64(timeDeltaNs) * 1e9), nil
 	}
 	total, err := convertToRate(last.Cpu.Usage.Total, cur.Cpu.Usage.Total)
 	if err != nil {
@@ -268,57 +269,3 @@ func ContainerSpecFromV1(specV1 *v1.ContainerSpec, aliases []string, namespace s
 	specV2.Namespace = namespace
 	return specV2
 }
-
-func instCpuStats(last, cur *v1.ContainerStats) (*CpuInstStats, error) {
-	if last == nil {
-		return nil, nil
-	}
-	if !cur.Timestamp.After(last.Timestamp) {
-		return nil, fmt.Errorf("container stats move backwards in time")
-	}
-	if len(last.Cpu.Usage.PerCpu) != len(cur.Cpu.Usage.PerCpu) {
-		return nil, fmt.Errorf("different number of cpus")
-	}
-	timeDelta := cur.Timestamp.Sub(last.Timestamp)
-	if timeDelta <= 100*time.Millisecond {
-		return nil, fmt.Errorf("time delta unexpectedly small")
-	}
-	// Nanoseconds to gain precision and avoid having zero seconds if the
-	// difference between the timestamps is just under a second
-	timeDeltaNs := uint64(timeDelta.Nanoseconds())
-	convertToRate := func(lastValue, curValue uint64) (uint64, error) {
-		if curValue < lastValue {
-			return 0, fmt.Errorf("cumulative stats decrease")
-		}
-		valueDelta := curValue - lastValue
-		return (valueDelta * 1e9) / timeDeltaNs, nil
-	}
-	total, err := convertToRate(last.Cpu.Usage.Total, cur.Cpu.Usage.Total)
-	if err != nil {
-		return nil, err
-	}
-	percpu := make([]uint64, len(last.Cpu.Usage.PerCpu))
-	for i := range percpu {
-		var err error
-		percpu[i], err = convertToRate(last.Cpu.Usage.PerCpu[i], cur.Cpu.Usage.PerCpu[i])
-		if err != nil {
-			return nil, err
-		}
-	}
-	user, err := convertToRate(last.Cpu.Usage.User, cur.Cpu.Usage.User)
-	if err != nil {
-		return nil, err
-	}
-	system, err := convertToRate(last.Cpu.Usage.System, cur.Cpu.Usage.System)
-	if err != nil {
-		return nil, err
-	}
-	return &CpuInstStats{
-		Usage: CpuInstUsage{
-			Total:  total,
-			PerCpu: percpu,
-			User:   user,
-			System: system,
-		},
-	}, nil
-}