From 3a3ec6f0213addb52abc29ee83e2d6a00e2292d3 Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Mon, 15 Oct 2018 11:03:15 +0200
Subject: [PATCH] Show available clusters when installed or updated
Before this commit updating Prometheus (e.g. adding alerts) made it
"updated" therefore not installed.
---
app/models/clusters/applications/jupyter.rb | 2 +-
app/models/clusters/cluster.rb | 5 +-
.../clusters/concerns/application_core.rb | 2 +-
.../clusters/concerns/application_status.rb | 4 +
.../project_services/prometheus_service.rb | 8 +-
.../_configuration_banner.html.haml | 2 +-
...showing-as-installed-even-though-it-is.yml | 5 ++
.../clusters/applications/ingress_spec.rb | 2 +-
.../clusters/applications/prometheus_spec.rb | 2 +-
.../clusters/applications/runner_spec.rb | 2 +-
spec/models/clusters/cluster_spec.rb | 7 +-
.../prometheus_service_spec.rb | 23 +++--
...luster_application_core_shared_examples.rb | 56 -------------
...ster_application_status_shared_examples.rb | 83 +++++++++++++++++++
14 files changed, 127 insertions(+), 76 deletions(-)
create mode 100644 changelogs/unreleased/51972-prometheus-not-showing-as-installed-even-though-it-is.yml
diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb
index 7be6a14f585..e43a0fd1786 100644
--- a/app/models/clusters/applications/jupyter.rb
+++ b/app/models/clusters/applications/jupyter.rb
@@ -19,7 +19,7 @@ module Clusters
def set_initial_status
return unless not_installable?
- if cluster&.application_ingress_installed? && cluster.application_ingress.external_ip
+ if cluster&.application_ingress_available? && cluster.application_ingress.external_ip
self.status = 'installable'
end
end
diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb
index d7011ef447a..20d53b8e620 100644
--- a/app/models/clusters/cluster.rb
+++ b/app/models/clusters/cluster.rb
@@ -43,8 +43,9 @@ module Clusters
delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true
delegate :rbac?, to: :platform_kubernetes, prefix: true, allow_nil: true
- delegate :installed?, to: :application_helm, prefix: true, allow_nil: true
- delegate :installed?, to: :application_ingress, prefix: true, allow_nil: true
+ delegate :available?, to: :application_helm, prefix: true, allow_nil: true
+ delegate :available?, to: :application_ingress, prefix: true, allow_nil: true
+ delegate :available?, to: :application_prometheus, prefix: true, allow_nil: true
enum platform_type: {
kubernetes: 1
diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb
index e3deedfb036..683b45331f6 100644
--- a/app/models/clusters/concerns/application_core.rb
+++ b/app/models/clusters/concerns/application_core.rb
@@ -15,7 +15,7 @@ module Clusters
def set_initial_status
return unless not_installable?
- self.status = 'installable' if cluster&.application_helm_installed?
+ self.status = 'installable' if cluster&.application_helm_available?
end
def self.application_name
diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb
index a9df59fc059..93bdf9c223d 100644
--- a/app/models/clusters/concerns/application_status.rb
+++ b/app/models/clusters/concerns/application_status.rb
@@ -66,6 +66,10 @@ module Clusters
end
end
end
+
+ def available?
+ installed? || updated?
+ end
end
end
end
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index 509e5b6089b..f141502d5df 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -26,7 +26,7 @@ class PrometheusService < MonitoringService
end
def editable?
- manual_configuration? || !prometheus_installed?
+ manual_configuration? || !prometheus_available?
end
def title
@@ -75,17 +75,17 @@ class PrometheusService < MonitoringService
RestClient::Resource.new(api_url) if api_url && manual_configuration? && active?
end
- def prometheus_installed?
+ def prometheus_available?
return false if template?
return false unless project
- project.clusters.enabled.any? { |cluster| cluster.application_prometheus&.installed? }
+ project.clusters.enabled.any? { |cluster| cluster.application_prometheus_available? }
end
private
def synchronize_service_state
- self.active = prometheus_installed? || manual_configuration?
+ self.active = prometheus_available? || manual_configuration?
true
end
diff --git a/app/views/projects/services/prometheus/_configuration_banner.html.haml b/app/views/projects/services/prometheus/_configuration_banner.html.haml
index 898b55e4b39..dfcb1c5d240 100644
--- a/app/views/projects/services/prometheus/_configuration_banner.html.haml
+++ b/app/views/projects/services/prometheus/_configuration_banner.html.haml
@@ -7,7 +7,7 @@
- else
.container-fluid
.row
- - if service.prometheus_installed?
+ - if service.prometheus_available?
.col-sm-2
.svg-container
= image_tag 'illustrations/monitoring/getting_started.svg'
diff --git a/changelogs/unreleased/51972-prometheus-not-showing-as-installed-even-though-it-is.yml b/changelogs/unreleased/51972-prometheus-not-showing-as-installed-even-though-it-is.yml
new file mode 100644
index 00000000000..73b035fca00
--- /dev/null
+++ b/changelogs/unreleased/51972-prometheus-not-showing-as-installed-even-though-it-is.yml
@@ -0,0 +1,5 @@
+---
+title: Show available clusters when installed or updated
+merge_request: 22356
+author:
+type: fixed
diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb
index c55953c8d22..1580ef36127 100644
--- a/spec/models/clusters/applications/ingress_spec.rb
+++ b/spec/models/clusters/applications/ingress_spec.rb
@@ -4,7 +4,7 @@ describe Clusters::Applications::Ingress do
let(:ingress) { create(:clusters_applications_ingress) }
include_examples 'cluster application core specs', :clusters_applications_ingress
- include_examples 'cluster application status specs', :cluster_application_ingress
+ include_examples 'cluster application status specs', :clusters_applications_ingress
before do
allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in)
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index f34b4ece8db..f9776acd4c8 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -4,7 +4,7 @@ describe Clusters::Applications::Prometheus do
include KubernetesHelpers
include_examples 'cluster application core specs', :clusters_applications_prometheus
- include_examples 'cluster application status specs', :cluster_application_prometheus
+ include_examples 'cluster application status specs', :clusters_applications_prometheus
describe '.installed' do
subject { described_class.installed }
diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb
index eda8d519f60..42448b42c8e 100644
--- a/spec/models/clusters/applications/runner_spec.rb
+++ b/spec/models/clusters/applications/runner_spec.rb
@@ -4,7 +4,7 @@ describe Clusters::Applications::Runner do
let(:ci_runner) { create(:ci_runner) }
include_examples 'cluster application core specs', :clusters_applications_runner
- include_examples 'cluster application status specs', :cluster_application_runner
+ include_examples 'cluster application status specs', :clusters_applications_runner
it { is_expected.to belong_to(:runner) }
diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb
index 2727191eb9b..34d321ec604 100644
--- a/spec/models/clusters/cluster_spec.rb
+++ b/spec/models/clusters/cluster_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'spec_helper'
describe Clusters::Cluster do
@@ -15,8 +17,9 @@ describe Clusters::Cluster do
it { is_expected.to delegate_method(:on_creation?).to(:provider) }
it { is_expected.to delegate_method(:active?).to(:platform_kubernetes).with_prefix }
it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix }
- it { is_expected.to delegate_method(:installed?).to(:application_helm).with_prefix }
- it { is_expected.to delegate_method(:installed?).to(:application_ingress).with_prefix }
+ it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix }
+ it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix }
+ it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix }
it { is_expected.to respond_to :project }
describe '.enabled' do
diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb
index 7afb1b4a8e3..f2cb927df37 100644
--- a/spec/models/project_services/prometheus_service_spec.rb
+++ b/spec/models/project_services/prometheus_service_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'spec_helper'
describe PrometheusService, :use_clean_rails_memory_store_caching do
@@ -83,13 +85,22 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
end
end
- describe '#prometheus_installed?' do
+ describe '#prometheus_available?' do
context 'clusters with installed prometheus' do
let!(:cluster) { create(:cluster, projects: [project]) }
let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
it 'returns true' do
- expect(service.prometheus_installed?).to be(true)
+ expect(service.prometheus_available?).to be(true)
+ end
+ end
+
+ context 'clusters with updated prometheus' do
+ let!(:cluster) { create(:cluster, projects: [project]) }
+ let!(:prometheus) { create(:clusters_applications_prometheus, :updated, cluster: cluster) }
+
+ it 'returns true' do
+ expect(service.prometheus_available?).to be(true)
end
end
@@ -98,7 +109,7 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) }
it 'returns false' do
- expect(service.prometheus_installed?).to be(false)
+ expect(service.prometheus_available?).to be(false)
end
end
@@ -106,13 +117,13 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
let(:cluster) { create(:cluster, projects: [project]) }
it 'returns false' do
- expect(service.prometheus_installed?).to be(false)
+ expect(service.prometheus_available?).to be(false)
end
end
context 'no clusters' do
it 'returns false' do
- expect(service.prometheus_installed?).to be(false)
+ expect(service.prometheus_available?).to be(false)
end
end
end
@@ -150,7 +161,7 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
context 'with prometheus installed in the cluster' do
before do
- allow(service).to receive(:prometheus_installed?).and_return(true)
+ allow(service).to receive(:prometheus_available?).and_return(true)
end
context 'when service is inactive' do
diff --git a/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
index 87d12a784ba..1f76b981292 100644
--- a/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_core_shared_examples.rb
@@ -11,60 +11,4 @@ shared_examples 'cluster application core specs' do |application_name|
expect(Clusters::Cluster::APPLICATIONS[subject.name]).to eq(described_class)
end
end
-
- describe 'status state machine' do
- describe '#make_installing' do
- subject { create(application_name, :scheduled) }
-
- it 'is installing' do
- subject.make_installing!
-
- expect(subject).to be_installing
- end
- end
-
- describe '#make_installed' do
- subject { create(application_name, :installing) }
-
- it 'is installed' do
- subject.make_installed
-
- expect(subject).to be_installed
- end
- end
-
- describe '#make_errored' do
- subject { create(application_name, :installing) }
- let(:reason) { 'some errors' }
-
- it 'is errored' do
- subject.make_errored(reason)
-
- expect(subject).to be_errored
- expect(subject.status_reason).to eq(reason)
- end
- end
-
- describe '#make_scheduled' do
- subject { create(application_name, :installable) }
-
- it 'is scheduled' do
- subject.make_scheduled
-
- expect(subject).to be_scheduled
- end
-
- describe 'when was errored' do
- subject { create(application_name, :errored) }
-
- it 'clears #status_reason' do
- expect(subject.status_reason).not_to be_nil
-
- subject.make_scheduled!
-
- expect(subject.status_reason).to be_nil
- end
- end
- end
- end
end
diff --git a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
index 765dd32f4ba..82f0dd5d00f 100644
--- a/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
+++ b/spec/support/shared_examples/models/cluster_application_status_shared_examples.rb
@@ -28,4 +28,87 @@ shared_examples 'cluster application status specs' do |application_name|
end
end
end
+
+ describe 'status state machine' do
+ describe '#make_installing' do
+ subject { create(application_name, :scheduled) }
+
+ it 'is installing' do
+ subject.make_installing!
+
+ expect(subject).to be_installing
+ end
+ end
+
+ describe '#make_installed' do
+ subject { create(application_name, :installing) }
+
+ it 'is installed' do
+ subject.make_installed
+
+ expect(subject).to be_installed
+ end
+ end
+
+ describe '#make_errored' do
+ subject { create(application_name, :installing) }
+ let(:reason) { 'some errors' }
+
+ it 'is errored' do
+ subject.make_errored(reason)
+
+ expect(subject).to be_errored
+ expect(subject.status_reason).to eq(reason)
+ end
+ end
+
+ describe '#make_scheduled' do
+ subject { create(application_name, :installable) }
+
+ it 'is scheduled' do
+ subject.make_scheduled
+
+ expect(subject).to be_scheduled
+ end
+
+ describe 'when was errored' do
+ subject { create(application_name, :errored) }
+
+ it 'clears #status_reason' do
+ expect(subject.status_reason).not_to be_nil
+
+ subject.make_scheduled!
+
+ expect(subject.status_reason).to be_nil
+ end
+ end
+ end
+ end
+
+ describe '#available?' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:trait, :available) do
+ :not_installable | false
+ :installable | false
+ :scheduled | false
+ :installing | false
+ :installed | true
+ :updating | false
+ :updated | true
+ :errored | false
+ :update_errored | false
+ :timeouted | false
+ end
+
+ with_them do
+ subject { build(application_name, trait) }
+
+ if params[:available]
+ it { is_expected.to be_available }
+ else
+ it { is_expected.not_to be_available }
+ end
+ end
+ end
end
--
GitLab