delete_on_termination=true attribute on new volume is not set on create

[gregburek]

I am using a config hash to create a new EBS volume and attach it to an existing server like so:

      new_ebs_config = {
                        'availability_zone'     => 'us-east-1d,
                        'size'                  => 20,
                        'device'                => "/dev/sdj",
                        'delete_on_termination' => true,
                        'type' => 'standard'
                        'server' => instance_id
      }
      new_volume = @ec2.volumes.create(new_ebs_config)

The delete_on_termination attribute is not being set on this new volume after it has been attached to the server. I have been using

          @ec2.modify_instance_attribute(
            @instance_id, {
              'BlockDeviceMapping.1.DeviceName'              => config['device'],
              'BlockDeviceMapping.1.Ebs.DeleteOnTermination' => config['delete_on_termination'].to_s
            }
          )

It would be nice to have it set after attach is called if server is part of the passed in config.

[geemus]

@gregburek - makes sense, I'd be happy to help you work on a pull request for it.

[karmi]

@geemus Seems like the bug is still present. Should I research it a bit and prepare a PR? We're bitten by this in the Elasticsearch Chef cookbook as linked by @abstrctn.

[geemus]

@karmi sounds great!

[karmi]

@geemus Looked around a bit:

• Seems like the proper approach would be to add some logic á la if @server and @delete_on_termination to the save or the attach method in aws/models/compute/volume.rb, which would use modify_instance_attribute as outlined above by @gregburek.
• Couple of the tests are failing when I run FOG_MOCK=yes bundle exec shindont (output)
• Are there any unit tests I might add the coverage for this addition?
• Should I add integration tests to volume_tests.rb?

[geemus]

I think that sounds reasonable. You may want to ensure you have an up-to-date checkout as we don't seem to be seeing those test failures on travis. Adding some tests would be great, integration with volume_tests would probably be sufficient. @gregburek - any further thoughts?

[gregburek]

This all sounds good to me! +1

[plribeiro3000]

@karmi You already have 2 issues related to this one: sous-chefs/elasticsearch#146 and sous-chefs/elasticsearch#172 . Do you need some help to make this fix hit master?

[plribeiro3000]

closed in favor of fog/fog-aws#6