From 73789fdfd2a6e34c9a05cc9e65930146e23b3195 Mon Sep 17 00:00:00 2001 From: Chris Baumbauer Date: Sat, 3 Nov 2018 14:43:48 -0700 Subject: [PATCH] Fix the way hostname is validated with the knative app --- .../clusters/stores/clusters_store.js | 6 +- app/models/clusters/applications/knative.rb | 6 +- .../20180912111628_add_knative_application.rb | 2 +- db/schema.rb | 2 +- .../clusters/applications/knative_spec.rb | 123 +----------------- spec/models/clusters/cluster_spec.rb | 2 +- 6 files changed, 17 insertions(+), 124 deletions(-) diff --git a/app/assets/javascripts/clusters/stores/clusters_store.js b/app/assets/javascripts/clusters/stores/clusters_store.js index 26566b60ba6..f4a016f3ef1 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: null + hostname: null, }, }, }; @@ -102,7 +102,9 @@ 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 : null; + 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 ecb42f6011e..d5cf3a4bb07 100644 --- a/app/models/clusters/applications/knative.rb +++ b/app/models/clusters/applications/knative.rb @@ -20,8 +20,6 @@ module Clusters default_value_for :version, VERSION default_value_for :hostname, nil - validates :hostname, presence: true - def chart 'knative/knative' end @@ -31,6 +29,10 @@ module Clusters end def install_command + if hostname.nil? + raise 'Hostname is required' + end + Gitlab::Kubernetes::Helm::InstallCommand.new( name: name, version: VERSION, diff --git a/db/migrate/20180912111628_add_knative_application.rb b/db/migrate/20180912111628_add_knative_application.rb index cbebc58c1e1..bfda6a945a7 100644 --- a/db/migrate/20180912111628_add_knative_application.rb +++ b/db/migrate/20180912111628_add_knative_application.rb @@ -13,7 +13,7 @@ class AddKnativeApplication < ActiveRecord::Migration t.datetime_with_timezone "updated_at", null: false t.integer "status", null: false t.string "version", null: false - t.string "hostname", null: false + t.string "hostname" t.text "status_reason" end end diff --git a/db/schema.rb b/db/schema.rb index 8c7b9d929db..4033b4a8c7c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -709,7 +709,7 @@ ActiveRecord::Schema.define(version: 20181101144347) do t.datetime_with_timezone "updated_at", null: false t.integer "status", null: false t.string "version", null: false - t.string "hostname", null: false + t.string "hostname" t.text "status_reason" end diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 09b023adcd3..6d72e3fba16 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -4,126 +4,15 @@ describe Clusters::Applications::Knative do let(:knative) { create(:clusters_applications_knative, hostname: 'example.com') } include_examples 'cluster application core 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 + include_examples 'cluster application status specs', :clusters_applications_knative describe '.installed' do subject { described_class.installed } - let!(:cluster) { create(:clusters_applications_knative, :installed, hostname: 'example.com') } + let!(:cluster) { create(:clusters_applications_knative, :installed) } before do - create(:clusters_applications_knative, :errored, hostname: 'example.com') + create(:clusters_applications_knative, :errored) end it { is_expected.to contain_exactly(cluster) } @@ -135,7 +24,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', hostname: 'example.com') } + let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.1.3') } it 'updates the application version' do expect(application.reload.version).to eq('0.1.3') @@ -146,10 +35,10 @@ describe Clusters::Applications::Knative do describe '#make_installed' do subject { described_class.installed } - let!(:cluster) { create(:clusters_applications_knative, :installed, hostname: 'example.com') } + let!(:cluster) { create(:clusters_applications_knative, :installed) } before do - create(:clusters_applications_knative, :errored, hostname: 'example.com') + create(:clusters_applications_knative, :errored) end it { is_expected.to contain_exactly(cluster) } diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 176eab3ce3a..10b9ca1a778 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, hostname: 'example.com') } + let!(:knative) { create(:clusters_applications_knative, cluster: cluster) } it 'returns a list of created applications' do is_expected.to contain_exactly(helm, ingress, prometheus, runner, jupyter, knative)