Addedd disable_api_termination option along with RSpecs.#427
Conversation
|
Done. |
| 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 |
| 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
4 participants
In-progress.