From 2ae3a68cd9a49bf3127e2f16c8801255a9b68424 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Wed, 20 Oct 2021 20:20:07 -0400 Subject: [PATCH 1/3] Drop DockerInfo/DockerImages in favor of directly using methods Signed-off-by: Davanum Srinivas --- cmd/internal/pages/docker.go | 4 ++-- manager/manager.go | 14 -------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/cmd/internal/pages/docker.go b/cmd/internal/pages/docker.go index 6a11e01b49..83cbefc359 100644 --- a/cmd/internal/pages/docker.go +++ b/cmd/internal/pages/docker.go @@ -78,7 +78,7 @@ func serveDockerPage(m manager.Manager, w http.ResponseWriter, u *url.URL) { } // Get Docker status - status, err := m.DockerInfo() + status, err := docker.Status() if err != nil { http.Error(w, fmt.Sprintf("failed to get docker info: %v", err), http.StatusInternalServerError) return @@ -86,7 +86,7 @@ func serveDockerPage(m manager.Manager, w http.ResponseWriter, u *url.URL) { dockerStatus, driverStatus := toStatusKV(status) // Get Docker Images - images, err := m.DockerImages() + images, err := docker.Images() if err != nil { http.Error(w, fmt.Sprintf("failed to get docker images: %v", err), http.StatusInternalServerError) return diff --git a/manager/manager.go b/manager/manager.go index 2e977159e8..c2547ea576 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -134,12 +134,6 @@ type Manager interface { CloseEventChannel(watchID int) - // Get status information about docker. - DockerInfo() (info.DockerStatus, error) - - // Get details about interesting docker images. - DockerImages() ([]info.DockerImage, error) - // Returns debugging information. Map of lines per category. DebugInfo() map[string][]string } @@ -1335,14 +1329,6 @@ func parseEventsStoragePolicy() events.StoragePolicy { return policy } -func (m *manager) DockerImages() ([]info.DockerImage, error) { - return docker.Images() -} - -func (m *manager) DockerInfo() (info.DockerStatus, error) { - return docker.Status() -} - func (m *manager) DebugInfo() map[string][]string { debugInfo := container.DebugInfo() From 074c853617cefecece8f2b67dbcc34c56cad5a23 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Wed, 20 Oct 2021 20:38:04 -0400 Subject: [PATCH 2/3] Remove dependency of manager package on "container/docker" package Signed-off-by: Davanum Srinivas --- manager/manager.go | 20 ++++++-------------- manager/manager_test.go | 5 ++--- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/manager/manager.go b/manager/manager.go index c2547ea576..0bbad1679d 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -31,7 +31,6 @@ import ( "github.com/google/cadvisor/cache/memory" "github.com/google/cadvisor/collector" "github.com/google/cadvisor/container" - "github.com/google/cadvisor/container/docker" "github.com/google/cadvisor/container/raw" "github.com/google/cadvisor/events" "github.com/google/cadvisor/fs" @@ -61,6 +60,9 @@ var eventStorageAgeLimit = flag.String("event_storage_age_limit", "default=24h", var eventStorageEventLimit = flag.String("event_storage_event_limit", "default=100000", "Max number of events to store (per type). Value is a comma separated list of key values, where the keys are event types (e.g.: creation, oom) or \"default\" and the value is an integer. Default is applied to all non-specified event types") var applicationMetricsCountLimit = flag.Int("application_metrics_count_limit", 100, "Max number of application metrics to store (per container)") +// The namespace under which Docker aliases are unique. +const DockerNamespace = "docker" + var HousekeepingConfigFlags = HouskeepingConfig{ flag.Duration("max_housekeeping_interval", 60*time.Second, "Largest interval to allow between container housekeepings"), flag.Bool("allow_dynamic_housekeeping", true, "Whether to allow the housekeeping interval to be dynamic"), @@ -593,7 +595,7 @@ func (m *manager) getAllDockerContainers() map[string]*containerData { // Get containers in the Docker namespace. for name, cont := range m.containers { - if name.Namespace == docker.DockerNamespace { + if name.Namespace == DockerNamespace { containers[cont.info.Name] = cont } } @@ -625,14 +627,14 @@ func (m *manager) getDockerContainer(containerName string) (*containerData, erro // Check for the container in the Docker container namespace. cont, ok := m.containers[namespacedContainerName{ - Namespace: docker.DockerNamespace, + Namespace: DockerNamespace, Name: containerName, }] // Look for container by short prefix name if no exact match found. if !ok { for contName, c := range m.containers { - if contName.Namespace == docker.DockerNamespace && strings.HasPrefix(contName.Name, containerName) { + if contName.Namespace == DockerNamespace && strings.HasPrefix(contName.Name, containerName) { if cont == nil { cont = c } else { @@ -1385,20 +1387,10 @@ func getVersionInfo() (*info.VersionInfo, error) { kernelVersion := machine.KernelVersion() osVersion := machine.ContainerOsVersion() - dockerVersion, err := docker.VersionString() - if err != nil { - return nil, err - } - dockerAPIVersion, err := docker.APIVersionString() - if err != nil { - return nil, err - } return &info.VersionInfo{ KernelVersion: kernelVersion, ContainerOsVersion: osVersion, - DockerVersion: dockerVersion, - DockerAPIVersion: dockerAPIVersion, CadvisorVersion: version.Info["version"], CadvisorRevision: version.Info["revision"], }, nil diff --git a/manager/manager_test.go b/manager/manager_test.go index 1e9e62c23d..2eba374e11 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -26,7 +26,6 @@ import ( "github.com/google/cadvisor/cache/memory" "github.com/google/cadvisor/collector" "github.com/google/cadvisor/container" - "github.com/google/cadvisor/container/docker" containertest "github.com/google/cadvisor/container/testing" info "github.com/google/cadvisor/info/v1" itest "github.com/google/cadvisor/info/v1/test" @@ -76,7 +75,7 @@ func createManagerAndAddContainers( // Add Docker containers under their namespace. if strings.HasPrefix(name, "/docker") { mif.containers[namespacedContainerName{ - Namespace: docker.DockerNamespace, + Namespace: DockerNamespace, Name: strings.TrimPrefix(name, "/docker/"), }] = cont } @@ -139,7 +138,7 @@ func createManagerAndAddSubContainers( // Add Docker containers under their namespace. if strings.HasPrefix(name, "/docker") { mif.containers[namespacedContainerName{ - Namespace: docker.DockerNamespace, + Namespace: DockerNamespace, Name: strings.TrimPrefix(name, "/docker/"), }] = cont } From d37a7239cbde9f49796cc6b246eb4af2678d095c Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Wed, 20 Oct 2021 20:53:57 -0400 Subject: [PATCH 3/3] try switching test to some other attribute Signed-off-by: Davanum Srinivas --- integration/tests/api/attributes_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/tests/api/attributes_test.go b/integration/tests/api/attributes_test.go index 17eeeb4209..13e828cc72 100644 --- a/integration/tests/api/attributes_test.go +++ b/integration/tests/api/attributes_test.go @@ -32,6 +32,6 @@ func TestAttributeInformationIsReturned(t *testing.T) { } vp := `\d+\.\d+\.\d+` - assert.True(t, assert.Regexp(t, vp, attributes.DockerVersion), - "Expected %s to match %s", attributes.DockerVersion, vp) + assert.True(t, assert.Regexp(t, vp, attributes.KernelVersion), + "Expected %s to match %s", attributes.KernelVersion, vp) }