Merge branch 'fix-deploy-board-multi-deployment-pods' into 'master'

Remove Duplicate Instances from Deploy Board

See merge request gitlab-org/gitlab!40768

config/feature_flags/development/deploy_boards_dedupe_instances.yml

+---
+name: deploy_boards_dedupe_instances
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40768
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/258214
+type: development
+group: group::progressive-delivery
+default_enabled: false

doc/user/project/deploy_boards.md

@@ -42,8 +42,9 @@ knowledge. In particular, you should be familiar with:
 - [Kubernetes canary deployments](https://kubernetes.io/docs/concepts/cluster-administration/manage-deployment/#canary-deployments)
 
 NOTE: **Note:**
-Apps that consist of multiple deployments are shown as duplicates on the deploy board.
-Follow [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/8463) for details.
+In GitLab 13.4 and earlier, apps that consist of multiple deployments are shown as
+duplicates on the deploy board. This is [fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/8463)
+in GitLab 13.5.
 
 ## Use cases
 

ee/changelogs/unreleased/fix-deploy-board-multi-deployment-pods.yml

+---
+title: Fix duplicate instances in deploy boards when multiple deployments have the same track
+merge_request: 40768
+author:
+type: fixed

ee/lib/gitlab/kubernetes/rollout_instances.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    class RolloutInstances
+      include ::Gitlab::Utils::StrongMemoize
+
+      def initialize(deployments, pods)
+        @deployments = deployments
+        @pods = pods
+      end
+
+      def pod_instances
+        pods = matching_pods + extra_pending_pods
+
+        pods.sort_by(&:order).map do |pod|
+          to_hash(pod)
+        end
+      end
+
+      private
+
+      attr_reader :deployments, :pods
+
+      def matching_pods
+        strong_memoize(:matching_pods) do
+          deployment_tracks = deployments.map(&:track)
+          pods.select { |p| deployment_tracks.include?(p.track) }
+        end
+      end
+
+      def extra_pending_pods
+        wanted_instances = sum_hashes(deployments.map { |d| { d.track => d.wanted_instances } })
+        present_instances = sum_hashes(matching_pods.map { |p| { p.track => 1 } })
+        pending_instances = subtract_hashes(wanted_instances, present_instances)
+
+        pending_instances.flat_map do |track, num|
+          Array.new(num, pending_pod_for(track))
+        end
+      end
+
+      def sum_hashes(hashes)
+        hashes.reduce({}) do |memo, hash|
+          memo.merge(hash) { |_key, memo_val, hash_val| memo_val + hash_val }
+        end
+      end
+
+      def subtract_hashes(hash_a, hash_b)
+        hash_a.merge(hash_b) { |_key, val_a, val_b| [0, val_a - val_b].max }
+      end
+
+      def pending_pod_for(track)
+        ::Gitlab::Kubernetes::Pod.new({
+          'status' => { 'phase' => 'Pending' },
+          'metadata' => {
+            'name' => 'Not provided',
+            'labels' => {
+              'track' => track
+            }
+          }
+        })
+      end
+
+      def to_hash(pod)
+        {
+          status: pod.status&.downcase,
+          pod_name: pod.name,
+          tooltip: "#{pod.name} (#{pod.status})",
+          track: pod.track,
+          stable: pod.stable?
+        }
+      end
+    end
+  end
+end

ee/lib/gitlab/kubernetes/rollout_status.rb

@@ -26,29 +26,41 @@ module Gitlab
         @status == :found
       end
 
-      def self.from_deployments(*deployments, pods_attrs: {})
-        return new([], status: :not_found) if deployments.empty?
+      def self.from_deployments(*deployments_attrs, pods_attrs: [])
+        return new([], status: :not_found) if deployments_attrs.empty?
 
-        deployments = deployments.map { |deploy| ::Gitlab::Kubernetes::Deployment.new(deploy, pods: pods_attrs) }
+        deployments = deployments_attrs.map do |attrs|
+          ::Gitlab::Kubernetes::Deployment.new(attrs, pods: pods_attrs)
+        end
         deployments.sort_by!(&:order)
-        new(deployments)
+
+        pods = pods_attrs.map do |attrs|
+          ::Gitlab::Kubernetes::Pod.new(attrs)
+        end
+
+        new(deployments, pods: pods)
       end
 
       def self.loading
         new([], status: :loading)
       end
 
-      def initialize(deployments, status: :found)
+      def initialize(deployments, pods: [], status: :found)
         @status       = status
         @deployments  = deployments
-        @instances    = deployments.flat_map(&:instances)
+
+        @instances = if ::Feature.enabled?(:deploy_boards_dedupe_instances)
+                       RolloutInstances.new(deployments, pods).pod_instances
+                     else
+                       deployments.flat_map(&:instances)
+                     end
 
         @completion =
           if @instances.empty?
             100
           else
             # We downcase the pod status in Gitlab::Kubernetes::Deployment#deployment_instance
-            finished = @instances.count { |instance| instance[:status] == Gitlab::Kubernetes::Pod::RUNNING.downcase }
+            finished = @instances.count { |instance| instance[:status] == ::Gitlab::Kubernetes::Pod::RUNNING.downcase }
 
             (finished / @instances.count.to_f * 100).to_i
           end

ee/spec/lib/gitlab/kubernetes/rollout_instances_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Kubernetes::RolloutInstances do
+  include KubernetesHelpers
+
+  def setup(deployments_attrs, pods_attrs)
+    deployments = deployments_attrs.map do |attrs|
+      ::Gitlab::Kubernetes::Deployment.new(attrs, pods: pods_attrs)
+    end
+
+    pods = pods_attrs.map do |attrs|
+      ::Gitlab::Kubernetes::Pod.new(attrs)
+    end
+
+    [deployments, pods]
+  end
+
+  describe '#pod_instances' do
+    it 'returns an instance for a deployment with one pod' do
+      deployments, pods = setup(
+        [kube_deployment(name: 'one', track: 'stable', replicas: 1)],
+        [kube_pod(name: 'one', status: 'Running', track: 'stable')]
+      )
+      rollout_instances = described_class.new(deployments, pods)
+
+      expect(rollout_instances.pod_instances).to eq([{
+        pod_name: 'one',
+        stable: true,
+        status: 'running',
+        tooltip: 'one (Running)',
+        track: 'stable'
+      }])
+    end
+
+    it 'returns a pending pod for a missing replica' do
+      deployments, pods = setup(
+        [kube_deployment(name: 'one', track: 'stable', replicas: 1)],
+        []
+      )
+      rollout_instances = described_class.new(deployments, pods)
+
+      expect(rollout_instances.pod_instances).to eq([{
+        pod_name: 'Not provided',
+        stable: true,
+        status: 'pending',
+        tooltip: 'Not provided (Pending)',
+        track: 'stable'
+      }])
+    end
+
+    it 'returns instances when there are two stable deployments' do
+      deployments, pods = setup([
+        kube_deployment(name: 'one', track: 'stable', replicas: 1),
+        kube_deployment(name: 'two', track: 'stable', replicas: 1)
+      ], [
+        kube_pod(name: 'one', status: 'Running', track: 'stable'),
+        kube_pod(name: 'two', status: 'Running', track: 'stable')
+      ])
+      rollout_instances = described_class.new(deployments, pods)
+
+      expect(rollout_instances.pod_instances).to eq([{
+        pod_name: 'one',
+        stable: true,
+        status: 'running',
+        tooltip: 'one (Running)',
+        track: 'stable'
+      }, {
+        pod_name: 'two',
+        stable: true,
+        status: 'running',
+        tooltip: 'two (Running)',
+        track: 'stable'
+      }])
+    end
+
+    it 'returns instances for two deployments with different tracks' do
+      deployments, pods = setup([
+        kube_deployment(name: 'one', track: 'mytrack', replicas: 1),
+        kube_deployment(name: 'two', track: 'othertrack', replicas: 1)
+      ], [
+        kube_pod(name: 'one', status: 'Running', track: 'mytrack'),
+        kube_pod(name: 'two', status: 'Running', track: 'othertrack')
+      ])
+      rollout_instances = described_class.new(deployments, pods)
+
+      expect(rollout_instances.pod_instances).to eq([{
+        pod_name: 'one',
+        stable: false,
+        status: 'running',
+        tooltip: 'one (Running)',
+        track: 'mytrack'
+      }, {
+        pod_name: 'two',
+        stable: false,
+        status: 'running',
+        tooltip: 'two (Running)',
+        track: 'othertrack'
+      }])
+    end
+
+    it 'sorts stable tracks after canary tracks' do
+      deployments, pods = setup([
+        kube_deployment(name: 'one', track: 'stable', replicas: 1),
+        kube_deployment(name: 'two', track: 'canary', replicas: 1)
+      ], [
+        kube_pod(name: 'one', status: 'Running', track: 'stable'),
+        kube_pod(name: 'two', status: 'Running', track: 'canary')
+      ])
+      rollout_instances = described_class.new(deployments, pods)
+
+      expect(rollout_instances.pod_instances).to eq([{
+        pod_name: 'two',
+        stable: false,
+        status: 'running',
+        tooltip: 'two (Running)',
+        track: 'canary'
+      }, {
+        pod_name: 'one',
+        stable: true,
+        status: 'running',
+        tooltip: 'one (Running)',
+        track: 'stable'
+      }])
+    end
+  end
+end

ee/spec/lib/gitlab/kubernetes/rollout_status_spec.rb

@@ -28,6 +28,7 @@ RSpec.describe Gitlab::Kubernetes::RolloutStatus do
 
   subject(:rollout_status) { described_class.from_deployments(*specs, pods_attrs: pods) }
 
+  shared_examples 'rollout status' do
     describe '#deployments' do
       it 'stores the deployments' do
         expect(rollout_status.deployments).to be_kind_of(Array)
@@ -193,34 +194,75 @@ RSpec.describe Gitlab::Kubernetes::RolloutStatus do
       end
     end
 
-  describe '#not_found?' do
+    describe '#found?' do
       context 'when the specs are passed' do
-      it { is_expected.not_to be_not_found }
+        it { is_expected.to be_found }
       end
 
       context 'when list of specs is empty' do
         let(:specs) { [] }
 
-      it { is_expected.to be_not_found }
+        it { is_expected.not_to be_found }
       end
     end
 
-  describe '#found?' do
+    describe '.loading' do
+      subject { described_class.loading }
+
+      it { is_expected.to be_loading }
+    end
+
+    describe '#not_found?' do
       context 'when the specs are passed' do
-      it { is_expected.to be_found }
+        it { is_expected.not_to be_not_found }
       end
 
       context 'when list of specs is empty' do
         let(:specs) { [] }
 
-      it { is_expected.not_to be_found }
+        it { is_expected.to be_not_found }
+      end
     end
   end
 
-  describe '.loading' do
-    subject { described_class.loading }
+  context 'deploy_boards_dedupe_instances is disabled' do
+    before do
+      stub_feature_flags(deploy_boards_dedupe_instances: false)
+    end
 
-    it { is_expected.to be_loading }
+    it_behaves_like 'rollout status'
+  end
+
+  context 'deploy_boards_dedupe_instances is enabled' do
+    before do
+      stub_feature_flags(deploy_boards_dedupe_instances: true)
+    end
+
+    it_behaves_like 'rollout status'
+
+    describe '#completion' do
+      it 'sets the completion percentage when a quarter of the pods are complete' do
+        deployments = [
+          kube_deployment(name: 'one', track: 'stable', replicas: 6),
+          kube_deployment(name: 'two', track: 'stable', replicas: 2)
+        ]
+        pods = create_pods(name: 'one', track: 'stable', count: 2)
+        rollout_status = described_class.from_deployments(*deployments, pods_attrs: pods)
+
+        expect(rollout_status.completion).to eq(25)
+      end
+
+      it 'sets the completion percentage when a third of the pods are complete' do
+        deployments = [
+          kube_deployment(name: 'one', track: 'stable', replicas: 2),
+          kube_deployment(name: 'two', track: nil, replicas: 7)
+        ]
+        pods = create_pods(name: 'one', track: 'stable', count: 2) + create_pods(name: 'two', track: nil, count: 1)
+        rollout_status = described_class.from_deployments(*deployments, pods_attrs: pods)
+
+        expect(rollout_status.completion).to eq(33)
+      end
+    end
   end
 
   def create_pods(name:, count:, track: nil, status: 'Running' )

ee/spec/models/ee/clusters/platforms/kubernetes_spec.rb

@@ -276,6 +276,28 @@ RSpec.describe Clusters::Platforms::Kubernetes do
         ])
       end
     end
+
+    context 'with multiple matching deployments' do
+      let(:deployments) do
+        [
+          kube_deployment(name: 'deployment-a', environment_slug: environment.slug, project_slug: project.full_path_slug, replicas: 2),
+          kube_deployment(name: 'deployment-b', environment_slug: environment.slug, project_slug: project.full_path_slug, replicas: 2)
+        ]
+      end
+
+      let(:pods) do
+        [
+          kube_pod(name: 'pod-a-1', environment_slug: environment.slug, project_slug: project.full_path_slug),
+          kube_pod(name: 'pod-a-2', environment_slug: environment.slug, project_slug: project.full_path_slug),
+          kube_pod(name: 'pod-b-1', environment_slug: environment.slug, project_slug: project.full_path_slug),
+          kube_pod(name: 'pod-b-2', environment_slug: environment.slug, project_slug: project.full_path_slug)
+        ]
+      end
+
+      it 'returns each pod once' do
+        expect(rollout_status.instances.map { |p| p[:pod_name] }).to eq(['pod-a-1', 'pod-a-2', 'pod-b-1', 'pod-b-2'])
+      end
+    end
   end
 
   describe '#calculate_reactive_cache_for' do

lib/gitlab/kubernetes/pod.rb

@@ -2,13 +2,47 @@
 
 module Gitlab
   module Kubernetes
-    module Pod
+    class Pod
       PENDING   = 'Pending'
       RUNNING   = 'Running'
       SUCCEEDED = 'Succeeded'
       FAILED    = 'Failed'
       UNKNOWN   = 'Unknown'
       PHASES    = [PENDING, RUNNING, SUCCEEDED, FAILED, UNKNOWN].freeze
+
+      STABLE_TRACK_VALUE = 'stable'
+
+      def initialize(attributes = {})
+        @attributes = attributes
+      end
+
+      def track
+        attributes.dig('metadata', 'labels', 'track') || STABLE_TRACK_VALUE
+      end
+
+      def name
+        metadata['name'] || metadata['generateName']
+      end
+
+      def stable?
+        track == STABLE_TRACK_VALUE
+      end
+
+      def status
+        attributes.dig('status', 'phase')
+      end
+
+      def order
+        stable? ? 1 : 0
+      end
+
+      private
+
+      attr_reader :attributes
+
+      def metadata
+        attributes.fetch('metadata', {})
+      end
     end
   end
 end