Addedd disable_api_termination option along with RSpecs. #427
Conversation
|
Done. |
| @@ -1,2 +1,4 @@ | |||
| source 'https://rubygems.org' | |||
| gemspec | |||
|
|
|||
| gem 'rack', '1.6.4' | |||
There was a problem hiding this comment.
Generally we want to use the most optimistic constraint that we can and not pin to a specific version in the Gemfile. Here is how we're handling the rack/ruby version problem in chef:
gem "rack", "< 2.0" # 2.0 requires Ruby 2.2+
By the way, we are starting soon on getting Chef on Ruby 2.2+
There was a problem hiding this comment.
right...will make the change
|
Is it possible to make disable_api_termination=true the default for an AWS account? If so, would using knife ec2 without knowing about this new feature start overriding that and setting disable_api_termination=false for those people unexpectedly? If not, 👍 on this PR as is. |
|
@btm The |
| @@ -1054,6 +1065,9 @@ def create_server_def | |||
| server_def[:block_device_mapping] = (server_def[:block_device_mapping] || []) << {'VirtualName' => "ephemeral#{i}", 'DeviceName' => device_name} | |||
| end | |||
|
|
|||
| ## Termination Protection cannot be enabled for spot instances ## | |||
| server_def[:disable_api_termination] = locate_config_value(:disable_api_termination) if locate_config_value(:spot_price).nil? | |||
There was a problem hiding this comment.
I'd drop if locate_config_value(:spot_price).nil? here. The check above should prevent this from being needed. If it didn't, this would cause odd behavior; the disable api termination feature would not be enabled even if set but we wouldn't tell the user that.
There was a problem hiding this comment.
@btm this check is just to prevent disable-api-termination option (irrespective of its value) from being getting passed in the server_definition of spot instance request as spot instance does not support this feature. The disable-api-termination option will only get passed in the server_definition of non-spot instance request. Also, even if user set this option disable-api-termination in a spot instance request then he/she will get error as spot-price and disable-api-termination options cannot be passed together as 'Termination Protection' cannot be enabled for spot instances.. You can check this validation code here.
There was a problem hiding this comment.
Okay. Might add clarity to set the comment to something like "Cannot pass disable_api_termination to API when using spot instances".
There was a problem hiding this comment.
sure...will change the comment.
|
👍 |
…stall failure on ruby version less than 2.2.
…uest with disable_api_termination option as termination protection cannot be enabled for spot instances.
…on option against the spot instance request for better clarity.
NimishaS
force-pushed
the
ali/add_disable_api_termination_option
branch
from
ca0dad4 to
b56f6d6
Compare
In-progress.