diff --git a/app/actions/droplet_create.rb b/app/actions/droplet_create.rb index ff3945615d9..603560051ad 100644 --- a/app/actions/droplet_create.rb +++ b/app/actions/droplet_create.rb @@ -49,11 +49,24 @@ def create_docker_droplet(build) droplet end - def create_buildpack_droplet(build) - droplet = droplet_from_build(build) - + def find_or_create_buildpack_droplet(build) + droplet = nil DropletModel.db.transaction do + existing = DropletModel.where(build_guid: build.guid).first + if existing + Steno.logger('droplet_existing').info("existing droplet found: #{existing.guid}") + if build.cnb_lifecycle? + existing.cnb_lifecycle_data ||= build.cnb_lifecycle_data + else + existing.buildpack_lifecycle_data ||= build.buildpack_lifecycle_data + end + existing.save_changes + return existing + end + + droplet = droplet_from_build(build) droplet.save + if build.cnb_lifecycle? droplet.cnb_lifecycle_data = build.cnb_lifecycle_data else diff --git a/app/controllers/runtime/stagings_controller.rb b/app/controllers/runtime/stagings_controller.rb index aa5400af27b..303db1b4cef 100644 --- a/app/controllers/runtime/stagings_controller.rb +++ b/app/controllers/runtime/stagings_controller.rb @@ -134,7 +134,7 @@ def droplet_from_build(build, guid) end def create_droplet_from_build(build) - DropletCreate.new.create_buildpack_droplet(build) + DropletCreate.new.find_or_create_buildpack_droplet(build) end def upload_path diff --git a/spec/unit/actions/droplet_create_spec.rb b/spec/unit/actions/droplet_create_spec.rb index 93f5584f553..ff40470b743 100644 --- a/spec/unit/actions/droplet_create_spec.rb +++ b/spec/unit/actions/droplet_create_spec.rb @@ -158,13 +158,13 @@ module VCAP::CloudController end end - describe '#create_buildpack_droplet' do + describe '#find_or_create_buildpack_droplet' do context 'buildpack lifecycle' do let!(:buildpack_lifecycle_data) { BuildpackLifecycleDataModel.make(build:) } it 'sets it on the droplet' do expect do - droplet_create.create_buildpack_droplet(build) + droplet_create.find_or_create_buildpack_droplet(build) end.to change { [DropletModel.count, Event.count] }.by([1, 1]) droplet = DropletModel.last @@ -195,6 +195,53 @@ module VCAP::CloudController }) end + it 'creates the droplet once and attaches lifecycle data' do + expect do + droplet_create.find_or_create_buildpack_droplet(build) + end.to change { [DropletModel.count, Event.count] }.by([1, 1]) + + droplet = DropletModel.last + expect(droplet.state).to eq(DropletModel::STAGING_STATE) + expect(droplet.app).to eq(app) + expect(droplet.package).to eq(package) + expect(droplet.build).to eq(build) + + buildpack_lifecycle_data.reload + expect(buildpack_lifecycle_data.droplet).to eq(droplet) + end + + it 'is idempotent on repeated calls (no duplicate droplet, same GUID, no extra event)' do + first = droplet_create.find_or_create_buildpack_droplet(build) + + expect do + second = droplet_create.find_or_create_buildpack_droplet(build) + expect(second.guid).to eq(first.guid) + end.not_to(change { [DropletModel.count, Event.count] }) + + # Ensure still only one droplet for this build + expect(DropletModel.where(build_guid: build.guid).count).to eq(1) + + buildpack_lifecycle_data.reload + expect(buildpack_lifecycle_data.droplet).to eq(first) + end + + it 'returns the pre-existing droplet when one already exists for the build' do + existing = DropletModel.make( + app: app, + package: package, + build: build, + state: DropletModel::STAGING_STATE + ) + buildpack_lifecycle_data.update(droplet: existing) + + expect do + returned = droplet_create.find_or_create_buildpack_droplet(build) + expect(returned.guid).to eq(existing.guid) + end.not_to(change { [DropletModel.count, Event.count] }) + + expect(DropletModel.where(build_guid: build.guid).first.guid).to eq(existing.guid) + end + context 'when the build does not contain created_by fields' do let(:build) do BuildModel.make( @@ -205,7 +252,7 @@ module VCAP::CloudController it 'sets the actor to UNKNOWN' do expect do - droplet_create.create_buildpack_droplet(build) + droplet_create.find_or_create_buildpack_droplet(build) end.to change { [DropletModel.count, Event.count] }.by([1, 1]) droplet = DropletModel.last @@ -230,7 +277,7 @@ module VCAP::CloudController it 'sets it on the droplet' do expect do - droplet_create.create_buildpack_droplet(build) + droplet_create.find_or_create_buildpack_droplet(build) end.to change { [DropletModel.count, Event.count] }.by([1, 1]) droplet = DropletModel.last @@ -271,7 +318,7 @@ module VCAP::CloudController it 'sets the actor to UNKNOWN' do expect do - droplet_create.create_buildpack_droplet(build) + droplet_create.find_or_create_buildpack_droplet(build) end.to change { [DropletModel.count, Event.count] }.by([1, 1]) droplet = DropletModel.last