[BUG] __salt__ not defined when running a custom runner

[brandon-sling]

Description got an error about __salt__ not being defined when running a custom runner

Setup

def execute(targets, fun, arg=None, tgt_type='compound'):
    import salt.loader

    if arg is None:
        ret = __salt__['salt.execute'](tgt=targets, fun=fun, tgt_type=tgt_type)
    else:
        ret = __salt__['salt.execute'](tgt=targets, fun=fun, arg=(arg,), tgt_type=tgt_type)
    return ret

Steps to Reproduce the behavior call this runner function from another custom runner to execute any salt execution module

Expected behavior I would expect this to work like it has in the past. salt has access to other runner functions of which salt.execute is one. This worked in the past but seemed to stop upon upgrading from 2017.7.? to 2019.2.3

Screenshots

root@p-gp2-salt-1:~$ salt-run test.get_version
Exception occurred in runner test.get_version(): Traceback (most recent call last):
  File "/srv/salt/runners/test.py", line 4, in test
    raw_targets = tools.execute(targets = '*', fun='pkg.version',  tgt_type = 'glob', arg = 'memcached')
  File "/srv/salt/runners/tools.py", line 16, in execute
    ret = __salt__['salt.execute'](tgt=targets, fun=fun, arg=(arg,), tgt_type=tgt_type)
NameError: name '__salt__' is not defined

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ``` root@p-gp2-salt-1:/srv/salt/runners# salt --versions-report Salt Version: Salt: 2019.2.3 Dependency Versions: cffi: 1.14.0 cherrypy: unknown dateutil: 2.4.2 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: Not Installed msgpack-pure: Not Installed msgpack-python: 0.4.6 mysql-python: 1.3.7 pycparser: 2.20 pycrypto: 2.6.1 pycryptodome: Not Installed pygit2: Not Installed Python: 3.5.2 (default, Oct 8 2019, 13:06:37) python-gnupg: 0.3.8 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: ISO-8859-1 machine: x86_64 release: 4.4.0-34-generic system: Linux version: Ubuntu 16.04 xenial ```

Additional context Add any other context about the problem here.

[max-arnold]

I can' reproduce it on 2019.2.3:

def execute():
    print(list(__salt__.keys()))

% sudo salt-run saltutil.sync_runners
- runners.myrun

% sudo salt-run myrun.execute

['myrun.execute', 'auth.del_token', 'auth.mk_token', 'cache.clear_all', 'cache.clear_git_lock', 'cache.clear_grains', 'cache.clear_mine', 'cache.clear_mine_func', 'cache.clear_pillar', 'cache.cloud', 'cache.fetch', 'cache.flush', 'cache.grains', 'cache.list', 'cache.mine', 'cache.pillar', 'cache.store', 'cloud.action', 'cloud.create', 'cloud.destroy', 'cloud.full_query', 'cloud.list_images', 'cloud.list_locations', 'cloud.list_sizes', 'cloud.map_run', 'cloud.profile', 'cloud.query', 'cloud.select_query', 'config.get', 'doc.execution', 'doc.runner', 'doc.wheel', 'error.error', 'event.send', 'fileserver.clear_cache', 'fileserver.clear_file_list_cache', 'fileserver.clear_lock', 'fileserver.dir_list', 'fileserver.empty_dir_list', 'fileserver.envs', 'fileserver.file_list', 'fileserver.lock', 'fileserver.symlink_list', 'fileserver.update', 'git_pillar.update', 'http.query', 'http.update_ca_bundle', 'jobs.active', 'jobs.exit_success', 'jobs.last_run', 'jobs.list_job', 'jobs.list_jobs', 'jobs.list_jobs_filter', 'jobs.lookup_jid', 'jobs.print_job', 'launchd.write_launchd_plist', 'lxc.cloud_init', 'lxc.find_guest', 'lxc.find_guests', 'lxc.freeze', 'lxc.info', 'lxc.init', 'lxc.list', 'lxc.purge', 'lxc.start', 'lxc.stop', 'lxc.unfreeze', 'manage.alived', 'manage.allowed', 'manage.bootstrap', 'manage.bootstrap_psexec', 'manage.down', 'manage.get_stats', 'manage.joined', 'manage.key_regen', 'manage.lane_stats', 'manage.list_not_state', 'manage.list_state', 'manage.not_alived', 'manage.not_allowed', 'manage.not_joined', 'manage.not_present', 'manage.not_reaped', 'manage.present', 'manage.reaped', 'manage.road_stats', 'manage.safe_accept', 'manage.status', 'manage.tagify', 'manage.up', 'manage.versions', 'mattermost.post_event', 'mattermost.post_message', 'mine.get', 'mine.update', 'network.wol', 'network.wollist', 'network.wolmatch', 'pagerduty.create_event', 'pagerduty.list_escalation_policies', 'pagerduty.list_incidents', 'pagerduty.list_maintenance_windows', 'pagerduty.list_policies', 'pagerduty.list_schedules', 'pagerduty.list_services', 'pagerduty.list_users', 'pagerduty.list_windows', 'pillar.show_pillar', 'pillar.show_top', 'pkg.list_upgrades', 'queue.delete', 'queue.get_event', 'queue.insert', 'queue.insert_runner', 'queue.list_items', 'queue.list_length', 'queue.list_queues', 'queue.pop', 'queue.process_queue', 'queue.process_runner', 'queue.tagify', 'reactor.add', 'reactor.delete', 'reactor.list', 'salt.cmd', 'salt.execute', 'saltutil.sync_all', 'saltutil.sync_cache', 'saltutil.sync_clouds', 'saltutil.sync_eauth_tokens', 'saltutil.sync_engines', 'saltutil.sync_fileserver', 'saltutil.sync_grains', 'saltutil.sync_modules', 'saltutil.sync_output', 'saltutil.sync_pillar', 'saltutil.sync_proxymodules', 'saltutil.sync_queues', 'saltutil.sync_renderers', 'saltutil.sync_returners', 'saltutil.sync_roster', 'saltutil.sync_runners', 'saltutil.sync_sdb', 'saltutil.sync_serializers', 'saltutil.sync_states', 'saltutil.sync_thorium', 'saltutil.sync_tops', 'saltutil.sync_utils', 'saltutil.sync_wheel', 'sdb.delete', 'sdb.get', 'sdb.get_or_set_hash', 'sdb.set', 'vmadm.get', 'vmadm.is_running', 'vmadm.list', 'vmadm.nodes', 'vmadm.reboot', 'vmadm.start', 'vmadm.stop', 'ssh.cmd', 'state.event', 'state.orch', 'state.orch_show_sls', 'state.orchestrate', 'state.orchestrate_high', 'state.orchestrate_show_sls', 'state.orchestrate_single', 'state.pause', 'state.resume', 'state.rm_pause', 'state.set_pause', 'state.sls', 'state.soft_kill', 'survey.diff', 'survey.hash', 'test.arg', 'test.get_opts', 'test.metasyntactic', 'test.raw_arg', 'test.sleep', 'test.stdout_print', 'test.stream', 'thin.generate', 'thin.generate_min', 'vault.generate_token', 'vault.show_policies', 'vault.unseal', 'virt.force_off', 'virt.host_info', 'virt.init', 'virt.list', 'virt.migrate', 'virt.next_host', 'virt.pause', 'virt.purge', 'virt.query', 'virt.reset', 'virt.resume', 'virt.start', 'virt.vm_info', 'winrepo.genrepo', 'winrepo.update_git_repos']

I suspect that your example is incomplete:

1. Unused import salt.loader statement
2. Call chain test.py:tools.execute() -> tools.py:execute() -> salt.py:execute()

So far it looks like you import tools.py from test.py. When you import a runner directly, the magic __salt__ dunder won't be injected.

You either need to call it via __salt__['tools.execute']() without importing, or use the salt.loader.runner() function (see an example).

[brandon-sling]

Sorry test.py is also a runner. The unused salt.loader import is indeed unused. I inherited this code from the previous employee and these runners haven't been touched in years. Last commit was late 2018.

test.py:

import tools

def get_version():
    raw_targets = tools.execute(targets = '*', fun='pkg.version',  tgt_type = 'glob', arg = 'memcached')
...

There is more but execution stops here.

I filed this because this setup had been working in 2017.7.8 and seemed to stop working after I updated our salt infrastructure to 2019.2.3

If this shouldn't have ever worked that's a different story. tools.py is a runner that has common utility functions for use by other runners.

EDIT: Test is a sanitized filename

[max-arnold]

I really have no idea how it worked for you in 2017.7.8:

% salt --version
salt 2017.7.8 (Nitrogen)

% cat /srv/salt/_runners/test.py

import tools

def execute():
    print(tools.execute())

% cat /srv/salt/_runners/tools.py

def execute():
    print(list(__salt__.keys()))

% salt-run saltutil.sync_runners

[INFO    ] Copying '/var/cache/salt/master/files/base/_runners/test.py' to '/var/cache/salt/master/extmods/runners/test.py'
[INFO    ] Copying '/var/cache/salt/master/files/base/_runners/tools.py' to '/var/cache/salt/master/extmods/runners/tools.py'
- runners.test
- runners.tools

% salt-run test.execute
Exception occurred in runner test.execute: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/client/mixins.py", line 382, in _low
    data['return'] = self.functions[fun](*args, **kwargs)
  File "/var/cache/salt/master/extmods/runners/test.py", line 4, in execute
    print(tools.execute())
  File "/var/cache/salt/master/extmods/runners/tools.py", line 2, in execute
    print(list(__salt__.keys()))
NameError: global name '__salt__' is not defined

Here is how to fix the test.py runner (remove the import tools statement, and call the second runner via __salt__['tools.execute'](....)):

% cat /srv/salt/_runners/test.py

def execute():
    print(__salt__['tools.execute']())

[sagetherage]

@garethgreenaway can you try to reproduce this, please?

[sagetherage]

Ok, some confusion on the Core team on what we are doing here, so explaining in an attempt to be explicit and transparent - this has not been reproduced and thus is blocked, we will attempt to get it reproduced in the Magnesium release cycle, however, that is not say we will fix it in Magnesium until we understand exactly what is needed. Apologies for the confusion and open to feedback on a different process.

[sagetherage]

Kicking this back into triage (no milestone) to see if I can have another engineer attempt at reproducing, I have not forgotten!

[cmcmarrow]

@max-arnold I still cant reproduce this. Can you please provided all the scripts in full and a .sh. Their might be something simple that we are overlooking.

[max-arnold]

@cmcmarrow I'm not sure why are you tagging me instead of the original reporter...

[sagetherage]

@brandon-sling we are not able to reproduce this. Can you please provided all the scripts in full and a .sh. Their might be something simple that we are overlooking.

[sagetherage]

clearing a few labels and assignee as Gareth's workload has recently changed a bit, do let us know @brandon-sling if you can provide the above. Thank you!

[brandon-sling]

tools.py

def execute(targets, fun, arg=None, tgt_type='compound'):
    '''
        runs salt.execute

        Input:
            targets: the salt target string
            fun: the function you would like to run. (e.g. varnish.ban_list)
            arg: Args for the function provided
    '''
    import salt.loader

    if arg is None:
        ret = __salt__['salt.execute'](tgt=targets, fun=fun, tgt_type=tgt_type)
    else:
        ret = __salt__['salt.execute'](tgt=targets, fun=fun, arg=(arg,), tgt_type=tgt_type)
    return ret

test.py

import tools
import sys
sys.path.append('/var/lib/salt/runners')
message = []

def clear_memcache(*args, **kwargs):
    '''
        will search out cmsapp memcached boxes and restart memcache
    '''

    def clear(targets, arg):
        arg = 'memcached'
        clear = tools.execute(targets, 'service.restart', arg)
        for host, result in clear.items():
            tools.color_print("\t\t{}: {}".format(host, result), "blue")
            message.append("\t{}: {}".format(host, result))

    targets = '*cmsapp*'
    target_list = []
    arg = 'service.restart memcached'

    message.append("Restarting memcache")

    raw_targets = tools.execute(targets=targets,
                                fun='pkg.version',
                                tgt_type='glob',
                                arg='memcached')

    for host, val in raw_targets.items():
        if val:
            target_list.append(host)
    target_list = tools.execute(','.join(target_list), 'test.ping')

    tools.color_print("\tMatch: {}\n".format(targets), "green")
    if kwargs.get('interactive', True):
        for host, status in target_list.items():
            if (status is True):
                tmp = "\t\t{}: {}".format(host, status)
                tools.color_print(tmp, 'blue')
            else:
                tmp = "\t\t{}: {}".format(host, status)
                tools.color_print(tmp, 'red')
            verify = raw_input("Would you like to continue? [n]y ").strip().lower()
            if verify == 'y':
                clear(targets, arg)
            else:
                tools.color_print("Exiting...\n", "warning")
                return ''
    else:
        clear(targets, arg)

    return ''

def clear_cache():
    clear_memcache(interactive=False)

and something like salt-run test.clear_cache is then run. This results in the error.

[s0undt3ch]

I think enough investigation work has been put into this and clearly notes the bad/wrong usage.

No code which is imported by Python's import machinery can expect any salt dunders to be available. If this worked before, then that was a bug and unintended behavior.

This is not a bug.