Salt: `docker_image.present: -sls:` should return formatted output

[brianthelion]

Description of Issue/Question

docker_image.present: -sls: is great.... except when the executed state fails. Then you get a giant blob of raw salt output that's impossible to read, hence debug.

Ideally, docker_image.present: -sls: would return formatted output as if the docker image were a minion. That's sort of what's going on in the sense that salt-call is just getting run inside a container, right?

At a minimum, the highstate formatter should be applied and the formatted string should be pretty-printed on the terminal.

Setup

Run-able states available upon request, but this issue should be fairly self-explanatory.

Steps to Reproduce Issue

See previous.

Versions Report

```salt-call --versions-report
Salt Version:
Salt: 2017.7.0-37-gc91a5e5

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: Not Installed
docker-py: 2.4.2
gitdb: 2.0.2
gitpython: 2.1.5
ioflo: Not Installed
Jinja2: 2.9.6
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.8
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.5.3 (default, Jan 19 2017, 14:11:04)
python-gnupg: Not Installed
PyYAML: 3.12
PyZMQ: 16.0.2
RAET: Not Installed
smmap: 2.0.3
timelib: Not Installed
Tornado: 4.5.1
ZMQ: 4.1.6

System Versions:
dist: Ubuntu 17.04 zesty
locale: UTF-8
machine: x86_64
release: 4.12.0-041200-generic
system: Linux
version: Ubuntu 17.04 zesty
```

[gtmanfred]

This would be great to have. I am going to mark it as a bug.

If you had time to submit a PR, we would greatly appreciate it.

Thanks,
Daniel

[brianthelion]

@gtmanfred I will be happy to, but I don't have much visibility into how this kind of thing gets implemented. Are we thinking __outputter__ in dockermod.py? Maybe @terminalmage has some direction he could give me.

[gtmanfred]

You might just be able to set.

__outputters__ = {
    'sls': 'highstate',
    'sls_build': 'highstate',
}

That is where I would start.

[brianthelion]

@gtmanfred Unfortunately, that didn't work. Just to be a little clearer, here's an example of what I'm up against:

local:
----------
          ID: docker.io
    Function: pkg.installed
      Result: True
     Comment: Package docker.io is already installed
     Started: 10:25:43.158642
    Duration: 67.859 ms
     Changes:   
----------
          ID: docker
    Function: pip.installed
      Result: True
     Comment: Python package docker was already installed
              All packages were successfully installed
     Started: 10:25:43.635224
    Duration: 2908.955 ms
     Changes:   
----------
          ID: foo.docker.sls_build:docker_image_present
    Function: docker_image.present
        Name: foo:latest
      Result: False
     Comment: Encountered error using sls foo.paste_compose for building foo:latest: {'file_|-/app/src/foo.service.users/apt-requirements.txt_|-/app/src/foo.service.users/apt-requirements.txt_|-exists': {'comment': ... 

   **** 200 LINE JSON BLOB ****

...}}}
     Started: 10:25:46.545565
    Duration: 38615.621 ms
     Changes:   
----------
          ID: foo.docker.sls_build:docker_container_running
    Function: docker_container.running
      Result: False
     Comment: One or more requisite failed: foo.docker.sls_build.foo.docker.sls_build:docker_image_present
     Changes:   

Summary for local
------------
Succeeded: 2
Failed:    2
------------
Total states run:     4
Total run time:  41.592 s

Note the 200-line JSON blob.

Any more thoughts you could offer on an implementation path would be super helpful. Thanks!

[gtmanfred]

Unfortunately that is my best guess at how to make it work.

I am surprised it didn't work.

On Tue, Jul 25, 2017 at 8:35 AM, brianthelion [e-mail]
wrote:

@gtmanfred https://github.com/gtmanfred Unfortunately, that didn't
work. Just to be a little clearer, here's an example of what I'm up against:

local:

      ID: docker.io
Function: pkg.installed
  Result: True
 Comment: Package docker.io is already installed
 Started: 10:25:43.158642
Duration: 67.859 ms

Changes:

      ID: docker
Function: pip.installed
  Result: True
 Comment: Python package docker was already installed
          All packages were successfully installed
 Started: 10:25:43.635224
Duration: 2908.955 ms

Changes:

      ID: foo.docker.sls_build:docker_image_present
Function: docker_image.present
    Name: foo:latest
  Result: False
 Comment: Encountered error using sls foo.paste_compose for building foo:latest: {'file_|-/app/src/foo.service.users/apt-requirements.txt_|-/app/src/foo.service.users/apt-requirements.txt_|-exists': {'comment': ...

* 200 LINE JSON BLOB *

...}}}
Started: 10:25:46.545565
Duration: 38615.621 ms

Changes:

      ID: foo.docker.sls_build:docker_container_running
Function: docker_container.running
  Result: False
 Comment: One or more requisite failed: foo.docker.sls_build.foo.docker.sls_build:docker_image_present
 Changes:

Summary for local

Succeeded: 2

Failed: 2

Total states run: 4
Total run time: 41.592 s

Any more thoughts you could offer on an implementation path would be super
helpful. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/issues/42493#issuecomment-317757568,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAssoTblR70G1Dv55wjzw2kJuIC-g4kPks5sRf0jgaJpZM4OgnFM
.

[brianthelion]

@gtmanfred Just to be clear about what I did:

# salt/modules/dockermod.py
...
__outputter__ = {
    'sls': 'highstate',
    'sls_build': 'highstate',
    }
# EOF

The result is that the is that the "[DEBUG]" output pretty-prints appropriately -- a BIG step in the right direction -- but the final highstate output doesn't.

[brianthelion]

@gtmanfred I still haven't found a good answer to this issue, but the problem would appear to be in how the pseudo-minion is running salt-call in the prenatal container:

[INFO    ] Executing command 'docker exec 229db3e04b4d8521c61b3dc6bd94e05ca3cc77ce806e276862446efb980a3462 env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/opt/bin:/usr/local/bin:/usr/local/sbin  python /tmp/salt.docker.00be11/salt-call --metadata --local --log-file /tmp/salt.docker.00be11/log --cachedir /tmp/salt.docker.00be11/cache --out json -l quiet -- grains.items' in directory '/home/bar'

Note the --out json. I assume this is getting executed via subprocess.call or similar and that the calling process is just sitting on stdout?

[gtmanfred]

It uses the container_resource.run to run the command

https://github.com/saltstack/salt/blob/2017.7/salt/modules/dockermod.py#L4566

Which will either use the cmd module and one of the functions to run it, or
will use the VT to run it.

https://github.com/saltstack/salt/blob/2017.7/salt/modules/container_resource.py#L215

On Wed, Aug 2, 2017 at 10:13 AM, brianthelion [e-mail]
wrote:

@gtmanfred https://github.com/gtmanfred I still haven't found a good
answer to this issue, but the problem would appear to be in how the
pseudo-minion is running salt-call in the prenatal container:

[INFO ] Executing command 'docker exec 229db3e04b4d8521c61b3dc6bd94e05ca3cc77ce806e276862446efb980a3462 env -i PATH=/bin:/usr/bin:/sbin:/usr/sbin:/opt/bin:/usr/local/bin:/usr/local/sbin python /tmp/salt.docker.00be11/salt-call --metadata --local --log-file /tmp/salt.docker.00be11/log --cachedir /tmp/salt.docker.00be11/cache --out json -l quiet -- grains.items' in directory '/home/bar'

Note the --out json. I assume this is getting executed via subprocess.call
or similar and that the calling process is just sitting on stdout?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/issues/42493#issuecomment-319721394,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAssoRZ8nngAmEZHipgrt6bmysUDA1Xzks5sUKAvgaJpZM4OgnFM
.

[brianthelion]

@gtmanfred It looks like raw stderr output from container_resource.run is just getting set as the comment field:

    if ret['retcode'] != 0:
        return {'result': False, 'comment': ret['stderr']}

See https://github.com/saltstack/salt/blob/2017.7/salt/modules/dockermod.py#L5294

Is the right thing to do to just pick an outputter and run it on the ret['stderr'] before the return?

[gtmanfred]

If it works, i would say go for it, and then it can be hashed out in the
pull request.

Thanks!
Daniel

On Thu, Aug 3, 2017 at 4:59 PM, brianthelion [e-mail]
wrote:

@gtmanfred https://github.com/gtmanfred It looks like raw stderr output
from container_resource.run is just getting set as the comment field:

if ret['retcode'] != 0:
    return {'result': False, 'comment': ret['stderr']}

See https://github.com/saltstack/salt/blob/2017.7/salt/modules/
dockermod.py#L5294

Is the right thing to do to just pick an outputter and run it on the
ret['stderr'] before the return?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/issues/42493#issuecomment-320112048,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAssoW9pqwUegNwvDSS7PO2HxqW9qHF_ks5sUlC2gaJpZM4OgnFM
.

[terminalmage]

Yeah, the nested outputter is what should be used @brianthelion:

import salt.output

ret['comment'] = salt.output.display_output(ret['stderr'], 'nested', __opts__)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[gtmanfred]

@brianthelion did you ever open a PR to fix this?

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[brianthelion]

@gtmanfred Unfortunately I don't remember what the outcome was here. Someone took the devops end of the project over and I moved on to management. I may be picking up the salt piece here again in January and will check on this issue when I do.