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

Support in data merging strategy for salt.modules.config.get like in pillar.stack #56187

Closed
baby-gnu opened this issue Feb 18, 2020 · 9 comments
Labels
info-needed waiting for more info
Milestone

Comments

@baby-gnu
Copy link

Description of Issue

salt.modules.config.get support merging strategy at call time with the merge parameter.

This is great for sls writer to decide how to use things but I think it should be better to make it a data driven behaviour like it's done in pillar.stack

This could be implemented at several levels but as the salt.modules.config.get use salt.utils.dictupdate.merge (using salt.utils.dictupdate.update for the recursive and overwrite strategies) it could be interesting to implement this in salt.utils.dictupdate for salt.slsutil to benefit from it.

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
           Salt: 3000
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.0
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 3.6.9 (default, Nov  7 2019, 10:44:02)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-74-generic
         system: Linux
        version: Ubuntu 18.04 bionic
@baby-gnu baby-gnu changed the title Support in data merging strategy for salt.modules.config.get like in pillar.stack Support in data merging strategy for salt.modules.config.get like in pillar.stack Feb 18, 2020
@waynew waynew added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Feb 19, 2020
@waynew waynew added this to the Blocked milestone Feb 19, 2020
@waynew
Copy link
Contributor

waynew commented Feb 19, 2020

Hi @baby-gnu thanks for the request - I've added it to our core team agenda and should be able to get back to you on Friday!

@waynew
Copy link
Contributor

waynew commented Feb 21, 2020

Hey @baby-gnu - could you elaborate on what you actually need here?

Typically configs should be relatively static and straightforward. Most dynamic behaviors can be specified with data in pillars.

What is the killer use-case here that can't be accomplished with pillars?

@waynew waynew added info-needed waiting for more info and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Feb 21, 2020
@baby-gnu
Copy link
Author

Hello @waynew.

What is the killer use-case here that can't be accomplished with pillars?

My feature request is to add merging capabilities to pillars (and anything queryable by salt.modules.config.get) the same way the pillar specific pillar.stack does.

When writing formulas, the author can choose the merging strategy of the salt.modules.config.get calls but then the user must override some formula files to choose something else.

I want to be able to define fine-grained merging strategies for my config, pillars, grains, … directly in the data (not in the code using the merge parameter of salt.modules.config.get).

For the context, in the formulas community, we are looking for minimizing the use of pillars since it can be quite costly on the master (this is a common complain against the formulas which abuse of pillars).

This formula proposal use salt.slsutil.merge to merge the values from different YAML files and it support a per YAML merging strategy with the file top level key strategy to configure salt.slsutil.merge, for example:

strategy: recurse
merge_list: true
values:
  key1: value1
  key2:
    subkey2:
      - 1
      - 2
      - 3

This feature request ask for the possiblity to define pillars, grains, etc. like:

__: merge-first
key1: value1
key2:
  subkey2:
    - '__': 'merge-last'
    - 1
    - 2
    - 3

With more fine-grained merging policies.

I think it's better to implement this in salt.utils.dictupdate to make it useful to salt.slsutil.merge too (and we could use it in our proposal)

Regards.

@stale
Copy link

stale bot commented Mar 22, 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 Mar 22, 2020
@baby-gnu
Copy link
Author

Do you need more information for this feature request?

@stale
Copy link

stale bot commented Mar 23, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Mar 23, 2020
@waynew
Copy link
Contributor

waynew commented Apr 15, 2020

@baby-gnu Sorry for the wait, things have been a bit crazy this past month for everyone. I'll make sure it's on our agenda for this week!

@baby-gnu
Copy link
Author

baby-gnu commented Oct 2, 2020

Thanks @waynew, seems things had been really crazy ;-)

@garethgreenaway
Copy link
Contributor

A similar feature request to #61141 which we've determined we do not want to support at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed waiting for more info
Projects
None yet
Development

No branches or pull requests

3 participants