Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Salt modules depends on utils are not loaded via salt loader #32500

Closed
acgupt opened this issue Apr 12, 2016 · 8 comments
Closed

Salt modules depends on utils are not loaded via salt loader #32500

acgupt opened this issue Apr 12, 2016 · 8 comments
Labels
Bug broken, incorrect, or confusing behavior cannot-reproduce cannot be replicated with info/context provided Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around stale
Milestone

Comments

@acgupt
Copy link

acgupt commented Apr 12, 2016

Description of Issue/Question

Salt Modules depends on utils are not loaded via salt loader.

Setup

We have a custom module and custom utils create at location _modules and _utils on master /srv/salt.
Modules are dependent on utils and loaded first in modules
After we do saltutil.sync_all to all minions . And modules and utils synced in the minion cache but failed to load.

Steps to Reproduce Issue

salt 'minion' sys.list_modules test*

or

salt 'minion' test_mod.echo

error in log: 2016-04-11 08:13:32,126 [salt.loaded.ext.module.test_mod][ERROR ][554][140137025505024] Got exception No module named test_utils: Traceback (most recent call last):
ImportError: No module named test_utils

This issue is reproducible in Salt version 2015.5.10 and 2015.8.8.2.

Versions Report

Salt Version:
Salt: 2015.8.8.2

Dependency Versions:
Jinja2: unknown
M2Crypto: 0.20.2
Mako: Not Installed
PyYAML: 3.11
PyZMQ: 14.5.0
Python: 2.6.6 (r266:84292, Jul 23 2015, 05:13:40)
RAET: Not Installed
Tornado: 4.2.1
ZMQ: 4.0.5
cffi: Not Installed
cherrypy: 3.2.2
dateutil: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
libgit2: Not Installed
libnacl: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pygit2: Not Installed
python-gnupg: Not Installed
smmap: Not Installed
timelib: Not Installed

System Versions:
dist: oracle 6.7
machine: x86_64
release: 3.8.13-68.3.4.el6uek.x86_64
system: Oracle Linux Server 6.7

Salt: 2015.5.10
Python: 2.6.6 (r266:84292, Jul 23 2015, 05:13:40)
Jinja2: unknown
M2Crypto: 0.20.2
msgpack-python: 0.4.6
msgpack-pure: Not Installed
pycrypto: 2.6.1
libnacl: Not Installed
PyYAML: 3.11
ioflo: Not Installed
PyZMQ: 14.5.0
RAET: Not Installed
ZMQ: 4.0.5
Mako: Not Installed
Tornado: Not Installed
timelib: Not Installed
dateutil: Not Installed

@cachedout
Copy link
Contributor

I don't believe __utils__ is supported on 2015.5. However, this does work correctly at the head of the 2015.8 branch:

/srv/salt/_modules/issue_32500.py

def echo():
    return __utils__['issue_32500.helper']()
#    return sorted([str(x) for x in __utils__.items()])

/srv/salt/_utils/issue_32500.py

def helper():
    return 'Helped'
mp@silver ...devel/salt/salt % sudo salt silver issue_32500.echo              (git)-[2015.8] 
silver:
    'issue_32500.echo' is not available.
ERROR: Minions returned with non-zero exit code
mp@silver ...devel/salt/salt % sudo salt silver saltutil.sync_modules         (git)-[2015.8] 
silver:
    - modules.demo
    - modules.issue_32500
    - modules.send_event
mp@silver ...devel/salt/salt % sudo salt silver issue_32500.echo              (git)-[2015.8] 
silver:
    The minion function caused an exception: Traceback (most recent call last):
      File "/home/mp/devel/salt/salt/minion.py", line 1075, in _thread_return
        return_data = func(*args, **kwargs)
      File "/var/cache/salt/minion/extmods/modules/issue_32500.py", line 2, in echo
        return __utils__['issue_32500.helper']()
      File "/home/mp/devel/salt/salt/loader.py", line 900, in __getitem__
        func = super(LazyLoader, self).__getitem__(item)
      File "/home/mp/devel/salt/salt/utils/lazy.py", line 93, in __getitem__
        raise KeyError(key)
    KeyError: 'issue_32500.helper'
mp@silver ...devel/salt/salt % sudo salt silver saltutil.sync_all             (git)-[2015.8] 
silver:
    ----------
    beacons:
    grains:
    log_handlers:
    modules:
    output:
    proxymodules:
    renderers:
    returners:
    sdb:
    states:
    utils:
        - utils.issue_32500
mp@silver ...devel/salt/salt % sudo salt silver issue_32500.echo              (git)-[2015.8] 
silver:
    Helped

@cachedout cachedout added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around cannot-reproduce cannot be replicated with info/context provided Core relates to code central or existential to Salt labels Apr 12, 2016
@cachedout cachedout added this to the Approved milestone Apr 12, 2016
@acgupt
Copy link
Author

acgupt commented Apr 12, 2016

Here are few additional test which makes module dependent on utils to load via salt loader and works fine

  • Copy utils from master to minions at location /srv/salt/utils and add utils_dirs configuration to /srv/salt/_utils in minion
  • salt-minion restart
  • calling utils with modifier __utils__ in module e.g. __ utils __ [ 'test_mod.echo' ]()
  • adding utils cache location to modules_dir configuration in minion config

Following is the example which we tried and failed to load. Please let us know if its not supported.

#test_mod.py
import logging
LOG = logging.getLogger(__name__)
LOG.setLevel(logging.DEBUG)
HAS_LIBS = False
try:    
    import test_utils
    HAS_LIBS = True
except ImportError as import_error:
    LOG.warning("Failed to load: %s", str(import_error.message))


def __virtual__():    
    return HAS_LIBS

def echo():
    return test_utils.echo()

#test_utils.py

def echo():
   return 'echo'

@cachedout
Copy link
Contributor

You need to access utils via __utils__ because that's the namespace where the loader injects them. It does not modify the python path to allow you to do a raw import as you are attempting to do in your example. That approach is unsupported.

@paclat
Copy link

paclat commented Apr 15, 2016

So let me describe the use case that we have in mind and see what the best way to accomplish this...

We have a number of actual modules and runners that we have written as various parts of a more complex system. In that system there are a number of "utility" modules that are shared across multiple of these modules and runners. Theses include things like an enumeration of status constants and the like.

What we would like is to have these shared modules installed once in the /srv/salt/_utils on the master and have them available for use in the runners and the modules. We originally experimented with copying these to both the _runners and the _modules directory. The problem with that solution is that they become "modules" and "runners" even though we don't really want them directly callable from the outside. Using utils is not so bad for making one or two dynamic calls, but it is a bit cumbersome for constants and the like.

The other option would be to install these into the site-packages on all of the places where we have salt-minions that use them. This seems to be not ideal as it limits the usefulness of the Salt module distribution to the minions.

What is confusing to me is that even though it was not a supported case it seemed to actually work when we happen to have a dynamic grain in the system? I am really perplexed that this somehow caused the _utils to be available to the python loader. The other odd thing is that even without the dynamic grains it will pick up the _utils once we restart the actual minions. The problem with rebooting minions as part of a state are documented elsewhere and seem to make the whole distribution process that we are trying to implement much less reliable.

@paclat

@cachedout
Copy link
Contributor

Hi @paclat

You have a lot of concerns here so let me see if I can restate a few and respond where I can.

I'm able to place a module in /srv/salt/_utils called my_utils.py, which contains functions my_first_util_func and my_second_util_func.

From modules inside /srv/salt/_modules, I am able to access the returns from these functions via __utils__['my_utils.myst_first_util_func'](), etc.

Is your concern that you're limited to access the returns of functions inside your custom utility module instead of having the entire namespace exported, so that you could declare constants independently of functions? Or is is that the workflow I'm describing above doesn't work for you at all?

Regarding the second part of your question, with the dynamic grains. I'm not sure I follow what's happening here. Could you post a step-by-step reproduction case that illustrates the issue? I might just be missing something obvious here. Thanks.

@cachedout cachedout self-assigned this Apr 18, 2016
@mephi42
Copy link
Contributor

mephi42 commented Nov 8, 2017

Was support for raw imports added in the meantime? I see the following in the doc:

https://docs.saltstack.com/en/latest/topics/utils/index.html

"... Also you could even write your utility modules in object oriented fashion ... And import them into other custom modules:

import mymodule"

but when I try to do this, I get an ImportError.

@mephi42
Copy link
Contributor

mephi42 commented Apr 18, 2018

I tried to implement this in #46841 myself, and while the result kind of sort of worked, there were nasty side-effects. The solution consisted of making sure utils modules are uploaded in the first place (I'm using salt-ssh exclusively and there it did not work - perhaps regular salt is different) by changing salt.client.ssh.mod_data(), and then making sure they are added to the search path by modifying salt.loader.{minion_mods,states}. The problem I observed was that built-in pkg state and module stopped working, presumably because they began colliding with their utils counterpart.

It would be great if someone more knowledgeable could point a better way to resolve this.

@stale
Copy link

stale bot commented Jan 9, 2020

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.

@stale stale bot added the stale label Jan 9, 2020
@stale stale bot closed this as completed Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior cannot-reproduce cannot be replicated with info/context provided Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around stale
Projects
None yet
Development

No branches or pull requests

4 participants