From 56054e3e31c250457dcae34bb6b4cfe0e8b08740 Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Sat, 2 Aug 2014 11:36:10 -0700 Subject: [PATCH] Remove SplitName(). This is possible thanks to the new libcontainer interface that allows the use of absolute paths. --- container/docker/factory.go | 7 +- container/docker/handler.go | 7 +- container/libcontainer/helpers.go | 50 -------------- container/libcontainer/helpers_test.go | 90 -------------------------- container/raw/handler.go | 8 +-- 5 files changed, 6 insertions(+), 156 deletions(-) delete mode 100644 container/libcontainer/helpers_test.go diff --git a/container/docker/factory.go b/container/docker/factory.go index 8382aec0..077826c3 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -18,6 +18,7 @@ import ( "flag" "fmt" "log" + "path" "regexp" "strconv" "strings" @@ -25,7 +26,6 @@ import ( "github.com/docker/libcontainer/cgroups/systemd" "github.com/fsouza/go-dockerclient" "github.com/google/cadvisor/container" - "github.com/google/cadvisor/container/libcontainer" "github.com/google/cadvisor/info" ) @@ -75,10 +75,7 @@ func (self *dockerFactory) CanHandle(name string) bool { return false } // Check if the container is known to docker and it is active. - _, id, err := libcontainer.SplitName(name) - if err != nil { - return false - } + id := path.Base(name) ctnr, err := self.client.InspectContainer(id) // We assume that if Inspect fails then the container is not known to docker. // TODO(vishh): Detect lxc containers and avoid handling them. diff --git a/container/docker/handler.go b/container/docker/handler.go index 040f74a7..a9d3fbd9 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -65,11 +65,8 @@ func newDockerContainerHandler( if handler.isDockerRoot() { return handler, nil } - parent, id, err := containerLibcontainer.SplitName(name) - if err != nil { - return nil, fmt.Errorf("invalid docker container %v: %v", name, err) - } - handler.parent = parent + id := path.Base(name) + handler.parent = path.Dir(name) handler.id = id ctnr, err := client.InspectContainer(id) // We assume that if Inspect fails then the container is not known to docker. diff --git a/container/libcontainer/helpers.go b/container/libcontainer/helpers.go index 525d5948..0b14565d 100644 --- a/container/libcontainer/helpers.go +++ b/container/libcontainer/helpers.go @@ -15,17 +15,12 @@ package libcontainer import ( - "bufio" - "path" - "path/filepath" - "strings" "time" "github.com/docker/libcontainer" "github.com/docker/libcontainer/cgroups" cgroupfs "github.com/docker/libcontainer/cgroups/fs" "github.com/google/cadvisor/info" - "github.com/google/cadvisor/utils/fs" ) // Get stats of the specified container @@ -88,48 +83,3 @@ func toContainerStats(libcontainerStats *libcontainer.ContainerStats) *info.Cont return ret } - -// Given a container name, returns the parent and name of the container to be fed to libcontainer. -func SplitName(containerName string) (string, string, error) { - parent, id := path.Split(containerName) - cgroupSelf, err := fs.Open("/proc/1/cgroup") - if err != nil { - return "", "", err - } - scanner := bufio.NewScanner(cgroupSelf) - - // Find how nested we are. Libcontainer takes container names relative to the current process. - subsys := []string{"memory", "cpu"} - nestedLevels := 0 - for scanner.Scan() { - line := scanner.Text() - elems := strings.Split(line, ":") - if len(elems) < 3 { - continue - } - for _, s := range subsys { - if elems[1] == s { - if elems[2] == "/" { - // We're running at root, no nesting. - nestedLevels = 0 - } else { - // Count how deeply nested we are. - nestedLevels = strings.Count(elems[2], "/") - } - break - } - } - } - if nestedLevels > 0 { - // we are running inside a docker container - upperLevel := strings.Repeat("../", nestedLevels) - parent = filepath.Join(upperLevel, parent) - } - - // Strip the last "/" - if parent[len(parent)-1] == '/' { - parent = parent[:len(parent)-1] - } - - return parent, id, nil -} diff --git a/container/libcontainer/helpers_test.go b/container/libcontainer/helpers_test.go deleted file mode 100644 index 36d6168b..00000000 --- a/container/libcontainer/helpers_test.go +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2014 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 libcontainer - -import ( - "testing" - - "code.google.com/p/gomock/gomock" - - "github.com/google/cadvisor/utils/fs" - "github.com/google/cadvisor/utils/fs/mockfs" -) - -var initCgroupsToParentAndID = []struct { - InitCgroupFileContent string - ContainerPath string - Parent string - Id string - Error error -}{ - { - ` -11:name=systemd:/ -10:hugetlb:/ -9:perf_event:/ -8:blkio:/ -7:freezer:/ -6:devices:/ -5:memory:/ -4:cpuacct:/ -3:cpu:/ -2:cpuset:/ -`, - "/", - "", - "", - nil, - }, - { - ` -11:name=systemd:/ -10:hugetlb:/ -9:perf_event:/ -8:blkio:/ -7:freezer:/ -6:devices:/ -5:memory:/docker/hash -4:cpuacct:/ -3:cpu:/docker/hash -2:cpuset:/ -`, - "/parent/id", - "../../parent", - "id", - nil, - }, -} - -func TestSplitName(t *testing.T) { - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - for _, testCase := range initCgroupsToParentAndID { - mfs := mockfs.NewMockFileSystem(mockCtrl) - mockfs.AddTextFile(mfs, "/proc/1/cgroup", testCase.InitCgroupFileContent) - fs.ChangeFileSystem(mfs) - parent, id, err := SplitName(testCase.ContainerPath) - if testCase.Error != nil { - if err == nil { - t.Fatalf("did not receive expected error.\ncontent:%v\n, path:%v\n, expected error:%v\n", testCase.InitCgroupFileContent, testCase.ContainerPath, testCase.Error) - } - continue - } - if testCase.Parent != parent || testCase.Id != id { - t.Errorf("unexpected parent or id:\ncontent:%v\npath:%v\nexpected parent: %v; recevied parent: %v;\nexpected id: %v; received id: %v", testCase.InitCgroupFileContent, testCase.ContainerPath, testCase.Parent, parent, testCase.Id, id) - } - } -} diff --git a/container/raw/handler.go b/container/raw/handler.go index ef5404e2..fc4fb69d 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -38,15 +38,11 @@ type rawContainerHandler struct { } func newRawContainerHandler(name string, cgroupSubsystems *cgroupSubsystems, machineInfoFactory info.MachineInfoFactory) (container.ContainerHandler, error) { - parent, id, err := libcontainer.SplitName(name) - if err != nil { - return nil, err - } return &rawContainerHandler{ name: name, cgroup: &cgroups.Cgroup{ - Parent: parent, - Name: id, + Parent: "/", + Name: name, }, cgroupSubsystems: cgroupSubsystems, machineInfoFactory: machineInfoFactory,