Skip to content

Recreate Deployment Strategy #4396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/actions/deployment_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,11 @@ def starting_process_instances(deployment, desired_instances)
else
desired_instances
end

[deployment.max_in_flight, starting_process_count].min
if deployment.strategy == DeploymentModel::RECREATE_STRATEGY
starting_process_count
else
[deployment.max_in_flight, starting_process_count].min
end
end

def log_rollback_event(app_guid, user_id, revision_id, strategy, max_in_flight, canary_steps)
Expand Down
6 changes: 5 additions & 1 deletion app/messages/deployment_create_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DeploymentCreateMessage < MetadataBaseMessage

validates_with NoAdditionalKeysValidator
validates :strategy,
inclusion: { in: %w[rolling canary], message: "'%<value>s' is not a supported deployment strategy" },
inclusion: { in: %w[rolling canary recreate], message: "'%<value>s' is not a supported deployment strategy" },
allow_nil: true
validate :mutually_exclusive_droplet_sources

Expand Down Expand Up @@ -113,6 +113,10 @@ def validate_scaling_options

def validate_max_in_flight
max_in_flight = options[:max_in_flight]
if max_in_flight && strategy == 'recreate'
errors.add(:'options.max_in_flight', 'is not a supported option for recreate deployment strategy')
return
end

return unless !max_in_flight.is_a?(Integer) || max_in_flight < 1

Expand Down
3 changes: 2 additions & 1 deletion app/models/runtime/deployment_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class DeploymentModel < Sequel::Model(:deployments)

DEPLOYMENT_STRATEGIES = [
ROLLING_STRATEGY = 'rolling'.freeze,
CANARY_STRATEGY = 'canary'.freeze
CANARY_STRATEGY = 'canary'.freeze,
RECREATE_STRATEGY = 'recreate'.freeze
].freeze

PROGRESSING_STATES = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Name | Type | Description
**status.details.error** | _string_ | Brief description of error encountered while deploying, if any. This field is cleared once the deployment progresses successfully.
**status.canary.steps.current** | _integer_ | The current canary step. Only available for deployments with strategy 'canary'. (experimental)
**status.canary.steps.total** | _integer_ | The total number of canary steps. Only available for deployments with strategy 'canary'. (experimental)
**strategy** | _string_ | Strategy used for the deployment; supported strategies are `rolling` and `canary` (experimental)
**strategy** | _string_ | Strategy used for the deployment; supported strategies are `rolling`, `canary` and `recreate` (experimental)
**options.max_in_flight** | _integer_ | The maximum number of new instances to deploy simultaneously
**options.web_instances** | _integer_ | The number of web instances the deployment will scale to
**options.memory_in_mb** | _integer_ | The amount of memory in megabytes to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process.
Expand Down
86 changes: 86 additions & 0 deletions lib/cloud_controller/deployment_updater/actions/recreate.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
require 'cloud_controller/deployment_updater/actions/scale_down_canceled_processes'
require 'cloud_controller/deployment_updater/actions/finalize'
require 'cloud_controller/deployment_updater/actions/down_scaler'
require 'cloud_controller/deployment_updater/actions/up_scaler'
require 'cloud_controller/diego/constants'

module VCAP::CloudController
module DeploymentUpdater
module Actions
class Recreate
attr_reader :deployment, :logger, :app, :target_total_instance_count, :interim_desired_instance_count

def initialize(deployment, logger, target_total_instance_count, interim_desired_instance_count=nil)
@deployment = deployment
@logger = logger
@app = deployment.app
@target_total_instance_count = target_total_instance_count
@interim_desired_instance_count = interim_desired_instance_count || target_total_instance_count
end

def call
logger.info("RECREATE Starting down scaler #{deployment.guid}")
down_scaler = DownScaler.new(deployment, logger, target_total_instance_count, instance_count_summary.routable_instances_count)
logger.info("RECREATE starting db transaction for #{deployment.guid}")
deployment.db.transaction do
return unless [DeploymentModel::DEPLOYING_STATE, DeploymentModel::PREPAUSED_STATE].include?(deployment.lock!.state)
return unless can_scale? || down_scaler.can_downscale?

logger.info("RECREATE lock the app for #{deployment.guid}")
app.lock!
logger.info("RECREATE lock the web_processes for #{deployment.guid}")
app.web_processes.each(&:lock!)
logger.info("RECREATE set status to active/deploying for #{deployment.guid}")
deployment.update(
status_value: DeploymentModel::ACTIVE_STATUS_VALUE,
status_reason: DeploymentModel::DEPLOYING_STATUS_REASON,
error: nil
)
logger.info("RECREATE scale down canceled processes for #{deployment.guid}")
ScaleDownCanceledProcesses.new(deployment).call
logger.info("RECREATE scale down web processes for #{deployment.guid}")
down_scaler.scale_down if down_scaler.can_downscale?
logger.info("are we finished scaling for #{deployment.guid}")
return true if finished_scaling?

logger.info("not finished scaling, scaling up web processes for #{deployment.guid}")
scale_up if can_scale?
end
false
rescue CloudController::Errors::ApiError # the instances_reporter re-raises InstancesUnavailable as ApiError
logger.info("skipping-deployment-update-for-#{deployment.guid}")
false
end

private

def scale_up
return unless can_scale?

deploying_web_process.update(instances: desired_new_instances)
deployment.update(last_healthy_at: Time.now)
end

def instance_count_summary
@instance_count_summary ||= instance_reporters.instance_count_summary(deploying_web_process)
end

def deploying_web_process
@deploying_web_process ||= deployment.deploying_web_process
end

def instance_reporters
CloudController::DependencyLocator.instance.instances_reporters
end

def can_scale
deploying_web_process.instances < interim_desired_instance_count && @routable_instances_count < interim_desired_instance_count
end

def finished_scaling
deploying_web_process.instances >= interim_desired_instance_count && @routable_instances_count >= interim_desired_instance_count
end
end
end
end
end
2 changes: 2 additions & 0 deletions lib/cloud_controller/deployment_updater/actions/scale.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ def initialize(deployment, logger, target_total_instance_count, interim_desired_
end

def call
logger.info("Starting down scaler #{deployment.guid}")
down_scaler = DownScaler.new(deployment, logger, target_total_instance_count, instance_count_summary.routable_instances_count)
logger.info("Starting up scaler #{deployment.guid}")
up_scaler = UpScaler.new(deployment, logger, interim_desired_instance_count, instance_count_summary)

deployment.db.transaction do
Expand Down
8 changes: 7 additions & 1 deletion lib/cloud_controller/deployment_updater/updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ def initialize(deployment, logger)

def scale
with_error_logging('error-scaling-deployment') do
finished = Actions::Scale.new(deployment, logger, deployment.desired_web_instances).call
if deployment.strategy == DeploymentModel::RECREATE_STRATEGY
logger.info("recreating deployment for -#{deployment.guid}")
finished = Actions::Recreate.new(deployment, logger, deployment.desired_web_instances).call
else
finished = Actions::Scale.new(deployment, logger, deployment.desired_web_instances).call
end

Actions::Finalize.new(deployment).call if finished
logger.info("ran-deployment-update-for-#{deployment.guid}")
end
Expand Down
59 changes: 59 additions & 0 deletions spec/unit/actions/deployment_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ module VCAP::CloudController
})
end

let(:recreate_message) do
DeploymentCreateMessage.new({
relationships: { app: { data: { guid: app.guid } } },
droplet: { guid: next_droplet.guid },
strategy: 'recreate',
options: { web_instances:, memory_in_mb:, disk_in_mb:, log_rate_limit_in_bytes_per_second: }
})
end

before do
app.update(droplet: original_droplet)
end
Expand Down Expand Up @@ -350,6 +359,32 @@ module VCAP::CloudController
).to contain_exactly(deployment.deploying_web_process.type)
end

it 'when the strategy is recreate' do
deployment = DeploymentCreate.create(app: app, message: recreate_message, user_audit_info: user_audit_info)
event = VCAP::CloudController::Event.find(type: 'audit.app.deployment.create')
expect(event).not_to be_nil
expect(event.actor).to eq('123')
expect(event.actor_type).to eq('user')
expect(event.actor_name).to eq('[email protected]')
expect(event.actor_username).to eq('braa')
expect(event.actee).to eq(app.guid)
expect(event.actee_type).to eq('app')
expect(event.actee_name).to eq(app.name)
expect(event.timestamp).to be
expect(event.space_guid).to eq(app.space_guid)
expect(event.organization_guid).to eq(app.space.organization.guid)
expect(event.metadata).to eq({
'droplet_guid' => next_droplet.guid,
'deployment_guid' => deployment.guid,
'type' => nil,
'revision_guid' => app.latest_revision.guid,
'request' => message.audit_hash,
'strategy' => 'recreate'
})

expect(deployment.deploying_web_process.instances).to eq(2)
end

context 'when the app does not have a droplet set' do
let(:app_without_current_droplet) { AppModel.make }
let(:next_droplet) { DropletModel.make(app: app_without_current_droplet, process_types: { 'web' => 'asdf' }) }
Expand Down Expand Up @@ -1509,6 +1544,30 @@ module VCAP::CloudController
end
end

context 'when the strategy is recreate' do
let(:strategy) { 'recreate' }

it 'creates the deployment with the recreate strategy' do
deployment = nil

expect do
deployment = DeploymentCreate.create(app:, message:, user_audit_info:)
end.to change(DeploymentModel, :count).by(1)

expect(deployment.strategy).to eq(DeploymentModel::RECREATE_STRATEGY)
end

it 'sets the deployment state to DEPLOYING' do
deployment = nil

expect do
deployment = DeploymentCreate.create(app:, message:, user_audit_info:)
end.to change(DeploymentModel, :count).by(1)

expect(deployment.state).to eq(DeploymentModel::DEPLOYING_STATE)
end
end

context 'when the strategy is nil' do
let(:strategy) { nil }

Expand Down
27 changes: 27 additions & 0 deletions spec/unit/messages/deployment_create_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ module VCAP::CloudController
expect(message).to be_valid
end

it 'can be recreate' do
body['strategy'] = 'recreate'
message = DeploymentCreateMessage.new(body)
expect(message).to be_valid
end

it 'is valid with nil strategy' do
body['strategy'] = nil
message = DeploymentCreateMessage.new(body)
Expand Down Expand Up @@ -125,6 +131,19 @@ module VCAP::CloudController
expect(message).to be_valid
end
end

context 'when set with recreate strategy' do
before do
body['options'] = { max_in_flight: 2 }
end

it 'is not valid' do
body['strategy'] = 'recreate'
message = DeploymentCreateMessage.new(body)
expect(message).not_to be_valid
expect(message.errors.full_messages).to include('Options max in flight is not a supported option for recreate deployment strategy')
end
end
end

describe 'web_instances' do
Expand Down Expand Up @@ -372,6 +391,14 @@ module VCAP::CloudController
expect(message.errors[:'options.canary']).to include('are only valid for Canary deployments')
end

it 'errors when strategy is set to recreate' do
body['options'] = { canary: {} }
body['strategy'] = 'recreate'
message = DeploymentCreateMessage.new(body)
expect(message).not_to be_valid
expect(message.errors[:'options.canary']).to include('are only valid for Canary deployments')
end

it 'errors when there is an unknown option' do
body['options'] = { foo: 'bar', baz: 'boo' }
message = DeploymentCreateMessage.new(body)
Expand Down
11 changes: 11 additions & 0 deletions spec/unit/presenters/v3/deployment_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,17 @@ module VCAP::CloudController::Presenters::V3
end
end
end

context 'when the strategy is recreate' do
before do
deployment.strategy = VCAP::CloudController::DeploymentModel::RECREATE_STRATEGY
end

it 'shows no canary status' do
result = DeploymentPresenter.new(deployment).to_hash
expect(result[:status][:canary]).to be_nil
end
end
end

describe 'options' do
Expand Down
Loading