Skip to content

add salt cloud support for block device tagging or root devices and such#48716

Merged
rallytime merged 11 commits intosaltstack:developfrom
thetoolsmith:develop
Jul 25, 2018
Merged

add salt cloud support for block device tagging or root devices and such#48716
rallytime merged 11 commits intosaltstack:developfrom
thetoolsmith:develop

Conversation

@thetoolsmith
Copy link
Copy Markdown
Contributor

This pull request adds the ability to set block device tags in salt cloud configuration for root volumes and such.

Currently, it seems that this is not supported in salt-cloud.
ref: #47097

This change allows for configuration of block_device_mappings in cloud configuration to support a tag dictionary like so:

  block_device_mappings:
    - DeviceName: /dev/sda1
       Ebs.VolumeSize: 10
       Ebs.VolumeType: gp2
       tag:    
         tag1: value
         tag2: value
    - DeviceName: /dev/sdb
       Ebs.VolumeSize: 35
       Ebs.VolumeType: gp2
       tag:    
         tag3: value
         tag4: value

Tests written?

No

Commits signed with GPG?

No

Additional notes:
I also have another implementation that does not use the existing block_device_mappings configuration. Instead it would add a new cloud configuration block_device_tags. However, this change separates the mappings from the tags and might not be ideal.
I'm thinking this pr is the best approach, but will look for feedback from the core Salt team.

Please review Salt's Contributing Guide for best practices.

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

@ghost ghost self-requested a review July 23, 2018 17:03
@thetoolsmith
Copy link
Copy Markdown
Contributor Author

build kicked off before I committed the corrected code.

@rallytime
Copy link
Copy Markdown
Contributor

@thetoolsmith
Copy link
Copy Markdown
Contributor Author

yeah, sorry about that. I pasted the code into the salt branch from my test system and missed the closing bracket. should have re-checked the code.
fix is committed.

Copy link
Copy Markdown
Contributor

@rallytime rallytime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the inline requests, can you also document this new behavior?

Thanks @thetoolsmith!

Comment thread salt/cloud/clouds/ec2.py Outdated
block_device_volume_id_map = {}

if ex_blockdevicetags:
for k, v in six.iteritems(ret['blockDeviceMapping']):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you switch these variables (and others below) to use 3 or more letters? That would be nice and match our Style Guide rules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure thing

Comment thread salt/cloud/clouds/ec2.py Outdated
transport=__opts__['transport']
)

salt.utils.cloud.wait_for_fun(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be the __utils__['cloud.x'] notation like you have above.

@thetoolsmith
Copy link
Copy Markdown
Contributor Author

changes have been comitted

@rallytime rallytime requested a review from gtmanfred July 25, 2018 15:00
@rallytime rallytime merged commit 816f7ed into saltstack:develop Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants