diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb index b31700b07fe..dad1d517ab9 100644 --- a/app/models/runtime/process_model.rb +++ b/app/models/runtime/process_model.rb @@ -13,7 +13,7 @@ require_relative 'buildpack' module VCAP::CloudController - class ProcessModel < Sequel::Model(:processes) # rubocop:disable Metrics/ClassLength + class ProcessModel < Sequel::Model(:processes) include Serializer plugin :serialization @@ -98,16 +98,6 @@ def runnable def diego where(diego: true) end - - def buildpack_type - inner_join(BuildpackLifecycleDataModel.table_name, app_guid: :app_guid). - select_all(:processes) - end - - def non_docker_type - inner_join(BuildpackLifecycleDataModel.table_name, app_guid: :app_guid). - select_all(:processes) - end end one_through_many :organization, diff --git a/lib/cloud_controller/diego/processes_sync.rb b/lib/cloud_controller/diego/processes_sync.rb index fa358128438..a3b00f34cef 100644 --- a/lib/cloud_controller/diego/processes_sync.rb +++ b/lib/cloud_controller/diego/processes_sync.rb @@ -26,7 +26,7 @@ def sync batched_processes_for_sync do |processes| processes.each do |process| process_guid = ProcessGuid.from_process(process) - diego_lrp = diego_lrps.delete(process_guid) + diego_lrp = diego_lrps.delete(process_guid) if diego_lrp.nil? to_desire.append(process.id) @@ -143,16 +143,10 @@ def batched_processes(ids) def processes(ids) processes = ProcessModel. - diego. - runnable. where(Sequel.lit("#{ProcessModel.table_name}.id IN ?", ids)). eager(:desired_droplet, :space, :service_bindings, { routes: :domain }, { app: :buildpack_lifecycle_data }) - if FeatureFlag.enabled?(:diego_docker) - processes.select_all(ProcessModel.table_name) - else - # `select_all` is called by `non_docker_type` - processes.non_docker_type - end + + processes.select_all(ProcessModel.table_name) end def processes_for_sync(last_id) @@ -163,6 +157,12 @@ def processes_for_sync(last_id) order(:"#{ProcessModel.table_name}__id"). limit(BATCH_SIZE) + unless FeatureFlag.enabled?(:diego_docker) + non_docker_app_guids = BuildpackLifecycleDataModel.select(:app_guid). + union(CNBLifecycleDataModel.select(:app_guid)) + processes = processes.where(Sequel.qualify(ProcessModel.table_name, :app_guid) => non_docker_app_guids) + end + processes.select(:"#{ProcessModel.table_name}__id", :"#{ProcessModel.table_name}__guid", :"#{ProcessModel.table_name}__version", :"#{ProcessModel.table_name}__updated_at") end diff --git a/spec/isolated_specs/processes_sync_spec.rb b/spec/isolated_specs/processes_sync_spec.rb index bbca2b18c40..b47f5ed78e8 100644 --- a/spec/isolated_specs/processes_sync_spec.rb +++ b/spec/isolated_specs/processes_sync_spec.rb @@ -56,6 +56,42 @@ module Diego expect(bbs_apps_client).not_to have_received(:stop_app) expect(bbs_apps_client).to have_received(:bump_freshness).once end + + context 'when the process is a docker app' do + let(:app) { AppModel.make(:docker, droplet: DropletModel.make(:docker)) } + let(:good_process) { ProcessModel.make(:docker, state: 'STARTED', app: app) } + + context 'when diego_docker is enabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: true) + end + + it 'does not touch lrps that are up to date and correct' do + allow(bbs_apps_client).to receive(:desire_app) + allow(bbs_apps_client).to receive(:update_app) + allow(bbs_apps_client).to receive(:stop_app) + + subject.sync + + expect(bbs_apps_client).not_to have_received(:desire_app) + expect(bbs_apps_client).not_to have_received(:update_app) + expect(bbs_apps_client).not_to have_received(:stop_app) + expect(bbs_apps_client).to have_received(:bump_freshness).once + end + end + + context 'when diego_docker is disabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: false) + end + + it 'deletes the LRP' do + allow(bbs_apps_client).to receive(:stop_app) + subject.sync + expect(bbs_apps_client).to have_received(:stop_app).with(ProcessGuid.from_process(good_process)) + end + end + end end context 'when a diego LRP is stale' do @@ -85,6 +121,72 @@ module Diego expect(bbs_apps_client).to have_received(:bump_freshness).once end + context 'when the process is a cnb app' do + let(:app) { AppModel.make(:cnb, droplet: DropletModel.make(:cnb)) } + let!(:stale_process) { ProcessModel.make(:cnb, state: 'STARTED', app: app) } + + before do + FeatureFlag.create(name: 'diego_cnb', enabled: true) + end + + context 'when diego_docker is disabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: false) + end + + it 'updates the stale lrp' do + allow(bbs_apps_client).to receive(:update_app) + subject.sync + expect(bbs_apps_client).to have_received(:update_app).with(stale_process, stale_lrp_scheduling_info) + expect(bbs_apps_client).to have_received(:bump_freshness).once + end + end + + context 'when diego_docker is enabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: true) + end + + it 'updates the stale lrp' do + allow(bbs_apps_client).to receive(:update_app) + subject.sync + expect(bbs_apps_client).to have_received(:update_app).with(stale_process, stale_lrp_scheduling_info) + expect(bbs_apps_client).to have_received(:bump_freshness).once + end + end + end + + context 'when the process is a docker app' do + let(:app) { AppModel.make(:docker, droplet: DropletModel.make(:docker)) } + let!(:stale_process) { ProcessModel.make(:docker, state: 'STARTED', app: app) } + + context 'when diego_docker is enabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: true) + end + + it 'updates the stale lrp' do + allow(bbs_apps_client).to receive(:update_app) + subject.sync + expect(bbs_apps_client).to have_received(:update_app).with(stale_process, stale_lrp_scheduling_info) + expect(bbs_apps_client).to have_received(:bump_freshness).once + end + end + + context 'when diego_docker is disabled' do + before do + FeatureFlag.create(name: 'diego_docker', enabled: false) + end + + it 'deletes the lrp' do + allow(bbs_apps_client).to receive(:stop_app) + subject.sync + expect(bbs_apps_client).to have_received(:stop_app).with(ProcessGuid.from_process(stale_process)) + expect(bbs_apps_client).to have_received(:bump_freshness).once + end + end + end + context 'when updating app fails' do # bbs_apps_client will raise ApiErrors as of right now, we should think about factoring that out so that # the background job doesn't have to deal with API concerns diff --git a/spec/unit/models/runtime/process_model_spec.rb b/spec/unit/models/runtime/process_model_spec.rb index 40c4b7951e7..8b8d0be3bd2 100644 --- a/spec/unit/models/runtime/process_model_spec.rb +++ b/spec/unit/models/runtime/process_model_spec.rb @@ -30,23 +30,6 @@ def expect_no_validator(validator_class) VCAP::CloudController::Seeds.create_seed_stacks end - describe 'dataset module' do - let!(:buildpack_process) { ProcessModel.make } - let!(:docker_process) { ProcessModel.make(:docker) } - - describe '#buildpack_type' do - it 'only returns processes associated with a buildpack app' do - expect(ProcessModel.buildpack_type.map(&:name)).to contain_exactly(buildpack_process.name) - end - end - - describe '#non_docker_type' do - it 'only returns processes not associated with a docker app' do - expect(ProcessModel.non_docker_type.map(&:name)).to contain_exactly(buildpack_process.name) - end - end - end - describe 'Creation' do subject(:process) { ProcessModel.new }