Skip to content

Saltclass pillars inheritance issues. #48895

Description

@rbthomp

Description of Issue/Question

Merged pillars from classes overwrites pillars defined in the node declaration. It looks to me that because expand_classes_in_order() is called after pillar data defined in the node.yml gets merged anything defined in the node.yml is overwriten. It seams to me that the node.yml should over write anything defined in a class.

Would it be more controlled if we didn't merge directly into saltdata['__pillar__']and returned all pillars in the pillars_dict? It is also noticed that expanded_dict_from_minion() gets called from get_tops as well. Dose this mean pillars get merged again when calling get_tops(), because pillars are getting merged to saltdata['__pillar__'] in expanded_dict_from_minion(). Not sure if this behavior is desired, or if pillars should only be updated when calling get_pillars().

Setup

I'm working off the saltclass.py from the development branch.

Steps to Reproduce Issue

Pillars from class:

pillars:
    test:
       data: classdata

PIllars from node:

pillars:
     test:
        data: nodedata

Will result in:

pillars:
    test:
       data: classdata

My Solution

My solution to this was to merge node pillar data after calling expand_classes_in_order()

def expanded_dict_from_minion(minion_id, salt_data):
    _file = ''
    saltclass_path = salt_data['path']
    # Start
    for root, dirs, files in salt.utils.path.os_walk(os.path.join(saltclass_path, 'nodes'), followlinks=True):
        for minion_file in files:
            if minion_file == '{0}.yml'.format(minion_id):
                _file = os.path.join(root, minion_file)

    # Load the minion_id definition if existing, else an empty dict
    node_dict = {}
    if _file:
        node_dict[minion_id] = render_yaml(_file, salt_data)
    else:
        log.warning('%s: Node definition not found', minion_id)
        node_dict[minion_id] = {}

    # Get 2 ordered lists:
    # expanded_classes: A list of all the dicts
    # classes_list: List of all the classes
    expanded_classes, classes_list, states_list = expand_classes_in_order(
                                                    node_dict[minion_id],
                                                    salt_data, [], {}, [])

    # Merge newly found pillars into existing ones
    dict_merge(salt_data['__pillar__'], node_dict[minion_id].get('pillars', []))

    # Here merge the pillars together
    pillars_dict = {}
    for exp_dict in expanded_classes:
        if 'pillars' in exp_dict:
            dict_merge(pillars_dict, exp_dict)

    return expanded_classes, pillars_dict, classes_list, states_list

Versions Report

Salt Version:
Salt: 2018.3.0

Dependency Versions:
cffi: 1.5.2
cherrypy: 3.6.0
dateutil: 2.4.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.8
libgit2: 0.24.0
libnacl: Not Installed
M2Crypto: 0.21.1
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.6
mysql-python: Not Installed
pycparser: 2.10
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: 0.24.0
Python: 2.7.13 (default, Jan 11 2017, 10:56:06) [GCC]
python-gnupg: Not Installed
PyYAML: 3.12
PyZMQ: 14.0.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.0.4

System Versions:
dist: SuSE 12 x86_64
locale: UTF-8
machine: x86_64
release: 4.4.120-92.70-default
system: Linux
version: SUSE Linux Enterprise Server 12 x86_64

CC: @olivier-mauras

Metadata

Metadata

Assignees

No one assigned

    Labels

    pending-discussionThe issue or pull request needs more discussion before it can be closed or merged

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions