From 7ddc75c41bca82f60d3c071a182c5eccf0899e02 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Fri, 26 Sep 2014 18:06:58 -0700 Subject: [PATCH] Squashed commit of the following: commit 6bf9fe89f638aa1c5f29bd03168311e2097fc8ad Author: Satnam Singh Date: Fri Sep 26 10:23:16 2014 -0700 Change error to warning during handling check. commit c58090718309247ae3edd561a7e76d612acbd966 Author: Satnam Singh Date: Fri Sep 26 10:21:41 2014 -0700 Decapatalise fmt.Errorf error messages. commit 3ecc5745d60b49b0b9b2573684cd27f34364cf3e Author: Satnam Singh Date: Fri Sep 26 10:19:15 2014 -0700 Fix misunderstanding about when CanHandle fails. commit adce0c5433946613890c28eec68c8b7798a45633 Author: Satnam Singh Date: Fri Sep 26 10:13:32 2014 -0700 Change the interface of CanHandle to return error information. --- container/docker/factory.go | 21 ++++++++++----------- container/factory.go | 8 ++++++-- container/factory_test.go | 4 ++-- container/raw/factory.go | 4 ++-- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/container/docker/factory.go b/container/docker/factory.go index 6b7271e6..d3328eaa 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -63,20 +63,19 @@ func (self *dockerFactory) NewContainerHandler(name string) (handler container.C } // Docker handles all containers under /docker -// TODO(vishh): Change the CanHandle interface to be able to return errors. -func (self *dockerFactory) CanHandle(name string) bool { +func (self *dockerFactory) CanHandle(name string) (bool, error) { // In systemd systems the containers are: /system.slice/docker-{ID} if self.useSystemd { if !strings.HasPrefix(name, "/system.slice/docker-") { - return false + return false, nil } } else if name == "/" { - return false + return false, nil } else if name == "/docker" { // We need the docker driver to handle /docker. Otherwise the aggregation at the API level will break. - return true + return true, nil } else if !strings.HasPrefix(name, "/docker/") { - return false + return false, nil } // Check if the container is known to docker and it is active. id := path.Base(name) @@ -84,10 +83,10 @@ func (self *dockerFactory) CanHandle(name string) bool { // We assume that if Inspect fails then the container is not known to docker. // TODO(vishh): Detect lxc containers and avoid handling them. if err != nil || !ctnr.State.Running { - return false + return false, fmt.Errorf("error inspecting container: %v", err) } - return true + return true, nil } func parseDockerVersion(full_version_string string) ([]int, error) { @@ -95,14 +94,14 @@ func parseDockerVersion(full_version_string string) ([]int, error) { version_re := regexp.MustCompile(version_regexp_string) matches := version_re.FindAllStringSubmatch(full_version_string, -1) if len(matches) != 1 { - return nil, fmt.Errorf("Version string \"%v\" doesn't match expected regular expression: \"%v\"", full_version_string, version_regexp_string) + return nil, fmt.Errorf("version string \"%v\" doesn't match expected regular expression: \"%v\"", full_version_string, version_regexp_string) } version_string_array := matches[0][1:] version_array := make([]int, 3) for index, version_string := range version_string_array { version, err := strconv.Atoi(version_string) if err != nil { - return nil, fmt.Errorf("Error while parsing \"%v\" in \"%v\"", version_string, full_version_string) + return nil, fmt.Errorf("error while parsing \"%v\" in \"%v\"", version_string, full_version_string) } version_array[index] = version } @@ -122,7 +121,7 @@ func Register(factory info.MachineInfoFactory) error { version_string := version.Get("Version") version, err := parseDockerVersion(version_string) if err != nil { - return fmt.Errorf("Couldn't parse docker version: %v", err) + return fmt.Errorf("couldn't parse docker version: %v", err) } for index, number := range version { if number > expected_version[index] { diff --git a/container/factory.go b/container/factory.go index 5386f577..e49bca33 100644 --- a/container/factory.go +++ b/container/factory.go @@ -26,7 +26,7 @@ type ContainerHandlerFactory interface { NewContainerHandler(name string) (ContainerHandler, error) // Returns whether this factory can handle the specified container. - CanHandle(name string) bool + CanHandle(name string) (bool, error) // Name of the factory. String() string @@ -55,7 +55,11 @@ func NewContainerHandler(name string) (ContainerHandler, error) { // Create the ContainerHandler with the first factory that supports it. for _, factory := range factories { - if factory.CanHandle(name) { + canHandle, err := factory.CanHandle(name) + if err != nil { + glog.V(1).Infof("Error trying to work out if we can hande %s: %v", name, err) + } + if canHandle { glog.V(1).Infof("Using factory %q for container %q", factory.String(), name) return factory.NewContainerHandler(name) } diff --git a/container/factory_test.go b/container/factory_test.go index b96caab5..45b7945d 100644 --- a/container/factory_test.go +++ b/container/factory_test.go @@ -30,8 +30,8 @@ func (self *mockContainerHandlerFactory) String() string { return self.Name } -func (self *mockContainerHandlerFactory) CanHandle(name string) bool { - return self.CanHandleValue +func (self *mockContainerHandlerFactory) CanHandle(name string) (bool, error) { + return self.CanHandleValue, nil } func (self *mockContainerHandlerFactory) NewContainerHandler(name string) (ContainerHandler, error) { diff --git a/container/raw/factory.go b/container/raw/factory.go index e20f5638..569c62c7 100644 --- a/container/raw/factory.go +++ b/container/raw/factory.go @@ -46,8 +46,8 @@ func (self *rawFactory) NewContainerHandler(name string) (container.ContainerHan } // The raw factory can handle any container. -func (self *rawFactory) CanHandle(name string) bool { - return true +func (self *rawFactory) CanHandle(name string) (bool, error) { + return true, nil } func Register(machineInfoFactory info.MachineInfoFactory) error {