Skip to content

Commit e347170

Browse files
committed
Merge branch '51972-prometheus-not-showing-as-installed-even-though-it-is' into 'master'
Resolve "Prometheus not showing as installed, even though it is" Closes #51972 See merge request gitlab-org/gitlab-ce!22356
2 parents 68d4dc8 + 3a3ec6f commit e347170

File tree

14 files changed

+127
-76
lines changed

14 files changed

+127
-76
lines changed

app/models/clusters/applications/jupyter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class Jupyter < ActiveRecord::Base
1919
def set_initial_status
2020
return unless not_installable?
2121

22-
if cluster&.application_ingress_installed? && cluster.application_ingress.external_ip
22+
if cluster&.application_ingress_available? && cluster.application_ingress.external_ip
2323
self.status = 'installable'
2424
end
2525
end

app/models/clusters/cluster.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ class Cluster < ActiveRecord::Base
4343

4444
delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true
4545
delegate :rbac?, to: :platform_kubernetes, prefix: true, allow_nil: true
46-
delegate :installed?, to: :application_helm, prefix: true, allow_nil: true
47-
delegate :installed?, to: :application_ingress, prefix: true, allow_nil: true
46+
delegate :available?, to: :application_helm, prefix: true, allow_nil: true
47+
delegate :available?, to: :application_ingress, prefix: true, allow_nil: true
48+
delegate :available?, to: :application_prometheus, prefix: true, allow_nil: true
4849

4950
enum platform_type: {
5051
kubernetes: 1

app/models/clusters/concerns/application_core.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ module ApplicationCore
1515
def set_initial_status
1616
return unless not_installable?
1717

18-
self.status = 'installable' if cluster&.application_helm_installed?
18+
self.status = 'installable' if cluster&.application_helm_available?
1919
end
2020

2121
def self.application_name

app/models/clusters/concerns/application_status.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ module ApplicationStatus
6666
end
6767
end
6868
end
69+
70+
def available?
71+
installed? || updated?
72+
end
6973
end
7074
end
7175
end

app/models/project_services/prometheus_service.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def show_active_box?
2626
end
2727

2828
def editable?
29-
manual_configuration? || !prometheus_installed?
29+
manual_configuration? || !prometheus_available?
3030
end
3131

3232
def title
@@ -75,17 +75,17 @@ def prometheus_client
7575
RestClient::Resource.new(api_url) if api_url && manual_configuration? && active?
7676
end
7777

78-
def prometheus_installed?
78+
def prometheus_available?
7979
return false if template?
8080
return false unless project
8181

82-
project.clusters.enabled.any? { |cluster| cluster.application_prometheus&.installed? }
82+
project.clusters.enabled.any? { |cluster| cluster.application_prometheus_available? }
8383
end
8484

8585
private
8686

8787
def synchronize_service_state
88-
self.active = prometheus_installed? || manual_configuration?
88+
self.active = prometheus_available? || manual_configuration?
8989

9090
true
9191
end

app/views/projects/services/prometheus/_configuration_banner.html.haml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
- else
88
.container-fluid
99
.row
10-
- if service.prometheus_installed?
10+
- if service.prometheus_available?
1111
.col-sm-2
1212
.svg-container
1313
= image_tag 'illustrations/monitoring/getting_started.svg'
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
title: Show available clusters when installed or updated
3+
merge_request: 22356
4+
author:
5+
type: fixed

spec/models/clusters/applications/ingress_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
let(:ingress) { create(:clusters_applications_ingress) }
55

66
include_examples 'cluster application core specs', :clusters_applications_ingress
7-
include_examples 'cluster application status specs', :cluster_application_ingress
7+
include_examples 'cluster application status specs', :clusters_applications_ingress
88

99
before do
1010
allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in)

spec/models/clusters/applications/prometheus_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
include KubernetesHelpers
55

66
include_examples 'cluster application core specs', :clusters_applications_prometheus
7-
include_examples 'cluster application status specs', :cluster_application_prometheus
7+
include_examples 'cluster application status specs', :clusters_applications_prometheus
88

99
describe '.installed' do
1010
subject { described_class.installed }

spec/models/clusters/applications/runner_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
let(:ci_runner) { create(:ci_runner) }
55

66
include_examples 'cluster application core specs', :clusters_applications_runner
7-
include_examples 'cluster application status specs', :cluster_application_runner
7+
include_examples 'cluster application status specs', :clusters_applications_runner
88

99
it { is_expected.to belong_to(:runner) }
1010

spec/models/clusters/cluster_spec.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# frozen_string_literal: true
2+
13
require 'spec_helper'
24

35
describe Clusters::Cluster do
@@ -15,8 +17,9 @@
1517
it { is_expected.to delegate_method(:on_creation?).to(:provider) }
1618
it { is_expected.to delegate_method(:active?).to(:platform_kubernetes).with_prefix }
1719
it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix }
18-
it { is_expected.to delegate_method(:installed?).to(:application_helm).with_prefix }
19-
it { is_expected.to delegate_method(:installed?).to(:application_ingress).with_prefix }
20+
it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix }
21+
it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix }
22+
it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix }
2023
it { is_expected.to respond_to :project }
2124

2225
describe '.enabled' do

spec/models/project_services/prometheus_service_spec.rb

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# frozen_string_literal: true
2+
13
require 'spec_helper'
24

35
describe PrometheusService, :use_clean_rails_memory_store_caching do
@@ -83,13 +85,22 @@
8385
end
8486
end
8587

86-
describe '#prometheus_installed?' do
88+
describe '#prometheus_available?' do
8789
context 'clusters with installed prometheus' do
8890
let!(:cluster) { create(:cluster, projects: [project]) }
8991
let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
9092

9193
it 'returns true' do
92-
expect(service.prometheus_installed?).to be(true)
94+
expect(service.prometheus_available?).to be(true)
95+
end
96+
end
97+
98+
context 'clusters with updated prometheus' do
99+
let!(:cluster) { create(:cluster, projects: [project]) }
100+
let!(:prometheus) { create(:clusters_applications_prometheus, :updated, cluster: cluster) }
101+
102+
it 'returns true' do
103+
expect(service.prometheus_available?).to be(true)
93104
end
94105
end
95106

@@ -98,21 +109,21 @@
98109
let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) }
99110

100111
it 'returns false' do
101-
expect(service.prometheus_installed?).to be(false)
112+
expect(service.prometheus_available?).to be(false)
102113
end
103114
end
104115

105116
context 'clusters without prometheus' do
106117
let(:cluster) { create(:cluster, projects: [project]) }
107118

108119
it 'returns false' do
109-
expect(service.prometheus_installed?).to be(false)
120+
expect(service.prometheus_available?).to be(false)
110121
end
111122
end
112123

113124
context 'no clusters' do
114125
it 'returns false' do
115-
expect(service.prometheus_installed?).to be(false)
126+
expect(service.prometheus_available?).to be(false)
116127
end
117128
end
118129
end
@@ -150,7 +161,7 @@
150161

151162
context 'with prometheus installed in the cluster' do
152163
before do
153-
allow(service).to receive(:prometheus_installed?).and_return(true)
164+
allow(service).to receive(:prometheus_available?).and_return(true)
154165
end
155166

156167
context 'when service is inactive' do

spec/support/shared_examples/models/cluster_application_core_shared_examples.rb

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -11,60 +11,4 @@
1111
expect(Clusters::Cluster::APPLICATIONS[subject.name]).to eq(described_class)
1212
end
1313
end
14-
15-
describe 'status state machine' do
16-
describe '#make_installing' do
17-
subject { create(application_name, :scheduled) }
18-
19-
it 'is installing' do
20-
subject.make_installing!
21-
22-
expect(subject).to be_installing
23-
end
24-
end
25-
26-
describe '#make_installed' do
27-
subject { create(application_name, :installing) }
28-
29-
it 'is installed' do
30-
subject.make_installed
31-
32-
expect(subject).to be_installed
33-
end
34-
end
35-
36-
describe '#make_errored' do
37-
subject { create(application_name, :installing) }
38-
let(:reason) { 'some errors' }
39-
40-
it 'is errored' do
41-
subject.make_errored(reason)
42-
43-
expect(subject).to be_errored
44-
expect(subject.status_reason).to eq(reason)
45-
end
46-
end
47-
48-
describe '#make_scheduled' do
49-
subject { create(application_name, :installable) }
50-
51-
it 'is scheduled' do
52-
subject.make_scheduled
53-
54-
expect(subject).to be_scheduled
55-
end
56-
57-
describe 'when was errored' do
58-
subject { create(application_name, :errored) }
59-
60-
it 'clears #status_reason' do
61-
expect(subject.status_reason).not_to be_nil
62-
63-
subject.make_scheduled!
64-
65-
expect(subject.status_reason).to be_nil
66-
end
67-
end
68-
end
69-
end
7014
end

spec/support/shared_examples/models/cluster_application_status_shared_examples.rb

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,87 @@
2828
end
2929
end
3030
end
31+
32+
describe 'status state machine' do
33+
describe '#make_installing' do
34+
subject { create(application_name, :scheduled) }
35+
36+
it 'is installing' do
37+
subject.make_installing!
38+
39+
expect(subject).to be_installing
40+
end
41+
end
42+
43+
describe '#make_installed' do
44+
subject { create(application_name, :installing) }
45+
46+
it 'is installed' do
47+
subject.make_installed
48+
49+
expect(subject).to be_installed
50+
end
51+
end
52+
53+
describe '#make_errored' do
54+
subject { create(application_name, :installing) }
55+
let(:reason) { 'some errors' }
56+
57+
it 'is errored' do
58+
subject.make_errored(reason)
59+
60+
expect(subject).to be_errored
61+
expect(subject.status_reason).to eq(reason)
62+
end
63+
end
64+
65+
describe '#make_scheduled' do
66+
subject { create(application_name, :installable) }
67+
68+
it 'is scheduled' do
69+
subject.make_scheduled
70+
71+
expect(subject).to be_scheduled
72+
end
73+
74+
describe 'when was errored' do
75+
subject { create(application_name, :errored) }
76+
77+
it 'clears #status_reason' do
78+
expect(subject.status_reason).not_to be_nil
79+
80+
subject.make_scheduled!
81+
82+
expect(subject.status_reason).to be_nil
83+
end
84+
end
85+
end
86+
end
87+
88+
describe '#available?' do
89+
using RSpec::Parameterized::TableSyntax
90+
91+
where(:trait, :available) do
92+
:not_installable | false
93+
:installable | false
94+
:scheduled | false
95+
:installing | false
96+
:installed | true
97+
:updating | false
98+
:updated | true
99+
:errored | false
100+
:update_errored | false
101+
:timeouted | false
102+
end
103+
104+
with_them do
105+
subject { build(application_name, trait) }
106+
107+
if params[:available]
108+
it { is_expected.to be_available }
109+
else
110+
it { is_expected.not_to be_available }
111+
end
112+
end
113+
end
31114
end

0 commit comments

Comments
 (0)