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

salt-cloud: allow map nodes to override provider #47648

Merged
merged 2 commits into from May 17, 2018

Conversation

Projects
None yet
3 participants
@mattp-
Copy link
Contributor

commented May 14, 2018

What does this PR do?

due to the way salt-cloud was previously pulling provider from only the profile,
it made it impossible to use map node overrides to override a provider. This
patch makes that possible.
The main usecase for this is having profiles that model your node type (db,
webapp, etc), without having to duplicate across providers. Your map can then
specify provider specific overrides whilst keeping profiles unique and DRY:

webapp:
  - node1:
    provider: ny:openstack
  - node2:
    provider: nj:openstack
  - node3:
    provider: toronto:openstack

etc.

What issues does this PR fix or reference?

fixes #45125
although not a direct solution, this could be used to solve the problems described in #40897 in a different way.

Previous Behavior

provider in map files were ignored

New Behavior

provider override in map file works as expected

Tests written?

Yes

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@salt-jenkins salt-jenkins requested a review from saltstack/team-cloud May 14, 2018

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented May 15, 2018

If you have a second to look at this too, that would be greatly appreciated

#45125

Thanks,
Daniel

@mattp-

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2018

@gtmanfred I'm pretty sure that bug is caused by https://github.com/bloomberg/salt/blob/4b8707054a8986899bc4530c843ad1f9e8fb8150/salt/cloud/__init__.py#L1951 not using salt.utils.dictupdate.update. I'll try to confirm/test tomorrow if I can find some spare cycles.

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented May 15, 2018

That would be greatly appreciated!

@wes-novack if you had a change to test matt's comment above and see if that fixes the bug for you, feedback would be greatly appreciated.

@rallytime
Copy link
Contributor

left a comment

Awesome!

@mattp- mattp- force-pushed the bloomberg:provider-override branch from 6b55131 to 9707b42 May 15, 2018

@mattp-

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2018

@gtmanfred my hunch was correct, updated to deepcopy and found a bug in the process. #45125 should be fixed with this PR now as well (with tests).

@mattp-

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2018

I will say though.. im not super familiar with salt-cloud. I'm unsure if there are any unintended consequences in just recursve-merging from provider->profile->map, but it works as described in #45125 now.

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented May 15, 2018

That is what we want, we want the map to override the pillar, to override the provider.

mattp- added some commits May 14, 2018

salt-cloud: allow map nodes to override provider
due to the way salt-cloud was previously pulling provider from only the profile,
it made it impossible to use map node overrides to override a provider. This
patch makes that possible.
The main usecase for this is having profiles that model your node type (db,
webapp, etc), without having to duplicate across providers. Your map can then
specify provider specific overrides whilst keeping profiles unique and DRY:

webapp:
  - node1:
    provider: ny:openstack
  - node2:
    provider: nj:openstack
  - node3:
    provider: toronto:openstack

etc.
salt.cloud: use salt.utils.dictupdate.update for provider
shallow copy was blasting things like minion_master in provider when grains
were provided in map and profile.

additionally noticed a bug while adding tests that map_data() is destructive on
self.rendered_map; we now deepcopy before map_data does its thing.

@mattp- mattp- force-pushed the bloomberg:provider-override branch from d985ee7 to 3dc821f May 16, 2018

@gtmanfred gtmanfred merged commit 567395f into saltstack:develop May 17, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #4955 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19012 — ABORTED
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #22889 — FAILURE
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #9925 — FAILURE
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25142 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17242 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #21872 — SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.