diff options
43 files changed, 442 insertions, 84 deletions
diff --git a/app/assets/javascripts/pages/snippets/show/index.js b/app/assets/javascripts/pages/snippets/show/index.js index 26936110402..5efcc901fde 100644 --- a/app/assets/javascripts/pages/snippets/show/index.js +++ b/app/assets/javascripts/pages/snippets/show/index.js @@ -5,9 +5,11 @@ import initNotes from '~/init_notes'; import snippetEmbed from '~/snippet/snippet_embed'; document.addEventListener('DOMContentLoaded', () => { - new LineHighlighter(); // eslint-disable-line no-new - new BlobViewer(); // eslint-disable-line no-new - initNotes(); - new ZenMode(); // eslint-disable-line no-new - snippetEmbed(); + if (!gon.features.snippetsVue) { + new LineHighlighter(); // eslint-disable-line no-new + new BlobViewer(); // eslint-disable-line no-new + initNotes(); + new ZenMode(); // eslint-disable-line no-new + snippetEmbed(); + } }); diff --git a/app/finders/clusters/knative_serving_namespace_finder.rb b/app/finders/clusters/knative_serving_namespace_finder.rb new file mode 100644 index 00000000000..d3db5be558c --- /dev/null +++ b/app/finders/clusters/knative_serving_namespace_finder.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Clusters + class KnativeServingNamespaceFinder + attr_reader :cluster + + def initialize(cluster) + @cluster = cluster + end + + def execute + cluster.kubeclient&.get_namespace(Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) + rescue Kubeclient::ResourceNotFoundError + nil + end + end +end diff --git a/app/finders/clusters/knative_version_role_binding_finder.rb b/app/finders/clusters/knative_version_role_binding_finder.rb index 06ec5ea557f..26f5492840a 100644 --- a/app/finders/clusters/knative_version_role_binding_finder.rb +++ b/app/finders/clusters/knative_version_role_binding_finder.rb @@ -9,9 +9,9 @@ module Clusters end def execute - cluster&.kubeclient&.get_cluster_role_bindings&.find do |resource| - resource.metadata.name == Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME - end + cluster.kubeclient&.get_cluster_role_binding(Clusters::Kubernetes::GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME) + rescue Kubeclient::ResourceNotFoundError + nil end end end diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index 085c6a04fa6..b718b55dd68 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -22,9 +22,28 @@ class DeploymentsFinder private + # rubocop: disable CodeReuse/ActiveRecord def init_collection - project.deployments + project + .deployments + .includes( + :user, + environment: [], + deployable: { + job_artifacts: [], + pipeline: { + project: { + route: [], + namespace: :route + } + }, + project: { + namespace: :route + } + } + ) end + # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord def sort(items) @@ -43,6 +62,9 @@ class DeploymentsFinder order_by = ALLOWED_SORT_VALUES.include?(params[:order_by]) ? params[:order_by] : DEFAULT_SORT_VALUE order_direction = ALLOWED_SORT_DIRECTIONS.include?(params[:sort]) ? params[:sort] : DEFAULT_SORT_DIRECTION - { order_by => order_direction } + { order_by => order_direction }.tap do |sort_values| + sort_values['id'] = 'desc' if sort_values['updated_at'] + sort_values['id'] = sort_values.delete('created_at') if sort_values['created_at'] # Sorting by `id` produces the same result as sorting by `created_at` + end end end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index 5e7fdd55cb6..91ffdac3273 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -84,13 +84,17 @@ module Clusters # ensures headers containing auth data are appended to original k8s client options options = kube_client.rest_client.options.merge(headers: kube_client.headers) Gitlab::PrometheusClient.new(proxy_url, options) - rescue Kubeclient::HttpError + rescue Kubeclient::HttpError, Errno::ECONNRESET, Errno::ECONNREFUSED # If users have mistakenly set parameters or removed the depended clusters, # `proxy_url` could raise an exception because gitlab can not communicate with the cluster. # Since `PrometheusAdapter#can_query?` is eargely loaded on environement pages in gitlab, # we need to silence the exceptions end + def configured? + kube_client.present? && available? + end + private def disable_prometheus_integration diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb index 9df77b565da..99da8b81398 100644 --- a/app/models/concerns/prometheus_adapter.rb +++ b/app/models/concerns/prometheus_adapter.rb @@ -16,6 +16,14 @@ module PrometheusAdapter raise NotImplementedError end + # This is a light-weight check if a prometheus client is properly configured. + def configured? + raise NotImplemented + end + + # This is a heavy-weight check if a prometheus is properly configured and accesible from GitLab. + # This actually sends a request to an external service and often it could take a long time, + # Please consider using `configured?` instead if the process is running on unicorn/puma threads. def can_query? prometheus_client.present? end diff --git a/app/models/environment.rb b/app/models/environment.rb index 327b1e594d7..fec1f034d63 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -193,11 +193,11 @@ class Environment < ApplicationRecord end def has_metrics? - available? && prometheus_adapter&.can_query? + available? && prometheus_adapter&.configured? end def metrics - prometheus_adapter.query(:environment, self) if has_metrics? + prometheus_adapter.query(:environment, self) if has_metrics? && prometheus_adapter.can_query? end def prometheus_status diff --git a/app/models/project.rb b/app/models/project.rb index 03a99577d5c..301fcfc8c98 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -374,9 +374,17 @@ class Project < ApplicationRecord scope :pending_delete, -> { where(pending_delete: true) } scope :without_deleted, -> { where(pending_delete: false) } - scope :with_storage_feature, ->(feature) { where('storage_version >= :version', version: HASHED_STORAGE_FEATURES[feature]) } - scope :without_storage_feature, ->(feature) { where('storage_version < :version OR storage_version IS NULL', version: HASHED_STORAGE_FEATURES[feature]) } - scope :with_unmigrated_storage, -> { where('storage_version < :version OR storage_version IS NULL', version: LATEST_STORAGE_VERSION) } + scope :with_storage_feature, ->(feature) do + where(arel_table[:storage_version].gteq(HASHED_STORAGE_FEATURES[feature])) + end + scope :without_storage_feature, ->(feature) do + where(arel_table[:storage_version].lt(HASHED_STORAGE_FEATURES[feature]) + .or(arel_table[:storage_version].eq(nil))) + end + scope :with_unmigrated_storage, -> do + where(arel_table[:storage_version].lt(LATEST_STORAGE_VERSION) + .or(arel_table[:storage_version].eq(nil))) + end # last_activity_at is throttled every minute, but last_repository_updated_at is updated with every push scope :sorted_by_activity, -> { reorder(Arel.sql("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC")) } diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 3e0606fd34a..2ca74b081aa 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -95,6 +95,10 @@ class PrometheusService < MonitoringService self_monitoring_project? && internal_prometheus_url? end + def configured? + should_return_client? + end + private def self_monitoring_project? diff --git a/app/models/upload.rb b/app/models/upload.rb index 8c409641452..650321e2793 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -23,6 +23,21 @@ class Upload < ApplicationRecord after_destroy :delete_file!, if: -> { uploader_class <= FileUploader } class << self + def inner_join_local_uploads_projects + upload_table = Upload.arel_table + project_table = Project.arel_table + + join_statement = upload_table.project(upload_table[Arel.star]) + .join(project_table) + .on( + upload_table[:model_type].eq('Project') + .and(upload_table[:model_id].eq(project_table[:id])) + .and(upload_table[:store].eq(ObjectStorage::Store::LOCAL)) + ) + + joins(join_statement.join_sources) + end + ## # FastDestroyAll concerns def begin_fast_destroy diff --git a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb index 0fea398d234..046046bf5a3 100644 --- a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb +++ b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb @@ -71,9 +71,9 @@ module Clusters end def knative_serving_namespace - kubeclient.core_client.get_namespaces.find do |namespace| - namespace.metadata.name == Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE - end + kubeclient.get_namespace(Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) + rescue Kubeclient::ResourceNotFoundError + nil end def create_role_or_cluster_role_binding diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index 36b4e00e8d5..c77b05e3ea8 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -4,13 +4,16 @@ - breadcrumb_title @snippet.to_reference - page_title "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets") -= render 'shared/snippets/header' +- if Feature.enabled?(:snippets_vue) + #js-snippet-view{ 'data-qa-selector': 'snippet_view' } +- else + = render 'shared/snippets/header' -.personal-snippets - %article.file-holder.snippet-file-content - = render 'shared/snippets/blob' + .personal-snippets + %article.file-holder.snippet-file-content + = render 'shared/snippets/blob' - .row-content-block.top-block.content-component-block - = render 'award_emoji/awards_block', awardable: @snippet, inline: true + .row-content-block.top-block.content-component-block + = render 'award_emoji/awards_block', awardable: @snippet, inline: true - #notes.limited-width-notes= render "shared/notes/notes_with_form", :autocomplete => false + #notes.limited-width-notes= render "shared/notes/notes_with_form", :autocomplete => false diff --git a/app/workers/hashed_storage/project_migrate_worker.rb b/app/workers/hashed_storage/project_migrate_worker.rb index 8c0ec97638f..0174467923d 100644 --- a/app/workers/hashed_storage/project_migrate_worker.rb +++ b/app/workers/hashed_storage/project_migrate_worker.rb @@ -14,7 +14,7 @@ module HashedStorage try_obtain_lease do project = Project.without_deleted.find_by(id: project_id) - break unless project + break unless project && project.storage_upgradable? old_disk_path ||= Storage::LegacyProject.new(project).disk_path diff --git a/changelogs/unreleased/28075-error-while-fetching-envs.yml b/changelogs/unreleased/28075-error-while-fetching-envs.yml new file mode 100644 index 00000000000..68d936f8f9c --- /dev/null +++ b/changelogs/unreleased/28075-error-while-fetching-envs.yml @@ -0,0 +1,5 @@ +--- +title: Added lightweight check when retrieving Prometheus metrics. +merge_request: 21099 +author: +type: performance diff --git a/changelogs/unreleased/35789-hashed-storage-attachments-migration-object-storage.yml b/changelogs/unreleased/35789-hashed-storage-attachments-migration-object-storage.yml new file mode 100644 index 00000000000..d868bdfe97a --- /dev/null +++ b/changelogs/unreleased/35789-hashed-storage-attachments-migration-object-storage.yml @@ -0,0 +1,6 @@ +--- +title: 'Hashed Storage attachments migration: exclude files in object storage as they + are all hashed already' +merge_request: 20338 +author: +type: changed diff --git a/changelogs/unreleased/36301-add-tie-breaker-id-sorting-to-deployments.yml b/changelogs/unreleased/36301-add-tie-breaker-id-sorting-to-deployments.yml new file mode 100644 index 00000000000..2457038bbce --- /dev/null +++ b/changelogs/unreleased/36301-add-tie-breaker-id-sorting-to-deployments.yml @@ -0,0 +1,5 @@ +--- +title: Optimize Deployments endpoint by preloading associations and make record ordering more consistent +merge_request: 20848 +author: +type: changed diff --git a/db/migrate/20191204070713_change_updated_at_index_and_add_index_to_id_on_deployments.rb b/db/migrate/20191204070713_change_updated_at_index_and_add_index_to_id_on_deployments.rb new file mode 100644 index 00000000000..450b276e689 --- /dev/null +++ b/db/migrate/20191204070713_change_updated_at_index_and_add_index_to_id_on_deployments.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class ChangeUpdatedAtIndexAndAddIndexToIdOnDeployments < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + PROJECT_ID_INDEX_PARAMS = [[:project_id, :id], order: { id: :desc }] + OLD_UPDATED_AT_INDEX_PARAMS = [[:project_id, :updated_at]] + NEW_UPDATED_AT_INDEX_PARAMS = [[:project_id, :updated_at, :id], order: { updated_at: :desc, id: :desc }] + + def up + add_concurrent_index :deployments, *NEW_UPDATED_AT_INDEX_PARAMS + + remove_concurrent_index :deployments, *OLD_UPDATED_AT_INDEX_PARAMS + + add_concurrent_index :deployments, *PROJECT_ID_INDEX_PARAMS + end + + def down + add_concurrent_index :deployments, *OLD_UPDATED_AT_INDEX_PARAMS + + remove_concurrent_index :deployments, *NEW_UPDATED_AT_INDEX_PARAMS + + remove_concurrent_index :deployments, *PROJECT_ID_INDEX_PARAMS + end +end diff --git a/db/schema.rb b/db/schema.rb index d33060f0e37..d0b4629d191 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_12_02_031812) do +ActiveRecord::Schema.define(version: 2019_12_04_070713) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -1339,10 +1339,11 @@ ActiveRecord::Schema.define(version: 2019_12_02_031812) do t.index ["environment_id", "iid", "project_id"], name: "index_deployments_on_environment_id_and_iid_and_project_id" t.index ["environment_id", "status"], name: "index_deployments_on_environment_id_and_status" t.index ["id"], name: "partial_index_deployments_for_legacy_successful_deployments", where: "((finished_at IS NULL) AND (status = 2))" + t.index ["project_id", "id"], name: "index_deployments_on_project_id_and_id", order: { id: :desc } t.index ["project_id", "iid"], name: "index_deployments_on_project_id_and_iid", unique: true t.index ["project_id", "status", "created_at"], name: "index_deployments_on_project_id_and_status_and_created_at" t.index ["project_id", "status"], name: "index_deployments_on_project_id_and_status" - t.index ["project_id", "updated_at"], name: "index_deployments_on_project_id_and_updated_at" + t.index ["project_id", "updated_at", "id"], name: "index_deployments_on_project_id_and_updated_at_and_id", order: { updated_at: :desc, id: :desc } end create_table "description_versions", force: :cascade do |t| diff --git a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb index b47238a3083..65445ab3fc4 100644 --- a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb +++ b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb @@ -8,7 +8,7 @@ module Gitlab def unmet? deployment_cluster.present? && deployment_cluster.managed? && - (missing_namespace? || missing_knative_version_role_binding?) + (missing_namespace? || need_knative_version_role_binding?) end def complete! @@ -23,8 +23,8 @@ module Gitlab kubernetes_namespace.nil? || kubernetes_namespace.service_account_token.blank? end - def missing_knative_version_role_binding? - knative_version_role_binding.nil? + def need_knative_version_role_binding? + !knative_serving_namespace.nil? && knative_version_role_binding.nil? end def deployment_cluster @@ -35,6 +35,14 @@ module Gitlab build.deployment.environment end + def knative_serving_namespace + strong_memoize(:knative_serving_namespace) do + Clusters::KnativeServingNamespaceFinder.new( + deployment_cluster + ).execute + end + end + def knative_version_role_binding strong_memoize(:knative_version_role_binding) do Clusters::KnativeVersionRoleBindingFinder.new( diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 2616a19fdaa..487dcd58d01 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -42,6 +42,7 @@ module Gitlab # Initialize gon.features with any flags that should be # made globally available to the frontend push_frontend_feature_flag(:suppress_ajax_navigation_errors, default_enabled: true) + push_frontend_feature_flag(:snippets_vue, default_enabled: false) end # Exposes the state of a feature flag to the frontend code. diff --git a/lib/gitlab/hashed_storage/rake_helper.rb b/lib/gitlab/hashed_storage/rake_helper.rb index 14727b03ce9..7965f165683 100644 --- a/lib/gitlab/hashed_storage/rake_helper.rb +++ b/lib/gitlab/hashed_storage/rake_helper.rb @@ -47,23 +47,13 @@ module Gitlab end # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def self.legacy_attachments_relation - Upload.joins(<<~SQL).where('projects.storage_version < :version OR projects.storage_version IS NULL', version: Project::HASHED_STORAGE_FEATURES[:attachments]) - JOIN projects - ON (uploads.model_type='Project' AND uploads.model_id=projects.id) - SQL + Upload.inner_join_local_uploads_projects.merge(Project.without_storage_feature(:attachments)) end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def self.hashed_attachments_relation - Upload.joins(<<~SQL).where('projects.storage_version >= :version', version: Project::HASHED_STORAGE_FEATURES[:attachments]) - JOIN projects - ON (uploads.model_type='Project' AND uploads.model_id=projects.id) - SQL + Upload.inner_join_local_uploads_projects.merge(Project.with_storage_feature(:attachments)) end - # rubocop: enable CodeReuse/ActiveRecord def self.relation_summary(relation_name, relation) relation_count = relation.count diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index b28664b2921..b01b4951738 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -146,7 +146,10 @@ module QA end def finished_loading? - has_no_css?('.fa-spinner', wait: Capybara.default_max_wait_time) + # The number of selectors should be able to be reduced after + # migration to the new spinner is complete. + # https://gitlab.com/groups/gitlab-org/-/epics/956 + has_no_css?('.gl-spinner, .fa-spinner, .spinner', wait: Capybara.default_max_wait_time) end def finished_loading_block? diff --git a/qa/qa/page/component/select2.rb b/qa/qa/page/component/select2.rb index 8fe6a4a75b3..1dd718a1d88 100644 --- a/qa/qa/page/component/select2.rb +++ b/qa/qa/page/component/select2.rb @@ -31,7 +31,7 @@ module QA end def wait_for_search_to_complete - has_css?('.select2-active') + has_css?('.select2-active', wait: 1) has_no_css?('.select2-active', wait: 30) end end diff --git a/qa/qa/page/settings/common.rb b/qa/qa/page/settings/common.rb index 2d7b41c76e1..bd1070158f0 100644 --- a/qa/qa/page/settings/common.rb +++ b/qa/qa/page/settings/common.rb @@ -6,7 +6,7 @@ module QA module Common # Click the Expand button present in the specified section # - # @param [Symbol] and `element` name defined in a `view` block + # @param [Symbol] element_name `element` name defined in a `view` block def expand_section(element_name) within_element(element_name) do # Because it is possible to click the button before the JS toggle code is bound @@ -14,6 +14,7 @@ module QA click_button 'Expand' unless has_css?('button', text: 'Collapse', wait: 1) has_content?('Collapse') + finished_loading? end yield if block_given? diff --git a/spec/features/snippets/internal_snippet_spec.rb b/spec/features/snippets/internal_snippet_spec.rb index 4ef3b0e5e7a..fd7ef71db15 100644 --- a/spec/features/snippets/internal_snippet_spec.rb +++ b/spec/features/snippets/internal_snippet_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe 'Internal Snippets', :js do let(:internal_snippet) { create(:personal_snippet, :internal) } + before do + stub_feature_flags(snippets_vue: false) + end + describe 'normal user' do before do sign_in(create(:user)) diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 2bd01be25e9..57264f97ddc 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -16,6 +16,7 @@ describe 'Comments on personal snippets', :js do let!(:other_note) { create(:note_on_personal_snippet) } before do + stub_feature_flags(snippets_vue: false) sign_in user visit snippet_path(snippet) diff --git a/spec/features/snippets/private_snippets_spec.rb b/spec/features/snippets/private_snippets_spec.rb index 9df4cd01103..37f45f22a27 100644 --- a/spec/features/snippets/private_snippets_spec.rb +++ b/spec/features/snippets/private_snippets_spec.rb @@ -6,6 +6,7 @@ describe 'Private Snippets', :js do let(:user) { create(:user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) end diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb index 82edda509c2..045afcf1c12 100644 --- a/spec/features/snippets/public_snippets_spec.rb +++ b/spec/features/snippets/public_snippets_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' describe 'Public Snippets', :js do + before do + stub_feature_flags(snippets_vue: false) + end + it 'Unauthenticated user should see public snippets' do public_snippet = create(:personal_snippet, :public) diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 450e520e293..9c686be012b 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -6,6 +6,10 @@ describe 'Snippet', :js do let(:project) { create(:project, :repository) } let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content) } + before do + stub_feature_flags(snippets_vue: false) + end + context 'Ruby file' do let(:file_name) { 'popen.rb' } let(:content) { project.repository.blob_at('master', 'files/ruby/popen.rb').data } diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb index 3e71a4e7879..0c3ca6f17c8 100644 --- a/spec/features/snippets/spam_snippets_spec.rb +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -7,6 +7,7 @@ describe 'User creates snippet', :js do before do stub_feature_flags(allow_possible_spam: false) + stub_feature_flags(snippets_vue: false) stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') Gitlab::CurrentSettings.update!( diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 9a141dd463a..b373264bbe4 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -8,6 +8,7 @@ describe 'User creates snippet', :js do let(:user) { create(:user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) visit new_snippet_path end diff --git a/spec/features/snippets/user_deletes_snippet_spec.rb b/spec/features/snippets/user_deletes_snippet_spec.rb index 217419a220a..35619b92561 100644 --- a/spec/features/snippets/user_deletes_snippet_spec.rb +++ b/spec/features/snippets/user_deletes_snippet_spec.rb @@ -10,6 +10,8 @@ describe 'User deletes snippet' do before do sign_in(user) + stub_feature_flags(snippets_vue: false) + visit snippet_path(snippet) end diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 51d9baf44bc..1d26660a4f6 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -12,6 +12,7 @@ describe 'User edits snippet', :js do let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, author: user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) visit edit_snippet_path(snippet) diff --git a/spec/features/snippets_spec.rb b/spec/features/snippets_spec.rb index 9df6fe7d16b..bc7fa161e87 100644 --- a/spec/features/snippets_spec.rb +++ b/spec/features/snippets_spec.rb @@ -6,11 +6,38 @@ describe 'Snippets' do context 'when the project has snippets' do let(:project) { create(:project, :public) } let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } + before do allow(Snippet).to receive(:default_per_page).and_return(1) - visit snippets_path(username: project.owner.username) + + visit project_snippets_path(project) end it_behaves_like 'paginated snippets' end + + describe 'rendering engine' do + let_it_be(:snippet) { create(:personal_snippet, :public) } + let(:snippets_vue_feature_flag_enabled) { true } + + before do + stub_feature_flags(snippets_vue: snippets_vue_feature_flag_enabled) + + visit snippet_path(snippet) + end + + it 'renders Vue application' do + expect(page).to have_selector('#js-snippet-view') + expect(page).not_to have_selector('.personal-snippets') + end + + context 'when feature flag is disabled' do + let(:snippets_vue_feature_flag_enabled) { false } + + it 'renders HAML application and not Vue' do + expect(page).not_to have_selector('#js-snippet-view') + expect(page).to have_selector('.personal-snippets') + end + end + end end diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb index f21bb068c24..be35a705b0d 100644 --- a/spec/finders/deployments_finder_spec.rb +++ b/spec/finders/deployments_finder_spec.rb @@ -32,13 +32,13 @@ describe DeploymentsFinder do let(:params) { { order_by: order_by, sort: sort } } - let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) } + let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now) } let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago) } - let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'patch', created_at: 2.days.ago, updated_at: 1.hour.ago) } + let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'patch', created_at: Time.now, updated_at: 1.hour.ago) } where(:order_by, :sort, :ordered_deployments) do - 'created_at' | 'asc' | [:deployment_3, :deployment_2, :deployment_1] - 'created_at' | 'desc' | [:deployment_1, :deployment_2, :deployment_3] + 'created_at' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] + 'created_at' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] 'id' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] 'id' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] 'iid' | 'asc' | [:deployment_3, :deployment_1, :deployment_2] @@ -57,5 +57,41 @@ describe DeploymentsFinder do end end end + + describe 'transform `created_at` sorting to `id` sorting' do + let(:params) { { order_by: 'created_at', sort: 'asc' } } + + it 'sorts by only one column' do + expect(subject.order_values.size).to eq(1) + end + + it 'sorts by `id`' do + expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) + end + end + + describe 'tie-breaker for `updated_at` sorting' do + let(:params) { { order_by: 'updated_at', sort: 'asc' } } + + it 'sorts by two columns' do + expect(subject.order_values.size).to eq(2) + end + + it 'adds `id` sorting as the second order column' do + order_value = subject.order_values[1] + + expect(order_value.to_sql).to eq(Deployment.arel_table[:id].desc.to_sql) + end + + it 'uses the `id DESC` as tie-breaker when ordering' do + updated_at = Time.now + + deployment_1 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_2 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_3 = create(:deployment, :success, project: project, updated_at: updated_at) + + expect(subject).to eq([deployment_3, deployment_2, deployment_1]) + end + end end end diff --git a/spec/frontend/fixtures/snippet.rb b/spec/frontend/fixtures/snippet.rb index 34a6fade9c9..33dfebac3e7 100644 --- a/spec/frontend/fixtures/snippet.rb +++ b/spec/frontend/fixtures/snippet.rb @@ -17,6 +17,7 @@ describe SnippetsController, '(JavaScript fixtures)', type: :controller do end before do + stub_feature_flags(snippets_vue: false) sign_in(admin) allow(Discussion).to receive(:build_discussion_id).and_return(['discussionid:ceterumcenseo']) end diff --git a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb index caa84faa9c1..74fa2be5930 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb @@ -38,28 +38,44 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do .and_return(double(execute: kubernetes_namespace)) end - context 'and the knative version role binding is missing' do + context 'and the knative-serving namespace is missing' do before do - allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new) - .and_return(double(execute: nil)) + allow(Clusters::KnativeServingNamespaceFinder).to receive(:new) + .and_return(double(execute: false)) end it { is_expected.to be_truthy } end - context 'and the knative version role binding already exists' do + context 'and the knative-serving namespace exists' do before do - allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new) + allow(Clusters::KnativeServingNamespaceFinder).to receive(:new) .and_return(double(execute: true)) end - it { is_expected.to be_falsey } - - context 'and the service_account_token is blank' do - let(:kubernetes_namespace) { instance_double(Clusters::KubernetesNamespace, service_account_token: nil) } + context 'and the knative version role binding is missing' do + before do + allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new) + .and_return(double(execute: nil)) + end it { is_expected.to be_truthy } end + + context 'and the knative version role binding already exists' do + before do + allow(Clusters::KnativeVersionRoleBindingFinder).to receive(:new) + .and_return(double(execute: true)) + end + + it { is_expected.to be_falsey } + + context 'and the service_account_token is blank' do + let(:kubernetes_namespace) { instance_double(Clusters::KubernetesNamespace, service_account_token: nil) } + + it { is_expected.to be_truthy } + end + end end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 2aeb7e5a990..cf6e3fff744 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -53,6 +53,16 @@ describe Clusters::Applications::Prometheus do end describe '#prometheus_client' do + shared_examples 'exception caught for prometheus client' do + before do + allow(kube_client).to receive(:proxy_url).and_raise(exception) + end + + it 'returns nil' do + expect(subject.prometheus_client).to be_nil + end + end + context 'cluster is nil' do it 'returns nil' do expect(subject.cluster).to be_nil @@ -98,12 +108,18 @@ describe Clusters::Applications::Prometheus do end context 'when cluster is not reachable' do - before do - allow(kube_client).to receive(:proxy_url).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) + it_behaves_like 'exception caught for prometheus client' do + let(:exception) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) } + end + end + + context 'when there is a socket error while contacting cluster' do + it_behaves_like 'exception caught for prometheus client' do + let(:exception) { Errno::ECONNREFUSED } end - it 'returns nil' do - expect(subject.prometheus_client).to be_nil + it_behaves_like 'exception caught for prometheus client' do + let(:exception) { Errno::ECONNRESET } end end end @@ -289,4 +305,28 @@ describe Clusters::Applications::Prometheus do end end end + + describe '#configured?' do + let(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + subject { prometheus.configured? } + + context 'when a kubenetes client is present' do + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + + it { is_expected.to be_truthy } + + context 'when it is not availalble' do + let(:prometheus) { create(:clusters_applications_prometheus, cluster: cluster) } + + it { is_expected.to be_falsey } + end + end + + context 'when a kubenetes client is not present' do + let(:cluster) { create(:cluster) } + + it { is_expected.to be_falsy } + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index e1e4af63ef3..826e5b41a5c 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -824,6 +824,14 @@ describe Environment, :use_clean_rails_memory_store_caching do context 'and no deployments' do it { is_expected.to be_truthy } end + + context 'and the prometheus adapter is not configured' do + before do + allow(environment.prometheus_adapter).to receive(:configured?).and_return(false) + end + + it { is_expected.to be_falsy } + end end context 'without a monitoring service' do @@ -858,6 +866,14 @@ describe Environment, :use_clean_rails_memory_store_caching do is_expected.to eq(:fake_metrics) end + + context 'and the prometheus client is not present' do + before do + allow(environment.prometheus_adapter).to receive(:promethus_client).and_return(nil) + end + + it { is_expected.to be_nil } + end end context 'when the environment does not have metrics' do diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 91e047774bf..986a44cc640 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -332,4 +332,40 @@ describe API::Deployments do end end end + + context 'prevent N + 1 queries' do + context 'when the endpoint returns multiple records' do + let(:project) { create(:project) } + + def create_record + create(:deployment, :success, project: project) + end + + def request_with_query_count + ActiveRecord::QueryRecorder.new { trigger_request }.count + end + + def trigger_request + get api("/projects/#{project.id}/deployments?order_by=updated_at&sort=asc", user) + end + + before do + create_record + end + + it 'succeeds' do + trigger_request + + expect(response).to have_gitlab_http_status(200) + + expect(json_response.size).to eq(1) + end + + it 'does not increase the query count' do + expect { create_record }.not_to change { request_with_query_count } + + expect(json_response.size).to eq(2) + end + end + end end diff --git a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb index a9e3e881fd4..1f520c4849d 100644 --- a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb +++ b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb @@ -22,7 +22,6 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do before do stub_kubeclient_discover(api_url) - stub_kubeclient_get_namespaces(api_url) stub_kubeclient_get_service_account_error(api_url, 'gitlab') stub_kubeclient_create_service_account(api_url) stub_kubeclient_get_secret_error(api_url, 'gitlab-token') @@ -31,6 +30,7 @@ describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do stub_kubeclient_get_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) stub_kubeclient_put_role_binding(api_url, "gitlab-#{namespace}", namespace: namespace) stub_kubeclient_get_namespace(api_url, namespace: namespace) + stub_kubeclient_get_namespace(api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) stub_kubeclient_get_service_account_error(api_url, "#{namespace}-service-account", namespace: namespace) stub_kubeclient_create_service_account(api_url, namespace: namespace) stub_kubeclient_create_secret(api_url, namespace: namespace) diff --git a/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb b/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb index b40861e5aaf..1ca3c50c46c 100644 --- a/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb +++ b/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb @@ -141,7 +141,7 @@ describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do before do cluster.platform_kubernetes.rbac! - stub_kubeclient_get_namespaces(api_url) + stub_kubeclient_get_namespace(api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) stub_kubeclient_get_role_binding_error(api_url, role_binding_name, namespace: namespace) stub_kubeclient_create_role_binding(api_url, namespace: namespace) stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_NAME, namespace: namespace) diff --git a/spec/workers/hashed_storage/project_migrate_worker_spec.rb b/spec/workers/hashed_storage/project_migrate_worker_spec.rb index f266c7dbe8c..4b1b5f84fd3 100644 --- a/spec/workers/hashed_storage/project_migrate_worker_spec.rb +++ b/spec/workers/hashed_storage/project_migrate_worker_spec.rb @@ -5,13 +5,13 @@ require 'spec_helper' describe HashedStorage::ProjectMigrateWorker, :clean_gitlab_redis_shared_state do include ExclusiveLeaseHelpers - describe '#perform' do - let(:project) { create(:project, :empty_repo, :legacy_storage) } - let(:lease_key) { "project_migrate_hashed_storage_worker:#{project.id}" } - let(:lease_timeout) { described_class::LEASE_TIMEOUT } - let(:migration_service) { ::Projects::HashedStorage::MigrationService } + let(:migration_service) { ::Projects::HashedStorage::MigrationService } + let(:lease_timeout) { described_class::LEASE_TIMEOUT } + describe '#perform' do it 'skips when project no longer exists' do + stub_exclusive_lease(lease_key(-1), 'uuid', timeout: lease_timeout) + expect(migration_service).not_to receive(:new) subject.perform(-1) @@ -19,32 +19,67 @@ describe HashedStorage::ProjectMigrateWorker, :clean_gitlab_redis_shared_state d it 'skips when project is pending delete' do pending_delete_project = create(:project, :empty_repo, pending_delete: true) + stub_exclusive_lease(lease_key(pending_delete_project.id), 'uuid', timeout: lease_timeout) expect(migration_service).not_to receive(:new) subject.perform(pending_delete_project.id) end - it 'delegates migration to service class when we have exclusive lease' do - stub_exclusive_lease(lease_key, 'uuid', timeout: lease_timeout) + it 'skips when project is already migrated' do + migrated_project = create(:project, :empty_repo) + stub_exclusive_lease(lease_key(migrated_project.id), 'uuid', timeout: lease_timeout) + + expect(migration_service).not_to receive(:new) + + subject.perform(migrated_project.id) + end + + context 'with exclusive lease available' do + it 'delegates migration to service class' do + project = create(:project, :empty_repo, :legacy_storage) + stub_exclusive_lease(lease_key(project.id), 'uuid', timeout: lease_timeout) - service_spy = spy + service_spy = spy - allow(migration_service) - .to receive(:new).with(project, project.full_path, logger: subject.logger) - .and_return(service_spy) + allow(migration_service) + .to receive(:new).with(project, project.full_path, logger: subject.logger) + .and_return(service_spy) - subject.perform(project.id) + subject.perform(project.id) - expect(service_spy).to have_received(:execute) + expect(service_spy).to have_received(:execute) + end + + it 'delegates migration to service class with correct path in a partially migrated project' do + project = create(:project, :empty_repo, storage_version: 1) + stub_exclusive_lease(lease_key(project.id), 'uuid', timeout: lease_timeout) + + service_spy = spy + + allow(migration_service) + .to receive(:new).with(project, project.full_path, logger: subject.logger) + .and_return(service_spy) + + subject.perform(project.id) + + expect(service_spy).to have_received(:execute) + end end - it 'skips when it cant acquire the exclusive lease' do - stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) + context 'with exclusive lease taken' do + it 'skips when it cant acquire the exclusive lease' do + project = create(:project, :empty_repo, :legacy_storage) + stub_exclusive_lease_taken(lease_key(project.id), timeout: lease_timeout) - expect(migration_service).not_to receive(:new) + expect(migration_service).not_to receive(:new) - subject.perform(project.id) + subject.perform(project.id) + end end end + + def lease_key(key) + "project_migrate_hashed_storage_worker:#{key}" + end end |