Resolve issues raised by Kamil

7 files changed, 176 insertions, 38 deletions

diff --git a/app/assets/javascripts/clusters/stores/clusters_store.js b/app/assets/javascripts/clusters/stores/clusters_store.js
index e9c580c5dfa..26566b60ba6 100644
--- a/app/assets/javascripts/clusters/stores/clusters_store.js
+++ b/app/assets/javascripts/clusters/stores/clusters_store.js
@@ -52,7 +52,7 @@ export default class ClusterStore {
           statusReason: null,
           requestStatus: null,
           requestReason: null,
-          hostname: ''
+          hostname: null
         },
       },
     };
@@ -102,7 +102,7 @@ export default class ClusterStore {
             ? `jupyter.${this.state.applications.ingress.externalIp}.nip.io`
             : '');
       } else if (appId === KNATIVE) {
-       this.state.applications.knative.hostname = serverAppEntry.hostname ? serverAppEntry.hostname : '';
+        this.state.applications.knative.hostname = serverAppEntry.hostname ? serverAppEntry.hostname : null;
       }
     });
   }
diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb
index 76d23870f4c..ecb42f6011e 100644
--- a/app/models/clusters/applications/knative.rb
+++ b/app/models/clusters/applications/knative.rb
@@ -18,18 +18,19 @@ module Clusters
       include ::Clusters::Concerns::ApplicationData
 
       default_value_for :version, VERSION
-      default_value_for :hostname, ''
+      default_value_for :hostname, nil
+
+      validates :hostname, presence: true
 
       def chart
         'knative/knative'
       end
 
-      def install_command
-        args = []
-        if !hostname.nil? && !hostname.eql?('')
-          args = ["domain=" + hostname]
-        end
+      def values
+        content_values.to_yaml
+      end
 
+      def install_command
         Gitlab::Kubernetes::Helm::InstallCommand.new(
           name: name,
           version: VERSION,
@@ -37,7 +38,6 @@ module Clusters
           chart: chart,
           files: files,
           repository: REPOSITORY,
-          setargs: args,
           script: install_script
         )
       end
@@ -46,8 +46,16 @@ module Clusters
         ['/usr/bin/kubectl', 'apply', '-f', ISTIO_CRDS]
       end
 
-      def client
-        cluster&.platform_kubernetes&.kubeclient&.core_client
+      private
+
+      def content_values
+        YAML.load_file(chart_values_file).deep_merge!(knative_configs)
+      end
+
+      def knative_configs
+        {
+          "domain" => hostname
+        }
       end
     end
   end
diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb
index aa5fddf1b44..09a4e494b64 100644
--- a/lib/gitlab/kubernetes/helm/install_command.rb
+++ b/lib/gitlab/kubernetes/helm/install_command.rb
@@ -4,16 +4,15 @@ module Gitlab
       class InstallCommand
         include BaseCommand
 
-        attr_reader :name, :files, :chart, :version, :repository, :setargs, :script
+        attr_reader :name, :files, :chart, :version, :repository, :script
 
-        def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, setargs: nil, script: nil)
+        def initialize(name:, chart:, files:, rbac:, version: nil, repository: nil, script: nil)
           @name = name
           @chart = chart
           @version = version
           @rbac = rbac
           @files = files
           @repository = repository
-          @setargs = setargs
           @script = script
         end
 
@@ -61,15 +60,13 @@ module Gitlab
           name_flag      = ['--name', name]
           namespace_flag = ['--namespace', Gitlab::Kubernetes::Helm::NAMESPACE]
           value_flag     = ['-f', "/data/helm/#{name}/config/values.yaml"]
-          args_flag      = optional_install_set_args_flag
 
           name_flag +
             optional_tls_flags +
             optional_version_flag +
             optional_rbac_create_flag +
             namespace_flag +
-            value_flag +
-            args_flag
+            value_flag
         end
 
         def optional_rbac_create_flag
@@ -80,16 +77,6 @@ module Gitlab
           %w[--set rbac.create=true,rbac.enabled=true]
         end
 
-        def optional_install_set_args_flag
-          return [] unless setargs
-
-          args = []
-          setargs.each do |s|
-            args.push("--set", s)
-          end
-          args
-        end
-
         def optional_version_flag
           return [] unless version
 
diff --git a/spec/javascripts/clusters/stores/clusters_store_spec.js b/spec/javascripts/clusters/stores/clusters_store_spec.js
index 0586724d451..34ed36afa5b 100644
--- a/spec/javascripts/clusters/stores/clusters_store_spec.js
+++ b/spec/javascripts/clusters/stores/clusters_store_spec.js
@@ -106,7 +106,7 @@ describe('Clusters Store', () => {
             statusReason: mockResponseData.applications[5].status_reason,
             requestStatus: null,
             requestReason: null,
-            hostname: '',
+            hostname: null,
           },
         },
       });
diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb
index 78124c13be9..09b023adcd3 100644
--- a/spec/models/clusters/applications/knative_spec.rb
+++ b/spec/models/clusters/applications/knative_spec.rb
@@ -1,18 +1,129 @@
 require 'rails_helper'
 
 describe Clusters::Applications::Knative do
-  let(:knative) { create(:clusters_applications_knative) }
+  let(:knative) { create(:clusters_applications_knative, hostname: 'example.com') }
 
   include_examples 'cluster application core specs', :clusters_applications_knative
-  include_examples 'cluster application status specs', :clusters_applications_knative
+
+  describe '#status' do
+    let(:cluster) { create(:cluster, :provided_by_gcp) }
+
+    subject { described_class.new(cluster: cluster) }
+
+    it 'sets a default status' do
+      expect(subject.status_name).to be(:not_installable)
+    end
+
+    context 'when application helm is scheduled' do
+      before do
+        create(:clusters_applications_helm, :scheduled, cluster: cluster)
+      end
+
+      it 'defaults to :not_installable' do
+        expect(subject.status_name).to be(:not_installable)
+      end
+    end
+
+    context 'when application is scheduled' do
+      before do
+        create(:clusters_applications_helm, :installed, cluster: cluster)
+      end
+
+      it 'sets a default status' do
+        expect(subject.status_name).to be(:installable)
+      end
+    end
+  end
+
+  describe 'status state machine' do
+    describe '#make_installing' do
+      subject { create(:clusters_applications_knative, :scheduled, hostname: 'example.com') }
+
+      it 'is installing' do
+        subject.make_installing!
+
+        expect(subject).to be_installing
+      end
+    end
+
+    describe '#make_installed' do
+      subject { create(:clusters_applications_knative, :installing, hostname: 'example.com') }
+
+      it 'is installed' do
+        subject.make_installed
+
+        expect(subject).to be_installed
+      end
+    end
+
+    describe '#make_errored' do
+      subject { create(:clusters_applications_knative, :installing, hostname: 'example.com') }
+      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(:clusters_applications_knative, :installable, hostname: 'example.com') }
+
+      it 'is scheduled' do
+        subject.make_scheduled
+
+        expect(subject).to be_scheduled
+      end
+
+      describe 'when was errored' do
+        subject { create(:clusters_applications_knative, :errored, hostname: 'example.com') }
+
+        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(:clusters_applications_knative, trait) }
+
+      if params[:available]
+        it { is_expected.to be_available }
+      else
+        it { is_expected.not_to be_available }
+      end
+    end
+  end
 
   describe '.installed' do
     subject { described_class.installed }
 
-    let!(:cluster) { create(:clusters_applications_knative, :installed) }
+    let!(:cluster) { create(:clusters_applications_knative, :installed, hostname: 'example.com') }
 
     before do
-      create(:clusters_applications_knative, :errored)
+      create(:clusters_applications_knative, :errored, hostname: 'example.com')
     end
 
     it { is_expected.to contain_exactly(cluster) }
@@ -24,7 +135,7 @@ describe Clusters::Applications::Knative do
     end
 
     context 'application install previously errored with older version' do
-      let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.1.3') }
+      let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.1.3', hostname: 'example.com') }
 
       it 'updates the application version' do
         expect(application.reload.version).to eq('0.1.3')
@@ -35,10 +146,10 @@ describe Clusters::Applications::Knative do
   describe '#make_installed' do
     subject { described_class.installed }
 
-    let!(:cluster) { create(:clusters_applications_knative, :installed) }
+    let!(:cluster) { create(:clusters_applications_knative, :installed, hostname: 'example.com') }
 
     before do
-      create(:clusters_applications_knative, :errored)
+      create(:clusters_applications_knative, :errored, hostname: 'example.com')
     end
 
     it { is_expected.to contain_exactly(cluster) }
@@ -54,11 +165,10 @@ describe Clusters::Applications::Knative do
       expect(subject.chart).to eq('knative/knative')
       expect(subject.version).to eq('0.1.3')
       expect(subject.files).to eq(knative.files)
-      expect(subject.setargs).to eq([])
     end
 
     context 'application failed to install previously' do
-      let(:knative) { create(:clusters_applications_knative, :errored, version: 'knative') }
+      let(:knative) { create(:clusters_applications_knative, :errored, version: 'knative', hostname: 'example.com') }
 
       it 'should be initialized with the locked version' do
         expect(subject.version).to eq('0.1.3')
diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb
index 10b9ca1a778..176eab3ce3a 100644
--- a/spec/models/clusters/cluster_spec.rb
+++ b/spec/models/clusters/cluster_spec.rb
@@ -314,7 +314,7 @@ describe Clusters::Cluster do
       let!(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) }
       let!(:runner) { create(:clusters_applications_runner, cluster: cluster) }
       let!(:jupyter) { create(:clusters_applications_jupyter, cluster: cluster) }
-      let!(:knative) { create(:clusters_applications_knative, cluster: cluster) }
+      let!(:knative) { create(:clusters_applications_knative, cluster: cluster, hostname: 'example.com') }
 
       it 'returns a list of created applications' do
         is_expected.to contain_exactly(helm, ingress, prometheus, runner, jupyter, knative)
diff --git a/spec/services/clusters/applications/create_service_spec.rb b/spec/services/clusters/applications/create_service_spec.rb
index 056db0c5486..a9985133b93 100644
--- a/spec/services/clusters/applications/create_service_spec.rb
+++ b/spec/services/clusters/applications/create_service_spec.rb
@@ -67,5 +67,38 @@ describe Clusters::Applications::CreateService do
         expect { subject }.to raise_error(Clusters::Applications::CreateService::InvalidApplicationError)
       end
     end
+
+    context 'knative application' do
+      let(:params) do
+        {
+          application: 'knative',
+          hostname: 'example.com'
+        }
+      end
+
+      before do
+        allow_any_instance_of(Clusters::Applications::ScheduleInstallationService).to receive(:execute)
+      end
+
+      it 'creates the application' do
+        expect do
+          subject
+
+          cluster.reload
+        end.to change(cluster, :application_knative)
+      end
+
+      it 'sets the hostname' do
+        expect(subject.hostname).to eq('example.com')
+      end
+    end
+
+    context 'invalid application' do
+      let(:params) { { application: 'non-existent' } }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(Clusters::Applications::CreateService::InvalidApplicationError)
+      end
+    end
   end
 end