Update version on :installed, not :installing

This makes this consistent with :updated. And also avoids a potential
issue where an install errors which means that that the recorded version
won't necessarily reflect the version that is actually installed.

9 files changed, 22 insertions, 89 deletions

diff --git a/app/models/clusters/concerns/application_version.rb b/app/models/clusters/concerns/application_version.rb
index 285c9d98935..e355de23df6 100644
--- a/app/models/clusters/concerns/application_version.rb
+++ b/app/models/clusters/concerns/application_version.rb
@@ -7,11 +7,7 @@ module Clusters
 
       included do
         state_machine :status do
-          after_transition any => [:installing] do |application|
-            application.update(version: application.class.const_get(:VERSION))
-          end
-
-          before_transition any => :updated do |application|
+          before_transition any => [:installed, :updated] do |application|
             application.version = application.class.const_get(:VERSION)
           end
         end
diff --git a/changelogs/unreleased/cluster_application_version_updated.yml b/changelogs/unreleased/cluster_application_version_updated.yml
new file mode 100644
index 00000000000..34fe55dcc5e
--- /dev/null
+++ b/changelogs/unreleased/cluster_application_version_updated.yml
@@ -0,0 +1,5 @@
+---
+title: Update cluster application version on updated and installed status
+merge_request: 24810
+author:
+type: other
diff --git a/spec/models/clusters/applications/cert_manager_spec.rb b/spec/models/clusters/applications/cert_manager_spec.rb
index 8e14abe098d..6b1bcce1081 100644
--- a/spec/models/clusters/applications/cert_manager_spec.rb
+++ b/spec/models/clusters/applications/cert_manager_spec.rb
@@ -4,20 +4,7 @@ describe Clusters::Applications::CertManager do
   let(:cert_manager) { create(:clusters_applications_cert_managers) }
 
   include_examples 'cluster application core specs', :clusters_applications_cert_managers
-
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_cert_managers, :scheduled, version: 'v0.4.0') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('v0.5.2')
-      end
-    end
-  end
+  include_examples 'cluster application status specs', :clusters_applications_cert_managers
 
   describe '#install_command' do
     let(:cluster_issuer_file) { { "cluster_issuer.yaml": "---\napiVersion: certmanager.k8s.io/v1alpha1\nkind: ClusterIssuer\nmetadata:\n  name: letsencrypt-prod\nspec:\n  acme:\n    server: https://acme-v02.api.letsencrypt.org/directory\n    email: admin@example.com\n    privateKeySecretRef:\n      name: letsencrypt-prod\n    http01: {}\n" } }
diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb
index 52c347229c6..811d0026535 100644
--- a/spec/models/clusters/applications/ingress_spec.rb
+++ b/spec/models/clusters/applications/ingress_spec.rb
@@ -26,20 +26,6 @@ describe Clusters::Applications::Ingress do
     it { is_expected.to contain_exactly(cluster) }
   end
 
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_ingress, :scheduled, version: '0.22.0') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('1.1.2')
-      end
-    end
-  end
-
   describe '#make_installed!' do
     before do
       application.make_installed!
diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb
index 391e5425384..b73a243f6e0 100644
--- a/spec/models/clusters/applications/jupyter_spec.rb
+++ b/spec/models/clusters/applications/jupyter_spec.rb
@@ -2,6 +2,7 @@ require 'rails_helper'
 
 describe Clusters::Applications::Jupyter do
   include_examples 'cluster application core specs', :clusters_applications_jupyter
+  include_examples 'cluster application status specs', :clusters_applications_jupyter
   include_examples 'cluster application helm specs', :clusters_applications_jupyter
 
   it { is_expected.to belong_to(:oauth_application) }
@@ -26,20 +27,6 @@ describe Clusters::Applications::Jupyter do
     end
   end
 
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_jupyter, :scheduled, version: 'v0.5') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('v0.6')
-      end
-    end
-  end
-
   describe '#install_command' do
     let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') }
     let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) }
diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb
index 35818be8deb..55443bb2d4f 100644
--- a/spec/models/clusters/applications/knative_spec.rb
+++ b/spec/models/clusters/applications/knative_spec.rb
@@ -34,20 +34,6 @@ describe Clusters::Applications::Knative do
     it { is_expected.to contain_exactly(cluster) }
   end
 
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.2.2') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('0.2.2')
-      end
-    end
-  end
-
   describe '#make_installed' do
     subject { described_class.installed }
 
diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb
index e50ba67c493..36a6e4c347b 100644
--- a/spec/models/clusters/applications/prometheus_spec.rb
+++ b/spec/models/clusters/applications/prometheus_spec.rb
@@ -19,20 +19,6 @@ describe Clusters::Applications::Prometheus do
     it { is_expected.to contain_exactly(cluster) }
   end
 
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_prometheus, :scheduled, version: '6.7.2') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('6.7.3')
-      end
-    end
-  end
-
   describe 'transition to installed' do
     let(:project) { create(:project) }
     let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb
index 8ad41e997c2..3e6daf8a167 100644
--- a/spec/models/clusters/applications/runner_spec.rb
+++ b/spec/models/clusters/applications/runner_spec.rb
@@ -9,20 +9,6 @@ describe Clusters::Applications::Runner do
 
   it { is_expected.to belong_to(:runner) }
 
-  describe '#make_installing!' do
-    before do
-      application.make_installing!
-    end
-
-    context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') }
-
-      it 'updates the application version' do
-        expect(application.reload.version).to eq('0.1.45')
-      end
-    end
-  end
-
   describe '.installed' do
     subject { described_class.installed }
 
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 bd12d94b2c2..af6147bed39 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
@@ -21,6 +21,10 @@ shared_examples 'cluster application status specs' do |application_name|
     context 'when application is scheduled' do
       before do
         create(:clusters_applications_helm, :installed, cluster: cluster)
+
+        if described_class == Clusters::Applications::Jupyter
+          create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1', cluster: cluster)
+        end
       end
 
       it 'sets a default status' do
@@ -58,6 +62,16 @@ shared_examples 'cluster application status specs' do |application_name|
 
         expect(subject.cluster.application_helm.version).to eq(Gitlab::Kubernetes::Helm::HELM_VERSION)
       end
+
+      it 'sets the correct version of the application' do
+        subject.update!(version: '0.0.0')
+
+        subject.make_installed!
+
+        subject.reload
+
+        expect(subject.version).to eq(subject.class.const_get(:VERSION))
+      end
     end
 
     describe '#make_updated' do