saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.15k stars 5.47k forks source link

__virtualname__ from salt.module.* also gets applied to runners! #34347

Closed sjorge closed 8 years ago

sjorge commented 8 years ago

Description of Issue/Question

salt.modules.vmadm (smartos_vmadm.py) has a virtualname of vmadm. However I am trying to write a runner (vmadm.py) but it fails to load.

The loader looks for smartos_vmadm.py.

[root@cronos /salt/config/.dynmod/runners]# ls -l
total 1
-rw-r--r-- 1 sjorge user 371 Jun 28 22:15 vmadm.py
[root@cronos /salt/config/.dynmod/runners]# salt-run vmadm.list
Exception occurred in runner vmadm.list: Traceback (most recent call last):
  File "/opt/salt/lib/python2.7/site-packages/salt/client/mixins.py", line 297, in low
    for mod_name in six.iterkeys(self.functions):
  File "/opt/salt/lib/python2.7/site-packages/salt/ext/six.py", line 577, in iterkeys
    return iter(d.iterkeys(**kw))
  File "/opt/salt/lib/python2.7/_abcoll.py", line 396, in iterkeys
    return iter(self)
  File "/opt/salt/lib/python2.7/site-packages/salt/utils/lazy.py", line 105, in __iter__
    self._load_all()
  File "/opt/salt/lib/python2.7/site-packages/salt/loader.py", line 1485, in _load_all
    self._load_module(name)
  File "/opt/salt/lib/python2.7/site-packages/salt/loader.py", line 1289, in _load_module
    with salt.utils.fopen(fpath, desc[1]) as fn_:
  File "/opt/salt/lib/python2.7/site-packages/salt/utils/__init__.py", line 1209, in fopen
    fhandle = open(*args, **kwargs)
IOError: [Errno 2] No such file or directory: '/opt/salt/lib/python2.7/site-packages/salt/runners/smartos_vmadm.py'
[root@cronos /salt/config/.dynmod/runners]# ls -l
total 3
-rw-r--r-- 1 sjorge user 371 Jun 28 22:15 smartos_vmadm.py
-rw-r--r-- 1 root   root 668 Jun 28 22:23 smartos_vmadm.pyc
[root@cronos /salt/config/.dynmod/runners]# salt-run vmadm.list
'vmadm.list' is not available.

The runner is still broken as well I just started with it, but I was wrestling with the loader for a long time now :(

Setup

n/a

Steps to Reproduce Issue

Find a module with a virtualname and create a runner with that name.

Versions Report

2016.3/develop

sjorge commented 8 years ago

Just got my hello world function to work.

To replicate: Create a test module (or find an existing one) which sets virtualname and has the virtual method return the name. (e.g. smartos_vmadm.py -> vmadm)

Create a runner module with the name of virtualname (e.g. vmadm.py), it will try to load smartos_vmadm.py from the runners dir instead of vmadm.py.

This shouldn't happen, a virtual name for a execution module should not affect a runner module (I think it effects state modules too, but it is super late so I am going to bed new)

Ch3LL commented 8 years ago

@sjorge I haven't been able to replicate this. Here is my test case maybe you can correct me if I am doing something wrong:

My module is an already existent module dig

Here is my runner:

 ch3ll@localhost  /srv/salt/_runners  cat dig.py
#!/usr/bin/python

__virtualname__ = 'dig'

def check(outputter=None, display_progress=True):
    print('Hello world')

I tried it with removing virtualname as well and it still worked.:

 ch3ll@localhost  /srv/salt/_runners  sudo salt-run dig.check -lwarning
Hello world
None
 ch3ll@localhost  /srv/salt/_runners  

Is there something im misunderstanding?

sjorge commented 8 years ago

Hey @Ch3LL,

In your case both the runner and the execution module have a virtualname and filename of dig.

My test case: salt/modules/smartos_vmadm.py (virtualname = vmadm) if I create /srv/salt/_runners/vmadm.py (virtualname = vmadm or no virtualname) it fails, the loader is looking for /srv/salt/_runners/smartos_vmadm.py in this case.

sjorge commented 8 years ago

@Ch3LL were you able to reproduce with the info from late june? I'm still seeing this locally.

Ch3LL commented 8 years ago

Thanks for bumping this for me. okay i updated my test case an i'm still having a hard time replicating. I have a docker container so you can see my test case:

  1. docker run -it -v /home/ch3ll/git/salt/:/testing/ ch3ll/issues:34347 /bin/bash (where /home/ch3ll/git/salt is a local cloned repo)
  2. salt-run vmadm.test and you will see the runner return:
[root@f9c7d705fb59 testing]# salt-run vmadm.test
[WARNING ] /testing/salt/grains/core.py:1493: DeprecationWarning: The "osmajorrelease" will be a type of an integer.

CUSTOM RUNNER

I have edited /testing/salt/modules/smartos_vmadm.py so it would load. Here are my changes:

[root@f9c7d705fb59 testing]# git diff
diff --git a/salt/modules/smartos_vmadm.py b/salt/modules/smartos_vmadm.py
index 007c8cb..318073b 100644
--- a/salt/modules/smartos_vmadm.py
+++ b/salt/modules/smartos_vmadm.py
@@ -48,15 +48,18 @@ def __virtual__():
     '''
     Provides vmadm on SmartOS
     '''
-    if salt.utils.is_smartos_globalzone() and _check_vmadm():
-        return __virtualname__
-    return (
-        False,
-        '{0} module can only be loaded on SmartOS computed nodes'.format(
-            __virtualname__
-        )
-    )
-
+ 
+    return __virtualname__
+#    if salt.utils.is_smartos_globalzone() and _check_vmadm():
+#        return __virtualname__
+#    return (
+#        False,
+#        '{0} module can only be loaded on SmartOS computed nodes'.format(
+#            __virtualname__
+#        )
+#    )
+def test():
+    return('Loaded execution')

 def _exit_status(retcode):
     '''

And i can show that i can run this execution module:

[root@f9c7d705fb59 testing]# salt-call --local vmadm.test
[WARNING ] /testing/salt/grains/core.py:1493: DeprecationWarning: The "osmajorrelease" will be a type of an integer.

local:
    Loaded execution

And here is my /srv/salt/_runners/vmadm.py:

[root@f9c7d705fb59 testing]# cat /srv/salt/_runners/vmadm.py
__virtualname__ = 'vmadm'

def test(outputter=None, display_progress=True):
    return('CUSTOM RUNNER')

I have tried it with removing virtulaname as well. Also here is my master config:

runner_dirs:
  - /srv/salt/_runners/

Do you have runner_dirs set in your master or are you syncing the runner a different way?

sjorge commented 8 years ago

I tried to replicated it with 2016.11.0rc1 and I cannot reproduce it with this version. I will close the issue.