App state is not updating in cf cli and appsman ui when the app is down (#4309)

* App state is not updating in cf cli and appsman ui when the app is down

In case of duplicate actual_lrp events(same index), CAPI should look at the since and pick the latest

* App state is not updating in cf cli and appsman ui when the app is down

Fix Rubocop error

* App state is not updating in cf cli and appsman ui when the app is down

Remove empty lines

* App state is not updating in cf cli and appsman ui when the app is down

Remove empty line

* App state is not updating in cf cli and appsman ui when the app is down

Code review changes

Author: nookala

lib/cloud_controller/diego/reporters/instances_stats_reporter.rb

@@ -45,14 +45,22 @@ def get_stats(desired_lrp, process)
       def actual_lrp_info(process, stats=nil, quota_stats=nil, log_cache_errors=nil, isolation_segment=nil, state=nil)
         # rubocop:enable Metrics/ParameterLists
         result = {}
+        lrp_instances = {}
+
         bbs_instances_client.lrp_instances(process).each do |actual_lrp|
           next unless actual_lrp.actual_lrp_key.index < process.instances
 
-          lrp_state = state || LrpStateTranslator.translate_lrp_state(actual_lrp)
+          # if an LRP already exists with the same index use the one with the latest since value
+          if lrp_instances.include?(actual_lrp.actual_lrp_key.index)
+            existing_lrp = lrp_instances[actual_lrp.actual_lrp_key.index]
+            next if actual_lrp.since < existing_lrp.since
+          end
 
+          lrp_state = state || LrpStateTranslator.translate_lrp_state(actual_lrp)
           info = build_info(lrp_state, actual_lrp, process, stats, quota_stats, log_cache_errors)
           info[:isolation_segment] = isolation_segment unless isolation_segment.nil?
           result[actual_lrp.actual_lrp_key.index] = info
+          lrp_instances[actual_lrp.actual_lrp_key.index] = actual_lrp
         end
 
         fill_unreported_instances_with_down_instances(result, process, flat: false)

spec/unit/lib/cloud_controller/diego/reporters/instances_stats_reporter_spec.rb

@@ -149,6 +149,61 @@ def make_actual_lrp(instance_guid:, index:, state:, error:, since:)
           end
         end
 
+        context 'when there are multiple lrps with the same index' do
+          let(:desired_instances) { 3 }
+          let(:bbs_actual_lrps_response) { [actual_lrp_1, actual_lrp_2, actual_lrp_3, actual_lrp_4, actual_lrp_5] }
+          let(:actual_lrp_1) do
+            make_actual_lrp(
+              instance_guid: '', index: 0, state: ::Diego::ActualLRPState::UNCLAIMED, error: 'some-details', since: two_days_ago_since_epoch_ns
+            ).tap do |actual_lrp|
+              actual_lrp.actual_lrp_net_info = lrp_1_net_info
+            end
+          end
+          let(:actual_lrp_2) do
+            make_actual_lrp(
+              instance_guid: 'instance-a', index: 0, state: ::Diego::ActualLRPState::RUNNING, error: 'some-details', since: two_days_ago_since_epoch_ns - 1000
+            ).tap do |actual_lrp|
+              actual_lrp.actual_lrp_net_info = lrp_1_net_info
+            end
+          end
+
+          let(:actual_lrp_3) do
+            make_actual_lrp(
+              instance_guid: '', index: 1, state: ::Diego::ActualLRPState::UNCLAIMED, error: 'some-details', since: two_days_ago_since_epoch_ns
+            ).tap do |actual_lrp|
+              actual_lrp.actual_lrp_net_info = lrp_1_net_info
+            end
+          end
+
+          let(:actual_lrp_4) do
+            make_actual_lrp(
+              instance_guid: 'instance-b', index: 1, state: ::Diego::ActualLRPState::CLAIMED, error: 'some-details', since: two_days_ago_since_epoch_ns - 1000
+            ).tap do |actual_lrp|
+              actual_lrp.actual_lrp_net_info = lrp_1_net_info
+            end
+          end
+
+          let(:actual_lrp_5) do
+            make_actual_lrp(
+              instance_guid: 'instance-c', index: 2, state: ::Diego::ActualLRPState::RUNNING, error: 'some-details', since: two_days_ago_since_epoch_ns
+            ).tap do |actual_lrp|
+              actual_lrp.actual_lrp_net_info = lrp_1_net_info
+            end
+          end
+
+          before do
+            allow(bbs_instances_client).to receive_messages(lrp_instances: bbs_actual_lrps_response, desired_lrp_instance: bbs_desired_lrp_response)
+          end
+
+          it 'shows all correct state for all instances' do
+            result, = instances_reporter.stats_for_app(process)
+            expect(result.length).to eq(3)
+            expect(result[0][:state]).to eq('DOWN')
+            expect(result[1][:state]).to eq('DOWN')
+            expect(result[2][:state]).to eq('RUNNING')
+          end
+        end
+
         context 'when a NoRunningInstances error is thrown for desired_lrp and it exists an actual_lrp' do
           let(:error) { CloudController::Errors::NoRunningInstances.new('No running instances ruh roh') }
           let(:expected_stopping_response) do