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

Exception when using Salt mine from pillar #11509

Closed
JensRantil opened this issue Mar 25, 2014 · 56 comments
Closed

Exception when using Salt mine from pillar #11509

JensRantil opened this issue Mar 25, 2014 · 56 comments
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt Documentation Relates to Salt documentation fixed-pls-verify fix is linked, bug author to confirm fix P3 Priority 3 Pillar severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@JensRantil
Copy link
Contributor

$ salt 'apt-mirror' state.highstate
apt-mirror:
    Data failed to compile:
----------
    Pillar failed to render with the following messages:
----------
    Rendering SLS 'apt-repo.eu-west-1' failed, render error:
Jinja error: 'master_uri'
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/salt/utils/templates.py", line 261, in render_jinja_tmpl
    output = jinja_env.from_string(tmplstr).render(**unicode_context)
  File "/usr/lib/python2.7/dist-packages/jinja2/environment.py", line 894, in render
    return self.environment.handle_exception(exc_info, True)
  File "<template>", line 1, in top-level template code
  File "/usr/lib/pymodules/python2.7/salt/modules/mine.py", line 182, in get
    auth = _auth()
  File "/usr/lib/pymodules/python2.7/salt/modules/mine.py", line 24, in _auth
    __context__['auth'] = salt.crypt.SAuth(__opts__)
  File "/usr/lib/pymodules/python2.7/salt/crypt.py", line 502, in __init__
    self.crypticle = self.__authenticate()
  File "/usr/lib/pymodules/python2.7/salt/crypt.py", line 514, in __authenticate
    self.opts.get('_safe_auth', True)
  File "/usr/lib/pymodules/python2.7/salt/crypt.py", line 345, in sign_in
    if self.opts['master_ip'] not in self.opts['master_uri']:
KeyError: 'master_uri'

; line 1

---
{% set ipaddrs = salt['mine.get']('apt-mirror', 'network.ipaddrs')[0] %}    <======================
{% if ipaddrs %}
apt-repo: http://{{ ipaddrs[0] }}
{% else %}
apt-repo: http://packages.somewhere.com
{% endif %}
[...]

---

This works:

$ salt 'apt-mirror' mine.get apt-mirror network.ipaddrs
apt-mirror:
    ----------

(no, I wasn't expecting any data, nor an exception)

@JensRantil
Copy link
Contributor Author

Maybe related to bf92ef7 in any way?

@basepi
Copy link
Contributor

basepi commented Mar 25, 2014

He's using opts.get in that commit you linked, so I don't think that's the problem.

In any case, we'll investigate this.

@basepi basepi added this to the Outstanding Bugs milestone Mar 25, 2014
@zerowolfgang
Copy link

I've run into the same master_uri key error problem using a similar mine.get call in a template.

@JensRantil
Copy link
Contributor Author

@zerowolfgang Did you make the mine.get call in a state template or pillar template?

@cachedout
Copy link
Contributor

That gist 404s for me,

On Fri, Mar 28, 2014 at 10:41 AM, Jens Rantil notifications@github.comwrote:

@zerowolfgang https://github.com/zerowolfgang Did you make the mine.get
call in a state template or pillar template?

Reply to this email directly or view it on GitHubhttps://github.com//issues/11509#issuecomment-38940490
.

@zerowolfgang
Copy link

@JensRantil Pillar. Could I expect the same error in a state template ?

@JensRantil
Copy link
Contributor Author

I don't know, but since I was experiencing this using pillar, too it's probably related to that.

@JensRantil
Copy link
Contributor Author

@cachedout I might have initially posted a gist. I removed it a put the actual error in this issue instead. Is that what you were referring to?

@zerowolfgang
Copy link

@JensRantil JFTR, I have confirmed that this is worky in a state file, i.e.:

some_managed_file:
  file.managed:
    - name: /etc/node_name
    - contents: {{ salt['mine.get']('node_name', 'network.ip_addrs').get('node_name')[ip_addr_index] }}

The same crashses with the master_uri KeyError in a pillar file, which is where I need it to be worky.

@cachedout
Copy link
Contributor

@JensRantil Yes, I think so. Looks like most of the relevant information is in the issue now so I'll take a peek this morning.

@cachedout
Copy link
Contributor

That error may be resolved in #11673. Please have a look at that patch and see if it resolves your concerns.

@cachedout
Copy link
Contributor

Also, I don't believe that querying mine data from a pillar is generally a supported behavior. Is this something that has worked for you in the past?

@JensRantil
Copy link
Contributor Author

Is this something that has worked for you in the past?

No, it's not. If it's not supported I propose printing out a message that it's not supported ;)

@zerowolfgang
Copy link

I've never tried it before either, but the docs seem to imply it's doable. JTFR there are those on the salt Google group, and a mention on SO, that claim they have it working in some fashion.

@JensRantil
Copy link
Contributor Author

@zerowolfgang Do you have a link to that discussion where they claim this?

@zerowolfgang
Copy link

@JensRantil Yes, and I've both misspoke and misread regarding it though :/ See http://goo.gl/V1JMQH - I guess this only reaffirms the use of mine.get in states. And I see now from your post on the 25th that you've spec'd mine_functions in a pillar file (as in the link) whereas I have not, but the invocations of mine.get have yielded the same error nonetheless, so perhaps @cachedout's last comment re supported behaviour is a factor here.
( There was another related groups post but I can't seem to find it now )

@cachedout
Copy link
Contributor

Yes, it does work in state files and that's widely supported. However, none of the core devs that I've talked to believe that mine.get inside pillars is a feature that's either documented or supported. I suppose we need to make a final decision on that issue but I wanted to at least alert you that this may be moved into feature work instead of bug fixing if robust pillar+mine integration is necessary.

That said, I agree with Jens that we need some form of documentation or alert notice to inform users about possible issues.

@zerowolfgang
Copy link

I'm relatively new to salt and am far from wading too deeply into the doco, so I have a question that pertains to the original mine.get attempt in pillar files: I reckon it's safe to assume that what would run as expected in a state file i.e mine.get, pillar.get, or grains.get, will fail if attempted in top level pillars due to not being supported given each really needs to be called in the context of a node state.
FTR, what I was trying to accomplish was to ascertain at/in a high level pillar file the value of a virtual interface that is not hard coded (a requirement requested by customer) that we are adding to nodes at deploy time such that we could then reference that value (via pillar.get) and consequently have global access to that any pertinent states files for the node.
This would be an nice and usable feature (methinks) at some point if doable - gets a +1 from me.

@cachedout
Copy link
Contributor

Hi @zerowolfgang. I'm not completely sure certain what you're trying to accomplish but it sounds like you're trying to gather some data from minions after they're deployed and then you want to be able to use that data in state files. If so, have you considered just using custom grains to accomplish what you want? http://docs.saltstack.com/en/latest/topics/targeting/grains.html#writing-grains

@basepi basepi modified the milestones: Approved, Outstanding Bugs Apr 21, 2014
@zerowolfgang
Copy link

@cachedout Yes what it sounds like is what I've attempted, evidently at the wrong level. I'll have a look at custom grains, so thanks for the suggestion. Sorry for the slow response. Cheers!

@cachedout
Copy link
Contributor

@zerowolfgang Sounds good. Please keep us posted so we can close this issue when appropriate. Thanks!

@zerowolfgang
Copy link

@JensRantil originally opened the issue.If he's satisfied with where things are, I'm good if he closes it as well. Danke !

@cachedout
Copy link
Contributor

@zerowolfgang Thanks for the quick reply. We'll see if @JensRantil wants to close this or leave it open. Thanks!

@JensRantil
Copy link
Contributor Author

@zerowolfgang @cachedout Custom grains are generally only loaded on minions on startup, right? If so, it is not enough for my requirements. Basically I'd want to set a variable conditionally based on another host exists or not. I could set this variable in a Jinja template, but since I'd like to reuse the variable, I would get rather messy with includes etc.

I think this issue can be closed after either:

  • mine.get calls are supported in pillar sls files; or
  • an error is printed that explains the mine calls are not supported in pillars (and with which we've taken a policy decision that it will not be supported).

@yakneens
Copy link
Contributor

+1 for mine in pillars.

@powellchristoph
Copy link

+1 for mine in pillars. Basically what @bersace said.

@DanyC97
Copy link

DanyC97 commented Aug 28, 2015

+1

@AusIV
Copy link

AusIV commented Sep 9, 2015

I've added the following to a custom execution module, allowing me to access the mine from pillars:

def pillar_mine_get(tgt, fun):
    mine_string = __salt__["cmd.run"]("salt-run mine.get '%s' %s --out=json"
        %( tgt, fun))
    return json.loads(mine_string)

This only supports minion_id globbing for lookups, but that could be pretty easily adapted if you need other target types.

In my pillars, I can use:

salt['my_module.pillar_mine_get']('host_glob', 'network.ip_addrs')

as a drop-in replacement for:

salt['mine.get']('host_glob', 'network.ip_addrs')

I'm interested to know if anyone has a better work-around - preferably one that doesn't involve forking the salt runner as a separate process.

I'd also warn that you need to be sure you can trust the input for 'tgt' and 'fun', as it's not being sanitized and I suspect someone could convince it to do malicious things. Anything that can invoke pillar_mine_get could also invoke cmd.run, but if you can't trust the inputs, don't use this solution.

@bersace
Copy link
Contributor

bersace commented Sep 9, 2015

@AusIV Very nice. Thanks for this workaround !

@whiteinge
Copy link
Contributor

@AusIV that is a good workaround.

In answer to your question about avoiding the shell-out process: the following should be possible now that #26648 is slated for the next 2015.5.6 point-release and the impending 2015.8 release:

{% set mine_data = salt['saltutil.runner']('mine.get', tgt='host_glob', fun='network.ip_addrs') %}

It uses the old-ish but not well-known saltutil.runner function. Unfortunately, prior to the above pull req, it shadowed the fun argument which the mine runner uses.

@genuss
Copy link
Contributor

genuss commented Sep 9, 2015

I tried to use something like this:
__salt__["cmd.run"]("salt-call mine.get %s %s" (tgt, fun))
but it was very unstable (was version 2014.7)

Does the solution with salt-run work as expected? I suppose, it must have the same issues like the one with salt-call

@AusIV
Copy link

AusIV commented Sep 10, 2015

@genuss: salt-call is a minion command, salt-run is a master command. Pillars get evaluated on the master, so if your master isn't also a minion salt-call won't work. salt-run, on the other hand, should always be present on the master (but you won't be able to use that module on formulas that run on minions).

Also, were you accounting for the fact that it's going to hand back a string, but it will be a serialized object? That's why I'm getting the output as json and parsing it.

@genuss
Copy link
Contributor

genuss commented Sep 10, 2015

It looks like you workaround works much better than mine! Thanks a lot.
And yes, I see why you getting out as json. But in 2014.7 it's not possible.

@jgmchan
Copy link

jgmchan commented Sep 12, 2015

+1 It would be great if it was possible to use mine.get in pillars. I have a state which gets all its config data from pillars and one of the bits of data is IP addresses from other servers in my environment.

Without this I think I may need to kludge it so that it gets this particular data from the state and everything else from the pillars.

@whiteinge whiteinge added fixed-pls-verify fix is linked, bug author to confirm fix Documentation Relates to Salt documentation labels Sep 13, 2015
@whiteinge
Copy link
Contributor

Calling the mine runner as detailed in this comment should be sufficient to close this issue out. Please test accordingly and update this issue if you encounter trouble.

2015.8 is due out shortly (likely this week) and the fix will be in 2015.5.6 as well.

We can update the docs to direct master-side operations (Pillar & Orchestrate) to the mine runner and minion-side operations to the mine execution module and I think this can finally be closed.

@JensRantil
Copy link
Contributor Author

👏 Thanks!

@bentwire
Copy link

@AusIV - I tried using your execution module example, but it isn't working. When I try to gen the pillar it says it can't find the module. No idea what I am doing wrong... The module is loaded on the minions fine, I see no errors in the minion logs and it shows it copying and loading the module...

@AusIV
Copy link

AusIV commented Sep 25, 2015

@bentwire: You need to have it set up in the salt master under extension_modules. See here. It's different than execution modules that run on minions, and it's an area of documentation that seems pretty lacking. I think one of my co-workers figured it out from a blog post somewhere, and I just know about it from seeing it done in our code base.

I have configured my salt master with:

extension_modules: /srv/modules

And the execution module is at /srv/modules/modules/my_utils.py.

@bentwire
Copy link

@AusIV - Awesome! I will try this. Thank you for the quick reply!

@wwentland
Copy link
Contributor

I really don't think that this is an acceptable solution as it forces users to fork formulas whenever they need to incorporate data from the mine. It is nice that it is easier to incorporate it this way, but it is far from a solution to the actual problem, namely that one cannot easily include dynamic data in the pillar as we conflate mine configuration and pillar rendering.

A step in the right direction would be the implementation of static pillars as discussed in #23910

@whiteinge
Copy link
Contributor

Totally agree. Arguments to a mine call should not be hard-coded in a publically available formula. But since you cannot reference Pillar from within Pillar, there's no good way to package such a formula other than docs.

@bersace
Copy link
Contributor

bersace commented Oct 6, 2015

Here we reuse pillar in pillars with Jinja.

{% import_yaml 'other/pillar.sls' as other -%}

formula:
  foo: 0
  bar: {{ other.other.bar }}
  baz: {{ salt['pillarmine.get']('func') }}

In order to get full settings out of pillar, we combine defaults.get, load_yaml and grains.filter_by to merge defaults and pillar.

{% set defaults = salt['defaults.get']('formula') -%}

{% load_pillar as pillars -%}
foo: 128
baz: {{ salt['pillarmine.get']('func') }}
{% endload -%}
{% set pillars = salt['grains.filter_by']({
    'default': defaults,
}, merge=pillars) -%}

formula:
  {{ pillars|yaml(False)|indent(2) }}

See #26903

@whiteinge
Copy link
Contributor

whiteinge commented Oct 6, 2015 via email

@bogdanr
Copy link
Contributor

bogdanr commented Oct 7, 2015

@bersace Can you please explain this a little bit more?
I would like to use network.ip_addrs extracted from a certain minion from mine and I still can figure out how to do that.

@bersace
Copy link
Contributor

bersace commented Oct 7, 2015

Here is the pillarmine module. I put it on extmods/modules and declare extmods as extension_modules config.

import logging
import yaml


logger = logging.getLogger(__name__)


def get(tgt, fun, tgt_type='glob'):
    cmd_run = __salt__['cmd.run']  # noqa
    cmd = "salt-run mine.get --out yaml '%s' %s tgt_type=%s" % (
        tgt, fun, tgt_type)
    logger.debug("Querying mine with %r", cmd)
    output = cmd_run(cmd)
    logger.debug("Got mine result %s", output)
    return yaml.load(output)

@bersace
Copy link
Contributor

bersace commented Oct 7, 2015

@bogdanr of course.

The goal is to feed formulas map.jinja with only namespaced pillars with pillar deduplication. No grains, no cross formula pillar reference, no mine.

So we want to have importable pillars containing defaults, regular pillars, mine and even grains logic. For this we use the power of jinja and salt modules to merge values in order and render the final values as yaml.

Note that modules in pillars are executed on master side. So you don't have _modules available.

# First, loads defaults from formula in jinja. This load `base:formula/defaults.yml` from `file_roots`, not pillar_roots.
{% set defaults = salt['defaults.get']('formula') -%}

# Then, loads regular pillar, namespaceless in jinja
{% load_pillar as pillars -%}
foo: 128
# You can use the hack in pillarmine module to fetch mine values from pillar.
baz: {{ salt['pillarmine.get']('func') }}
{% endload -%}
# Now, merge defaults and pillars to get final pillar
{% set pillars = salt['grains.filter_by']({
    'default': defaults,
}, merge=pillars) -%}

# Finally, render pillar namespaced.
formula:
  {{ pillars|yaml(False)|indent(2) }}

You can now reuse pillars in another pillar in these way :

# Import the yaml in jinja. You know that with the preceding logic, you have final values, including defaults.
{% import_yaml 'formula.sls' as formulap -%}
{% set formula = formulap.formula -%}

bar:
  # Reuse a value just like you do in a formula
  name: {{ formula.foo }}

You can also directly import jinja variables, but that's a new API you have to respect, and it does not work on all pillars.

{% from 'formula.sls' import pillars as formula -%}

bar:
  name: {{ formula.foo }}

@cybacolt
Copy link

regarding #11509 (comment):

incase anyone is having the same problem - if you have minions that require a significant amount of mine data, this method will timeout (saltReqTimeoutError) when the minion runs state.highstate.

until mine data becomes available in pillar, a hack is to cache the mine data on cron on the salt master. this is detailed here: #21403 (comment)

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 Core relates to code central or existential to Salt Documentation Relates to Salt documentation fixed-pls-verify fix is linked, bug author to confirm fix P3 Priority 3 Pillar severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests