saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.21k stars 5.48k forks source link

On salt-ssh, cmd.script does not copy scripts to remote server / cp.cache_* broken on salt-ssh #48067

Open guptasakshi01 opened 6 years ago

guptasakshi01 commented 6 years ago

while executing script on target server using salt-ssh i am getting following error cmd:- sudo salt-ssh minion cmd.script source=salt://test.sh minion:

cache_error:
    True
pid:
    0
retcode:
    1
stderr:
stdout:

Setup

test.sh script is present at master on the /srv/salt/ We are executing command considering script will be executed remotely on target server(minion).

Versions Report

Salt Version: Salt: 2017.7.1

Dependency Versions: cffi: 1.11.5 cherrypy: 3.5.0 dateutil: 2.7.0 docker-py: Not Installed gitdb: 0.6.4 gitpython: 1.0.1 ioflo: Not Installed Jinja2: 2.8 libgit2: Not Installed libnacl: Not Installed M2Crypto: Not Installed Mako: 1.0.3 msgpack-pure: Not Installed msgpack-python: 0.4.6 mysql-python: 1.3.7 pycparser: 2.18 pycrypto: 2.6.1 pycryptodome: Not Installed pygit2: Not Installed Python: 2.7.12 (default, Dec 4 2017, 14:50:18) python-gnupg: Not Installed PyYAML: 3.11 PyZMQ: 15.2.0 RAET: Not Installed smmap: 0.9.0 timelib: Not Installed Tornado: 4.2.1 ZMQ: 4.1.4

System Versions: dist: Ubuntu 16.04 xenial locale: UTF-8 machine: x86_64 release: 4.4.0-112-generic system: Linux version: Ubuntu 16.04 xenial

gtmanfred commented 6 years ago

I am unable to replicate this behavior.

[root@salt ~]# cat /srv/salt/test.sh
#!/usr/bin/env bash
echo 'Hello, World!'
[root@salt ~]# salt-ssh -i \* cmd.script source=salt://test.sh
server:
    ----------
    pid:
        1744
    retcode:
        0
    stderr:
    stdout:
        Hello, World!
[root@salt ~]# salt-ssh --versions-report
Salt Version:
           Salt: 2017.7.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: Not Installed
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Apr 11 2018, 07:36:10)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: Not Installed
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 5.0.2
            ZMQ: Not Installed

System Versions:
           dist: centos 7.5.1804 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-862.3.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.5.1804 Core

It is working correctly for me. and copying the script over to the minion and then running the command.

sumeetisp commented 6 years ago

Minion based approach works but when i am using salt ssh there is no output.

minion:
    ----------
    cache_error:
        True
    pid:
        0
    retcode:
        1
    stderr:
    stdout:

Any way to debug when using salt ssh?

sumeetisp commented 6 years ago

The only different in output is cache error.

gtmanfred commented 6 years ago

I am unable to replicate this at all, can you provide me more information on your setup? any config changes you have made from the basic setup?

I am not able to replicate this, and it shouldn't be happening, so I don't know where the problem is for you.

It would also be worth while to share the -l debug output of the salt-ssh command.

Daniel

liucy1983 commented 6 years ago

i got the same problem. there is my debug info.

salt-ssh --no-host-keys --user=root --passwd=xxxx --roster=scan 10.0.10.200 cmd.script salt://5848f863e4b051f7b59fa4ce.sh -l debug [DEBUG ] Reading configuration from /etc/salt/master [DEBUG ] Including configuration from '/etc/salt/master.d/salt-api.conf' [DEBUG ] Reading configuration from /etc/salt/master.d/salt-api.conf [DEBUG ] Using cached minion ID from /etc/salt/minion_id: openstack [DEBUG ] Configuration file path: /etc/salt/master [WARNING ] Insecure logging configuration detected! Sensitive data may be logged. [DEBUG ] MasterEvent PUB socket URI: /var/run/salt/master/master_event_pub.ipc [DEBUG ] MasterEvent PULL socket URI: /var/run/salt/master/master_event_pull.ipc [DEBUG ] salt.utils.network.ip_to_host(u'10.0.10.200') failed: [Errno 1] Unknown host [DEBUG ] LazyLoaded scan.targets [DEBUG ] Matched minions: {u'10.0.10.200': {u'host': u'10.0.10.200', u'port': 22}} [DEBUG ] LazyLoaded roots.envs [DEBUG ] Could not LazyLoad roots.init: 'roots.init' is not available. [DEBUG ] Updating roots fileserver cache [DEBUG ] LazyLoaded local_cache.prep_jid [DEBUG ] Adding minions for job 20180706104235103147: [u'10.0.10.200'] [DEBUG ] Could not LazyLoad cmd.script: 'cmd.script' is not available. [DEBUG ] Performing shimmed, blocking command as follows: cmd.script salt://5848f863e4b051f7b59fa4ce.sh [DEBUG ] Executed SHIM command. Command logged to TRACE [DEBUG ] Child Forked! PID: 30898 STDOUT_FD: 13 STDERR_FD: 15 [DEBUG ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE [DEBUG ] RETCODE 10.0.10.200: 0 [DEBUG ] LazyLoaded nested.output 10.0.10.200:

cache_error:
    True
pid:
    0
retcode:
    1
stderr:
stdout:

[DEBUG ] Initializing new IPCClient for path: /var/run/salt/master/master_event_pull.ipc [DEBUG ] Sending event: tag = salt/job/20180706104235103147/ret/10.0.10.200; data = {u'fun_args': [u'salt://5848f863e4b051f7b59fa4ce.sh'], u'jid': u'20180706104235103147', u'return': {u'stderr': u'', u'pid': 0, u'retcode': 1, u'cache_error': True, u'stdout': u''}, u'retcode': 0, u'_stamp': '2018-07-06T02:42:38.132159', u'fun': u'cmd.script', u'id': u'10.0.10.200'}

Salt Version: Salt: 2018.3.1

Dependency Versions: cffi: 1.11.2 cherrypy: unknown dateutil: Not Installed docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed ioflo: Not Installed Jinja2: 2.7.2 libgit2: Not Installed libnacl: Not Installed M2Crypto: Not Installed Mako: Not Installed msgpack-pure: Not Installed msgpack-python: 0.4.6 mysql-python: Not Installed pycparser: 2.14 pycrypto: 2.6.1 pycryptodome: Not Installed pygit2: Not Installed Python: 2.7.5 (default, Aug 4 2017, 00:39:18) python-gnupg: Not Installed PyYAML: 3.10 PyZMQ: 14.7.0 RAET: Not Installed smmap: Not Installed timelib: Not Installed Tornado: 4.2.1 ZMQ: 4.0.5

System Versions: dist: centos 7.4.1708 Core locale: UTF-8 machine: x86_64 release: 3.10.0-693.2.2.el7.x86_64 system: Linux version: CentOS Linux 7.4.1708 Core

The-Loeki commented 5 years ago

Same problem 2019.2.0rc2

Related/probably duplicate: #11352

gtmanfred commented 5 years ago

@saltstack/team-triage can yall take a look at this?

The-Loeki commented 5 years ago

Single State cmd.script works fine btw (???)

bastion:~/.salt % sssh minion1 cmd.script salt://myscript.sh
minion1:
    ----------
    cache_error:
        True
    pid:
        0
    retcode:
        1
    stderr:
    stdout:
bastion:~/.salt % sssh minion1 state.single name=domyscript cmd.script source=salt://myscript.sh
minion1:
----------
          ID: domyscript 
    Function: cmd.script
      Result: True
     Comment: Command 'domyscript' run
     Started: 11:43:08.510654
    Duration: 7694.469 ms
     Changes:   
              ----------
              pid:
                  4653
              retcode:
                  0
              stderr:
                 <OUTPUT FOLLOWS>
waynew commented 5 years ago

I'll take a look at this today

waynew commented 5 years ago

Okay, yeah, I got the same behavior using salt-ssh mini -i cmd.script source=salt://test.sh -l debug. Using a couple of Docker images this my debug output:

[root@fa7d47a25c89 /]# salt-ssh mini -i cmd.script source=salt://test.sh -l debug
[DEBUG   ] Reading configuration from /etc/salt/master
[DEBUG   ] Configuration file path: /etc/salt/master
[WARNING ] Insecure logging configuration detected! Sensitive data may be logged.
[DEBUG   ] LazyLoaded flat.targets
[DEBUG   ] LazyLoaded jinja.render
[DEBUG   ] LazyLoaded yaml.render
[DEBUG   ] compile template: /etc/salt/roster
[DEBUG   ] Jinja search path: ['/var/cache/salt/master/files/base']
[PROFILE ] Time (in seconds) to render '/etc/salt/roster' using 'jinja' renderer: 0.00340509414673
[DEBUG   ] Rendered data from file: /etc/salt/roster:
# Sample salt-ssh config file
#web1:
#  host: 192.168.42.1 # The IP addr or DNS hostname
#  user: fred         # Remote executions will be executed as user fred
#  passwd: foobarbaz  # The password to use for login, if omitted, keys are used
#  sudo: True         # Whether to sudo to root, not enabled by default
#web2:
#  host: 192.168.42.2
mini:
  host: 172.17.0.5
  user: root
  passwd: test

[DEBUG   ] Results of YAML rendering:
OrderedDict([('mini', OrderedDict([('host', '172.17.0.5'), ('user', 'root'), ('passwd', 'test')]))])
[PROFILE ] Time (in seconds) to render '/etc/salt/roster' using 'yaml' renderer: 0.00223207473755
[DEBUG   ] Matched minions: {'mini': OrderedDict([('host', '172.17.0.5'), ('user', 'root'), ('passwd', 'test')])}
[DEBUG   ] LazyLoaded roots.envs
[DEBUG   ] Could not LazyLoad roots.init
[DEBUG   ] Updating roots fileserver cache
[DEBUG   ] LazyLoaded local_cache.prep_jid
[DEBUG   ] Adding minions for job 20190207204022735139: ['mini']
[DEBUG   ] Could not LazyLoad cmd.script
[DEBUG   ] Performing shimmed, blocking command as follows:
cmd.script source=salt://test.sh
[DEBUG   ] Executed SHIM command. Command logged to TRACE
[DEBUG   ] Child Forked! PID: 159  STDOUT_FD: 10  STDERR_FD: 12
[DEBUG   ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE
[DEBUG   ] RETCODE 172.17.0.5: 0
[DEBUG   ] LazyLoaded nested.output
mini:
    ----------
    cache_error:
        True
    pid:
        0
    retcode:
        1
    stderr:
    stdout:

I also tried salt-call cmd.script salt://test.sh using the minion'd source, which also failed. Working on tracking down the ultimate cause.

waynew commented 5 years ago

Interesting - I just added /srv/salt/test.sh to the minion and calling it worked... but with the contents of that script and not the one stored on the master. I suspect the cause is a lack of /srv/salt on the minion.

waynew commented 5 years ago

Huh. Nope - I was wrong, it looks like it's not passing the script to the minion...

waynew commented 5 years ago

Okay, I'm not quite sure exactly why, but I've managed to isolate the behavior. Using this Dockerfile for the minion:

FROM ubuntu:18.04

RUN apt-get update && apt-get install -y openssh-server
RUN mkdir /var/run/sshd
RUN echo 'root:test' | chpasswd
RUN sed -i 's/.*PermitRootLogin prohibit-password/PermitRootLogin yes/' /etc/ssh/sshd_config

# SSH login fix. Otherwise user is kicked off after login
RUN sed 's@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g' -i /etc/pam.d/sshd

ENV NOTVISIBLE "in users profile"
RUN echo "export VISIBLE=now" >> /etc/profile

RUN apt-get install -y python-minimal

EXPOSE 22
CMD ["/usr/sbin/sshd", "-D"]

And this Dockerfile for the master

FROM ubuntu:18.04 as testing
MAINTAINER Wayne Werner <wwerner@saltstack.com>

RUN echo "tzdata tzdata/Areas select Etc" >/tmp/fnord
RUN echo "tzdata tzdata/Zones/Etc select UTC" >>/tmp/fnord
RUN debconf-set-selections /tmp/fnord
RUN apt-get update && apt-get install -y tzdata
RUN echo "deb http://archive.ubuntu.com/ubuntu bionic main universe multiverse restricted" > /etc/apt/sources.list && \
echo "deb http://archive.ubuntu.com/ubuntu bionic-security main universe multiverse restricted" >> /etc/apt/sources.list && \
apt-get update && \
apt-get install -y git && \
apt-get upgrade -y -o DPkg::Options::=--force-confold

## Add the Salt PPA
RUN echo "deb http://repo.saltstack.com/apt/ubuntu/18.04/amd64/latest bionic main" > /etc/apt/sources.list.d/saltstack.list
RUN apt-get install -y -o DPkg::Options::=--force-confold  software-properties-common curl && \
curl https://repo.saltstack.com/apt/ubuntu/18.04/amd64/latest/SALTSTACK-GPG-KEY.pub | apt-key add - && \
apt-get update

## Install Salt Dependencies
RUN apt-get install -y -o DPkg::Options::=--force-confold \
python \
python-yaml \
python-m2crypto \
python-crypto \
msgpack-python \
python-zmq \
python-jinja2 \
python-requests \
salt-ssh

RUN echo "auto_accept: True" >> /etc/salt/master

# TODO will I need to futz with the master config?
#ADD default_minion /etc/salt/minion

FROM testing
## Install dev tools
RUN apt-get install -y ed vim-tiny python-pip
RUN pip install pudb

WORKDIR /srv/salt

Run the minion like so:

docker run -d -P --name test_sshd salt-sshd-ubuntu-minion:18.04   # or whatever you've tagged it

And the master:

docker run --rm -it --link test_sshd salt-ssh:ubuntu1804  # or whatever you've tagged it

Then create a script in /srv/salt/ like so:

#!/bin/sh
echo "This might work!"

And try to run it:

salt-ssh --user=root --passwd=test --roster=scan $TEST_SSHD_PORT_22_TCP_ADDR -i cmd.script salt://test.sh

This fails with a cache error. Sad. All is not lost, however!

salt-ssh --user=root --passwd=test --roster=scan $TEST_SSHD_PORT_22_TCP_ADDR -i cp.get_file salt://test.sh /srv/salt/ makedirs=True

Now re-run the cmd.script and it works!

From what I've been able to tell the problem is that the minion never asks the master for the file - I've got some tests I want to run about caching, but ultimately you can at least work around it by copying the file directly to /srv/salt/ on the minion (which seems kind of weird, considering that it puts the salt sources under /var/tmp from what I'm able to see. Especially odd if you do cp.cache_file - which only works after you've copied the file to /srv/salt/ on the minion.)

The-Loeki commented 5 years ago

ow gosh darnit @waynew I didn't have the time to deal with yet another bug in the salt-ssh minefield, but your enthousiasm has truly inspired me :1st_place_medal:

So here's where I'm starting. The thing that freaks me out is that the single state does work. States basically do some check&control juju before actually calling the modules themselves. And lo & behold: https://github.com/saltstack/salt/blob/2018.3/salt/states/cmd.py#L1152 Here's State cmd.script running module cmd.script just like I'm doing from the CLI. Except this time it does work.

You're quite right in that in one instance the file is copied over, and the other is not. That's because salt-ssh has some magic unicorn stuff I don't know about yet. But that's where I'm headed over looking ;)

The-Loeki commented 5 years ago

Especially odd if you do cp.cache_file - which only works after you've copied the file to /srv/salt/ on the minion.)

well whaddayaknow https://github.com/saltstack/salt/blob/2018.3/salt/modules/cmdmod.py#L2328

waynew commented 5 years ago

Okay, so you can run cp.cache_file but it just copies whatever is in /srv/salt, and doesn't appear to use it for anything.

To verify:

From what I'm led to understand - salt-ssh should be grabbing the file on run, zipping it up, and shipping it to the minion. For some reason this is not happening with cmd.script, only with cp.get_file.

The-Loeki commented 5 years ago

The Salt state prepackages itself into a state pkg, it takes an entirely different path and I've therefore ignored it for now.

The thing is, salt-ssh overloads the cp module here: https://github.com/saltstack/salt/blob/2018.3/salt/client/ssh/wrapper/cp.py

The real cp module and it's functions eventually end up in https://github.com/saltstack/salt/blob/2018.3/salt/fileclient.py#L189 et. al.

So my current hunch is __salt__['cp.get_file'] is the proper salt-ssh one, and this one is still the one from fileclient: https://github.com/saltstack/salt/blob/2018.3/salt/fileclient.py#L476

The-Loeki commented 5 years ago

:( I can think of 3 fixes:

The latter at first sight seems so obvious I wonder why this hasn't happened before; it looks like not every module-internal call to salt:// might go through cp, but must go through fileclient by definition.

Bad news is all are non-trivial fixes so again unfortunately beyond my current timetable :(

The-Loeki commented 5 years ago

thing that also has me stumped now is how @gtmanfred could not replicate the behaviour. Should've been exactly the same looking at the code from his versions report???

The-Loeki commented 5 years ago
[root@bassie ~]# salt-ssh -l debug -t 'bassie*' file.get_source_sum /tmp/bla source=salt://bastion/mgr.sh source_hash=salt://mgr.sum
bassie.ams2:
    ----------
    retcode:
        1
    stderr:
        Error running 'file.get_source_sum': Source hash file salt://mgr.sum not found
    stdout:

Uses cp.cache_file too for the checksum. I feel we should up the ante on this bug, it's pretty odd this one kept under the radar for so long ;) might solve a lot of subtle issues as cp.cache_file gets called around a lot.

Also @waynew, I see you can change the description right? I think the description should be changed to something along the lines of cp.cache_* broken on salt-ssh.

waynew commented 5 years ago

@The-Loeki That might be true. TBH, the ssh client in fileclient seemed like the most consistent approach (I started in on this on Friday as well, just trying to grok what was going on - I'm entirely new to salt-ssh). In some conversation with others I was led to understand that the way salt-ssh do is (as you pointed out) wraps get_cache and maybe uses that to zip up the file and passes it off to the client?

But I did notice that the client does try using its own fileclients - if it were possible to just have a ssh fileclient that can ask the master for the file over ssh, that would totally fix this issue (and probably many others in salt-ssh). I don't know what that takes to do, though.

The-Loeki commented 5 years ago

Well look at the fileclient.py file; it already starts with a single getter that picks names from a dict. What starts after is a Python object with some common stuff in it, and some subclasses that make it a remote or a local one.

Basically one would have to copy the stuff from the remote one and replace it's functions with most of the related fucntions in the overload-cp.py, fix & make everything nice to work, toss out the old cp.py overload, adjust the unit testing & docs.

Now that I'm writing all of this down it looks like a huge job, but it's manageable ;)

The-Loeki commented 5 years ago

Looking at that overloaded thing by the way there's definitely no prepackaging or something going on.

So for module calls the flow is direct, as opposed to executing states, which actually do prepackage themselves as archives, including calls & all (that's some true black magic if you ask me ;), haven't looked at that piece yet)

The-Loeki commented 5 years ago

huh

@terminalmage I'm still not saying I'm up for fixxing this, but can we bother you for some wisdom regarding a direction here?

The-Loeki commented 5 years ago

OK new idea: https://github.com/saltstack/salt/blob/2018.3/salt/modules/cp.py#L157 How 'bout we overload either _client() or _mk_client in there? Can we easily pass in a new type of fileclient that way?

The-Loeki commented 5 years ago
[ salt.git]$ [theloeki@murphy salt]$ grep -IrE 'cp\.[a-z]+_[a-z]+' salt/modules | sed -E 's|([^:]+):.*(cp\.[a-z]+_[a-z]+).*|\2 : \1|g' | sort -Vu | grep -Ev -e 'get_(file|dir|url)' -e 'cache_local' -e 'list_master' -e 'route*' -e 'has_*' -e 'is_cached'
cp.cache_dir : salt/modules/cp.py
cp.cache_dir : salt/modules/textfsm_mod.py
cp.cache_dir : salt/modules/win_pkg.py
cp.cache_files : salt/modules/cp.py
cp.cache_file : salt/modules/aptly.py
cp.cache_file : salt/modules/aptpkg.py
cp.cache_file : salt/modules/archive.py
cp.cache_file : salt/modules/cmdmod.py
cp.cache_file : salt/modules/container_resource.py
cp.cache_file : salt/modules/cp.py
cp.cache_file : salt/modules/debconfmod.py
cp.cache_file : salt/modules/dockermod.py
cp.cache_file : salt/modules/dpkg.py
cp.cache_file : salt/modules/file.py
cp.cache_file : salt/modules/jenkinsmod.py
cp.cache_file : salt/modules/k8s.py
cp.cache_file : salt/modules/kapacitor.py
cp.cache_file : salt/modules/kubernetes.py
cp.cache_file : salt/modules/lxc.py
cp.cache_file : salt/modules/mysql.py
cp.cache_file : salt/modules/nspawn.py
cp.cache_file : salt/modules/pip.py
cp.cache_file : salt/modules/pkg_resource.py
cp.cache_file : salt/modules/reg.py
cp.cache_file : salt/modules/rpm.py
cp.cache_file : salt/modules/selinux.py
cp.cache_file : salt/modules/solarispkg.py
cp.cache_file : salt/modules/ssh.py
cp.cache_file : salt/modules/textfsm_mod.py
cp.cache_file : salt/modules/virt.py
cp.cache_file : salt/modules/virtualenv_mod.py
cp.cache_file : salt/modules/win_certutil.py
cp.cache_file : salt/modules/win_pkg.py
cp.cache_file : salt/modules/win_pki.py
cp.cache_master : salt/modules/cp.py
cp.cache_master : salt/modules/saltcheck.py
cp.get_template : salt/modules/cmdmod.py
cp.get_template : salt/modules/cp.py
cp.get_template : salt/modules/debconfmod.py
cp.get_template : salt/modules/dockermod.py
cp.get_template : salt/modules/junos.py
cp.list_minion : salt/modules/cp.py
cp.list_states : salt/modules/cp.py
cp.push_dir : salt/modules/cp.py
cp.push_dir : salt/modules/openscap.py
cp.stat_file : salt/modules/cp.py
cp.stat_file : salt/modules/file.py
waynew commented 5 years ago

So these are all the modules that are (probably) affected by this issue? 😞

The-Loeki commented 5 years ago

Something like that yeah ;) I haven't looked through the list thoroughly though.

I've toyed around a bit and just PR'ed a WIP POC thingy. Is borken in all ways except get_file & cache_file by injecting a very slightly modified fileclient.

Edit: Should now behave the same or better as original + cache_* & template fixes

The-Loeki commented 5 years ago

Salt pplz seem however really busy with 2019.2 so might be a while before they respond?

The-Loeki commented 5 years ago

Related: #50525

The-Loeki commented 5 years ago

Related: #50196

The-Loeki commented 5 years ago

Related: #50351

max-arnold commented 5 years ago

Is #31531 also related?

waynew commented 5 years ago

@The-Loeki Did you have some code somewhere that I could look at to see if/how it solves this problem?

mchugh19 commented 5 years ago

For the issue mentioned in the title' the solution will likely require a wrapper to handle the file sync: https://docs.saltstack.com/en/develop/topics/development/modules/ssh_wrapper.html

While it would be nice to have cp.cachedir and the rest fully wrapped, those wrapper functions are not utilized by unwrapped functions. So just creating cp.cache* won't fix cmd.script, there will still need to be a cmd.py wrapper to setup the bits needed for the script function.

The-Loeki commented 5 years ago

@waynew negative, nothing more than what's already in PR #51636.

As @mchugh19 notes (see more discussion in the PR) the wrapper wraps incompletely, it wraps only explicitly wrapped functions (if you still follow).

The PoC I did in #51636 basically lowers the wrapping one level in the code in the wrapper itself. This makes it quite easy & elegant to 'wrap' functions, again, in the wrapper itself, solving a few cases and improving the wrappers quality.

The real problem however is that the wrapped functions aren't the only entrypoints; others call the same functions in such a way that the wrapper in it's current form will never be able to catch.

After digging around I figured that the method I used in #51636 would be easiest to port deeper into the salt code so the 'wrap' could possibly be completely forgone (as the SSH get's basically become a special case of Salt Fileserver gets). -BUT- of course that leads to the conundrum in https://github.com/saltstack/salt/issues/48067#issuecomment-462475250 I haven't had any feedback from the Salt people and so that's where we are on this ;)

stale[bot] commented 4 years ago

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.

max-arnold commented 4 years ago

Bump

stale[bot] commented 4 years ago

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

stale[bot] commented 4 years ago

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.

max-arnold commented 4 years ago

Bump

stale[bot] commented 4 years ago

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

xiaoxiaoming9 commented 3 years ago

[rss@server_web_10 tmp]$ salt-ssh --version salt-ssh 2017.7.0 (Nitrogen)

salt-ssh -l debug -i "192.128.100.77" cmd.run "mkdir -p /home/mycapitaltrade/lc"

[DEBUG ] LazyLoaded roots.envs [DEBUG ] Could not LazyLoad roots.init: 'roots.init' is not available. [DEBUG ] Updating roots fileserver cache [DEBUG ] LazyLoaded local_cache.prep_jid [DEBUG ] Adding minions for job 20210310194215111549: ['192.168.100.77'] [DEBUG ] Could not LazyLoad cmd.run: 'cmd.run' is not available. [DEBUG ] Performing shimmed, blocking command as follows: cmd.run mkdir -p /home/mycapitaltrade/lc [DEBUG ] Executed SHIM command. Command logged to TRACE [DEBUG ] Child Forked! PID: 24446 STDOUT_FD: 10 STDERR_FD: 12 [DEBUG ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE

Why does it get stuck for so long after the last step,Can someone help solve this,millions of thanks

The above frequency is an occasional occurrence,It leads to a big head

NeteaseWright commented 3 years ago

Same here when deploying salt ssh inside docker compose, with the following debug log:

[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] LazyLoaded roots.envs
[DEBUG   ] Could not LazyLoad roots.init: 'roots.init' is not available.
[DEBUG   ] Updating roots fileserver cache
[DEBUG   ] LazyLoaded local_cache.prep_jid
[DEBUG   ] Adding minions for job 20210511082242925098: ['m1', 'm2']
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] Override  __grains__: <module 'salt.loaded.int.wrapper.grains' from '/usr/local/lib/python3.9/site-packages/salt/client/ssh/wrapper/grains.py'>
[DEBUG   ] Override  __grains__: <module 'salt.loaded.int.wrapper.grains' from '/usr/local/lib/python3.9/site-packages/salt/client/ssh/wrapper/grains.py'>
[DEBUG   ] Could not LazyLoad cmd.script: 'cmd.script' is not available.
[DEBUG   ] Performing shimmed, blocking command as follows:
cmd.script salt://sb.sh
[DEBUG   ] Could not LazyLoad cmd.script: 'cmd.script' is not available.
[DEBUG   ] Performing shimmed, blocking command as follows:
cmd.script salt://sb.sh
[DEBUG   ] Executed SHIM command. Command logged to TRACE
[DEBUG   ] Executed SHIM command. Command logged to TRACE
[DEBUG   ] Child Forked! PID: 98  STDOUT_FD: 11  STDERR_FD: 13
[DEBUG   ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE
[DEBUG   ] Child Forked! PID: 99  STDOUT_FD: 13  STDERR_FD: 15
[DEBUG   ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE
[DEBUG   ] RETCODE salt-minion1: 0
[DEBUG   ] RETCODE salt-minion2: 0
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] LazyLoaded nested.output
m1:
    ----------
    cache_error:
        True
    pid:
        0
    retcode:
        1
    stderr:
    stdout:
[DEBUG   ] Using pkg_resources to load entry points
[DEBUG   ] LazyLoaded nested.output
m2:
    ----------
    cache_error:
        True
    pid:
        0
    retcode:
        1
    stderr:
    stdout: