Commit a1b56a54 authored by Jason Goodman's avatar Jason Goodman Committed by Thong Kuah

Return each pod only once for deploy board with multiple deployments

Fixes an issue where pods were returned multiple times
parent c94aa77f
---
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
...@@ -42,8 +42,9 @@ knowledge. In particular, you should be familiar with: ...@@ -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) - [Kubernetes canary deployments](https://kubernetes.io/docs/concepts/cluster-administration/manage-deployment/#canary-deployments)
NOTE: **Note:** NOTE: **Note:**
Apps that consist of multiple deployments are shown as duplicates on the deploy board. In GitLab 13.4 and earlier, apps that consist of multiple deployments are shown as
Follow [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/8463) for details. duplicates on the deploy board. This is [fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/8463)
in GitLab 13.5.
## Use cases ## Use cases
......
---
title: Fix duplicate instances in deploy boards when multiple deployments have the same track
merge_request: 40768
author:
type: fixed
# 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
...@@ -26,29 +26,41 @@ module Gitlab ...@@ -26,29 +26,41 @@ module Gitlab
@status == :found @status == :found
end end
def self.from_deployments(*deployments, pods_attrs: {}) def self.from_deployments(*deployments_attrs, pods_attrs: [])
return new([], status: :not_found) if deployments.empty? 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) deployments.sort_by!(&:order)
new(deployments)
pods = pods_attrs.map do |attrs|
::Gitlab::Kubernetes::Pod.new(attrs)
end
new(deployments, pods: pods)
end end
def self.loading def self.loading
new([], status: :loading) new([], status: :loading)
end end
def initialize(deployments, status: :found) def initialize(deployments, pods: [], status: :found)
@status = status @status = status
@deployments = deployments @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 = @completion =
if @instances.empty? if @instances.empty?
100 100
else else
# We downcase the pod status in Gitlab::Kubernetes::Deployment#deployment_instance # 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 (finished / @instances.count.to_f * 100).to_i
end end
......
# 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
...@@ -276,6 +276,28 @@ RSpec.describe Clusters::Platforms::Kubernetes do ...@@ -276,6 +276,28 @@ RSpec.describe Clusters::Platforms::Kubernetes do
]) ])
end end
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 end
describe '#calculate_reactive_cache_for' do describe '#calculate_reactive_cache_for' do
......
...@@ -2,13 +2,47 @@ ...@@ -2,13 +2,47 @@
module Gitlab module Gitlab
module Kubernetes module Kubernetes
module Pod class Pod
PENDING = 'Pending' PENDING = 'Pending'
RUNNING = 'Running' RUNNING = 'Running'
SUCCEEDED = 'Succeeded' SUCCEEDED = 'Succeeded'
FAILED = 'Failed' FAILED = 'Failed'
UNKNOWN = 'Unknown' UNKNOWN = 'Unknown'
PHASES = [PENDING, RUNNING, SUCCEEDED, FAILED, UNKNOWN].freeze 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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment