Question: using salt-sproxy as a replacement for salt-ssh #179

dmacvicar · 2020-10-11T01:34:12Z

I have tried to replace my salt-ssh usage with salt-sproxy. This means a plain git repo, with a Saltfile like described here. I had mixed success.

Changing my Saltfile from:

salt-ssh:
  config_dir: etc/salt
  max_procs: 30
  wipe_ssh: True

to

salt-sproxy:
  config_dir: etc/salt
  max_procs: 30
  wipe_ssh: True

Added some paramers to etc/salt/master

diff --git a/etc/salt/master b/etc/salt/master
index 3395db0..b81a197 100644
--- a/etc/salt/master
+++ b/etc/salt/master
@@ -1,3 +1,7 @@
+roster: file
+sync_proxy: true
+sync_executors: true
+

For some reason ssh keys and user did not work, so I added root/password to etc/salt/roster

After applying a pillar ssh.sls to all machines with the content:

proxy:
  proxytype: ssh

I got it "working". But I could only call test.ping. Anything with arguments did not return anything. After reading salt-ssh code I noticed the client was constructed differently and the args/kwargs where passed in the client. Changing it to:

diff --git a/salt_sproxy/_proxy/ssh.py b/salt_sproxy/_proxy/ssh.py
index f729b21..9a44c2f 100644
--- a/salt_sproxy/_proxy/ssh.py
+++ b/salt_sproxy/_proxy/ssh.py
@@ -105,8 +105,18 @@ def _prep_conn(opts, fun, *args, **kwargs):
     fsclient = salt.fileclient.FSClient(opts)
     # TODO: Have here more options to simplify the usage, through features like
     # auto-expand the path to the priv key, auto-discovery, etc.
+    argv = [fun]
+    argv.extend([salt.utils.json.dumps(arg) for arg in args])
+    argv.extend(
+        [
+            "{0}={1}".format(
+                salt.utils.stringutils.to_str(key), salt.utils.json.dumps(val)
+            )
+            for key, val in six.iteritems(kwargs)
+        ]
+    )
     conn = salt.client.ssh.Single(
-        opts, [fun], opts['id'], fsclient=fsclient, **opts['proxy']
+        opts, argv, opts['id'], fsclient=fsclient, **opts['proxy']
     )
     conn.args = args
     conn.kwargs = kwargs

allowed me to call more functions. I did not succeed with state.apply but at least cmd.run is working with and without the patch. grain.items works but I swear it broke in some of the combinations I tested.

Without the patch I get:

[ERROR   ] [rpi02] Passed invalid arguments: run() missing 1 required positional argument: 'cmd'.

and the module documentation as output.

Note: I haven't tested running this from a proper master.

The text was updated successfully, but these errors were encountered:

mirceaulinic · 2020-10-14T13:46:23Z

Hi @dmacvicar. I've never used the Saltfile before, so first things first - let's clarify the state.apply issue, which I'm not able to reproduce. I have the following setup:

$ tree /srv/salt
/srv/salt/
|-- pillar/
|   |-- ssh.sls
|   `-- top.sls
|-- states/
    |-- files.sls
    |-- pkgs.sls
    |-- templates/
    |   `-- etc/
    |       `-- salt/
    |           `-- minion.jinja
    `-- top.sls

/srv/salt/pillar/top.sls

base:
  '*':
    - ssh

/srv/salt/pillar/ssh.sls

proxy:
  proxytype: ssh
  user: root
  priv: /home/mircea/.ssh/id_ed25519

/etc/salt/master

pillar_roots:
  base:
  - /srv/salt/pillar

file_roots:
  base:
    - /srv/salt/states

sync_proxy: true
sync_executors: true

roster: file
roster_file: /etc/salt/roster

/etc/salt/roster

srv:
  host: <IP address>

/srv/salt/states/top.sls

base:
  '*':
    - pkgs
    - files

/srv/salt/states/pkgs.sls

vim:
  pkg.installed

ack:
  pkg.installed

/srv/salt/states/pkgs.sls

/etc/salt/minion:
  file.managed:
    - source: salt://templates/etc/salt/minion.jinja
    - makedirs: true

With that, I can run without any issues:

$ salt-sproxy srv state.apply
srv:
----------
          ID: vim
    Function: pkg.installed
      Result: True
     Comment: All specified packages are already installed
     Started: 13:41:58.981253
    Duration: 39.485 ms
     Changes:   
----------
          ID: ack
    Function: pkg.installed
      Result: True
     Comment: All specified packages are already installed
     Started: 13:41:59.020987
    Duration: 24.168 ms
     Changes:   
----------
          ID: /etc/salt/minion
    Function: file.managed
      Result: True
     Comment: File /etc/salt/minion is in the correct state
     Started: 13:41:59.048389
    Duration: 25.753 ms
     Changes:   

Summary for srv
------------
Succeeded: 3
Failed:    0
------------
Total states run:     3
Total run time:  89.406 ms

Can you confirm you're seeing the same with this setup? Also, what does salt-sproxy -V shows?

dmacvicar · 2020-10-16T12:24:22Z

oh. I got it.

[DEBUG   ] Performing shimmed, blocking command as follows:
state.pkg /var/tmp/.root_a4e8a0_salt/salt_state.tgz test=True pkg_sum=43319bf17d4d87dadfdee87653880b0e37ea4b0246bb4f08d60e46493c13bc95 hash_type=sha256
[DEBUG   ] Executed SHIM command. Command logged to TRACE
[DEBUG   ] Child Forked! PID: 28653  STDOUT_FD: 13  STDERR_FD: 20
[DEBUG   ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE
[INFO    ] Terminating the process for rpi03, as it didn't reply within 60 seconds
[DEBUG   ] LazyLoaded highstate.output
rpi03:
    Minion did not return. [No response]
ERROR: Minions returned with non-zero exit code

I did not notice the timeout. -t 200 and it worked.

dmacvicar · 2020-10-16T12:48:59Z

Much of the time is spent resolving...

[DEBUG   ] Guessing ID. The id can be explicitly set in /etc/salt/minion
[DEBUG   ] Found minion id from generate_minion_id(): linux-1cur
[DEBUG   ] Grains refresh requested. Refreshing grains.
...
[DEBUG   ] Unable to resolve address 192.168.100.1: [Errno 0] Resolver Error 0 (no error)
[DEBUG   ] Unable to resolve address fe80::97f0:32a2:3e0a:8a91: [Errno 0] Resolver Error 0 (no error)
[DEBUG   ] Elapsed time getting FQDNs: 5.010853052139282 seconds

mirceaulinic · 2020-10-16T14:57:39Z

Elapsed time getting FQDNs: 5.010853052139282 seconds

Ugh, that's huge. In my testing it was much smaller (under 0.5s, but still not negligible). I seem to recall some issues with this a few years ago, there might be a way to disable collecting the FQDN grain.

mirceaulinic · 2020-10-16T15:07:14Z

Yep, seems to be this one: https://docs.saltstack.com/en/latest/ref/grains/all/salt.grains.core.html#salt.grains.core.fqdns. I put enable_fqdns_grains: false in the Master config, and it did pick it up, and prevented collecting the FQDN Grains. -t 200 seems excessive though, but guess it's somewhat normal particularly for state.highstate and sorts. Wondering if we should enforce (internally) to -t 0 (i.e., wait till the device replies) on state.* functions? 🤔

dmacvicar · 2020-10-16T15:34:30Z

I think it should keep as "salty" as possible to avoid surprises with defaults.

@mirceaulinic Everything seems to work now. Is my patch something that fixes anything for you? Should I do a PR?

mirceaulinic · 2020-10-23T10:53:14Z

I think this is fine, if everything works well. I'll leave this thread open however, to hear more thoughts on the following:

Wondering if we should enforce (internally) to -t 0 (i.e., wait till the device replies) on state.* functions?

I personally incline to do so (as the salt native command does this, IIRC), but at the same time agree on the surprising defaults point.

mirceaulinic · 2020-10-23T11:08:14Z

Actually, opened #182 for this. Thanks for the report @dmacvicar!

dmacvicar · 2020-10-23T13:33:32Z

@mirceaulinic It was not clear to me if I need to open a PR with the patch to add the args to the salt.client.ssh.Single constructor.
That patch was required to get the command to return anything, otherwise it would return the module exec documentation like if I had called the function with missing arguments.

mirceaulinic · 2020-10-23T13:44:57Z

Hey @dmacvicar - I'm not sure I'm following, I thought you said it was working fine? But feel free to open a PR regardless, cheers.

dmacvicar · 2020-10-23T14:12:28Z

@mirceaulinic In order to be able to execute any function with arguments, I need to apply this patch:

diff --git a/salt_sproxy/_proxy/ssh.py b/salt_sproxy/_proxy/ssh.py
index f729b21..9a44c2f 100644
--- a/salt_sproxy/_proxy/ssh.py
+++ b/salt_sproxy/_proxy/ssh.py
@@ -105,8 +105,18 @@ def _prep_conn(opts, fun, *args, **kwargs):
     fsclient = salt.fileclient.FSClient(opts)
     # TODO: Have here more options to simplify the usage, through features like
     # auto-expand the path to the priv key, auto-discovery, etc.
+    argv = [fun]
+    argv.extend([salt.utils.json.dumps(arg) for arg in args])
+    argv.extend(
+        [
+            "{0}={1}".format(
+                salt.utils.stringutils.to_str(key), salt.utils.json.dumps(val)
+            )
+            for key, val in six.iteritems(kwargs)
+        ]
+    )
     conn = salt.client.ssh.Single(
-        opts, [fun], opts['id'], fsclient=fsclient, **opts['proxy']
+        opts, argv, opts['id'], fsclient=fsclient, **opts['proxy']
     )
     conn.args = args
     conn.kwargs = kwargs

Now, I arrived to this change noticing how salt-ssh and salt-sproxy construct the object. I am not sure if it is correct or not, but the patch makes things work for me.

Without the patch, calling eg. cmd.run returns the documentation:

[DEBUG   ] VT: Salt-SSH SHIM Terminal Command executed. Logged to TRACE
[DEBUG   ] RETCODE rpi03: 1
[ERROR   ] [rpi03] Passed invalid arguments: run() missing 1 required positional argument: 'cmd'.
...
[DEBUG   ] LazyLoaded localfs.init_kwargs
[DEBUG   ] Caching Pillar for rpi03
[DEBUG   ] LazyLoaded localfs.init_kwargs
[DEBUG   ] LazyLoaded nested.output
rpi03:
    Execute the passed command and return the output as a string

        :param str cmd: The command to run. ex: ``ls -lart /home``

        :param str cwd: The directory from which to execute the command. Defaults
            to the home directory of the user specified by ``runas`` (or the user
            under which Salt is running if ``runas`` is not specified).

        :param str stdin: A string of standard input can be specified for the
            command to be run using the ``stdin`` parameter. This can be useful in
            cases where sensitive information must be read from standard input.

        :param str runas: Specify an alternate user to run the command. The default
            behavior is to run as the user under which Salt is running.

mirceaulinic · 2020-10-23T14:28:20Z

Sorry, I thought you said "everything seems to work now" in #179 (comment) ? Or you were referring to something else?

dmacvicar · 2020-10-23T14:58:17Z

Once I applied the patch, I got most calls to work, except state.apply, which turned to be a timeout.
With a proper timeout, got everything to work, but only if the patch is applied. The original PR description shows the sequence.

mirceaulinic · 2020-10-23T15:32:02Z

Hrm, interesting... in the above, with the config I shared, I tried with the current develop (without the patch you shared) - and it seems to work just fine. Could it be related to the use of the Saltfile?

mirceaulinic · 2020-10-23T15:42:06Z

Anyhow, yes, please open a PR, cheers. @dmacvicar

dmacvicar · 2020-10-25T16:28:56Z

PR #185

dmacvicar added the question Further information is requested label Oct 11, 2020

mirceaulinic closed this as completed Oct 23, 2020

mirceaulinic reopened this Oct 23, 2020

dmacvicar mentioned this issue Oct 25, 2020

Fix calling module functions with arguments when using a Saltfile #185

Merged

dmacvicar closed this as completed Oct 25, 2020

mirceaulinic added this to the 2020.10.0 milestone Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Question: using salt-sproxy as a replacement for salt-ssh #179

Question: using salt-sproxy as a replacement for salt-ssh #179

dmacvicar commented Oct 11, 2020

mirceaulinic commented Oct 14, 2020

dmacvicar commented Oct 16, 2020

dmacvicar commented Oct 16, 2020

mirceaulinic commented Oct 16, 2020

mirceaulinic commented Oct 16, 2020

dmacvicar commented Oct 16, 2020 •

edited

mirceaulinic commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020 •

edited

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 25, 2020

Question: using salt-sproxy as a replacement for salt-ssh #179

Question: using salt-sproxy as a replacement for salt-ssh #179

Comments

dmacvicar commented Oct 11, 2020

mirceaulinic commented Oct 14, 2020

dmacvicar commented Oct 16, 2020

dmacvicar commented Oct 16, 2020

mirceaulinic commented Oct 16, 2020

mirceaulinic commented Oct 16, 2020

dmacvicar commented Oct 16, 2020 • edited

mirceaulinic commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020 • edited

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

mirceaulinic commented Oct 23, 2020

dmacvicar commented Oct 25, 2020

dmacvicar commented Oct 16, 2020 •

edited

dmacvicar commented Oct 23, 2020 •

edited