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

Implement includes for the mongo ext_pillar. #34566

Closed
wants to merge 2 commits into from

Conversation

@pcdummy
Copy link
Contributor

pcdummy commented Jul 8, 2016

What does this PR do?

A WIP feature which adds the possibility to include files from other pillars for the mongo ext_pillar.

Please give feedback, i have the following questions:

  • Is it usefull, something that we want in core?
  • Is there a better way to handle from which environment we include future includes instead of
    defining it per include.

This is an example of such a mongodb document that i use with includes:

{
    "_id" : "srv02.pcdummy.lan",
    "include" : [ 
        {
            "file" : "roles.base.server",
            "saltenv" : "pcdummy"
        }, 
        {
            "file" : "roles.base.lxc",
            "saltenv" : "pcdummy"
        }, 
        {
            "file" : "roles.base.postfix-relayclient",
            "saltenv" : "pcdummy"
        }
    ],
    "lxc" : {
        "default_conf" : [ 
            {
                "lxc:network:type" : "veth"
            }, 
            {
                "lxc:network:link" : "lanbr0"
            }, 
            {
                "lxc:network:flags" : "up"
            }, 
            {
                "lxc:network:hwaddr" : "00:16:3e:04:xx:xx"
            }
        ],
        "users" : {
            "lxd" : {
                "interfaces" : {
                    "lanbr0" : {
                        "type" : "veth",
                        "count" : 100
                    }
                }
            }
        }
    }
}

Tests written?

No, there are no tests for the mongo ext_pillar.

Signed-off-by: Rene Jochum rene@jochums.at

@pcdummy pcdummy force-pushed the pcdummy:mongo_include branch from 0024e51 to d1e9d64 Jul 8, 2016
@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 9, 2016

@cachedout
Copy link
Contributor

cachedout commented Jul 11, 2016

I can see how this would be useful. I would like to have documentation included in this PR though, before we merge it.

cc: @thatch45 @terminalmage

@rallytime
Copy link
Contributor

rallytime commented Jul 11, 2016

Go Go Jenkins!

if 'include' in result:

if not isinstance(result['include'], list):
msg = ('MongoDB include in Document {0!r} '

This comment has been minimized.

Copy link
@terminalmage

terminalmage Jul 11, 2016

Collaborator

Please do not use the repr flag here, change this line to:

msg = ('MongoDB include in document \'{0}\' '

This comment has been minimized.

Copy link
@pcdummy

pcdummy Jul 12, 2016

Author Contributor

done.

@pcdummy pcdummy force-pushed the pcdummy:mongo_include branch 2 times, most recently from a687eef to 6d998f4 Jul 12, 2016
Signed-off-by: Rene Jochum <rene@jochums.at>
@pcdummy pcdummy force-pushed the pcdummy:mongo_include branch from 6d998f4 to 6ad5587 Jul 12, 2016
@pcdummy pcdummy changed the title WIP: Implement includes for the mongo ext_pillar. Implement includes for the mongo ext_pillar. Jul 12, 2016
@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 12, 2016

docs added.

@pcdummy pcdummy force-pushed the pcdummy:mongo_include branch 3 times, most recently from ad05d62 to 6ad5587 Jul 12, 2016
@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 12, 2016

Have written another howto about my latest patch here:
https://rene.jochums.at/having-fun-with-saltstack-and-ext_pillar-mongo

Signed-off-by: Rene Jochum <rene@jochums.at>
@pcdummy pcdummy force-pushed the pcdummy:mongo_include branch from e6d7bf4 to 726eed6 Jul 12, 2016
@cachedout
Copy link
Contributor

cachedout commented Jul 15, 2016

@thatch45
Copy link
Member

thatch45 commented Jul 18, 2016

@pcdummy I am hesitant to bring this in, it seems to be a special use case that is very specific. I think that this should be in salt-contrib or a separate repo for now before we merge it into mainline.

@cachedout
Copy link
Contributor

cachedout commented Jul 18, 2016

I'm in the same camp as @thatch45 here. I'd rather this be maintained outside of salt-core. Bear in mind that we do pass in the compiled pillar into ext pillars, which is how the majority of these use cases are handled: https://docs.saltstack.com/en/latest/topics/development/external_pillars.html#ext-pillar

@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 18, 2016

@thatch45 @cachedout Fine, will bring it in a extra repo and include over gitfs. 👍

@pcdummy pcdummy closed this Jul 18, 2016
@cachedout
Copy link
Contributor

cachedout commented Jul 18, 2016

@pcdummy Thanks for understanding. Much appreciated. :]

@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 18, 2016

Have to ask again, whats about my first patch @cachedout @thatch45 6ad5587

Can this get included into salt-core ?

@pcdummy pcdummy reopened this Jul 18, 2016
@cachedout
Copy link
Contributor

cachedout commented Jul 19, 2016

@pcdummy We would prefer to have it in contrib.

@pcdummy
Copy link
Contributor Author

pcdummy commented Jul 19, 2016

ok :)

@pcdummy pcdummy closed this Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.