Bug 1328074

Summary:	docker plugin - invalid 'docker logs' execution
Product:	Red Hat Enterprise Linux 7	Reporter:	Lukáš Zachar <lzachar>
Component:	sos	Assignee:	Pavel Moravec <pmoravec>
Status:	CLOSED CURRENTRELEASE	QA Contact:	BaseOS QE - Apps <qe-baseos-apps>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	7.2	CC:	agk, bmr, gavin, plambri, sbradley
Target Milestone:	beta
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
URL:	https://github.com/sosreport/sos/issues/813
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2016-11-11 08:16:27 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

Description Lukáš Zachar 2016-04-18 11:36:49 UTC

Description of problem:

docker plugin for sos runs 'docker logs', but this command requires container name as parameter. 
At least once it is executed without the parameter resulting with error message in the output.

Version-Release number of selected component (if applicable):
sos-3.2-35.el7_2.3.noarch

How reproducible:
always

Steps to Reproduce:
1. sos -o docker
2. see resulting sos_commands/docker/docker_logs

Actual results:
docker: "logs" requires 1 argument.
See 'docker logs --help'.

Usage:  docker logs [OPTIONS] CONTAINER

Fetch the logs of a container

Expected results:
valid logs or not run

Additional info:
# sosreport -o docker -k docker.all=true
prints logs for all available containers, but even then the empty/invalid one is present.

Comment 1 Pavel Moravec 2016-05-07 12:46:54 UTC

Thanks for spotting this redundant file.

Docker plugin in sos collects logs of running containers:

        ps_cmd = "{0} ps".format(self.docker_bin)
        if self.get_option('all'):
            ps_cmd = "{0} -a".format(ps_cmd)

        result = self.get_command_output(ps_cmd)
        if result['status'] == 0:
            result['output'] = result['output'].split("\n")
            for line in result['output'][1:]:
                container_id = line.split(" ")[0]
                self.add_cmd_output([
                    "{0} logs {1}".format(self.docker_bin, container_id)
                ])

I.e. it collects "docker ps" output and for each container running / present in the output, it collects "docker logs containerID". But the parsing of "docker ps" adds an empty line, such that "for line" cycle is executed for an empty line as well. Thus sosreport collects output of "docker logs " command.

So two possible patches can fix it:

diff --git a/sos/plugins/docker.py b/sos/plugins/docker.py
index aab558d..0a587d2 100644
--- a/sos/plugins/docker.py
+++ b/sos/plugins/docker.py
@@ -46,7 +46,7 @@ class Docker(Plugin):
 
         result = self.get_command_output(ps_cmd)
         if result['status'] == 0:
-            for line in result['output'].splitlines()[1:]:
+            for line in result['output'].splitlines()[1:-1]:
                 container_id = line.split(" ")[0]
                 self.add_cmd_output([
                     "{0} logs {1}".format(self.docker_bin, container_id)

(assumes the empty line is there every time - if not, the latest container wont have collected logs)

or

diff --git a/sos/plugins/docker.py b/sos/plugins/docker.py
index aab558d..c6e6262 100644
--- a/sos/plugins/docker.py
+++ b/sos/plugins/docker.py
@@ -47,10 +47,11 @@ class Docker(Plugin):
         result = self.get_command_output(ps_cmd)
         if result['status'] == 0:
             for line in result['output'].splitlines()[1:]:
-                container_id = line.split(" ")[0]
-                self.add_cmd_output([
-                    "{0} logs {1}".format(self.docker_bin, container_id)
-                ])
+                if lines:
+                    container_id = line.split(" ")[0]
+                    self.add_cmd_output([
+                        "{0} logs {1}".format(self.docker_bin, container_id)
+                    ])
 
 
 class RedHatDocker(Docker, RedHatPlugin):

Comment 2 Pavel Moravec 2016-05-25 14:13:52 UTC

Realized this is already fixed in upstream sos:

https://github.com/sosreport/sos/commit/ce6cd3ae9dc89dd1de5122add016ce2df60b6cee#diff-2f00a07bb807d7d02ad231c73dfd16e6

Comment 3 Pavel Moravec 2016-11-11 08:16:27 UTC

This has been fixed in upstream sos 3.3 we rebased to by [1] in RHEL7.3. Errata [2] should resolve this bug.

Please test [2] and in case it does not address the reported problem properly, reopen this BZ.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1293044
[2] https://rhn.redhat.com/errata/RHBA-2016-2380.html