From 3a3ec6f0213addb52abc29ee83e2d6a00e2292d3 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Mon, 15 Oct 2018 11:03:15 +0200 Subject: 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 +- app/models/clusters/concerns/application_core.rb | 2 +- app/models/clusters/concerns/application_status.rb | 4 ++ app/models/project_services/prometheus_service.rb | 8 +-- .../prometheus/_configuration_banner.html.haml | 2 +- ...-not-showing-as-installed-even-though-it-is.yml | 5 ++ spec/models/clusters/applications/ingress_spec.rb | 2 +- .../clusters/applications/prometheus_spec.rb | 2 +- spec/models/clusters/applications/runner_spec.rb | 2 +- spec/models/clusters/cluster_spec.rb | 7 +- .../project_services/prometheus_service_spec.rb | 23 ++++-- .../cluster_application_core_shared_examples.rb | 56 --------------- .../cluster_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 -- cgit v1.2.1