How to call other state in custom state code?

[Colstuwjx]

Hi, I'm writing a custom state in /srv/salt/_states/git_refresh.py, and I just want to call file.managed state in this py.

What should I do?? Thanks.

[tbaker57]

Hi,

I've never tried it, but I believe you should be able to call the function salt'file.managed' from within your custom state with the appropriate args and it should run that state and return a dict as per the usual state output (return data).

cheers, Tim

[tbaker57]

oops, that should be

__salt__['file.managed'](...)

[tbaker57]

ok last try before I give up and have a beer

__salt__['file.managed'](...)

[basepi]

That actually won't work -- __salt__ is only loaded with the execution module functions, not the state module functions.

We don't currently provide a mechanism by which you can call states from other states -- states are designed to be fairly standalone, and the modules those states use to implement their behavior are available.

That said, this could be useful, so perhaps we should add a mechanism by which this could be accomplished. I'll have to think on it.

[thatch45]

There is a very hacky way to do it, we need to add states into the loader cross caller. The hacky way is CRAZY HACKY, I don't remember exactly how to do it, but you use sys.modules to look it up off the python stack. We should open this as a feature request

[basepi]

Done.

[steverweber]

i use the salt.loader

import salt.loader

def present(name, infoblox_host_record=None, inventory_mfcf_record=None):
    ret = {'name': name, 'result': False, 'comment': '', 'changes': {}}
    __state__ = salt.loader.states(__opts__, __salt__)

    # STATE: infoblox_host_record
    # ----------------------------------------------
    if infoblox_host_record:
        sub_ret = __state__['infoblox_host_record.present'](name=name, data=infoblox_host_record)
        if sub_ret['result'] == False or sub_ret['result'] == None:
            return sub_ret
        if sub_ret['changes']:
            ret['changes'].update({'1- infoblox_host_record.present': sub_ret['changes']})

[blaffoy]

steverweber, thanks for posting that workaround. When using it I have a problem that some states rely on the global dictionary env being defined, but it isn't available in the state dictionary loaded through salt.loader.states. Any idea how I might use salt.loader.states in such a way that the env dictionary is available?

edit: I figured out that I could do something like this:

__states__ = salt.loader.states(__opts__, __salt__)
__states__['file.managed'].func_globals['__env__'] = __env__

It's very hacky, but it seems to work.

[steverweber]

thanks for the edit. I was about to look into it :)

[thusoy]

This would be intensly useful, a lot of custom states are wrappers around some very common states like file.managed, which at the moment is around 250 lines of code. Being able to re-use this instead of writing it yourself is an enormous gain and actually makes it feasible to implement custom states for a lot of stuff, which currently isn't an option because of the amount of code needed.

The workaround by steverweber and tensorproduct works like a charm until something there's built-in support for this.

Edit: After a little more testing, the above method does work in isolation, but it causes other stuff to fail in weird ways, like pkgrepo.present failing with KeyErrors due to 'os_family' not being present in grains and similar. However, the following method seems to work without side-effects:

from salt.states.file import managed as file_managed

def my_state(name):
    for dunder_dict in 'env salt opts grains pillar'.split():
        dunder = '__%s__' % dunder_dict
        file_managed.func_globals[dunder] = globals()[dunder]
    ret = file_managed(...)

[rvora]

@basepi @thatch45 Solution by @thusoy looks great and it seems to work. Do you think this is the way to go until SaltStack officially supports calling states from other states?

Thanks - this is a very useful feature hope you implement it in core SaltStack.

[multani]

I'm currently doing something gross like @thusoy solution. There's also some discussion in #3513, which also adds some bits about the grains, @thusoy you might be interested to give it a try maybe?

[basepi]

Refs #23927

[jacksontj]

This will be a little weird to make, as it will make somewhat of a circular dep. Right now the loader builds on top of the layers (basically) grains -> modules -> states (where states can be replaced with almost all module types). If we want to add states to modules, then we'll have to figure out how to get a reference to that exact dict -- which i just circular references :/

The alternative is to optimize the state.* functions- such that you can cross call to the state module which will do all the state magic on its own-- the key will be to not re-load the state module's highstate object on each call-- since that is slow :)

[thusoy]

@jacksontj I think the desire is more towards re-using states in other states, than states in modules (if I understood your point correctly). Are there any downsides to the approach listed above? Here's the same approach, but refactored into a context manager and with some more error checking:

import sys
import contextlib
import importlib

@contextlib.contextmanager
def salt_state(qualified_state, _globals):
    try:
        module_name, func_name = qualified_state.split('.', 1)
    except ValueError:
        message = ("Argument to salt_state needs to be of the form "
        "'<module>.<function>' (was '{}')".format(qualified_state))
        _, _, traceback = sys.exc_info()
        raise ValueError(message), None, traceback

    try:
        module = importlib.import_module('salt.states.{}'.format(module_name))
    except ImportError:
        message = "No state with the name '{}'".format(module_name)
        _, _, traceback = sys.exc_info()
        raise ValueError(message), None, traceback

    func = getattr(module, func_name, None)
    if func is None:
        raise ValueError("'{}' state does not have a function '{}'".format(
            module_name, func_name))

    for dunder_dict in 'env salt opts grains pillar instance_id'.split():
        dunder_name = '__%s__' % dunder_dict
        dunder = _globals.get(dunder_name)
        # instance_id is not on older salt versions, thus the check
        if dunder is not None:
            func.func_globals[dunder_name] = dunder

    yield func

You could then use it like this:

def my_custom_state(name):
  with salt_state('file.managed', globals()) as file_managed:
    file_ret = file_managed(name)
  <..>

Shall I make a PR out of this or is there some better way? I tried to get rid of having to pass globals() as an argument to the context manager, since it's a bit ugly. I tried by using a decorator instead, thinking I could fetch the dunder dicts from the decorated function when called, and inject the state with the globals applied into its scope, but the decorated got some of the dunders applied to itself, so it got a bit hacky, and I think it messed with the decorated function. Anyway, the one above works and is the least hacky alternative I managed when trying to put the function outside the custom state.

[jacksontj]

Oh, if the ask is soely to cross call state modules from other states this is actually trivial. We'd just need to add a pack (in the loader) which referenced the rest of the states-- maybe something like __states__. Then in your state you could do something like:

def custom_state(name):
    __states__['file.managed'](name)
    # insert your stuff here

Does that cover what you were thinking?

[thusoy]

@jacksontj That would be brilliant, would cover all of my use cases at least.

[rvora]

That would cover my use case as well. Thanks.

Rajul Vora - typos by Android On Jun 21, 2015 12:25 AM, "Tarjei Husøy" notifications@github.com wrote:

@jacksontj https://github.com/jacksontj That would be brilliant, would cover all of my use cases at least.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-113870767.

[jacksontj]

I'll crank that out Monday morning then :) On Jun 21, 2015 11:12 AM, "Rajul Vora" notifications@github.com wrote:

That would cover my use case as well. Thanks.

Rajul Vora - typos by Android On Jun 21, 2015 12:25 AM, "Tarjei Husøy" notifications@github.com wrote:

@jacksontj https://github.com/jacksontj That would be brilliant, would cover all of my use cases at least.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-113870767.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-113937468.

[jacksontj]

PR opened (#24865). I added a test and some simple documentation.

[jacksontj]

Just curious, for your use cases why don't requisites work? If your state module requires cross calling to a state it seems almost like you should just use requisites? I'd just be very cautious about monolithic state modules

[rvora]

Thomas:

Here's my use case. In short, we are moving the complexity from Jinja templates to a custom state where an attribute (version) of the state method (pkg.installed) needs to be computed.

CICD pipeline creates a new salt-master for the entire multi-VM environment (say, my-qa-environment).

It also creates a pillar file that specifies which version of an RPM should be installed so that particular build of the environment can be reproduced 10 days later (when many components may have newer versions of RPM). Think of this as bill of materials for a release.

release_versions: comp1: version: 1.0-001 comp2: version: 1.0-020

Now, this could be easily done with pkg.installed and specifying version attribute using Pillar look up.

However, sometimes a specific developer workflow requires overriding this version on a per-component basis. Developing on a feature branch for comp1? Use the latest feature branch RPM for comp1 but everything else comes from the pillar file.

Even that can be accomplished using some Jinja template logic. But then all this logic has to be repeated all over the place for each state file that deals with each component. Any change in logic now has to be maintained in multiple state files. That should be avoided.

So the solution is to write my own state that calls pkg.installed. Below example is contrived and doesn't have all the complexity but makes the point.

File: comp1.sls (and similarly for comp2.sls, comp3.sls) comp1: mypkg.installed: []

File: _states/mypkg.py def installed(name, **kwargs):

#..... some logic to determine version to be installed ....
kwargs['version'] = desired_version

return  __states__['pkg.installed'](name, **kwargs)

On Mon, Jun 22, 2015 at 9:18 AM, Thomas Jackson notifications@github.com wrote:

Just curious, for your use cases why don't requisites work? If your state module requires cross calling to a state it seems almost like you should just use requisites? I'd just be very cautious about monolithic state modules

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-114168998.

[thusoy]

@jacksontj My use case is deploying initfiles, so my custom state checks the init script running on the box, and passes the correct script the file.managed. Thus, I can do like this:

nginx:
  init_script.managed:
    upstart: salt://nginx/init-upstart
    sysvinit: salt://nginx/init-sysvinit

  service.running:
    - watch:
      - init_script: nginx

The state itself is quite short:

def managed(name, **kwargs):
    """ Make sure an init script is present. Set the name of the service, and a file
    source for each of the different init systems you want to support.
    """
    ret = {'name': name, 'changes': {}, 'result': False, 'comment': ''}
    init_system = get_init_system()

    args = {}
    if init_system == 'upstart':
        args['name'] = '/etc/init/%s.conf' % name
        args['mode'] = '0644'
    elif init_system == 'sysvinit':
        args['name'] = '/etc/init.d/%s' % name
        args['mode'] = '0755'

    if init_system in kwargs:
        args['source'] = kwargs[init_system]
        file_ret = __states__['file.managed'](**args)
        ret['comment'] = file_ret['comment']
        ret['result'] = file_ret['result']
        ret['changes'] = file_ret['changes']
    else:
        ret['comment'] = 'No source file for present init system given: %s' % init_system
    return ret

Could this be done easier with requisites?

[jacksontj]

@rvora that case does make some sense, although it seems to be a better fit with an execution module function. I'm thinking you should make a module that gets the version (instead of a state)-- then the users can use jinja and set the version to something like salt['custom.version']('name of thing').

As for @thusoy You could do the same thing with requisites, and it would be more pluggable/reusable. If your state instead looked like:

/etc/init.d/upstart:
  file.managed:
    - mode: 644
    - source: salt://nginx/init-upstart

/etc/init.d/sysvinit:
  file.managed:
    - mode: 755
    - source: salt://nginx/init-upstart

nginx:
  service.running:
    - watch:
      - file: /etc/init.d/sysvinit
      - file: /etc/init.d/upstart

This will manage the 2 init files, setting them to the correct contents and permissions-- and on change of either restart nginx.

This setup is a bit more pluggable (you can move these around, override them, etc.) and is a bit easier to follow since it breaks the various states up into manageable blocks.

[thusoy]

@jacksontj I don't agree that's a better solution, both files would be created even if only one of them is needed. Thus you'd need to check which init system is running, and you'd need to duplicate those checks and both files (three files if you also need to support systemd) for every state that sets up a running service. Putting this in one location greatly reduces duplication, which sounds much more reusable to me, but I think we're talking about different kinds of re-usability.

In any case I don't think the best design of this precise problem is very interesting for the issue at hand, custom states will often need to either wrap many existing states or combine several of them, I think being able to re-use them is essential to avoid duplication of effort. This could probably also make many states shipped with salt easier, see f. ex cron.file (which basically re-implements file.managed).

Anyway, I stumbled across something that I think solves the problem: state.single. At least for me this works:

def managed(name):
  state_ret = __salt__['state.single']('file.managed', *args)
  file_ret = state_ret.values()[0]
  <...>

The only difference from calling file.managed directly is that state.single wraps the return value, so you have to dig it out with .values()[0], but that's not a big problem. Is there any reason why this shouldn't generally work? If it does it's just a matter of adding a note to the custom state docs and this could be closed.

Edit: Removed question about pluggability, reading comprehension fail on my part.

[rvora]

@jacksontj https://github.com/jacksontj Your proposed solution using custom module to compute the version would have side effects. pkg.installed has many options that I want to purposely hide for my use-case (e.g. pkgs list or source). So creating a custom state allows me to support only the attributes I want to support and then call pkg.installed underneath to get things done.

In Puppet, you can create a custom class (with possible inheritance) to hide complexity of underlying resource primitives.

Its a very useful design pattern that you should try to support - even if there might be other ways to skin the cat. People can choose whichever way makes sense to them.

I haven't heard any argument as to why this would be a bad direction for SaltStack - so I am assuming you are still open to implementing this feature.

Rajul

On Wed, Jun 24, 2015 at 9:41 AM, Tarjei Husøy notifications@github.com wrote:

@jacksontj https://github.com/jacksontj I don't agree that's a better solution, both files would be created even if only one of them is needed. Thus you'd need to check which init system is running, and you'd need to duplicate those checks and both files (three files if you also need to support systemd) for every state that sets up a running service. Putting this in one location greatly reduces duplication, which sounds much more reusable to me, but I think we're talking about different kinds of re-usability. How do you think this is less pluggable? Publishing the custom state in a repo would allow it to be re-used by lots of other states if they just include the repo as a gitfs backend, wouldn't that be pluggable?

In any case I don't think the best design of this precise problem is very interesting for the issue at hand, custom states will often need to either wrap many existing states or combine several of them, I think being able to re-use them is essential to avoid duplication of effort. This could probably also make many states shipped with salt easier, see f. ex cron.file (which basically re-implements file.managed).

Anyway, I stumbled across something that I think solves the problem: state.single. At least for me this works:

def managed(name): state_ret = salt['state.single']('file.managed', *args) file_ret = state_ret.values()[0] <...>

The only difference from calling file.managed directly is that state.single wraps the return value https://github.com/saltstack/salt/blob/develop/salt/modules/state.py#L1088, so you have to dig it out with .values()[0], but that's not a big problem. Is there any reason why this shouldn't generally work? If it does it's just a matter of adding a note to the custom state docs and this could be closed.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-114937464.

[thusoy]

@rvora Try the solution I just posted, use __salt__['state.single']('pkg.installed', **kwargs) and see if it works for you.

[jacksontj]

Both of your use cases make sense, and I'm definitely for this feature-- I'm just trying to understand the use case better :) for @rvora we thought about doing something similar but we decided we didn't want to manage the complexity to the users.

Calling state.single will work but will be... slow and a bit awkward. You can definitely use it until the __states__ feature lands in a salt release though.

[ryan-lane]

What's the current status on this? We're heavily using state.single and it's killing us. There's examples of core modules using this:

https://github.com/saltstack/salt/blob/develop/salt/states/boto_asg.py#L411

https://github.com/saltstack/salt/blob/develop/salt/states/boto_asg.py#L615

We have more incoming changes that heavily rely on this. The reasoning behind this is that some AWS resources should include management of other AWS resources inside of the same state. For instance, a dynamodb state that also manages its own cloudwatch alarms and datapipelines (for backup schedules). Each call to state.single causes all modules to be refreshed. If we have 2-3 alarms and a datapipeline called from a dynamodb state, it can take minutes to just do simple things.

[ryan-lane]

The archive and dockerio states are also examples of core modules doing this.

[basepi]

We should investigate adding this for Boron, it makes a lot of sense.

[basepi]

Added in #26747.

I guess I was under the impression this was going to be much harder than it actually was. Once I started looking into it it was super straightforward. I haven't done extensive testing, but I have done basic testing.

I'm embarrassed we waited this long at this point.

[ryan-lane]

Awesome. I'll give this a try! On Aug 28, 2015 10:23 AM, "Colton Myers" notifications@github.com wrote:

Added in #26747 https://github.com/saltstack/salt/pull/26747.

I guess I was under the impression this was going to be much harder than it actually was. Once I started looking into it it was super straightforward. I haven't done extensive testing, but I have done basic testing.

I'm embarrassed we waited this long at this point.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/21214#issuecomment-135838584.

[thatch45]

This used to be MUCH harder to do, this is crazy easy now because of how the new LazyLoader works.

[basepi]

Ah, yes, this is definitely a side effect of the LazyLoader.

[thusoy]

@basepi I think your fix is identical to what @jacksontj attempted earlier. I tried running this on my machines, but I end up with NameError: global name '__instance_id__' is not defined when calling file.managed from my custom state, any idea how to fix?

[basepi]

Thanks @thusoy. I have a couple of ideas on how to fix that, I'll look into it soon.

[basepi]

__states__ issues (file.managed, etc) should be fixed: https://github.com/saltstack/salt/pull/27725