Skip to content

Commit

Permalink
Don't set a default minimum healthy percentage of 100% anymore,
Browse files Browse the repository at this point in the history
so either the default of 90% will be used, or the percentage set
in the instance maintenance policy for the ASG.

Introduce the properties `min_healthy_percentage` and `max_healthy_percentage`
on the autoscale definitions for tuning these settings.

The property `healthy_percentage` is still supported but
will give a deprecation warning, and will be removed somewhere in the future.

Gem `aws-sdk-autoscaling` >= 1.100.0 is now required to support
setting the maximum healthy percentage.
  • Loading branch information
ppostma committed Dec 22, 2023
1 parent 78dd5fd commit 875b252
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,6 @@ RSpec/IndexedLet:

RSpec/MultipleExpectations:
Max: 5

Style/GuardClause:
Enabled: false
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ PATH
remote: .
specs:
capistrano-asg-rolling (0.4.1)
aws-sdk-autoscaling (~> 1, >= 1.67.0)
aws-sdk-autoscaling (~> 1, >= 1.100.0)
aws-sdk-ec2 (~> 1)
capistrano (~> 3)
concurrent-ruby (~> 1)
Expand Down
2 changes: 1 addition & 1 deletion capistrano-asg-rolling.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'rspec', '~> 3.0'
spec.add_development_dependency 'webmock', '~> 3.11'

spec.add_dependency 'aws-sdk-autoscaling', '~> 1', '>= 1.67.0'
spec.add_dependency 'aws-sdk-autoscaling', '~> 1', '>= 1.100.0'
spec.add_dependency 'aws-sdk-ec2', '~> 1'
spec.add_dependency 'capistrano', '~> 3'
spec.add_dependency 'concurrent-ruby', '~> 1'
Expand Down
21 changes: 16 additions & 5 deletions lib/capistrano/asg/rolling/autoscale_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ class AutoscaleGroup
def initialize(name, properties = {})
@name = name
@properties = properties

if properties[:healthy_percentage]
properties[:min_healthy_percentage] = properties.delete(:healthy_percentage)

Kernel.warn('WARNING: the property `healthy_percentage` is deprecated and will be removed in a future release. Please update to `min_healthy_percentage`.')
end
end

def exists?
Expand All @@ -42,8 +48,12 @@ def instance_warmup_time
aws_autoscaling_group.health_check_grace_period
end

def healthy_percentage
properties.fetch(:healthy_percentage, 100)
def min_healthy_percentage
properties.fetch(:min_healthy_percentage, nil)
end

def max_healthy_percentage
properties.fetch(:max_healthy_percentage, nil)
end

def start_instance_refresh(launch_template)
Expand All @@ -58,9 +68,10 @@ def start_instance_refresh(launch_template)
},
preferences: {
instance_warmup: instance_warmup_time,
min_healthy_percentage: healthy_percentage,
skip_matching: true
}
skip_matching: true,
min_healthy_percentage: min_healthy_percentage,
max_healthy_percentage: max_healthy_percentage
}.compact
).instance_refresh_id
rescue Aws::AutoScaling::Errors::InstanceRefreshInProgress => e
raise Capistrano::ASG::Rolling::InstanceRefreshFailed, e
Expand Down
36 changes: 31 additions & 5 deletions spec/capistrano/asg/rolling/autoscale_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,34 @@
end
end

describe '#healthy_percentage' do
describe '#min_healthy_percentage' do
context 'when no value is set' do
it 'returns the default healthy percentage (100)' do
expect(group.healthy_percentage).to eq(100)
it 'returns nil' do
expect(group.min_healthy_percentage).to be_nil
end
end

context 'when a value is set' do
subject(:group) { described_class.new('test-asg', healthy_percentage: 50) }
subject(:group) { described_class.new('test-asg', min_healthy_percentage: 50) }

it 'returns the value set in the configuration (50)' do
expect(group.healthy_percentage).to eq(50)
expect(group.min_healthy_percentage).to eq(50)
end
end
end

describe '#max_healthy_percentage' do
context 'when no value is set' do
it 'returns nil' do
expect(group.max_healthy_percentage).to be_nil
end
end

context 'when a value is set' do
subject(:group) { described_class.new('test-asg', max_healthy_percentage: 110) }

it 'returns the value set in the configuration (110)' do
expect(group.max_healthy_percentage).to eq(110)
end
end
end
Expand Down Expand Up @@ -108,6 +124,16 @@
expect { group.start_instance_refresh(template) }.to raise_error(Capistrano::ASG::Rolling::InstanceRefreshFailed, 'An Instance Refresh is already in progress and blocks the execution of this Instance Refresh.')
end
end

context 'when min and max healthy percentage is set' do
subject(:group) { described_class.new('test-asg', min_healthy_percentage: 50, max_healthy_percentage: 110) }

it 'calls the API to start instance refresh with the given healthy percentages' do
group.start_instance_refresh(template)
expect(WebMock).to have_requested(:post, /amazonaws.com/)
.with(body: /Action=StartInstanceRefresh&AutoScalingGroupName=test-asg&DesiredConfiguration.LaunchTemplate.LaunchTemplateId=lt-1234567890&DesiredConfiguration.LaunchTemplate.Version=1&Preferences.InstanceWarmup=300&Preferences.MaxHealthyPercentage=110&Preferences.MinHealthyPercentage=50&Preferences.SkipMatching=true&Strategy=Rolling/).once
end
end
end

describe '#instances' do
Expand Down

0 comments on commit 875b252

Please sign in to comment.